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

pull achow101 wants to merge 12 commits into bitcoin:master from achow101:importdescriptors-without-priv changing 12 files +637 −279
  1. 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

  2. DrahtBot added the label Wallet on May 25, 2026
  3. DrahtBot commented at 11:55 PM on May 25, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    Concept ACK rkrux, polespinasa

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35630 (test: Add importdescriptors rpc error test coverage by polespinasa)
    • #35619 (test: ExtendedPrivateKey follow-ups by rkrux)
    • #35579 (wallet: reserve walletrescan before checking wallet is at the tip by polespinasa)
    • #34861 (wallet: Add importdescriptors interface by polespinasa)
    • #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
    • #33392 (wallet, rpc: add UTXO set check and incremental rescan to importdescriptors by musaHaruna)
    • #32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • harded -> hardened [misspelled in the derivation comment; this affects readability]
    • substition -> substitution [misspelled in the test log message]

    <sup>2026-07-08 21:14:08</sup>

  4. DrahtBot added the label Needs rebase on May 26, 2026
  5. achow101 force-pushed on May 26, 2026
  6. DrahtBot removed the label Needs rebase on May 26, 2026
  7. 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.

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

    seems like a good idea, concept ACK

    Will review after #34861 is merged

  9. DrahtBot added the label Needs rebase on Jun 12, 2026
  10. achow101 force-pushed on Jun 13, 2026
  11. DrahtBot added the label CI failed on Jun 13, 2026
  12. DrahtBot commented at 3:24 AM on June 13, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/27453755862/job/81154189319</sub> <sub>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”).</sub>

    <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>

  13. DrahtBot removed the label Needs rebase on Jun 13, 2026
  14. achow101 force-pushed on Jun 19, 2026
  15. DrahtBot removed the label CI failed on Jun 19, 2026
  16. DrahtBot added the label Needs rebase on Jun 26, 2026
  17. achow101 force-pushed on Jun 27, 2026
  18. DrahtBot removed the label Needs rebase on Jun 27, 2026
  19. DrahtBot added the label Needs rebase on Jul 3, 2026
  20. wallet: Add ImportDescriptorRequest and ImportDescriptorResult structs
    Add two new structs used for creating requests and getting response when importing descriptors.

These structs replace the UniValue-based interface of ProcessDescriptorImport()
in a subsequent commit.
    d9c92d0eda
  21. wallet: rpc: Use std::optional in GetImportTimestamp a713f985c7
  22. 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.
    6f49869c3c
  23. 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.
    d6913898a4
  24. wallet: refactor: make is_ranged no longer an optional 5663cea6b7
  25. 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
    219bac2775
  26. 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.
    7455e061d6
  27. 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.
    b83b253ecf
  28. wallet: Add an importDescriptors() interface for the wallet b1d82f8232
  29. 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.
    142b578c8f
  30. 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.
    b6c3417d50
  31. 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.
    8791d4f71b
  32. achow101 force-pushed on Jul 8, 2026
  33. DrahtBot removed the label Needs rebase on Jul 8, 2026
Contributors
achow101DrahtBotrkruxpolespinasa
Labels
Wallet
Linked (view graph)
#27336 Should be able to import an xpub descriptor to a privkey-enabled wallet if the wallet has the privkeys#34861 wallet: Add importdescriptors interface#35645 Multisig Wizard tracking issue
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-09 06:47 UTC