Hello ๐ folks, Iโm Kevkevin. Iโm an open-source developer and reporter for Insider Edition. Last week I reviewed several pull requests from the Bitcoin Core repo. One merge this week puts the functional-test dependencies in one table in test/README.md. pip, apt, brew, winget, pkg, pkgin, and pkg_add each get a row. The columns are Python, sqlite3, zmq, capnp, and lsof. Under pip the ZMQ module is pyzmq. Under apt it is python3-zmq. The FreeBSD, NetBSD, OpenBSD, macOS, MSVC Windows, and Unix build notes point at that table.
Another merge changes what Assert, Assume, and CHECK_NONFATAL print when the condition is false. The old line was the condition by itself, so pindex != nullptr read as if that were the state the node had observed. StrFormatFailedCheck wraps it. The report is `pindex != nullptr` check failed.
Thursdayโs #bitcoin-core-dev meeting took the fuzzing and QML working groups, a second Windows code signer, and a stateless rewrite of private broadcast. v32.0 is tagged. The final binaries are not in the download directory.
Merged PRโs
Every week, several changes are officially added to Bitcoin Core. This week, 40 changes were merged. Here are some I found interesting this week.
The per-OS test-suite install snippets in the FreeBSD, NetBSD, OpenBSD, macOS, and MSVC Windows build notes are gone. Each file now links to one table in test/README.md, and doc/build-unix.md gets the same link. The table has a row for each package manager (pip, apt, brew, winget, pkg, pkgin, and pkg_add) and the package name for Python, sqlite3, zmq, capnp, and lsof. Under pip the ZMQ module is pyzmq. Under apt it is python3-zmq.
If an optional dependency is not installed, the tests that need it are skipped, not failed. The from-source pycapnp recipe is gone, and the pip row is the named pycapnp.
A message of Internal bug detected: pindex != nullptr does not say whether that comparison held. StrFormatFailedCheck now wraps the condition, so a failed Assert, Assume, or CHECK_NONFATAL reports `pindex != nullptr` check failed.
NONFATAL_UNREACHABLE now honors the same abort flag as CHECK_NONFATAL. Both go through the new internal_non_fatal_helper, which aborts if G_ABORT_ON_FAILED_ASSUME is set (a debug build) and otherwise throws NonFatalCheckError. util_check_tests, util_not_null_tests, and rpc_misc.py expect the new text.
There are always changes being updated and reviewed in real-time. Here are some notable PRโs that are still up and looking for reviews.
Private broadcast on master drives its attempts from node state, and that has already produced a series of careful fixes. This PR replaces it with jobs that do not consult addrman, the mempool, or -onlynet. A job is one broadcast of one transaction. It resolves DNS seed names through Tor, then delivers over BIP324 v2 connections on its own proxy streams, which do not show up in getpeerinfo.
The same job runs inside bitcoind -privatebroadcast or in the standalone bitcoin-privbcast. Part 4 adds one-parent-one-child package relay. The logic is about 4,600 net lines outside consensus, with the rest tests and documentation. He is asking for a concept ACK and a decision on whether parts 1 through 3 are the v33 goal.
The primary failure mode of the in-master private broadcast is that it relies on node state for operation, and this has led to a number of โcareful codingโ fixes.
IRC meeting notes
Every week on Thursday, there is an IRC meeting. Here are some short notes from that meeting.
--- Topic 1 ---
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: #topic Fuzzing WG Update (eugenesiegel, marcofleon)
๐บ๐ฎ๐ฟ๐ฐ๐ผ๐ณ๐น๐ฒ๐ผ๐ป: no update from me
๐ฒ๐๐ด๐ฒ๐ป๐ฒ๐๐ถ๐ฒ๐ด๐ฒ๐น: currently just working on getting bedrock into shape so we can run fuzzamoto with it. some faults I added found some really minor database consistency issues
๐ฒ๐๐ด๐ฒ๐ป๐ฒ๐๐ถ๐ฒ๐ด๐ฒ๐น: like e.g. a node having a power loss at a super specific point during -reindex prevents the node from starting up again. should things like that be fixed?
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: if it's a loud failure, i wouldn't personally consider that worth spending a lot of energy or complexity on
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: silent consensus failures/money losing behaviour is a different thing of course
๐๐ฎ๐ป๐ฐ๐: might it also be OS disk corruption?
๐ฒ๐๐ด๐ฒ๐ป๐ฒ๐๐ถ๐ฒ๐ด๐ฒ๐น: I don't think it's a disk corruption issue, just things not being persisted in the proper order. It wasn't clear to me what guarantees the codebase should have when there's no consensus failure or money loss
๐ฏ๐ฟ๐๐ป๐ผ๐ฒ๐ฟ๐ด: perhaps it depends on how complex the fix would be?
๐ฒ๐๐ด๐ฒ๐ป๐ฒ๐๐ถ๐ฒ๐ด๐ฒ๐น: I think the fixes are pretty small. I don't think it's likely that anybody will run into these issues though
--- Topic 2 ---
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: #topic QML GUI WG Update (johnny9dev)
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: We're working through the feedback from last week
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: We've got a PR to clarify storage choices during onboarding
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: Priority is getting the staging branch ready
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: I am increasingly interested in making a pruned wallet setup work and have started looking into txindex and compact block filters
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: You mean pruned rescans?
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: Yeah, pruned rescans. Block retrieval. I think that becomes a high priority for the QML project once the staging branch is settled
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: eugenesiegel: worth at least opening an issue for those. May be a symptom of a bug that could open up more serious inconsistencies. As far as I know we should always be able to recover from a crash
๐ ๐๐ฟ๐ฐ๐ต[๐บ]: One argument is that a change this large should merge very early in a release cycle, right after branch-off, so the new GUI would only be considered for v34. The new GUI has better testing and more eyes, and I'm uncomfortable releasing the old GUI while hardly anyone is looking at it. Maybe we should talk about getting it into v33
๐ต๐ฒ๐ฏ๐ฎ๐๐๐ผ: it seems premature until the staging branch is ready
๐ท๐ผ๐ต๐ป๐ป๐๐ต๐ฑ๐ฒ๐: My current view is that we'll have the PR by the end of the year. Likely end of November or early December
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: I'm not sure merging a huge amount of new code is necessarily safer, so I don't think we should feel time pressure because of that
๐ ๐๐ฟ๐ฐ๐ต[๐บ]: Sure, let's revisit when the staging branch is available
๐ต๐ฒ๐ฏ๐ฎ๐๐๐ผ: agree
--- Topic 3 ---
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: #topic QA WG Update (brunoerg)
๐ฏ๐ฟ๐๐ป๐ผ๐ฒ๐ฟ๐ด: no updates this week
--- Topic 4 ---
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: #topic another windows code signer (achow101)
๐ฎ๐ฐ๐ต๐ผ๐๐ญ๐ฌ๐ญ: Windows code signing is done by only me, which is a single point of failure. If anyone is interested in being a code signer, message me
๐ฎ๐ฐ๐ต๐ผ๐๐ญ๐ฌ๐ญ: Needs to be someone who currently builds, or will commit to building, all releases and release candidates shortly after tagging
--- Topic 5 ---
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: #topic Reimplement private broadcast as node state-less jobs + package relay support (#36608) (instagibbs)
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: I've been working on a node state-less variant of private broadcast. I think it's mature enough to be considered as a replacement for v33. It makes some opinionated decisions so the privacy properties are easier to reason about
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: It's a feature swap on the same sendrawtransaction path, with 1p1c package relay support, plus a standalone tool that is a thin wrapper around the same job
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: Roughly +4,600 net lines of logic, all outside consensus. The rest is tests and documentation. In-master net_processing private broadcast goes away
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: Short term I'm looking for concept and approach ACKs, and a general buy-in that we want this for v33. Also: do we want the standalone tool, or should it fold into bitcoin-util? And should package relay be a follow-up PR?
๐ฎ๐ป๐ฑ๐ฟ๐ฒ๐๐๐ผ๐๐ต_: I haven't looked in depth, but I'm not sure a 17k line PR is realistic for v33. Conceptual feedback on the previous branch doesn't look addressed
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: I responded to those comments. It's ~4.6k lines net of logic. The rest is tests
๐ฐ๐ณ๐ถ๐ฒ๐น๐ฑ๐: This overlaps my multiprocess p2p work. I'll take a look
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: This is a big lift for a single feature, but it's more attractive if it comes with a better architecture
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: Maybe it's two releases, but I don't think it has to be. I think it's a removal of risk
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: I agree with the approach: fix the node-state leaks structurally, then consider carving out exceptions for UX, instead of the opposite
๐ฎ๐ป๐ฑ๐ฟ๐ฒ๐๐๐ผ๐๐ต_: Can we quantify the remaining risks? There were several private-broadcast fixes in v32. What gaps are left, given it's experimental and the guarantees were softened?
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: The risk is sharing node state, which leaks in dozens of ways. The part to debate is the addrman-less design. addrman is heavily attacker-influenced, and there are tradeoffs
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: We can't guarantee perfect privacy. If private broadcast is supposed to provide any limited guarantee against an adversary, separating it from node state is a requirement
๐ฎ๐ป๐ฑ๐ฟ๐ฒ๐๐๐ผ๐๐ต_: Private broadcast already protects a user's privacy against linkinglion. Default broadcast exposes the transaction origin even with BIP324
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: Current private broadcast can expose origin too, if someone cares enough to attack. I don't think we should offer a feature branded as privacy that doesn't hold up to that bar
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: We can continue off-channel
๐ฑ๐ฎ๐ฟ๐ผ๐๐ถ๐ผ๐ฟ: On the tool: I don't see why we need a new binary. bitcoin-util is fine. A separate tool is a small cost and a useful canary against reintroducing node state. On the split: package relay can be a follow-up, but the swap itself should not be split, or master ends up with two implementations
๐ถ๐ป๐๐๐ฎ๐ด๐ถ๐ฏ๐ฏ๐: I started with the tool so I wouldn't re-add node state to the jobs, then reused that library inside the node
๐๐๐ถ๐ฐ๐ธ๐ถ๐ฒ๐-๐: agreed with darosior on not splitting the swap
--- Topic 6 ---
๐ณ๐ฎ๐ป๐พ๐๐ฎ๐ธ๐ฒ: v32.0 has been tagged
๐ณ๐ฎ๐ป๐พ๐๐ฎ๐ธ๐ฒ: The security policy update is merged: https://github.com/bitcoin-core/bitcoincore.org/pull/1281
๐ณ๐ฎ๐ป๐พ๐๐ฎ๐ธ๐ฒ: PR to enrol Core in the Anthropic OSS scanner: https://github.com/anthropics/oss-scanner/pull/1018
Releases
v32.0is tagged: github.com/bitcoin/bitcoin/releases/tag/v32.0 (tagged by sedited). The final binaries are not published yet.
Thank you for reading. Be sure to tune in again next week for your updates on Bitcoin Core!
If there are any comments, suggestions, or errors, do not hesitate to reach out or comment.