#36245 wallet: start descriptor top up at range_start instead of index 0

ReviewabilityReviewsAgreementSize
#36245 kriss39 wallet: start descriptor top up at range_start instead of index 0Ready0Crickets6 + 37 tests
  • Prevent ranged descriptor imports from expanding keys starting from zero instead of the start index
  • Avoid extreme import delays and memory exhaustion when using high range start values
  • Fix inconsistent ownership tracking where transactions appear owned but lose ownership on reload
  • Ready for review
  • No review verdicts recorded
  • No reviews or comments yet
  • 6 lines added or modified outside tests
  • 37 lines added or modified in tests
  • 3 lines removed in total
  • 2 files, 1 commits
Wallet#8 of 92
P2 · bug fix
  • P2 because it fixes descriptor imports deriving from zero instead of the requested range start
  • Prevents extreme import slowdowns and potential memory exhaustion from deriving up to 2^31 keys
  • Eliminates transient ownership bugs where imported transactions lose IsMine status across restarts
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

Restricts descriptor expansion in `DescriptorScriptPubKeyMan::TopUpWithDB()` to start at `range_start` instead of index 0, and records the derived index directly rather than incrementing an offset from -1. Also aligns `DescriptorScriptPubKeyMan::Load()` to set `m_max_cached_index` to the actual index loaded.

Problem: When importing a descriptor with range `[N, M]`, the wallet expands and tracks keys starting from index 0 instead of `N`. This causes import times to scale with M rather than the range size M - N, falsely treats scriptPubKeys below N as `IsMine` until wallet restart (causing rescanned transactions below N to lose ownership on reload), can trigger derivation of up to 2^31 keys for high range starts, and causes redundant re-expansions on active wallet reload due to miscalculated `m_max_cached_index`.

Evidence of importance:

  • Author benchmark: 'Importing wpkh(xpub/0/*) with "range": [600000, 600009] into a blank watch-only wallet goes from 19.5s to 0.09s on this machine.' (PR description)
  • Author notes vulnerability to excessive derivation: 'Since ParseDescriptorRange() only bounds the size of the range and not its start, "range": [2146483648, 2147483647] is accepted and expands 2^31 indexes into m_map_script_pub_keys before the RPC returns.' (PR description)

Risk of a mistake: medium (wallet). Flaws in descriptor address derivation or caching could cause wallets to fail to detect incoming transactions or misreport wallet ownership.

Changed since the previous assessment: No functional differences; patch rebased or re-formatted.

Reviewability

Ready

Nothing known that would invalidate a review now.

Agreement

Crickets

no substantive comment either way

Objections, suggestions, and questions: none recorded.

Files

37 lines under test/bench/ci.

  • test/functional/wallet_importdescriptors.py +37/-0
  • src/wallet/scriptpubkeyman.cpp +6/-3

Card

The short text the ranking pass compares across PRs.

Bounds descriptor key expansion during import top-ups to start at range_start rather than index 0, and records the derived index directly. Currently, importing a descriptor with range [N, M] derives and caches all keys from 0 to M, making import cost O(M) instead of O(M - N) and causing keys below N to appear as owned during import only to disappear when the wallet is reloaded. The author reports that importing a 10-key range starting at index 600,000 drops from 19.5 seconds to 0.09 seconds, and prevents pathological large range starts from attempting to derive up to 2^31 keys.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-11T01:20, confidence high, input hash 508bbf235045286b
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Wallet

Categories

Wallet

P2 · bug fix

Fixes a correctness and reliability bug where ranged descriptor imports starting at index N derive keys from 0, causing import times to scale with M rather than M - N (benchmark: 19.5s to 0.09s for [600000, 600009]), potential memory exhaustion on high range starts up to 2^31, and inconsistent `IsMine` tracking across restarts.

Membership: Matches Wallet label, modifies `src/wallet/scriptpubkeyman.cpp` descriptor expansion logic and adds functional test in `test/functional/wallet_importdescriptors.py`.

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