💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1954834091)
> I should be setting main_only to true,

I think that works? Will think more on it.

> GetDescendants

Missed that one!

💬 sr-gi commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#issuecomment-2657136142)
@tdb3 @danielabrozzoni can you give this another look? The changes should be trivial

💬 ryanofsky commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954836649)
In commit "Have createNewBlock() wait for a tip" (854766b886b6c2ee564b867db64bdbce80064b3b)

Another alternative but probably not better is:

```diff
- if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {};
+ if (!static_cast<Mining*>(this)->waitTipChanged(uint256::ZERO)) return {};
```

I think in longer term after we add an initial waitNext() method and make improvements to it, it will be natural to move more functionality out of Mining class methods i
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2657139469)
(there seems to be a delay in Github processing my latest push)

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2657139983)
Ah, I didn't test with libFuzzer. I guess to support that I'll have to port https://github.com/bitcoin/bitcoin/blob/c242fa5be358150d83c2446896b6f4c45c6365e9/test/fuzz/test_runner.py#L170

I'll do that tomorrow.

✅ fanquake closed a pull request: "cmake: Install man pages for configured targets only"
(https://github.com/bitcoin/bitcoin/pull/31765)

💬 fanquake commented on pull request "cmake: Install man pages for configured targets only":
(https://github.com/bitcoin/bitcoin/pull/31765#issuecomment-2657151761)
Closing given the concept ACK in the other thread.

💬 fanquake commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2657172457)
https://github.com/bitcoin/bitcoin/pull/31662/checks?check_run_id=37171033299:
```bash
[10:53:16.049] gmake[2]: Entering directory '/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu'
[10:53:16.093] [ 72%] Linking CXX executable fuzz
[10:53:16.093] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz && /usr/bin/cmake -E cmake_link_script CMakeFiles/fuzz.dir/link.txt --verbose=1
[10:53:16.117] /usr/bin/clang++-20 -ftrivial-auto-var-init=pattern -O2 -g -fsanitize=fuz
...

💬 sipa commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2657179844)
Mild concept ACK. I think having all binaries in one place is a nice, but not critical improvement. If we're doing it, we should do it now, though.

💬 ryanofsky commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954865233)
In commit "rpc: handle shutdown during long poll and wait methods" (0c06c8108fb43be317d86199750ecd3b7db9a5d2)

I think this code is not quite right because it is assuming that if `getTip()` returns null it means the node is shutting down, when in reality it is starting up. Also the IsRPCRunning() call unnecessarily complicates things and is incorrect as we found previously in #30635. Would also be nice to reduce number of variables used in this code and remove CHECK_NONFATAL calls that can't t
...

💬 glozow commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2657198949)
@hebasto @theuni can you open a PR to prune the gcc stuff for v29?

💬 sipa commented on pull request "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports":
(https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2657199198)
I suggest treating the broken GCC-based coverage as a 29.0 blocker, as it seems it's been effectively unusable anyway.

Adding an LLVM-based coverage mechanism then seems like an independent improvement.

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1954895162)
I'll defer to you, but IIUC we'll do ~1 orphan processing per message processing step, so it might take a bit more to process all 24 from orphanage? Alternatively e could query the *first* tx and wait until the descendant count hits DEFAULT_ANCESTOR_LIMIT

👍 stickies-v approved a pull request: "cmake: add a component for each binary"
(https://github.com/bitcoin/bitcoin/pull/31844#pullrequestreview-2615433035)
tACK 686ebcfe93efa9ecca9b94e78da23a36c3b01f90

Being able to specify install components for all targets is helpful, and this implementation seems very clean. Part of me annoyed is that the consistency isn't there for kernel (`bitcoinkernel` target and `libbitcoinkernel` component), but it seems like that's conventional for libraries so I'll suck it up.

💬 stickies-v commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#discussion_r1954741365)
I think this is now duplicated and can be removed?

🤔 danielabrozzoni reviewed a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758#pullrequestreview-2615727315)
Whoops, I had forgotten about this one :)

re-ACK 0a02e7fdeaca26455b3710ef76b11101ac816e53

💬 hodlinator commented on pull request "test: Remove stale gettime test":
(https://github.com/bitcoin/bitcoin/pull/31846#issuecomment-2657262496)
Would probably have leaned towards just updating the comment and leaving the test just out of paranoia, even though so much is different with the `std::chrono` implementation that anything similar is unlikely to re-occur. Getting rid of lines to maintain has value too.

💬 theuni commented on pull request "depends: add missing Darwin objcopy":
(https://github.com/bitcoin/bitcoin/pull/31840#issuecomment-2657264201)
@hebasto Agreed as long as it's trivial. From what I can tell `llvm-install-name-tool` is just as available as the rest of the tools, so even though it's annoying because it's not actually necessary, I don't see much harm in setting it. @fanquake ?

This required a similar hack for https://github.com/bitcoin/bitcoin/pull/30434 as well.

💬 brunoerg commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2657278221)
> q: isn't the test just wrong?. `min_viable_change` is initialized to the future input fee without increasing the window by one as we do in the actual code. Thats why there is an overlap here. See the wallet code:
>
> [bitcoin/src/wallet/spend.cpp](https://github.com/bitcoin/bitcoin/blob/c242fa5be358150d83c2446896b6f4c45c6365e9/src/wallet/spend.cpp#L1131-L1132)
>
> Lines 1131 to 1132 in [c242fa5](/bitcoin/bitcoin/commit/c242fa5be358150d83c2446896b6f4c45c6365e9)
>
> const auto change_spend_f
...

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2657285774)
Rebased c6658d675d0bb945f53dc62f7e9b95c7bba973bb -> b2108240dec5d858d17af798fdc79037f894786d ([`pr/subtree.15`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.15) -> [`pr/subtree.16`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.16), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.15-rebase..pr/subtree.16)) to fix conflict with #31834. Also added another fix for #30975 and documented & unhid CAPNP_EXECUTABLE variable as suggested

re: https://github.com/bi
...