Libevent removal next release and BnB rewrite? - This Week in Bitcoin Core #46
Delete Libevent and make coin selection faster...
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.
This week in Bitcoin Core, there was a discussion in the IRC weekly meeting of attempting to remove the Libevent dependency in favor of our own HTTP and socket-handling implementation in Bitcoin Core. Also, this week, Murch had his PR merged, which rewrote the BnB coin selection algorithm.
Merged PR’s
Every week, several changes are officially added to Bitcoin Core. This week, multiple changes were merged. Here are some I found interesting this week.
test: Add test on skip heights in CBlockIndex by optout21
Optout21 authored and had his PR merged which added additional test coverage for the internal functions GetSkipHeight() and CBlockIndex::BuildSkip().
He found that the skip logics and these functions were not well documented, so adding test coverage would firstly add coverage, but also document how they are intended to function. As always, additional test coverage is always welcomed to the coverage as it helps reduce issues in the future.coinselection: Optimize BnB exploration by murchandamus
This week, Murch had his PR merged, which rewrote the BnB coin selection algorithm. He created this to skip the duplicate evaluation of previously visited input selections.
This update is a pure optimization of the algorithm to reduce visiting inputs more than we needed to, and also, as fewer nodes are visited, this approach would enumerate more possible combinations than the original implementation, given the same limit for iterations.
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](https://github.com/bitcoin/bitcoin/pull/25665/commits/d0d2535cac193a448395fbb1d32df072b2e491a3) as an initial application. Followup PRs [#25722](https://github.com/bitcoin/bitcoin/pull/25722) and [#29700](https://github.com/bitcoin/bitcoin/pull/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: #32427 has been getting a bunch of review over the past week and has accrued a few ACKs
sedited: For a change that will break backwards compatibility and completely changes one of our persistence layers, I think it requires still more buy-in.
willcl-ark: yes would be nice to see more Concept ACK-ers (and others!) review that one
sedited: So I’d like to invite more people to come review it.
stickies-v: anything else?
sedited: that’s allstickies-v: #topic Benchmarking WG Update (l0rinc, andrewtoth)
l0rinc_: I pushed a tiny optimization to https://github.com/bitcoin-core/secp256k1/pull/1859 resulting in up to ~12% signature validation speedup by force-inlining int128 mul/sqr helpers.
l0rinc_: Reproducers and Windows/MSVC experts are very welcome.
l0rinc_: adndrewtoth’s https://github.com/bitcoin-core/leveldb-subtree/pull/61 was also merged and cherry-picked to previous releases.
l0rinc_: A followup was just pushed to #35465 - it does the compaction at the assumevalid boundary to guarantee uniqueness. It’s similar to darosior’s suggestion last week, just easier to implement and explain.
l0rinc_: that’s it from mestickies-v: #topic QML GUI WG Update (johnny9dev)
johnny9dev: Nothing major to report. Making a lot of progress with 4 actively contributing now
johnny9dev: only a few more items to get merged before we want to start getting feedback
johnny9dev: https://github.com/orgs/bitcoin-core/projects/1/views/2
johnny9dev: still on track to have unsigned builds before the end of June
johnny9dev: that is all for nowstickies-v: #topic Libevent removal (pinheadmz, fjahr)
fjahr: pinheadmz isn’t here today but full focus is on review of #35182. IMO it’s getting very close and it would be great if we can get it in early in the release cycle.
fjahr: That’s it.
stickies-v: Anything else to discuss?
sedited: Might talk about #35465?
darosior: I don’t think compacting is particularly important in the first place, so take my nit with a grain of salt, but using assumevalid instead of minchainwork means that people actually validating the historical chain won’t benefit from the compaction?
instagibbs: that was my concern too
darosior: From a quick skim through the commits, it seems that this check would never be true for people running with -assumevalid=0 https://github.com/bitcoin/bitcoin/pull/35465/changes/dbf076273476d121a6d04c8a05dfcc5492e68b98#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R3418
l0rinc_: the compaction doesn’t necessarily result in a measurable speedup - except maybe smaller files, so a few more can fit into the OS memory. But without compaction (especially on HDD) we were about ~15% faster
sipa: hi
l0rinc_: “it seems that this check would never be true for people running with -assumevalid=0” - good point, it’s why originally this was triggered by getting out of IBD mode. I’ll find something, thanks for the hint.
stickies-v: looks like we’re done here? thanks for the conversation, folks
l0rinc_: I want to avoid on events that can happen multiple times, e.g. when someone starts their node weekly - we don’t want every one of these resulting in a compactionRead 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