#35041 descriptor: speed-up `Parse` (xpub/xpriv) in ~30%
https://github.com/bitcoin/bitcoin/pull/35041 · · +67/-28 in 5 files, 2 commits · labels: Descriptors
Goal
- Speed up descriptor parsing for extended keys and checksum calculations
- Accelerates wallet fuzzing harnesses and parsing of complex descriptor setups
This PR optimizes descriptor parsing by avoiding redundant base58 decoding for extended keys and replacing a linear charset search in checksum calculation with a lookup table. It introduces DecodeExtKeyOrPubKey to parse both xpub and xpriv keys in a single base58 decode pass and transitions DecodeBase58 routines to accept std::string_view.
Problem: ParsePubkeyInner unconditionally called both DecodeExtKey and DecodeExtPubKey, executing two separate base58 decoding and SHA256 passes per key. Additionally, DescriptorChecksum performed linear scans over its input character set, slowing down descriptor parsing and wallet fuzzing harnesses.
Category: Wallet (#49 of 84)
P3 · speedup
- P3 because it yields a verified 30% to 36% speedup when parsing extended keys
- Benefits wallet fuzz targets and complex multisig descriptor setups
- Descriptor parsing is rarely a primary bottleneck for standard wallet users
P3 because it delivers a verified 30-36% performance improvement for parsing descriptors containing extended keys. As hodlinator noted, 'author of this PR motivates it through the concrete second-order effect of speeding up fuzz targets.' While useful for fuzzing and complex multisig descriptor setups, descriptor parsing is rarely a primary bottleneck for wallet users, making it worthwhile but deferrable.
Membership: Touches descriptor parsing and key decoding logic in src/script/descriptor.cpp and src/key_io.cpp under Descriptors label.
Factors: security/stability 0, bug 0, performance 2, user value 1, leverage 1
Reviewability: Ready
- Ready for review: clean state, passing CI, and material feedback addressed
The code is in a clean state, CI passes, and all material review comments have been resolved.
Author status: Active; addressed review comments and force-pushed updates
Resolved concerns:
- Whether base58 optimization approaches previously rejected applied here; hodlinator noted this operates at a cleaner higher-level abstraction and addresses fuzzing performance
- Use of std::string_view to clean up call sites and avoid string copies, addressed by author
- Lookup table type size and constexpr static qualifiers raised by hodlinator, mostly resolved in subsequent force push
Agreement: Strong
- Strong support with independent benchmarks confirming a 37% speedup (hodlinator)
- Concept approval without stated reasons (sedited)
- Implementation suggestions regarding string_view were adopted (maflcko)
Strong: hodlinator approved with independent benchmarks confirming a 36.7% speedup; sedited concept ACKed.
Reviewers verified the benchmark results and supported the change, especially for speeding up wallet fuzz targets, with only minor unresolved nits left.
- sedited Concept ACKed
- hodlinator tested, benchmarked, and approved (re-ACK after string_view updates)
- maflcko provided suggestions regarding string_view which were adopted
Review verdicts (DrahtBot): 1
- ACK: hodlinator
- Concept ACK: sedited
Files
0 lines under test/bench/ci.
- src/base58.cpp +15/-13
- src/key_io.cpp +22/-3
- src/script/descriptor.cpp +17/-7
- src/key_io.h +10/-3
- src/base58.h +3/-2
Card
This PR optimizes descriptor parsing by avoiding redundant base58 decoding and replacing a linear checksum charset search with a 256-entry lookup table. It solves performance overhead in parsing xpub/xpriv descriptors, which noticeably slows down wallet fuzz testing and large multisig setups. Benchmarks provided by both the author and a reviewer demonstrate a 30% to 36% speedup. The PR is fully reviewable and currently has strong reviewer support including an approval.