💬 sipa commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937451223)
This is not the case. It's just that block template building will be much faster, but operationally nothing changes: you have to build a template to know what the fees will be. Of course, it'd be possible to create a way to run the template-building code without actually remembering the template, and only keeping track of the fees, which would be even faster, but that could be done right now too.

I think eventually, the correct approach is to have a background thread that continuously creates
...

✅ achow101 closed an issue: "incrementalrelayfee configuration option should be present in bitcoin.conf"
(https://github.com/bitcoin/bitcoin/issues/31769)

💬 achow101 commented on issue "incrementalrelayfee configuration option should be present in bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/31769#issuecomment-2627557208)
All command line options, hidden or not, can be set in the bitcoin.conf.

The help for incrementalrelayfee can be seen if `-help-debug` is additionally passed.

💬 theuni commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1937464559)
Good point. Move it down below language selection, and also now it's only set if not specified by the user.

💬 theuni commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2627582147)
Updated to address hebasto's comment, and removed comments about it being a speedup. Should be good to go now.

This speeds things up significantly for me in some cases where I'm only making specific targets. I think it should be a general speedup in other cases too, but sure, it's more about the dependency reduction.

⚠️ fanquake opened an issue: "test: 32-bit Clang `ipc_test` failure at `-O0`"
(https://github.com/bitcoin/bitcoin/issues/31772)
Noticed as part of this branch #29796, however it can also be reproduced with master (8fa10edcd1706a1f0dc9d8c3adbc8efa3c7755bf) by reproducing the equivalent CI & setting C(XX)FLAGS to -O0:
```bash
make -C depends/ MULTIPROCESS=1 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_USDT=1 CFLAGS="-O0" CXXFLAGS="-O0" DEBUG=1 -j19 HOST=i686-pc-linux-gnu
cmake -B build --toolchain /root/ci_scratch/depends/i686-pc-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang;-m32' -DCMAKE_CXX_COMPILER='cl
...

💬 fanquake commented on pull request "[RFC] Align debugging flags to `-O0`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2627584482)
I've pushed up a patch that should work around the broken qt code (although it's unclear that 32-bit Clang builds are something we need to support for the gui).

Note that this branch is likely going to fail a different way now, in the `ipc_tests`, see #31772.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937491375)
I was hoping we could simply have a variable that tracks the total fees for the top 1 vMB, which is constantly updated as things are inserted and removed from the mempool.

💬 sipa commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937500659)
It's probably fast enough that that can be done as an add-on, updated after every mempool change, but limited to every N milliseconds or so.

💬 ismaelsadeeq commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937503273)
@sipa, I imagine that an RBF over any transaction in the last built block template be an indicator that the previously built template improve.

And since we would be using fee rate diagram for RBF's after cluster mempool, we could use use RBF's as determinant?

💬 sr-gi commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#discussion_r1937515621)
I don't have a strong opinion on this.

I've checked and looks like no other test uses this, or re-defines it. I'd lean towards leaving it locally to avoid bloating `p2p.py`, but I'm open to move it if there is support for it

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937516539)
You mean "never generate a template" when run from the tests, right? Why is that? The default timeout is `inf` so `now < deadline` should be `true`?

> So it has to use <=, but without the break here the test will freeze, because time doesn't move.

Right, but it is not too difficult to advance the time in unit tests.

💬 tdb3 commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#discussion_r1937519743)
Works for me. If/when the file is touched again, it could be moved then.

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937522651)
Functionality-wise I think the behavior of the production code is ok. I am concerned about its readability and maintainability. It just looks confusing (to me).

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937527468)
Agreed about `MillisecondsDouble::max()`. Note here that we rely to have such a max value that when we add to it the result is the same max value (not overflow or cause unexpected things).

💬 sipa commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937550966)
Just discussed this with @ismaelsadeeq a bit more.

The hard part is not tracking improvements to the mempool (that is ultimately what all of cluster mempool is, making this explicit), but know which improvements affect the top 1 MvB of the mempool and to what extent. My thinking was that we can't really maintain an index for that, as doing so would be less efficient than just recomputing when it's needed.

But maybe that is not the case. We could keep track at all times of (a) the sum of th
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937555381)
@ryanofsky do I understand correctly that for the `.capnp` there's no difference when making something an `std::optional`?

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937566000)
Changed the commit message.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937568342)
Changed to "nothing"

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937577466)
I turned `Assert` into `Assume`. When comparing an std::optional<uint256> with a <uint256> it will be considered unequal if the optional is empty. https://en.cppreference.com/w/cpp/utility/optional/operator_cmp (24)

This would trigger a spurious `tip_changed`. But since it can only happen during shutdown, it's not a problem. And not fatal in any case.