wallet: Allow importing of descriptors without private keys when the wallet has the private keys #35377

achow101 commented at 11:55 PM on May 25, 2026: member

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. This PR makes it so that the wallet will lookup and substitute the private keys for their respective public keys in such descriptors, thus enabling importing of public descriptors into wallets with private keys.

The underlying mechanism is that the wallet retrieves all of the pubkeys from the descriptor and checks to see if any of them have private keys in any ScriptPubKeyMan. Additionally, if the descriptor has a xpub with key origin info, we will check if any xprvs known to the wallet have a matching fingerprint and derive to the specified xpub. If so, the origin + xpub are replaced with the single xprv with the origin derivation path prepended to the key expression's derivation path.

Possible future work is to allow the xpub substitution to work when the wallet has some child in the key origin, i.e. the wallet xprv has a key origin that is a prefix of the key origin specified for an xpub in the descriptor. Currently this kind of substitution is not being done, only root master xprvs will be substituted.

If a wallet does not have the private keys for a public descriptor, the import is still disallowed.

This PR is based on #34861 to avoid an annoying rebase.

Closes #27336

DrahtBot added the label Wallet on May 25, 2026

DrahtBot commented at 11:55 PM on May 25, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35377.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	rkrux, polespinasa, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36143 (descriptor: add CreateMultisigDescriptor() by rxbryan)
#36133 (wallet: store multipath descriptor by Sjors)
#35998 (wallet: Handle or explicitly ignore WalletBatch write failures by achow101)
#35989 (wallet: fix crash on importdescriptors with a range ending at 2^31-1 by shuv-amp)
#35752 (wallet: make encryption state updates atomic by l0rinc)
#35619 (test: ExtendedPrivateKey follow-ups by rkrux)
#35436 (wallet: Add addHDkey interface by pseudoramdom)
#34861 (wallet: Add importdescriptors interface by polespinasa)
#34681 (wallet: move rescan logic into ChainScanner and wallet/scan by Eunovo)
#34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
#31668 (Added rescan option for import descriptors by saikiran57)
#29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

// Whether ' or h is used in harded derivation -> // Whether ' or h is used in hardened derivation [“harded” is misspelled]
self.log.info("... xpub substition") -> self.log.info("... xpub substitution") [“substition” is misspelled]

2026-08-26 21:02:55

DrahtBot added the label Needs rebase on May 26, 2026

achow101 force-pushed on May 26, 2026

DrahtBot removed the label Needs rebase on May 26, 2026

rkrux commented at 10:28 AM on June 5, 2026: contributor

Definite Concept ACK 2e6d8d0 because it allows the users to not deal with private keys manually.

polespinasa commented at 10:15 AM on June 8, 2026: member

seems like a good idea, concept ACK

Will review after #34861 is merged

DrahtBot added the label Needs rebase on Jun 12, 2026

achow101 force-pushed on Jun 13, 2026

DrahtBot added the label CI failed on Jun 13, 2026

DrahtBot commented at 3:24 AM on June 13, 2026: contributor

🚧 At least one of the CI tasks failed. Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/27453755862/job/81154189319 LLM reason (✨ experimental): CI failed because IWYU reported a header include issue (it modified src/script/descriptor.h and deliberately exited with “Failure generated from IWYU”).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label Needs rebase on Jun 13, 2026

achow101 force-pushed on Jun 19, 2026

DrahtBot removed the label CI failed on Jun 19, 2026

DrahtBot added the label Needs rebase on Jun 26, 2026

achow101 force-pushed on Jun 27, 2026

DrahtBot removed the label Needs rebase on Jun 27, 2026

DrahtBot added the label Needs rebase on Jul 3, 2026

achow101 force-pushed on Jul 8, 2026

DrahtBot removed the label Needs rebase on Jul 8, 2026

DrahtBot added the label Needs rebase on Jul 14, 2026

achow101 force-pushed on Jul 31, 2026

achow101 force-pushed on Aug 5, 2026

DrahtBot removed the label Needs rebase on Aug 6, 2026

DrahtBot added the label CI failed on Aug 6, 2026

DrahtBot commented at 1:23 AM on August 6, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/31056225146/job/92474198450 LLM reason (✨ experimental): CI failed due to a C++ build error in src/script/descriptor.cpp (invalid access to MuSigPubkeyProvider::m_participants plus a related std::equal compilation error).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

achow101 force-pushed on Aug 8, 2026

DrahtBot removed the label CI failed on Aug 8, 2026

DrahtBot added the label Needs rebase on Aug 11, 2026

wallet: rpc: Use std::optional in GetImportTimestamp ca3b84ca7a

wallet: lower the minimum timestamp to 0

Timestamp 0 is allowed and it does not seem a reason to have a minimum timestamp > 0
other than some carried value from legacy wallet.

Lowering simplifies importdescriptors because, before this change, if a timestamp 0 was set
by the user it would be overwritten by the minimum timestamp, but error messages should still
report timestamp 0.

3e18c7885b

wallet: refactor: make is_ranged no longer an optional 4d62c0c223

wallet: Add ImportDescriptorRequest structs

ImportDescriptorRquest will replace the UniValue-based interface of ProcessDescriptorImport(),
which will allow to extract the functionality to other interfaces such as the GUI, etc,
in a subsequent commit.

bddd917fc7

wallet: rpc: refactor: Extract UniValue processing from ProcessDescriptorImport

Adds a new function ProcessUniValueDescriptor that translates from UniValue to ImportDescriptorRequest.
Refactors ProcessDescriptorImport to work with ImportDescriptorRequest objects instead of UniValue.
Refactors RPCMethod importdescriptors() to use ProcessUniValueDescriptor.

The commit introduces a small behavior change to make label handling more consistent.
Previously, an unset label on a descriptor that cannot have a label would eventually be converted
to an empty string. However, if the user explicitly set the label to an empty string,
the importdescriptors RPC call would fail because the descriptor does not support labels.
This inconsistency does not make sense, since an empty string (or null) effectively means that
no label was set. With this change, null labels and empty strings are treated as valid and are
considered equivalent to having no label at all.

ecb622b6cf

wallet: Add ImportError struct and new WalletError codes

ImportError struct will substitute the UniValue based response of ProcessDescriptorImport
in a subsequent commit. This will allow other interfaces such as the GUI to consume those function.

Also add new error codes to the WalletError codes enum, this is necessary in order to propagate
error codes to the new interfaces and to translate errors to the rpc wallet error codes.

5eeb912466

wallet: rpc: refactor: ProcessDescriptorImport returns ImportDescriptorResult

Make ProcessDescriptorImport return a ImportDescriptorResult object instead of a UniValue.

This commit gets rid of a try-catch that generates a big diff of identation changes,
it is recommended to review it with --ignore-space-change

9deb9f4f65

wallet: rename ProcessDescriptorImport to ImportDescriptor and add ProcessDescriptorsImport

Rename ProcessDescriptorImport() to ImportDescriptor().
ImportDescriptor() takes a single descriptor request and imports it into the wallet.

Add ProcessDescriptorsImport() (descriptors in plural) to handle wallet locking and rescanning
over a vector of ImportDescriptorRequest items. It is also in charge of collecting all responses
of imports from ImportDescriptor.

This allows a next commit to extract ImportDescriptor() and
ProcessDescriptorsImport() out of the RPC code so they can be used by
other future interfaces.

6e081b6a30

wallet: Move ImportDescriptor and ProcessDescriptorsImport to imports.cpp

Extract ImportDescriptor() and ProcessDescriptorsImport() out of backup.cpp and move them into imports.cpp so other future interfaces
can use them without needing to know about RPC code.

The commit can be reviewed with the --color-moved=dimmed-zebra option for an easier review.

fa6a1015e6

wallet: add CheckDescriptorRangeBounds

Co-authored-by: w0xlt <woltx@protonmail.com>

ProcessUniValueDescriptor() checks for the descriptor range correctness, but since a
future commit adds an interface to import descriptors, those checks must be extracted
from RPC code. This commit adds a helper function to check ranges in descriptors and
checks for the range inside ImportDescriptor().
ProcessUniValueDescriptor keeps also the check as other RPC calls other than importdescriptors
also needs to check for ranges. This introduces a double check on the RPC path,
but the cost of it is minimal and can be accepted.

49f74f7874

wallet: Add an importDescriptors() interface for the wallet 9ebb6ff259

descriptor: Implement SubstituteMasterExtPubs

SubstituteMasterExtPubs replaces Origin + BIP32 inside of a descriptor
when a master xprv is provided that matches the origin and derives the
BIP32 xpub.

0516febd17

wallet: Substitute known keys when importing a descriptor

When an imported descriptor contains pubkeys for which the wallet knows
the private keys, substitute those pubkeys for the privkeys so that the
descriptor can be imported. This allows such descriptors without private keys
to be imported into wallets with private keys enabled.

de4616b86f

descriptor, musig: Return participants in GetPubKeys 04b14bd8f0

test: Test importdescriptors with descriptors without privkeys

Test that importdescriptors can import descriptors that don't have
private keys, but the wallet already has the private key for them.

8d439772a7

achow101 force-pushed on Aug 26, 2026

DrahtBot removed the label Needs rebase on Aug 26, 2026

Sjors commented at 6:24 PM on August 31, 2026: member

Concept ACK, I've been using some version of this in https://github.com/Sjors/bitcoin/pull/91. I'll try to switch to exactly this PR after #34861 lands and you've rebased.

Sjors commented at 6:00 PM on September 2, 2026: member

An alternative approach, using bits of #36133, is to give Parse access to the wallet's extended keys: 2026/09/parse-known-keys (2 commits)

Although it works fine when you give it all keys, I further simplified it by only considering hd keys.