Bitcoin Core Github
42 subscribers
127K links
Download Telegram
🤔 rkrux reviewed a pull request: "doc: Use multipath descriptors in descriptors.md and linked test"
(https://github.com/bitcoin/bitcoin/pull/34100#pullrequestreview-3593332537)
Concept ACK 912d837326145c304662ca29a01ccd1d240946e8

Multi-path convention is indeed convenient and preferred.
💬 rkrux commented on pull request "doc: Use multipath descriptors in descriptors.md and linked test":
(https://github.com/bitcoin/bitcoin/pull/34100#discussion_r2631352329)
Nit, feel free to ignore but I think reading `/0` instead of just `0` would be more relatable given the context.

```diff
diff --git a/doc/descriptors.md b/doc/descriptors.md
index 6c600af5ba..1b79c99fe6 100644
--- a/doc/descriptors.md
+++ b/doc/descriptors.md
@@ -67,8 +67,8 @@ Output descriptors currently support:
- `wsh(sortedmulti(1,xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/1/0/*,xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3Zu
...
💬 rkrux commented on pull request "doc: Use multipath descriptors in descriptors.md and linked test":
(https://github.com/bitcoin/bitcoin/pull/34100#discussion_r2631357406)
This test fails now, at least in my system:

```zsh
2025-12-18T14:35:06.064047Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main
self.run_test()
~~~~~~~~~~~~~^^
File "/Users/rkrux/projects/bitcoin/./build/test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py", line 95, in run_test
psbt = multisig.walletcreatefundedpsbt(inputs=[],
...
👍 rkrux approved a pull request: "test: address self-announcement"
(https://github.com/bitcoin/bitcoin/pull/34039#pullrequestreview-3593348031)
ACK 1841bf9

Thanks for the co-author attribution.
📝 hebasto opened a pull request: "cmake: Add missed `Boost::headers`"
(https://github.com/bitcoin/bitcoin/pull/34103)
On Fedora 43:
```
$ gmake -C depends NO_LIBEVENT=1 NO_QT=1 NO_ZMQ=1 NO_WALLET=1 NO_USDT=1 NO_IPC=1
$ cmake -B build -DBoost_ROOT=depends/x86_64-pc-linux-gnu --preset dev-mode
<snip>
[363/1092] Building CXX object src/ipc/CMakeFiles/bitcoin_ipc.dir/capnp/mining.cpp.o
FAILED: [code=1] src/ipc/CMakeFiles/bitcoin_ipc.dir/capnp/mining.cpp.o
/usr/bin/ccache /usr/lib64/ccache/c++ -DKJ_USE_FIBERS -I/home/hebasto/dev/bitcoin/build/src -I/home/hebasto/dev/bitcoin/src -I/home/hebasto/dev/bitcoin/bu
...
💬 hebasto commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670657739)
> > Is it a good time to consider bumping of the Boost minimum supported version?
>
> I think there are users out there on Ubuntu:22.04 and Debian Bookworm, so breaking them seems a bit too early?

FWIW, they could workaround using depends. For example, see https://github.com/bitcoin/bitcoin/pull/34103.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3670670860)
I tend to agree with @fanquake. Running the CI locally should be easy and supported. We don't want to end up in a place where the CI is basically just a prayer toward Microsoft/GHA to please run the scripts and to please run them correctly.

I think the open questions are:

* Why does the GHA CI *not* catch the issue seen in local runs?
* Which test is responsible for the issues in local runs, and what is the fix?
💬 l0rinc commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670680873)
regardless of the workaround, I don't mind reverting to make it work with old boost again - we can enable the new clang-tidy rule without them as well.
I have reverted the above changes in https://github.com/bitcoin/bitcoin/pull/34095 - can someone please validate that it fixed the build with older boost?
💬 hebasto commented on pull request "depends: capnp 1.3.0":
(https://github.com/bitcoin/bitcoin/pull/34102#issuecomment-3670687077)
Concept ACK.
💬 l0rinc commented on pull request "refactor: enable `readability-container-contains` clang-tidy rule":
(https://github.com/bitcoin/bitcoin/pull/34095#issuecomment-3670692392)
Reverted the boost changes (already merged and new ones), clang-tidy should still pass.
💬 l0rinc commented on pull request "scripted-diff: [doc] Unify stale copyright headers":
(https://github.com/bitcoin/bitcoin/pull/34084#issuecomment-3670703429)
ACK fa4cb13b52030c2e55c6bea170649ab69d75f758
⚠️ fanquake opened an issue: "p2p: `seed.bitcoin.sipa.be` is down"
(https://github.com/bitcoin/bitcoin/issues/34104)
```bash
./check-dnsseeds.py
* mainnet
FAIL seed.bitcoin.sipa.be
OK dnsseed.bluematt.me (31 results)
OK dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us (35 results)
OK seed.bitcoin.jonasschnelli.ch (24 results)
<snip>
```
💬 fanquake commented on issue "p2p: `seed.bitcoin.sipa.be` is down":
(https://github.com/bitcoin/bitcoin/issues/34104#issuecomment-3670729702)
cc @sipa
👋 fanquake's pull request is ready for review: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095)
💬 theuni commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3670774799)
Post-merge ACK 5f5c1ea01955d277581f6c2acbeb982949088960. Nits aside, both of these are nice cleanups.
💬 fanquake commented on pull request "kernel: revert accidentally removed copyright header":
(https://github.com/bitcoin/bitcoin/pull/34105#issuecomment-3670801358)
ACK 85314dc0bf871226c0e43446bb79f49630d15f4a
🚀 fanquake merged a pull request: "kernel: revert accidentally removed copyright header"
(https://github.com/bitcoin/bitcoin/pull/34105)
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2631512181)
I was assuming that directly calling `ActivateBestChain` wouldn't work in a fuzz test due to certain things not being mocked out, but I wonder if it's possible to instead call it and not have to re-implement it here?
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3670814276)
reACK https://github.com/bitcoin/bitcoin/commit/db2d39f642979f929261e5f1cd67f0c2f2ca045f