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. Here’s what I found notable.
Merged PR’s
Every week, several changes are officially added to Bitcoin Core. This week, 17 changes were merged. Here are some I thought were interesting from this week.
Do you remember AssumeUTXO? This change is an attempt to make AssumeUTXO more maintainable. Ofsky does this by not locking cs_main for a long time when the snapshot block is connected. cs_main isa mutex that locks up a resource for the rest of the application, minimizing the number of times we need to hold it, is an improvement.
Ofsky’s change is only a refactor, so there is no actual logic being modified, and in this change, he modifies the language around AssumeUTXO to make it less confusing. Overall, this change makes it easier for developers to maintain AssumeUTXO.
Again, another refactor, this time L0rinc saw that there is this function contains In C++20, this lets us find a value in an array or list in a more succinct manner than we currently do.
It makes sense to use a built-in library rather than a bunch of other variants. This change makes the code more readable and consistent in the Bitcoin Core codebase.
Be sure to read the last few editions, and you’ll understand why this change just makes sense. In the last few editions, there were changes to the logs to use LogInfo, LogWarning, and LogError. So now there is really no use for LogPrintLevel since it is easy to break and a bit confusing to understand how it works.
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.
Exploit modern simd to calculate 2/4/6/8/16 states at a time depending on the size of the input.
This demonstrates a 2x speedup on x86-64 and 3x for arm+neon. Platforms which require runtime detection (avx2/avx512) improve performance even further, and will come as a follow-up.
Rather than hand-writing assembly or using arch-specific intrinsics, this is written using compiler built-ins understood by gcc and clang.
IRC meeting notes
Every week on Thursday, there is an IRC meeting. Here are some short notes from that meeting.
Fuzzing WG Update (dergoegge)
dergoegge: There are a bunch of updates on fuzzamoto, I made some notes here:
IR extended to cover addr relay, bloom filters, compact blocks, taproot (still a PR).
Probing mutators: conceptually similar to cmp hooking that generic byte array fuzzer do, but we observe node state instead of comparisons to enable smarter mutations
Maybe the most relevant to you all, is that hopefully soon you can write fuzzamoto tests using the functional test framework
That’s probably it, unless someone else also has updates on something fuzzing related
Kernel WG Update (sedited)
sedited: stickies-v has been working on a spec for a kernel logger: https://github.com/bitcoin/bitcoin/issues/34062 . His most recent approach for that splits the existing logger into a new util::log that implements string formatting and the existing higher level functions for buffering, writing to file, etc.
We’ve also been discussing a new non-locking chain data structure as a replacement/extension of CChain that uses copy-on-write.
The end goal is to have a chain that does not require locking to read. A similar approach could be taken for the block tree map too.
Also been experimenting a bit with introducing more kernel header api-based utilities to better dog food it.
l0rinc_: is it a logarithmic COW? (never thought I’ll write this sentence)
sedited: l0rinc_ something like that is the aim
Benchmarking WG Update (l0rinc, andrewtoth)
l0rinc_: I’m mentioning PRs that are tangentially related to benchmarking and IBD performance to help with their visibility (not necessarily because I was involved).
Raimo pushed #34044 which was meant to speed up `-reindex` a bit, but based on the usage it turned out to be a dead end.
Cory opened #34083 - a vectorized ChaCha implementation, resulting in 2-3x speedup - no IBD benchmarks yet.
Rob experimented with different ways to store the hints for #34004 and addressed a few review concerns.
#32414 was merged recently, so reindex-chainstate benchmarks will behave slightly different now - especially for bigger dbcaches.
Most of our low-end benchmarking servers were offline this week for maintenance, but on the remaining ones we have experimented with: benchmarking #31132 in a cloud environment with network-connected storage; since we need a lot less memory now for the same performance after #31132, we tried to see how a sorted map would perform instead of SipHashed std::unordered_map - it would be 26-60% slower; we tried turning off cache reallocation during IBD flushes - seems to result in ~4% speedup; wanted to see whether the speedup introduced by pooled allocation is still relevant - removing it would still slow validation by 8-12%; next step is to see if we can fine-tune it instead; currently measuring how much difference a SipHash-1-3 would make compared to the current SipHash-2-4; I have finally rented a Windows benchmarking server, so we might soon see how well it performs there.
https://github.com/bitcoin/bips/pull/2055 was opened to limit the number of SP recipients to scan for to `(2^32) - 1` to allow devs set appropriate datatypes in their implementation.
There is a proposed sp descriptor in https://groups.google.com/g/bitcoindev/c/bP6ktUyCOJI that introduces two new key types for silent payments (”spscan1q...” and “spspend1q”) and looks something like “sp([deadbeef/352’/0’/0’]spscan1q...,900000,1,5,10)”.
pinheadmz: this libevent issue brought to my attention by darosior
fjahr: To me this doesn’t really change much. I didn’t expect we would be seeing a 2.2 release before we remove it no matter how long the process takes. Now we might see a 2.2 but won’t want to upgrade to it anyway :p
pinheadmz: they are looking for a new maintainer, which makes sense since the lib hasnt cut a release this decade.
fjahr: Not sure if it had even been worth the effort trying to leverage 2.2 if it came out today even without the new concerns, given the work already going into the httpserver.
pinheadmz: fjahr agreed just wanted to bring to attention, the risk this deependency poses is ticking up slightly
and next pr for review is #32061, which i recently refactored to address concerns broughtup at coredev by cfields. so now, there is no sockman “lite” or otherwise, httpserver does its own sockets. im rebasing again today to fix conflicts with master then ill run some more benchmarks as well as the integration tests with various RPC consuming libraries like electrs
