#35377 wallet: Allow importing of descriptors without private keys when the wallet has the private keys
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35377 wallet: Allow importing of descriptors without private keys when the wallet has the private keys | Waiting on author | 0 | Blocked | 140 + 74 tests | |||
|
|
|
|
| |||
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
Allows importing descriptors that omit private keys into private-key-enabled wallets when the wallet already holds those keys. During import, the wallet inspects its existing descriptor scriptpubkey managers for known private keys and master xprvs, substitutes matching origin-annotated xpubs (including participants inside musig expressions) with master keys, and supplies matching single private keys to the import keyset.
Problem: Importing a public descriptor into a wallet with private keys enabled fails unless the descriptor explicitly contains the private keys. Users are forced to export and expose sensitive private key material in RPC calls even when the wallet already possesses the required master xprv or individual keys.
Evidence of importance:
- Closes issue #27336 ('Should be able to import an xpub descriptor to a privkey-enabled wallet if the wallet has the privkeys')
- Description notes: 'Currently importing a descriptor to a wallet that has private keys requires the descriptor to include the private keys. This is not ideal as it means exposing private key material.'
Risk of a mistake: high (wallet). Flaws in key derivation or extended key substitution during descriptor import could cause the wallet to track addresses it cannot spend from or associate the wrong private keys with addresses, leading to loss of funds.
Changed since the previous assessment: No functional changes to the implementation; PR was rebased following the merge of base PR #34861.
Reviewability
Paused · Waiting on author
Open objections from Sjors, fametrano with no author reply for 10 days; review would repeat what has been asked.
Waiting on (from the thread read):
- author: address duplicate descriptor creation on re-import and missing array include reported by fametrano 2026-10-22
Agreement
Blocked
objection that needs a change, nobody has replied (fametrano)
Thread notes: Added fametrano's test results reporting duplicate descriptor creation on re-import when a master key is present, along with an IWYU missing header failure.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| Sjors (member) | suggestion, approach | Consider an alternative approach giving Parse access to the wallet's extended keys (or only hd keys) | open | answered | no | 2026-09-02: 'An alternative approach, using bits of #36133, is to give `Parse` access to the wallet's extended keys' |
| polespinasa (member) | suggestion, style | Simplify fingerprint comparison using xpub.id_key_fingerprint() != origin_pub->m_origin.fingerprint | answered · author replied done | answered | yes | 2026-09-30: 'I think this should work and would be cleaner?' Status from: 2026-09-30: achow101: 'Done' |
| polespinasa (member) | suggestion, approach | Compare only pubkeys rather than all metadata (depth, fingerprint) to avoid omitting valid keys from other wallets | answered · author explained that specificity avoids surprises when visually different xpubs get private keys | answered | yes | 2026-09-30: 'shouldn't we only compare the pubkey? What if some metadata such as depth, fingerprint, etc. is missing (or is 0)? They key still a valid key but we would silently omit it.' Status from: 2026-09-30: achow101: 'I think we want to be as specific as possible here to avoid any unexpected surprises.' |
| fametrano (contributor) | objection, correctness | Re-importing a descriptor the wallet already holds creates a duplicate entry with a new descriptor ID when the master key is present | open | changed | no | 2026-10-22: 'with the master key in the wallet, re-importing a descriptor the wallet already holds adds a duplicate.' |
| fametrano (contributor) | objection, maintenance | Missing #include <array> in descriptor.cpp causes an IWYU failure | open | changed | no | 2026-10-22: 'The IWYU failure is this PR's: the new `KeyFingerprint` comparison needs `#include <array>` in `descriptor.cpp`.' |
Support:
- rkrux (member) (Concept ACK, 2026-06-05): allows the users to not deal with private keys manually
- polespinasa (member) (Concept ACK, 2026-06-08): seems like a good idea [not substantive]
- Sjors (member) (Concept ACK, 2026-08-31): has been using a version of this in Sjors/bitcoin#91 and plans to switch to this PR
- polespinasa (member) (2026-09-30) [read]: did a first review, looks pretty good [not substantive]
Participants:
- rkrux (member, support, 1 statements 2026-06-05 to 2026-06-05): Concept ACK noting it allows users not to deal with private keys manually
- polespinasa (member, support, 4 statements 2026-06-08 to 2026-09-30): Concept ACK and reviewed descriptor substitution logic with suggestions
- Sjors (member, support, 2 statements 2026-08-31 to 2026-09-02): Concept ACK; proposed an alternative parsing approach using extended keys
- fametrano (contributor, objection, 1 statements 2026-10-22 to 2026-10-22): found that re-importing a descriptor with the master key in the wallet adds a duplicate, and noted an IWYU failure
Files
78 lines under test/bench/ci.
- src/script/descriptor.cpp +77/-15
- test/functional/wallet_importdescriptors.py +73/-4
- src/wallet/wallet.cpp +31/-0
- src/wallet/imports.cpp +24/-0
- src/script/descriptor.h +5/-0
- src/wallet/wallet.h +3/-0
- src/wallet/test/walletload_tests.cpp +1/-0
Card
The short text the ranking pass compares across PRs.
Allows importing public descriptors into private-key-enabled wallets when the wallet already holds the corresponding private keys. Previously, Bitcoin Core rejected public descriptors on privkey-enabled wallets, forcing users to expose and pass sensitive private keys via RPC even if the wallet already stored the master xprv. The wallet now inspects existing ScriptPubKeyMans, substituting matching master xprvs into origin-annotated xpubs and attaching matching individual keys. Closes #27336.
Data
thread read through 2026-10-22 (10 statements), head b58383aca6
labels: Wallet, CI failed
Categories
Wallet
P2 · user request (ranking pass: Moved down: long-requested usability/opsec improvement, ranked below the correctness fixes.)
Allows importing public descriptors into a wallet that already has the required private keys without forcing the user to re-export and expose private keys over RPC. This resolves long-standing feature request #27336 ('Should be able to import an xpub descriptor to a privkey-enabled wallet if the wallet has the privkeys') and improves operational security for descriptor workflows.
Membership: Labeled Wallet and modifies wallet descriptor import logic in src/wallet/imports.cpp and src/wallet/wallet.cpp.
Factors: security/stability 1, bug 0, performance 0, user value 2, leverage 1