💬 maflcko commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#issuecomment-2763285550)
I've run `cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ partially_downloaded_block 128` for about 300 times and it passed. So hopefully this is good enough for now. (In theory the scheduler thread may still be woken spuriously, even if there is no work, but the only solution to that would be to disable it completely for all fuzz targets that don't need it.)

💬 pablomartin4btc commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2019776473)
Yup, fixed it, thanks!

💬 pablomartin4btc commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#issuecomment-2763311638)
_<ins>Updates</ins>:_
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2019486082).

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019792536)
> I guess the advantage of the on-the-fly heap would be that you may never have to calculate the ordering for some chunks in the middle of the mempool

This is possible even with the existing index-based structure, by allowing chunk feerates to be indeterminate, and index them by a known lower and upper bound. That would necessitate two precomputed indexes (one for block building, sorted by the upper bound; one for eviction, sorted by the lower bound), and relinearizing whatever cluster was f
...

📝 hebasto opened a pull request: "depends: Switch from multilib to platform-specific toolchains"
(https://github.com/bitcoin/bitcoin/pull/32162)
Using the multilib GCC toolchain, as currently documented in [`depends/README.md`](https://github.com/bitcoin/bitcoin/blob/4c1906a500cacab385b09e780b54271b0addaf4b/depends/README.md), has several issues, such as:

1. The [`g++-multilib`](https://packages.ubuntu.com/noble/g++-multilib) package conflicts with platform-specific cross-compiler packages. This means it is not possible to cross compile for `i686` and other platforms using the same set of installed packages.

2. The [`g++-multilib`]
...

📝 hebasto converted_to_draft a pull request: "depends: Switch from multilib to platform-specific toolchains"
(https://github.com/bitcoin/bitcoin/pull/32162)
Using the multilib GCC toolchain, as currently documented in [`depends/README.md`](https://github.com/bitcoin/bitcoin/blob/4c1906a500cacab385b09e780b54271b0addaf4b/depends/README.md), has several issues, such as:

1. The [`g++-multilib`](https://packages.ubuntu.com/noble/g++-multilib) package conflicts with platform-specific cross-compiler packages. This means it is not possible to cross compile for `i686` and other platforms using the same set of installed packages.

2. The [`g++-multilib`]
...

💬 laanwj commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2019820641)
Thanks!

The receive flow [here](https://www.infradead.org/~tgr/libnl/doc/core.html#core_recv) seems to indicate:
- Process packet
- If `NLM_F_MULTI` is set and the packet is not `NLMSG_DONE`, repeat

What i'm mostly worried about is that the current code will hang if `NLMSG_DONE` never comes, which seems to be the case for non-multi responses, which have one data packet.

But it may be that the `NETLINK_ROUTE` response to `NLM_F_DUMP` is always multi-packet. That empirically seems to be
...

👋 hebasto's pull request is ready for review: "depends: Switch from multilib to platform-specific toolchains"
(https://github.com/bitcoin/bitcoin/pull/32162)

💬 polespinasa commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2019840095)
> Wouldn't it be better to set this to `static_cast<uint32_t>(nHeight +99 );`?
That would make the transaction invalid and unable to be included in a block until the `nHeight + 99` block is not a condition on when it can be spent, but when it is valid (therefore it can be mined).

💬 arejula27 commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2019840538)
True :(

💬 arejula27 commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2019842504)
That's true, I confused it with CLTV.

💬 bigspider commented on pull request "OP_CHECKCONTRACTVERIFY":
(https://github.com/bitcoin/bitcoin/pull/32080#issuecomment-2763808758)
Renamed the `flags` parameter to `mode`, matching the analogous change in the BIP draft.
Rebased to the latest master.

Still trying to locally reproduce the unit test failures.

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019899624)
Sorry, that the builder *doesn't* outlive the graph. I was testing to see what happens when it *does* outlive it, and it's not crashing currently.

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019900371)
feel free to resolve

💬 mabu44 commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019943385)
nit: sub**s**titute

💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968139)
Thanks! Fixed.

💬 Prabhat1308 commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#issuecomment-2764219862)
re-ACK [`56f271e`](https://github.com/bitcoin/bitcoin/pull/32134/commits/56f271e9b9c82f40054d63d4b638584bd2faef00)

💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968258)
commit message still contains `seen_multipath`

💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968705)
this implicit empty constructor was always weird to me, a more explicit and uniform alternative could be:
```suggestion
substitutes = {};
```

💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019970351)
To me `.emplace()` appears more specific. Assigning `{}` seems to make it `std::nullopt`, leading to test failures.

💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019969499)
It also contains:

> Rename it to seen_substitutes to better describe what it does, now that the context implies its involved in multipath.