✅ fanquake closed a pull request: "Fix legacy migration bug"
(https://github.com/bitcoin/bitcoin/pull/32161)
(https://github.com/bitcoin/bitcoin/pull/32161)
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2762968613)
Seeing this output (for all translations) while Guix building. Is it an issue:
```bash
[ 52%] Automatic MOC and UIC for target bitcoinqt
AutoUic: Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, but has failed to switch to one.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
<snip>
[ 52%] Generating locale/bitcoin_fi.qm
Detected locale "C" with character encoding "ANSI_X3.4-19
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2762968613)
Seeing this output (for all translations) while Guix building. Is it an issue:
```bash
[ 52%] Automatic MOC and UIC for target bitcoinqt
AutoUic: Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, but has failed to switch to one.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
<snip>
[ 52%] Generating locale/bitcoin_fi.qm
Detected locale "C" with character encoding "ANSI_X3.4-19
...
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019656793)
Yeah, I think I considered an on-the-fly heap at some point, and it's probable that that actually has overall lower runtime costs than this (due do better memory locality and less allocation overhead than a tree-structured set), but being to do the bulk of the work ahead of time is nice.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019656793)
Yeah, I think I considered an on-the-fly heap at some point, and it's probable that that actually has overall lower runtime costs than this (due do better memory locality and less allocation overhead than a tree-structured set), but being to do the bulk of the work ahead of time is nice.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2763012132)
Guix Build (x86_64):
```bash
6ea4a76be3383337e57d6a12450bd589776ebb3fd0d9161347766ef845241e13 guix-build-c4861570e468/output/aarch64-linux-gnu/SHA256SUMS.part
3eb7656483dfe47fa6b7cf40bceb3decda73474c813edb224d42840adb8b49d6 guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu-debug.tar.gz
0aa522010efd138d78eeac0a8ea15df469298c50afaa2451dece78c564546cac guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu.tar.gz
5e1e833
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2763012132)
Guix Build (x86_64):
```bash
6ea4a76be3383337e57d6a12450bd589776ebb3fd0d9161347766ef845241e13 guix-build-c4861570e468/output/aarch64-linux-gnu/SHA256SUMS.part
3eb7656483dfe47fa6b7cf40bceb3decda73474c813edb224d42840adb8b49d6 guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu-debug.tar.gz
0aa522010efd138d78eeac0a8ea15df469298c50afaa2451dece78c564546cac guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu.tar.gz
5e1e833
...
💬 ajtowns commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019711805)
Thinking some more about this... 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 (if they get rbf'ed, or conflicted by a block tx, or if they expire, or if they get cpfp'ed and thus brought to the front of the mempool, rather than just waiting until the best block feerate drops to their level, or you stop and restart your bitcoind before they get mined). Probably few enough txs hit those special
...
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019711805)
Thinking some more about this... 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 (if they get rbf'ed, or conflicted by a block tx, or if they expire, or if they get cpfp'ed and thus brought to the front of the mempool, rather than just waiting until the best block feerate drops to their level, or you stop and restart your bitcoind before they get mined). Probably few enough txs hit those special
...
📝 fanquake locked a pull request: "5e8bc97cdc6dd8d7be10f0ac8c1b46d2c2fd1547"
(https://github.com/bitcoin/bitcoin/pull/32127)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/32127)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
📝 fanquake locked a pull request: "Fix broken link"
(https://github.com/bitcoin/bitcoin/pull/32146)
Fix broken link
(https://github.com/bitcoin/bitcoin/pull/32146)
Fix broken link
💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2763240357)
> Seeing this output (for all translations) while Guix building. Is it an issue:
Likely a spurious warning, the translations appear to be compiled fine nevertheless. But to be 100% sure it doesn't make a difference, we could try to install the UTF-8 locale inside guix and see if it makes any difference to the output.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2763240357)
> Seeing this output (for all translations) while Guix building. Is it an issue:
Likely a spurious warning, the translations appear to be compiled fine nevertheless. But to be 100% sure it doesn't make a difference, we could try to install the UTF-8 locale inside guix and see if it makes any difference to the output.
💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019761226)
> Do you mean the lld instruction?
No, I mean we already have
```bash
brew install llvm lld
```
a few lines below, I don't see the point of announcing the same in the doc a few paragraphs before, too.
> keep the instruction here as well and maybe adapt the lld part
What would be the advantage of sprinkling all the dependencies throughout multiple separate brew commands?
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019761226)
> Do you mean the lld instruction?
No, I mean we already have
```bash
brew install llvm lld
```
a few lines below, I don't see the point of announcing the same in the doc a few paragraphs before, too.
> keep the instruction here as well and maybe adapt the lld part
What would be the advantage of sprinkling all the dependencies throughout multiple separate brew commands?
💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019761404)
That limit likely caused us to miss the typo (splitting between related words), it's why I have also reordered it minimally to be structured by meaning instead.
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019761404)
That limit likely caused us to miss the typo (splitting between related words), it's why I have also reordered it minimally to be structured by meaning instead.
💬 willcl-ark 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_r2019761455)
I don't think it is, but like you am unsure about what the guarantees of the protocol are here.
I kind of reverse engineered this looking at miniupnpc and netlink sources, along with an `strace` of `ip route show`, which is where the repeated `recv` calls jumped out to me as a difference between our code and other tools.
This approach simply relies on receiving an `NLMSG_DONE` to signal the end of the response and break. This should handle both single and multi-part messages. Here's a snip
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2019761455)
I don't think it is, but like you am unsure about what the guarantees of the protocol are here.
I kind of reverse engineered this looking at miniupnpc and netlink sources, along with an `strace` of `ip route show`, which is where the repeated `recv` calls jumped out to me as a difference between our code and other tools.
This approach simply relies on receiving an `NLMSG_DONE` to signal the end of the response and break. This should handle both single and multi-part messages. Here's a snip
...
🚀 hebasto merged a pull request: "cmake: Avoid fuzzer "multiple definition of `main'" errors"
(https://github.com/bitcoin/bitcoin/pull/31992)
(https://github.com/bitcoin/bitcoin/pull/31992)
💬 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.)
(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!
(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).
(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
...
(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`]
...
(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`]
...
(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
...
(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)
(https://github.com/bitcoin/bitcoin/pull/32162)