wallet: Add addHDkey interface #35436

pull pseudoramdom wants to merge 3 commits into bitcoin:master from pseudoramdom:addhdkey-interface changing 8 files +166 −32

pseudoramdom commented at 9:20 PM on June 1, 2026: none
This PR adds a wallet interface for addhdkey.

The motivation is same as #34861 - while we have addhdkey RPC, Bitcoin Core GUI does not use the RPC interface. Having a dedicated wallet interface is helpful for GUI when performing multisig setup. When used in tandem with a similar interface for derivehdkey (#32784), the GUI can produce a shareable xpub during multisig setup.

Key changes:
- Introduce wallet::AddHDKey() helper in a new hd_keys.{h,cpp}. The core logic from RPC is moved to this helper.
- Introduce wallet::AddHDKeyResult which is a struct that wraps the resulting master xpub and its fingerprint.
- Introduce a concrete error enum wallet::AddHDKeyError. Caller can use AddHDKeyErrorString() to convert the error to translatable user-facing messages.
- Update addhdkey RPC to use the above wallet helper. The RPC also exposes the master fingerprint in hex as fingerprint field.
  
  <details> <summary>Sample Output</summary>
  
  $ build/bin/bitcoin-cli -rpcwallet=Alice addhdkey { "fingerprint": "d2f7b7da", "xpub": "tpubD6NzVbkrYhZ4YKrGrgkAbx4HuqDxKDMzJMsipFMd2VzFaaxLMBXZ9uzqCmbpRXWHVoS2Q6WgY4K1mUbvdhcDQEDs6VjNHQmnTcHQKAyDzUf" }
  
  </details>
- Add interfaces::Wallet::addHDKey(), a new interface that GUI can call. Only the fingerprint is returned, the master xpub is intentionally not returned.
- Update test/functional/wallet_hd.py to assert the new fingerprint field.
DrahtBot added the label Wallet on Jun 1, 2026
DrahtBot commented at 9:20 PM on June 1, 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/35436.



Reviews

See the guideline for information on the review process.

Type Reviewers

Concept ACK w0xlt

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:
- #bitcoin-core/gui/872 (Menu action to export a watchonly wallet by achow101)
- #35444 (wallet: make descriptor SPKM mutex non-recursive by w0xlt)
- #35377 (wallet: Allow importing of descriptors without private keys when the wallet has the private keys by achow101)
- #35201 (Refactor: Updated signMessage to use util::Expected and removed SigningResult::OK by kevkevinpal)
- #34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
- #34861 (wallet: Add importdescriptors interface by polespinasa)
- #34617 (fees: wallet: remove block policy fee estimator internals from wallet by ismaelsadeeq)
- #34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation by ismaelsadeeq)
- #32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)
- #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.
DrahtBot added the label CI failed on Jun 1, 2026
DrahtBot commented at 10:42 PM on June 1, 2026: contributor


🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26782790523/job/78951203063</sub> <sub>LLM reason (✨ experimental): CI failed due to a new circular dependency detected in the lint check (“interfaces/wallet.h -> wallet/hd_keys -> wallet/wallet -> interfaces/wallet”).</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>
pseudoramdom force-pushed on Jun 2, 2026

Type	Reviewers
Concept ACK	w0xlt

wallet: Introduce AddHDKey helper in wallet

Introduce a wallet-layer helper for adding an HD key to a descriptor wallet. This mirrors the existing `addhdkey` RPC behavior. Subsequent commits will convert the addhdkey RPC into a thin wrapper and call this helper.

The helper returns a `util::Expected<AddHDKeyResult, AddHDKeyError>` carrying the master fingerprint and master xpub on success or a concrete error enum on failure.

fc40c6c312

wallet, test: Refactor `addhdkey` RPC to use `wallet::AddHDKey` helper

Replace `addhdkey` RPC with a thin wrapper over the `wallet::AddHDKey` helper.
Additionally, expose the the master fingerprint in the RPC response.

Also update `test_addhdkey` to validate fingerprint.

6342e56630

wallet: Expose addHDKey via the Wallet interface

Add interfaces::Wallet::addHDKey() which calls the wallet::AddHDKey helper.
The master xpub is intentionally not surfaced through this interface and it returns only the master fingerprint.

edae6b048c

pseudoramdom force-pushed on Jun 2, 2026
DrahtBot removed the label CI failed on Jun 2, 2026
pseudoramdom marked this as ready for review on Jun 2, 2026
w0xlt commented at 8:58 PM on June 2, 2026: contributor

Concept ACK

in src/interfaces/wallet.h:108 in edae6b048c

 104 | @@ -102,6 +105,9 @@ class Wallet
 105 |      //! Get public key.
 106 |      virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
 107 |  
 108 | +    //! Add a new HD key to the wallet. Returns the master fingerprint only.

w0xlt commented at 9:52 PM on June 3, 2026:

nit: That is the fingerprint of the HD key being added.

    //! Add a new HD key to the wallet. Returns the added key's fingerprint only.

in src/wallet/rpc/wallet.cpp:857 in edae6b048c

 853 | @@ -853,6 +854,7 @@ RPCMethod addhdkey()
 854 |          RPCResult{
 855 |              RPCResult::Type::OBJ, "", "",
 856 |              {
 857 | +                {RPCResult::Type::STR_HEX, "fingerprint", "Master key fingerprint as hex"},

w0xlt commented at 9:54 PM on June 3, 2026:

nit: same as above

                {RPCResult::Type::STR_HEX, "fingerprint", "Fingerprint of the HD key that was added as hex"},

in src/wallet/hd_keys.cpp:54 in edae6b048c

  49 | +    } else {
  50 | +        CKey seed_key = GenerateRandomKey();
  51 | +        hdkey.SetSeed(seed_key);
  52 | +    }
  53 | +
  54 | +    LOCK(wallet.cs_wallet);

w0xlt commented at 6:07 AM on June 11, 2026:

Consider taking LOCK(wallet.cs_wallet) before the IsLocked() check rather than after key generation. It matches the existing convention in the wallet RPCs, where LOCK(pwallet->cs_wallet) is consistently taken before EnsureWalletIsUnlocked(*pwallet) (e.g. signmessage.cpp, backup.cpp, spend.cpp).

diff --git a/src/wallet/hd_keys.cpp b/src/wallet/hd_keys.cpp
index 20dbf647a2..b7be8d246c 100644
--- a/src/wallet/hd_keys.cpp
+++ b/src/wallet/hd_keys.cpp
@@ -35,6 +35,8 @@ bilingual_str AddHDKeyErrorString(AddHDKeyError error)
 
 util::Expected<AddHDKeyResult, AddHDKeyError> AddHDKey(CWallet &wallet, const std::optional<CExtKey> &existing_key)
 {
+    LOCK(wallet.cs_wallet);
+
     if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
         return util::Unexpected{AddHDKeyError::PRIVATE_KEYS_DISABLED};
     }
@@ -51,8 +53,6 @@ util::Expected<AddHDKeyResult, AddHDKeyError> AddHDKey(CWallet &wallet, const st
         hdkey.SetSeed(seed_key);
     }
 
-    LOCK(wallet.cs_wallet);
-
     std::string descriptor_str = "unused(" + EncodeExtKey(hdkey) + ")";
     FlatSigningProvider keys;
     std::string error;

Contributors

Labels

Linked (view graph)

#32784 wallet: derivehdkey RPC to get xpub at arbitrary path #34861 wallet: Add importdescriptors interface

wallet: Add addHDkey interface #35436

Code Coverage & Benchmarks

Reviews

Conflicts