#35989 wallet: fix crash on importdescriptors with a range ending at 2^31-1
https://github.com/bitcoin/bitcoin/pull/35989 · · +65/-2 in 4 files, 3 commits · labels: Wallet, Needs rebase
Goal
- Prevent node crashes and wallet corruption when importing descriptors with large ranges or keypools
- Return clean RPC errors instead of triggering assertion failures and aborting bitcoind
Fixes an assertion failure that aborts the node when calling importdescriptors with a range ending at 2^31-1 or without a range when -keypool exceeds INT32_MAX. Also avoids signed integer overflow in DescriptorScriptPubKeyMan::TopUpWithDB when calculating the target range end, and adds functional tests covering both cases.
Problem: WalletDescriptor stores descriptor ranges with an exclusive 32-bit signed integer end, but importdescriptors computes range_end in int64_t without upper-bound validation. When the inclusive endpoint is 2^31-1 or -keypool is excessively large, range_end wraps to negative numbers, corrupting the stored record and aborting the node via an assertion during keypool top-up.
Category: Wallet (#5 of 84)
P2 · bug fix
- P2 because it fixes an assertion abort and database corruption caused by extreme descriptor ranges
- Prevents persisting inverted range records to disk before terminating the node
Fixes an assertion failure that crashes bitcoind during importdescriptors and leaves a malformed descriptor record in the wallet database. While triggered only on edge-case range boundaries or extreme -keypool settings, eliminating daemon crashes and signed integer overflow in wallet logic represents clear stability value.
Membership: Touches src/wallet/rpc/backup.cpp, src/wallet/scriptpubkeyman.cpp, and wallet functional tests.
Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0
Reviewability: Stale: Needs rebase
- Needs rebase due to merge conflicts with master
The branch has merge conflicts with current master and is labeled Needs rebase.
Author status: Active; replied thoroughly to reviewer suggestions and pushed a regression test requested by jeanpablojp.
Resolved concerns:
- molnard noted potential follow-ups regarding recovery of already-corrupted records and centralizing -keypool upper bounds; both author and molnard agreed these belong in follow-up PRs.
Agreement: Strong
- Strong consensus on preventing node aborts with no open objections
- Verified by reproducing crashes on master and testing clean error handling (jeanpablojp, molnard)
- Verified watch-only wallet behavior and node stability (kriss39)
- Agreed that repairing pre-existing corrupted wallet records is follow-up work (molnard)
Strong: Multiple reviewers reproduced the crash, reviewed the fix, and confirmed it resolves the abort without objection.
Multiple contributors reviewed the code, reproduced the abort on master, and provided ACKs. No blocking criticisms or approach disagreements exist.
- molnard (2026-09-02): 'ACK def4fb0101... The changes are straightforward and focused on fixing the reported issues.'
- kriss39 (2026-09-14): 'tACK def4fb0101... Without the first two commits the new importdescriptors case takes the node down as expected.'
Objections: none enumerated.
Support:
- jeanpablojp: Reproduced the crash both ways on master.
- molnard: Verified that importing [2147483647,2147483647], importing with large -keypool, and keypoolrefill terminate master but return clean RPC errors on this PR.
- kriss39: Verified tests pass, tested manually that invalid imports are rejected cleanly, and confirmed master aborts without the fix.
- vicjuma: Concept ACK with before/after testing. [not substantive]
Participants: jeanpablojp (support), molnard (support), kriss39 (support), vicjuma (support), polespinasa (neutral)
State derived from the lists: substantive support, no open objection (jeanpablojp, molnard, kriss39)
Review verdicts (DrahtBot): 2
- ACK: molnard, kriss39
- Concept ACK: jeanpablojp, vicjuma
Files
50 lines under test/bench/ci.
- test/functional/wallet_importdescriptors.py +28/-0
- test/functional/wallet_keypool.py +22/-0
- src/wallet/scriptpubkeyman.cpp +9/-2
- src/wallet/rpc/backup.cpp +6/-0
Card
This PR fixes an assertion abort in DescriptorScriptPubKeyMan::TopUpWithDB when importdescriptors is called with an inclusive range ending at 2^31-1 or without a range under an oversized -keypool configuration. Because WalletDescriptor stores ranges in int32_t with an exclusive endpoint, 2^31 overflows to INT32_MIN and writes an inverted range to the wallet before asserting. It also prevents signed integer overflow in keypool top-up calculations. Review feedback is strongly positive with multiple tested ACKs, though the PR currently needs a rebase.