#35041 descriptor: speed-up `Parse` (xpub/xpriv) in ~30%

ReviewabilityReviewsAgreementSize
#35041 brunoerg descriptor: speed-up `Parse` (xpub/xpriv) in ~30%Waiting on author1Strong71
  • Speed up descriptor parsing by roughly 30% to improve descriptor loading and fuzzing
  • Eliminate redundant work when parsing extended keys and computing descriptor checksums
  • Ready for review
  • Strong support citing a 36.7% benchmarked speedup and fuzzing benefits (hodlinator)
  • Concept approval without stated reasons (sedited)
  • Nonblocking suggestion to add an argument precondition or use an unsigned length (hodlinator)
  • 71 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 28 lines removed in total
  • 5 files, 2 commits
Wallet#71 of 92
P3 · speedup
  • P3 because descriptor parsing runs roughly 30% faster on large multisig descriptors
  • Sub-millisecond savings per descriptor benefit fuzz targets but remain deferrable for normal wallet use
  • Ranking pass: Moved down: sub-millisecond per-descriptor gain mainly benefits fuzzing, not normal wallet use.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Improves descriptor parsing performance by combining extended key decoding (`xpub`/`xprv`) into a single Base58Check decode pass. It also replaces the O(N) linear character scan in descriptor checksum computation with a precomputed 256-byte lookup table, and updates Base58 and key decoding APIs to accept `std::string_view` to avoid string allocations.

Problem: When parsing extended keys, the descriptor parser unconditionally called both `DecodeExtKey()` and `DecodeExtPubKey()`, executing two separate Base58Check passes (including double-SHA256 hashing) even though only one prefix could match. In addition, descriptor checksumming performed linear scans over the character set for every character, and key decoding functions required `const std::string&`, causing redundant heap allocations and slowing down wallet fuzzing and descriptor parsing.

Evidence of importance:

  • Author provides microbenchmark results parsing a large multisig Taproot descriptor showing parse time reduced from 127,812 ns/op to 90,253 ns/op (~29.4% speedup) (PR description).

Risk of a mistake: medium (wallet, other). A flaw in Base58 decoding or extended key parsing could cause valid descriptors or keys to be rejected or parsed incorrectly by the wallet.

Changed since the previous assessment: The Base58 decoding helper now caps the initial buffer allocation size using `max_ret_len - zeroes`.

Reviewability

Paused · Waiting on author

Open objection from hodlinator with no author reply for 11 days; review would repeat what has been asked.

Waiting on (from the thread read):

  • author: respond to hodlinator's suggestion regarding max_ret_len precondition 2026-10-21

Agreement

Strong

substantive support, no open objection (hodlinator)

Thread notes: Added hodlinator's suggestion to add a precondition or make max_ret_len unsigned, and recorded their latest re-ACK.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
l0rinc (member)question, approachwould it help if earlier base58 optimization attempts were revived?answered · author explained prior PRs focused on algorithm rewrite whereas this PR optimizes usageansweredyes2026-04-09: 'would it help if we revived them?'
Status from: 2026-04-10: brunoerg: 'I think their focus is to improve/rewrites the algorithm itself, here is more about the way it's been used.'
sedited (member)suggestion, maintenancemake the lookup table constexprfixed · fix pushed · author made it constexpr and pushedansweredyes2026-06-07: 'Shouldn't really matter, but I thinks this could be constexpr too.'
Status from: 2026-06-10: brunoerg: 'Done.'
hodlinator (member)suggestion, maintenancechange INPUT_CHARSET_POS element type to int8_t to better match range and decrease sizeaccepted · fix pushed · author implemented int8_t change; hodlinator acknowledged feedback was appliedansweredyes2026-08-25: 'Change element type of `INPUT_CHARSET_POS` to `int8_t` to better match range of possible values and decrease size.'
Status from: 2026-09-02: hodlinator: 'Thanks for applying most of my feedback!'
maflcko (member)suggestion, interfacepass a string_view to clean up callersfixed · fix pushed · author updated function to accept string_viewansweredyes2026-09-01: 'Could also pass a string view in here to clean up the callers and then derive `psz` and `end` only in the first line of this function?'
Status from: 2026-09-02: brunoerg: 'Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/35041#discussion_r3906003696 (string view)'
sedited (member)suggestion, safetyclamp to max size to prevent 32-bit overflowfixed · fix pushed · author force-pushed fix clamping to max sizeansweredyes2026-10-19: 'Could take this opportunity and make this operation 32bit overflow-safe by clamping to max size'
Status from: 2026-10-19: brunoerg: 'Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/35041/changes/59daebcf05db41d83f9d1974b5c79be3eaf62770#r'
hodlinator (member)suggestion, maintenanceadd a precondition assert for max_ret_len >= 0 or make it unsigned instead of std::max(..., 0)openansweredno2026-10-21: 'Could instead add a precondition for the argument (or make it unsigned)'

Support:

  • sedited (member) (Concept ACK, 2026-06-07): Concept ACK [not substantive]
  • hodlinator (member) (ACK, 2026-08-25) [both]: Cleaner higher-level optimization than previously rejected PRs, concrete benefit for fuzz targets, and benchmarked 36.7% speedup in ParseDescriptor
  • hodlinator (member) (ACK, 2026-09-02): re-ACK ddcacd25e735dad4868bab7f618961a363f23239 [not substantive]
  • hodlinator (member) (ACK, 2026-10-21): re-ACK 61cb3d966a855a551307889daea9c45c2c1db203 [not substantive]

Participants:

  • l0rinc (member, question, 1 statements 2026-04-09 to 2026-04-09): asked whether earlier base58 optimization attempts should be revived
  • sedited (member, support, 3 statements 2026-06-07 to 2026-10-19): Concept ACK; suggested constexpr and clamping to prevent 32-bit overflow
  • hodlinator (member, support, 10 statements 2026-08-25 to 2026-10-21): ACK with benchmark showing 36.7% speedup; suggested int8_t table and max_ret_len precondition; re-ACKed
  • maflcko (member, neutral, 1 statements 2026-09-01 to 2026-09-01): suggested passing string_view to simplify callers

Files

0 lines under test/bench/ci.

  • src/base58.cpp +19/-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

The short text the ranking pass compares across PRs.

This pull request speeds up descriptor parsing for descriptors containing extended public and private keys. The parser previously called both DecodeExtKey and DecodeExtPubKey on each extended key, redundantly running Base58Check and double-SHA256 passes twice per key, while descriptor checksums performed linear charset scans. This PR combines the key decodes into a single Base58Check pass, replaces checksum charset lookups with a 256-byte table, and switches Base58 string APIs to std::string_view. The author reports a 29.4% parsing speedup on a large multisig descriptor in microbenchmarks.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:14, confidence high, input hash 7a82165061b69dba
thread read through 2026-10-21 (25 statements), head 61cb3d966a
labels: Descriptors

Categories

Wallet

P3 · speedup (ranking pass: Moved down: sub-millisecond per-descriptor gain mainly benefits fuzzing, not normal wallet use.)

Optimizing descriptor parsing by ~30% via unified Base58Check decoding and character lookup tables is a solid optimization with real value for fuzzing and descriptor loading, but the sub-millisecond savings per descriptor are reasonably deferrable. Reviewers and author confirm a 29–37% speedup on `ParseDescriptor` microbenchmarks (from 127 µs to 90 µs on large multisig descriptors) with concrete benefit for descriptor fuzz targets.

Membership: Touches descriptor parsing and extended key decoding in `src/script/descriptor.cpp` and `src/key_io.cpp`, which are core to wallet key and descriptor management.

Factors: security/stability 0, bug 0, performance 1, user value 1, leverage 1