This Week in Bitcoin Core #45
This week Bitcoin developers are working as normal...
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.
Merged PR’s
Every week, several changes are officially added to Bitcoin Core. This week, 25 changes were merged. Here are some I found interesting this week.
refactor: disentangle miner startup defaults from runtime options by Sjors
Sjors isolates the miner’s initial startup configuration from its ongoing runtime options.The Issue: The block template builder was heavily tangled up with global node state. This made writing clean tests or reasoning about the code a pain.
The Fix: Pulls those startup defaults out into an independent structure so it can be tested in isolation.
The Big Picture: This is the structural groundwork for the Multiprocess (IPC) mining interface. By separating these options, a standalone mining process will eventually be able to run cleanly on its own without needing to read deep into
bitcoind‘s internal memory state.
wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively by achow101
In this PR, Ava Chow tackles critical technical debt by explicitly documenting and enforcing the threading and locking rules for the validation interface. This interface handles notifications like blocks connecting, disconnecting, or transactions entering the mempool but previously its concurrency requirements were ambiguous, leaving the door open for potential data races or deadlocks.
By formalizing these thread safety guarantees and adding strict assertions directly into the code, this update prevents future developers from introducing subtle concurrency bugs. It is a stability and code-quality improvement, making sure the node’s background notification system remains robust as Core continues to move toward a more modular architecture.
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.
refactor: Add util::Result failure types and ability to merge result values by ryanofsky
Add `util::Result` support for returning more error information and make use of it in LoadChainstate method as an initial application. Followup PRs #25722 and #29700 use it more broadly to return errors and warnings from wallet and kernel functions as well..
IRC meeting notes
Every week on Thursday, there is an IRC meeting. Here are some short notes from that meeting.
stickies-v: #topic Fuzzing WG Update (dergoegge)
dergoegge: no updatestickies-v: #topic Kernel WG Update (sedited)
sedited: Some review activity happening on #32427 again, and looking for more.
sedited: I rebased my branch for splitting the mempool/policy code from validation (and thus from the kernel library): https://github.com/sedited/bitcoin/tree/mempoolout_rebase
sedited: Still not happy with the code for the split though, the interface for accesing back into the mempool and grabing its locks is very clunky.
stickies-v: nice, will get back to reviewing 32427 asap, would be a nice improvement for kernel
sedited: not sure how interested people are in this split in the first place. The code-organizational benefits are kind of nice, but beyond that the utility for the Bitcoin Core software are a bit questionable.
darosior: "Mempool eviction? -Yes but the one you are thinking about"
darosior: not*
sedited: indeed :)
darosior: If it's a win for the Kernel library, and Bitcoin Core is no worse off, that seems fine?
stickies-v: yeah i think it's a pretty big improvement for kernel users
sedited: I might open it as a RFC PR , keeping it rebased is a challenge, but it's also kind of nice to ensure on a regular basis that such a split is possible in the first place.
stickies-v: an RFC issue might make more sense?
sedited: not sure, I think most would agree that it is a good thing on the face of it, so I more think it's the refactoring price that needs to be gauged here.
stickies-v: linking a branch from an issue seems fine, but then at least you're not wasting time rebasing / addressing nits
stickies-v: anyway
stickies-v: anything else here?
sedited: that's allstickies-v: #topic Benchmarking WG Update (l0rinc, andrewtoth)
andrewtoth_: l0rinc is out but he said he is working on an automated compaction PR
andrewtoth_: I relayed darosior's idea to use minimum chain work as a trigger
andrewtoth_: I also have come around to thinking a ~weekly automated compaction would be good
sedited: what would be wrong with just repeatedly checking isibd?
andrewtoth_: that would trigger every startup too, which would not be ideal
sipa: andrewtoth_: 1 week worth of blocks is mayhe 2 GB of block data, a full rewrite every week would add a multiple of that
sipa: or do we necessarily already have much morr
andrewtoth_: sipa: the chainstate adds < 200MB a week to the db if uncompacted
andrewtoth_: err I'm not sure how you mean block data to be relevant here?
sipa: yeah, i'm just trying to gauge how much a weekly full rewrite of the chainstate would add in terms of disk activity
darosior: For disk IO i think?
andrewtoth_: I think today it will be like 10-15 GB of write IO per full compaction
sipa: yes, which is a multiple of what i expect we'd do without
Murch[m]: Why once per week rather than say, once per month?
andrewtoth_: we can definitely bikeshed on how often to do it. could be monthly
stickies-v: what about thriweekly
sipa: rewrite at the end of IBD, once, is more impactful i think
darosior: stickies-v: hah
sipa: stickies-v: whenever the last block hash ends in 12 zero bits
sedited: ^^
andrewtoth_: yes agreed end of IBD is important. but, saving users an extra 11% for a full compaction every so often seems worth it?
Murch[m]: sipa: Probably better if not everyone does it at the same height
b10c: murch: agree
darosior: Once a month seems fine, and then no need to do it at the end of IBD
sipa: yeah, sorry, i didn't mean to start a bikeshed here... i'm just not very convinced doing it regularly adds much, and probably adds a relatively large amount of disk activity still
sipa: Murch[m]: absolutely, it was a joke :)
andrewtoth_: i think post-IBD alone is important, since a user might shut off and never keep it running for a whole month after that
Murch[m]: E.g., do it once after IBD finishes, then schedule it for a random height 3500–5000 blocks later again
Murch[m]: Would probably be bad if all nodes did it at the same height
andrewtoth_: Murch: I think that's nearly the approach l0rinc is taking now
darosior: andrewtoth_: you would advise doing both? I was hoping for either or
Murch[m]: From reading the topic last week, my impression was that the db is bloated by several GB at the end of a full IBD
Murch[m]: So, tracking that you started a new IBD and doing it once when you have caught up to the chaintip sounds pretty useful?
andrewtoth_: i was thinking both, yes...
darosior: If we do it once at the end of IBD, how much do you expect it would help to do it regularly on top? Like if it's done once around block 950k, how much disk space would doing it around block 1M would really save?
andrewtoth_: if I had to choose one, it would be post-IBD
andrewtoth_: 11% of disk space
andrewtoth_: we save ~30% doing it immediately after IBD, then it's a slow creep up to 11% total
andrewtoth_: a periodic compaction will trim that 11%
Murch[m]: and then ~200 MB per week, roughly?
andrewtoth_: Murch: up until 11%, it gets capped
darosior: Ok. No opinion here. I don't think it matters much either way.
andrewtoth_: I feel like post-IBD can be the initial change, then we can bikeshed more on periodic compaction.
Murch[m]: Well, every week seems a bit aggressive, but either way it would be a huge improvement over doing it every hour
andrewtoth_: ok, thanks for the feedback everyone
andrewtoth_: otherwise, got some great review on #35295, thanks! will address that soon
andrewtoth_: that's it from mestickies-v: #topic Libevent removal (pinheadmz, fjahr)
pinheadmz: #35182 is real close. addresed some nits yesterday and restarted integration testing. already have a few small follow-ups in the queue.
pinheadmz: And libevent removal from client was merged yay
pinheadmz: Hoping to merge with enough time before release to find anything wrong
pinheadmz: All from mestickies-v: #topic QML GUI WG Update (johnny9dev)
johnny9dev: We continue to be on track for our “Preview” unsigned builds in June . The related work for that can be found at https://github.com/orgs/bitcoin-core/projects/1/views/2 and I updated it this morning so it is accurate. We have wallet updates from pseudoramdom and epicleafies and node error dialogs and block status updates from me in review. Afterwards the high runners are updating the project to v31 and validating the settings and
johnny9dev: onboarding is compatible with bitcoin-qt installs.
johnny9dev: That’s all from me
stickies-v: alright, thanks for the updates everyone
stickies-v: anything else to discuss?
abubakarsadiq: novo asked me to relay silent payments update
instagibbs: IIUC point release for 31.x is held up on #35319, so let’s try and get that done?
abubakarsadiq: https://github.com/bitcoin-core/secp256k1/pull/1765 has to be merged before we can merge silent payments on Core, but reviews are welcome on https://github.com/bitcoin/bitcoin/pull/35301 and https://github.com/bitcoin/bitcoin/pull/35302.
stickies-v: yeah good shout out instagibbs
Murch[m]: One follow-up to the compaction: if the mechanism is to schedule it for specific heights, you could schedule the first compaction for a height slightly after the chaintip of the best header chain once the node has acquired one, but delay the compaction until we are no longer in IBD.
Murch[m]: Or maybe just scheduling it for the regular ~x thousand block interval but waiting until you are no longer in IBD would be enough to do one after IBD
eugenesiegel: I tried making a functional test that built on top of p2p_private_broadcast.py, but it did not go well. I had issues with the Socks5 proxy and reconnection code, hopefully vasild has a better approach
instagibbs: vasild any ETA on your version of the test, even with cleanups TODO
andrewtoth_: Murch: we need to make sure we only trigger it one time, so using a gauge like previous work was < minimum now work is > minimum could work for that
andrewtoth_: otherwise it will trigger every startup
stickies-v: will wrap up the meeting here, but conversations can of course continueRead here for the full meeting
Releases
No releases this week
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