Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 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.
💬 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.
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019971150)
ok, if that's deliberate, please resolve this
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019971282)
My mistake, thanks for checking
👍 l0rinc approved a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134#pullrequestreview-2727715824)
utACK 56f271e9b9c82f40054d63d4b638584bd2faef00