wallet: derivehdkey RPC to get xpub at arbitrary path #32784

Sjors commented at 11:04 AM on June 20, 2025: member

Adds a derivehdkey RPC that returns an xpub, or optionally the xprv, at an arbitrary BIP32 path (with at least one hardened step), derived from a wallet HD key.

The main use case is coordinating a multisig setup, where each participant shares an xpub derived at a hardened path (e.g. m/87h/0h/0h) distinct from their default single-signature descriptors. See the (updated) doc/multisig-tutorial.md and (updated) functional test to see how that workflow improves.

The first commits are some helpful helpers:

key: add DeriveExtKey() helper - performs the actual derivation
test: move parse_hd_keypath test to bip32_tests - from psbt_wallet_tests
Have ParseHDKeypath handle h derivation marker
util: reject out-of-range BIP32 keypath indices - ParseHDKeypath would previously map overflowing values without h to hardened.
fuzz: check ParseHDKeypath/WriteHDKeypath round-trip
rpc: ParsePathBIP32 helper
refactor: add hardened derivation helper - HasHardenedDerivation(), to enforce the "at least one hardened step" rule
wallet: generalize GetActiveHDPubKeys helper - extracts code from gethdkeys which derivehdkey needs
wallet: add GetExtKey helper - reconstruct an xprv from a wallet xpub (analog of GetKey()); behavior-preserving prep, also simplifies gethdkeys.

Meat and potatoes:

rpc: add derivehdkey - the RPC itself, plus the UnusedKey filter on GetHDPubKeys that drives key selection.
test: use derivehdkey in M-of-N multisig demo - rewrites the functional multisig test to use the RPC and <0;1> syntax.
doc: use derivehdkey in multisig tutorial - same for the prose tutorial.

DrahtBot renamed this:
~~wallet: derivehdkey RPC to get xpub at arbitrary path~~
wallet: derivehdkey RPC to get xpub at arbitrary path
on Jun 20, 2025

DrahtBot added the label Wallet on Jun 20, 2025

DrahtBot commented at 11:04 AM on June 20, 2025: 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/32784.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	rkrux, pseudoramdom

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:

#35444 (wallet: make descriptor SPKM mutex non-recursive by w0xlt)
#35170 (test: Better test coverage for legacy ParseHDKeypath() by optout21)
#35069 (Refactor keypath parser by pythcoiner)
#34502 (wallet: remove most asserts of WALLET_FLAG_DESCRIPTORS flag by rkrux)
#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.

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:

[test/functional/wallet_derivehdkey.py] assert xpub_info["xpub"] != xpub_info_2["xpub"] -> assert_not_equal(xpub_info["xpub"], xpub_info_2["xpub"])

2026-07-01 12:58:24

DrahtBot added the label CI failed on Jun 20, 2025

DrahtBot commented at 11:47 AM on June 20, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/44475268764 LLM reason (✨ experimental): The CI failure is caused by errors from the lint check 'py_lint', specifically due to unused imports flagged by ruff.

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

Sjors force-pushed on Jun 20, 2025

DrahtBot removed the label CI failed on Jun 20, 2025

in src/rpc/util.h:None in 30d5e5ccf7 outdated

 151 | @@ -152,6 +152,14 @@ std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value);
 152 |  /** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */
 153 |  std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider, const bool expand_priv = false);
 154 |  
 155 | +//! Get extended key and origin info for a given path
 156 | +//! @params[in] path The BIP 32 path

DrahtBot commented at 9:41 AM on June 25, 2025:

In util.h: “@params[in] path” → “@param[in] path” [Doxygen tag typo]

Sjors force-pushed on Jun 26, 2025

Sjors force-pushed on Jun 27, 2025

in doc/multisig-tutorial.md:None in 35eec44491 outdated

 137 |  
 138 | -./build/bin/bitcoin-cli  -signet -rpcwallet="multisig_wallet_01" getwalletinfo
 139 | +./build/bin/bitcoin rpc  -signet -rpcwallet="multisig_wallet_01" listdescriptors
 140 |  ```
 141 |  
 142 | +The `<0;1>` notation in `desc` caused the creation of two descriptors. One uses the chain 0 for external addressesd the other the chain 1 for internal ones (change).

DrahtBot commented at 4:37 AM on July 2, 2025:

addressesd -> addresses [extra “d” makes “addresses” misspelled]

rkrux commented at 11:11 AM on July 18, 2025: contributor

Very nice, Concept ACK. I will review this PR.

Sjors force-pushed on Aug 1, 2025

DrahtBot added the label Needs rebase on Sep 25, 2025

Sjors commented at 10:15 AM on September 26, 2025: member

(will rebase this later)

Sjors force-pushed on Jan 5, 2026

Sjors commented at 4:05 AM on January 5, 2026: member

Rebased after #32821 landed and #29136 was rebased. Re-tested by having an LLM follow the tutorial.

DrahtBot removed the label Needs rebase on Jan 5, 2026

Sjors force-pushed on Feb 2, 2026

DrahtBot added the label CI failed on Apr 28, 2026

DrahtBot commented at 12:39 PM on April 28, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/25052455086/job/73383604351 LLM reason (✨ experimental): CI failed because the CMake build errored while compiling src/wallet (bitcoin_wallet target), stopping with make/gmake exit code 2.

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

Sjors commented at 3:30 PM on April 28, 2026: member

I pushed an extra commit 9814b3350df3ec8381a04189d66e5a20d9414ab0 to pre-empt IWYU warnings (at least in https://github.com/Sjors/bitcoin/pull/91, probably here too). That caused CI to pick up the latest master, which fails as expected. I'll push again after #29136 is updated.

Sjors force-pushed on May 1, 2026

Sjors commented at 11:28 AM on May 1, 2026: member

Rebased after #29136 updated and squashed 9814b3350df3ec8381a04189d66e5a20d9414ab0 IWYU fixup into the first commit.

DrahtBot removed the label CI failed on May 1, 2026

Sjors force-pushed on May 14, 2026

Sjors commented at 12:47 PM on May 14, 2026: member

Rebased now that #29136 landed.

Sjors marked this as ready for review on May 14, 2026

sedited requested review from polespinasa on May 22, 2026

pseudoramdom commented at 5:27 PM on June 1, 2026: none

Concept ACK

Hi @Sjors, could the implementation move out of the RPC into a wallet::DeriveHDKey() helper, similar to #34861 ? The use case I have in mind is multisig setup in gui-qml, where each cosigner needs to share a derived xpub. A single interfaces::Wallet method that combines addhdkey & derivehdkey would let the GUI get a shareable xpub in one call.

pseudoramdom commented at 9:30 PM on June 1, 2026: none

A single interfaces::Wallet method that combines addhdkey & derivehdkey would let the GUI get a shareable xpub in one call.

After chatting with @achow101, these might be separate interfaces. We'll still need the logic to be moved out of the RPC for derivehdkey. I have PR for addHDKey interface https://github.com/bitcoin/bitcoin/pull/35436

in src/rpc/util.cpp:1388 in a45777992b outdated

1381 | @@ -1379,6 +1382,31 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
1382 |      return ret;
1383 |  }
1384 |  
1385 | +std::optional<std::pair<CExtKey, KeyOriginInfo>> DeriveExtKey(const CExtKey& ext_key, const std::vector<uint32_t>& path) {
1386 | +    CExtKey descendant = ext_key;
1387 | +    KeyOriginInfo origin;
1388 | +    origin.clear(); // Prevent spurious uninitialized variable warning

w0xlt commented at 11:16 PM on June 2, 2026:

derivehdkey("m") currently leaves the origin fingerprint as [00000000] because the fingerprint is only populated inside the child-derivation loop, which does not run for an empty path.

diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 9cae5b8ecd..1754d26cfa 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -1386,14 +1386,11 @@ std::optional<std::pair<CExtKey, KeyOriginInfo>> DeriveExtKey(const CExtKey& ext
     CExtKey descendant = ext_key;
     KeyOriginInfo origin;
     origin.clear(); // Prevent spurious uninitialized variable warning
+    const CKeyID id = ext_key.key.GetPubKey().GetID();
+    std::copy(id.begin(), id.begin() + sizeof(origin.fingerprint), origin.fingerprint);
     origin.path = path;
-    bool first = true;
     for (uint32_t i : path) {
         if (!descendant.Derive(descendant, i)) return std::nullopt;
-        if (first) {
-            memcpy(origin.fingerprint, descendant.vchFingerprint, 4);
-            first = false;
-        }
     }
     return std::make_pair(descendant, origin);
 }
diff --git a/src/rpc/util.h b/src/rpc/util.h
index d00920a9eb..70bde19dfe 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -159,7 +159,7 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
 //! Get extended key and origin info for a given path
 //! [@param](/bitcoin-bitcoin/contributor/param/)[in] ext_key The extended private key to derive from
 //! [@param](/bitcoin-bitcoin/contributor/param/)[in] path The BIP 32 path
-//! [@return](/bitcoin-bitcoin/contributor/return/) the resulting extended private key and origin info (blank if path is empty)
+//! [@return](/bitcoin-bitcoin/contributor/return/) the resulting extended private key and origin info
 std::optional<std::pair<CExtKey, KeyOriginInfo>> DeriveExtKey(const CExtKey& ext_key, const std::vector<uint32_t>& path);
 
 //! Parse BIP32 path
diff --git a/test/functional/wallet_derivehdkey.py b/test/functional/wallet_derivehdkey.py
index 118726b94b..0bedc985a8 100755
--- a/test/functional/wallet_derivehdkey.py
+++ b/test/functional/wallet_derivehdkey.py
@@ -35,6 +35,8 @@ class WalletDeriveHDKeyTest(BitcoinTestFramework):
         xpub_info = wallet.derivehdkey("m")
         assert "xprv" not in xpub_info
         xpub = xpub_info["xpub"]
+        root_fingerprint = wallet.derivehdkey("m/0")["origin"][1:9]
+        assert_equal(xpub_info["origin"], f"[{root_fingerprint}]")
 
         xpub_info = wallet.derivehdkey("m", private=True)
         xprv = xpub_info["xprv"]

Sjors commented at 6:54 AM on June 24, 2026:

5cedf369d976da64d022c377f9e75cfa0b5978c5: I think this lets me drop origin.clear() too, so I'll try that.

in src/wallet/rpc/wallet.cpp:1030 in a45777992b

1025 | +                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Private key for %s is not known", EncodeExtPubKey(xpub)));
1026 | +            }
1027 | +            CExtKey xprv(xpub, *key);
1028 | +
1029 | +            std::optional<std::pair<CExtKey, KeyOriginInfo>> child{DeriveExtKey(xprv, path)};
1030 | +            CHECK_NONFATAL(child);

w0xlt commented at 11:34 PM on June 2, 2026:

Could derivehdkey() return RPC_INVALID_PARAMETER when DeriveExtKey() returns null for a user-provided path, such as one exceeding BIP32’s maximum depth?

That would avoid surfacing invalid input through CHECK_NONFATAL(), which looks more like an internal bug path.

diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index f086ead493..87a860dbf8 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -1027,7 +1027,9 @@ RPCMethod derivehdkey()
             CExtKey xprv(xpub, *key);
 
             std::optional<std::pair<CExtKey, KeyOriginInfo>> child{DeriveExtKey(xprv, path)};
-            CHECK_NONFATAL(child);
+            if (!child) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Unable to derive HD key at the requested path");
+            }
 
             UniValue res{UniValue::VOBJ};
 
diff --git a/test/functional/wallet_derivehdkey.py b/test/functional/wallet_derivehdkey.py
index 118726b94b..13220a5aba 100755
--- a/test/functional/wallet_derivehdkey.py
+++ b/test/functional/wallet_derivehdkey.py
@@ -35,6 +35,13 @@ class WalletDeriveHDKeyTest(BitcoinTestFramework):
         xpub_info = wallet.derivehdkey("m")
         assert "xprv" not in xpub_info
         xpub = xpub_info["xpub"]
+        too_deep_path = "m/" + "/".join(["0"] * 256)
+        assert_raises_rpc_error(
+            -8,
+            "Unable to derive HD key at the requested path",
+            wallet.derivehdkey,
+            too_deep_path,
+        )
 
         xpub_info = wallet.derivehdkey("m", private=True)
         xprv = xpub_info["xprv"]

w0xlt commented at 11:36 PM on June 2, 2026: contributor

A few review comments:

Sjors commented at 10:08 AM on June 5, 2026: member

I'll mark this draft pending #35436.

Sjors marked this as a draft on Jun 5, 2026

DrahtBot added the label Needs rebase on Jun 8, 2026

Sjors force-pushed on Jun 23, 2026

Sjors commented at 6:57 PM on June 23, 2026: member

I tried basing this on #35436, but the current version has too many issues. So instead I just rebased on master and applied @w0xlt's suggestions. Ready for review again.

(I do still think it's good to move implementation code out of RPC, but this can be done later)

Sjors marked this as ready for review on Jun 23, 2026

DrahtBot removed the label Needs rebase on Jun 23, 2026

in src/util/bip32.cpp:20 in 6e0efac98d outdated

  16 | @@ -17,7 +17,7 @@ bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypa
  17 |      std::stringstream ss(keypath_str);
  18 |      std::string item;
  19 |      bool first = true;
  20 | -    while (std::getline(ss, item, '/')) {
  21 | +    while (std::getline(ss, item, '/') || std::getline(ss, item, 'h')) {

Sjors commented at 7:03 AM on June 24, 2026:

6e0efac98d3325e9d699ad1a92bd2a5324fc41bd: I'm adding test coverage in the next push.

in src/wallet/rpc/wallet.cpp:1006 in dc57236474

1001 | +                        std::set<CPubKey> desc_pubkeys;
1002 | +                        std::set<CExtPubKey> desc_xpubs;
1003 | +                        w_desc.descriptor->GetPubKeys(desc_pubkeys, desc_xpubs);
1004 | +                        for (const CExtPubKey& xpub : desc_xpubs) {
1005 | +                            std::string desc_str;
1006 | +                            bool ok = desc_spkm->GetDescriptorString(desc_str, false);

Sjors commented at 7:18 AM on June 24, 2026:

In dc57236474f13ce3e62081bbb763dc90be1c9a13 rpc: add derivehdkey: I copy-pasted this check from gethdkeys, but it's not needed here. Will drop in the next push.

in src/wallet/rpc/wallet.cpp:1013 in dc57236474

1008 | +                            wallet_xpubs.emplace_back(xpub);
1009 | +                        }
1010 | +                    }
1011 | +
1012 | +                    if (wallet_xpubs.size() == 0) {
1013 | +                        throw JSONRPCError(RPC_INVALID_PARAMETER, "No active or unused(KEY) descriptor found");

Sjors commented at 7:26 AM on June 24, 2026:

In dc57236474f13ce3e62081bbb763dc90be1c9a13 rpc: add derivehdkey: I'm switching this to RPC_INVALID_ADDRESS_OR_KEY.

Sjors force-pushed on Jun 24, 2026

Sjors commented at 7:49 AM on June 24, 2026: member

The above inline comments were from before I pushed and announce some small changes. I also slightly improved the tutorial.

in src/wallet/rpc/wallet.cpp:954 in b3ec19f720

 949 | +            const std::shared_ptr<const CWallet> wallet = GetWalletForJSONRPCRequest(request);
 950 | +            if (!wallet) return UniValue::VNULL;
 951 | +
 952 | +            if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
 953 | +                throw JSONRPCError(RPC_WALLET_ERROR, "derivehdkey is not available for non-descriptor wallets");
 954 | +            }

rkrux commented at 10:21 AM on June 24, 2026:

In b3ec19f720f9ddfd77fe831aadb9c9f8c3ab1d76 "rpc: add derivehdkey"

diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index dc2fcdb207..c299a91f16 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -949,10 +949,6 @@ RPCMethod derivehdkey()
             const std::shared_ptr<const CWallet> wallet = GetWalletForJSONRPCRequest(request);
             if (!wallet) return UniValue::VNULL;
 
-            if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
-                throw JSONRPCError(RPC_WALLET_ERROR, "derivehdkey is not available for non-descriptor wallets");
-            }
-
             if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
                 // Watch-only wallets can't contain unused(KEY) descriptors
                 throw JSONRPCError(RPC_WALLET_ERROR, "derivehdkey is not available for watch-only wallets");

I think this check was relevant when the legacy wallets could be loaded, but it's not possible now and we should avoid this check in the wallet code now.

in doc/release-notes-32784.md:4 in b3ec19f720

   0 | @@ -0,0 +1,5 @@
   1 | +Wallet
   2 | +------
   3 | +
   4 | +- A new `derivehdkey` RPC is available to obtain an xpub or xpriv for any given BIP32 path.

rkrux commented at 11:22 AM on June 24, 2026:

In b3ec19f "rpc: add derivehdkey"

s/xpriv/xprv

in doc/release-notes-32784.md:5 in b3ec19f720

   0 | @@ -0,0 +1,5 @@
   1 | +Wallet
   2 | +------
   3 | +
   4 | +- A new `derivehdkey` RPC is available to obtain an xpub or xpriv for any given BIP32 path.
   5 | +  The hd key can then be imported as e.g. part of a multisig descriptor. (#22341)

rkrux commented at 11:25 AM on June 24, 2026:

In b3ec19f "rpc: add derivehdkey"

I feel this use case of making the core wallet more flexible in a multi-sig setup can be mentioned in some detail here.

Sjors commented at 12:54 PM on June 24, 2026:

I expanded the note and also referenced the tutorial.

in src/wallet/rpc/wallet.cpp:970 in b3ec19f720

 965 | +
 966 | +            LOCK(wallet->cs_wallet);
 967 | +
 968 | +            // Standard derivation paths from the HD seed contain at least one
 969 | +            // hardened step, so just always unlock the wallet.
 970 | +            EnsureWalletIsUnlocked(*wallet);

rkrux commented at 11:35 AM on June 24, 2026:

In b3ec19f "rpc: add derivehdkey"

The non-standard paths can contain zero hardened steps. I believe it's trivial to check for hardened-ness in the derivation path so that we can unlock the wallet conditionally? In case of zero hardened steps, the private keys need not be brought into the picture.

Sjors commented at 11:52 AM on June 24, 2026:

Do we know a single use case for a path that's unhardened from the seed? It just seems unsafe and not worth supporting.

rkrux commented at 12:06 PM on June 24, 2026:

Do we know a single use case for a path that's unhardened from the seed?

I'm not aware.

It just seems unsafe and not worth supporting.

Nice, not supporting gels well with the other suggestion #32784 (review) where we will mandate hardened steps in the path.

in src/wallet/rpc/wallet.cpp:933 in b3ec19f720

 928 | +        + HELP_REQUIRING_PASSPHRASE,
 929 | +        {
 930 | +            {"path", RPCArg::Type::STR, RPCArg::Optional::NO, "BIP 32 derivation path."},
 931 | +            {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", {
 932 | +                {"private", RPCArg::Type::BOOL, RPCArg::Default{false}, "Show private key"},
 933 | +                {"hdkey", RPCArg::Type::STR, RPCArg::DefaultHint{"Either the HD key used by all other active descriptors, or an unused(KEY) descriptor"}, "The HD key that the wallet knows the private key of, listed using 'gethdkeys', to use for derivation"},

rkrux commented at 11:38 AM on June 24, 2026:

In b3ec19f "rpc: add derivehdkey"

Regarding the option of displaying the derived/child private key: I'm concerned about the accidental leakage of the parent/root private key in case a non-hardened derivation path is used. The risk is non-obvious, that's why listdescriptors private=True also doesn't display the child private key, but it instead shows the master private key with the derivation path (which as well should not be an option here for this RPC I believe).

https://github.com/bitcoin/bitcoin/blob/d84fc352cbc1363df5cd6024a22e73fc63283e4f/src/wallet/scriptpubkeyman.cpp#L1560-L1565

If the sole motivation of this PR is to make the core wallet flexible in multi-sig setups where only the xpubs need to be shared with other participants, then let's not provide the private option altogether?

Either the HD key used by all other active descriptors, or an unused(KEY) descriptor"

If only the unused descriptor was considered, then this risk could have been smaller but given that the HD key used by all other active descriptors is also considered, then I feel the risk is quite high.

rkrux commented at 11:39 AM on June 24, 2026:

If there is any other reason to display the child private key, then I really feel we should mandate hardened-ness in the derivation path.

Sjors commented at 11:58 AM on June 24, 2026:

then let's not provide the private option altogether?

You need the xprv when importing a descriptor where Core is one of the signers. Unfortunately we don't recognize an imported xpub as our own when it derives from our hd key. That requires something like https://github.com/Sjors/bitcoin/pull/91/changes/9f57f8705b71e06c868a59d28cb2998976df6162.

we should mandate hardened-ness in the derivation path

That's fine by me.

Sjors commented at 12:54 PM on June 24, 2026:

I introduced a HasHardenedDerivation helper (extracted from descriptor code) and use it to enforce that there's at least one hardened derivation.

rkrux commented at 1:10 PM on June 24, 2026:

You need the xprv when importing a descriptor where Core is one of the signers. Unfortunately we don't recognize an imported xpub as our own when it derives from our hd key. That requires something like https://github.com/Sjors/bitcoin/commit/9f57f8705b71e06c868a59d28cb2998976df6162.

Can we add that commit in this PR or raise a separate PR containing that commit (making this one dependent on that one)? I'd prefer to avoid returning any kind of private key (master or derived) just for setting up a multi-sig wallet where the core wallet is one of the signers - I'm not a proponent of making the user handle their private keys manually. It seems quite reasonable for the wallet to detect any derived xpub that's being imported.

Sjors commented at 2:35 PM on June 24, 2026:

I don't think we should make this PR more complicated. I'll look into making it a standalone PR, but would prefer to not make this one dependent on it.

We generally allow exporting private keys. Note that the multisig tutorial only uses the xpub, so we're not encouraging it. I'd like to eventually add an example of using a Bitcoin Core hot key, but that requires additional changes (e.g. #33112).

Sjors commented at 2:39 PM on June 24, 2026:

Also it doesn't seem like 9f57f8705b71e06c868a59d28cb2998976df6162 is enough, we probably also need 0a11232c137ceb6c5b9f4a894baff8d1d1042b13, 90ae3563559e1516807c2d46e74432c378629fb0 and 7bde38b0fde9f91821e7b9cb011761c17d96e58c. All of which are heavily vibed coded bits of #91, so they'll need cleanup in any case. You're welcome to give it a try by the way.

rkrux commented at 3:01 PM on June 24, 2026:

If multiple commits are required, then I guess it's fine to not increase the diff of this PR.

in src/wallet/rpc/wallet.cpp:981 in b3ec19f720

 976 | +                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unable to parse HD key. Please provide a valid xpub");
 977 | +                }
 978 | +            }
 979 | +
 980 | +            // If hdkey was not specified, try to look it up. First consider the
 981 | +            // active descriptors. Otherwise look for an unused(KEY) descriptor.

rkrux commented at 11:43 AM on June 24, 2026:

In b3ec19f "rpc: add derivehdkey"

Can we switch the order here? I'd prefer to use the unused descriptors (if present) instead of combining it with the main hd key that powers all the active descriptors. This might encourage the usage of the addhdkey RPC as well while providing a clean separation.

Sjors commented at 12:55 PM on June 24, 2026:

Done, that makes much more sense.

in src/rpc/util.cpp:1379 in 43c7770b63 outdated

1375 | @@ -1374,6 +1376,18 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
1376 |      return ret;
1377 |  }
1378 |  
1379 | +std::optional<std::pair<CExtKey, KeyOriginInfo>> DeriveExtKey(const CExtKey& ext_key, const std::vector<uint32_t>& path) {

rkrux commented at 11:46 AM on June 24, 2026:

In 43c7770b63b46c06d27a09616f13c6178034c8cc "rpc: add DeriveExtKey() helper"

This seems like a method that could prove to be generic in the future. Can we put this in src/key.cpp instead so that its presence is highlighted - it doesn't need to be a part of the CExtKey class. I'm unable to think why its presence should be restricted within src/rpc/ only.

Sjors commented at 12:02 PM on June 24, 2026:

src/key.cpp is part of the kernel, which we shouldn't add things to that aren't needed for validation. So we'd have to add src/common/key.{h,cpp}. I think that's better done as a refactor if someone wants to reuse this outside of RPC.

rkrux commented at 12:19 PM on June 24, 2026:

src/key.cpp is part of the kernel, which we shouldn't add things to that aren't needed for validation

I checked in src/kernel/CMakeLists.txt that only src/pubkey.cpp is a part of it and not src/key.cpp. Which kind of makes sense(?) because validation requires the Verify method that belongs in CPubKey, and not in CKey/CExtKey.

https://github.com/bitcoin/bitcoin/blob/d84fc352cbc1363df5cd6024a22e73fc63283e4f/src/kernel/CMakeLists.txt#L47

Sjors commented at 1:03 PM on June 24, 2026:

Good point, let me see if I can move it... (just missed this in the last push)

Sjors commented at 1:30 PM on June 24, 2026:

Done in 0973920308c300d2584ff2ddb4c06e83c8d09fc6, also added a direct test.

rkrux commented at 11:47 AM on June 24, 2026: contributor

Partial code review ff0ade21fc67e9a7ec84f30ea2bea81d5946a2e5

The above inline comments were from before I pushed and announce some small changes.

Can these comments be resolved now? They came across confusing to me at the first glance.

Sjors force-pushed on Jun 24, 2026

key: add DeriveExtKey() helper

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>

0973920308

Sjors force-pushed on Jun 24, 2026

DrahtBot added the label CI failed on Jun 24, 2026

Sjors commented at 1:31 PM on June 24, 2026: member

Ok, this should address the last review round: #32784#pullrequestreview-4561263861

DrahtBot removed the label CI failed on Jun 24, 2026

in src/wallet/rpc/wallet.cpp:1022 in 63450ca4b2

1017 | +
1018 | +                    xpub = *active_xpubs.begin();
1019 | +                }
1020 | +            }
1021 | +
1022 | +            std::optional<CKey> key{wallet->GetKey(xpub.pubkey.GetID())};

pseudoramdom commented at 11:27 PM on June 25, 2026:

In 63450ca4b218aede226e00df5df5a0267f9f76b6 rpc: add derivehdkey

IIUC this is checking if the wallet has the private key for xpub.pubkey. Should we verify that the supplied xpub exactly matches one of the wallet’s known descriptor xpubs before using it.

Sjors commented at 5:51 PM on June 26, 2026:

CExtPubKey xpub comes from the hdkey argument of the RPC call. It's the xpub for the master xprv. It's not expected to appear in regular wallet descriptors, because those use xpubs at a deeper derivation path (usually account level). The only exception is the new unused() descriptor.

However it does make sense to require that the xpub matches either an unused() descriptor or an xpub corresponding to an hd key. I added that check plus a test (63450ca4b218aede226e00df5df5a0267f9f76b6 -> ...). I first extracted two news helpers: GetExtKey in b40725823822275139315631e9f9f914eb0c87c0 and GetHDPubKeys in f488a7711074001523b7ab6f97fc7e173c1f059f.

Sjors force-pushed on Jun 26, 2026

Sjors commented at 5:51 PM on June 26, 2026: member

Addressed @pseudoramdom's feedback #32784 (review). This adds a safety check and gets rid of code duplication in the process. I also expanded the PR description.

Some commits could be a standalone refactor pull request if the review burden is too much, but they all towards the goal of adding derivehdkey.

in src/rpc/util.cpp:1383 in d0b9579c08 outdated

1374 | @@ -1374,6 +1375,15 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
1375 |      return ret;
1376 |  }
1377 |  
1378 | +std::vector<uint32_t> ParsePathBIP32(const std::string& path)
1379 | +{
1380 | +    std::vector<uint32_t> out;
1381 | +    if (!ParseHDKeypath(path, out)) {
1382 | +        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid BIP32 keypath");
1383 | +    }

w0xlt commented at 12:33 AM on July 1, 2026:

ParsePathBIP32() currently accepts m/2147483648, which is 0x80000000. Since BIP32 uses that high bit to mark hardened derivation, the RPC ends up treating this as m/0h instead of rejecting it.

Suggestion:

diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index ab27a84a67..d1770f0bdf 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -29,6 +29,7 @@
 
 #include <algorithm>
 #include <iterator>
+#include <sstream>
 #include <string_view>
 #include <tuple>
 #include <utility>
@@ -1381,6 +1382,25 @@ std::vector<uint32_t> ParsePathBIP32(const std::string& path)
     if (!ParseHDKeypath(path, out)) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid BIP32 keypath");
     }
+
+    // BIP32 child indices must fit in 31 bits; the high bit is reserved for
+    // the hardened derivation marker.
+    std::stringstream ss(path);
+    std::string item;
+    bool first{true};
+    while (std::getline(ss, item, '/')) {
+        if (first && item == "m") {
+            first = false;
+            continue;
+        }
+        if (item.ends_with('\'') || item.ends_with('h')) item.pop_back();
+
+        const auto number{ToIntegral<uint32_t>(item)};
+        if (!number || *number > 0x7fffffffU) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid BIP32 keypath");
+        }
+        first = false;
+    }
     return out;
 }
 
diff --git a/test/functional/wallet_derivehdkey.py b/test/functional/wallet_derivehdkey.py
index 5ed8cc0eb0..32287c9464 100755
--- a/test/functional/wallet_derivehdkey.py
+++ b/test/functional/wallet_derivehdkey.py
@@ -44,6 +44,13 @@ class WalletDeriveHDKeyTest(BitcoinTestFramework):
             wallet.derivehdkey,
             "m/87/0/0",
         )
+        for path in ["m/2147483648", "m/2147483648h", "m/2147483648'"]:
+            assert_raises_rpc_error(
+                -8,
+                "Invalid BIP32 keypath",
+                wallet.derivehdkey,
+                path,
+            )
         assert_raises_rpc_error(
             -5,
             "No active or unused(KEY) descriptor found",

</details>

Sjors commented at 10:03 AM on July 1, 2026:

Good catch! Fixed in f086c97ab56d04f48297cb0ec1029ff61f2ff772 (in bip32.cpp)

test: move parse_hd_keypath test to bip32_tests

ParseHDKeypath() lives in util/bip32, so its unit test belongs in
bip32_tests rather than psbt_wallet_tests. Pure move, no changes to the
test itself; subsequent commits extend it in its new home.

58f484d2dc

Have ParseHDKeypath handle h derivation marker e9accda0de

util: reject out-of-range BIP32 keypath indices

ParseHDKeypath() parsed each path element with ToIntegral<uint32_t>, so
a bare decimal >= 2^31 (e.g. "m/2147483648" == 0x80000000) was silently
treated as "m/0h".

This commit rejects such overflow instead.

f086c97ab5

rpc: ParsePathBIP32 helper fcd155b37d

refactor: add hardened derivation helper e34e643dbb

wallet: generalize GetActiveHDPubKeys helper

GetHDPubKeys() centralizes the descriptor xpub lookup used by gethdkeys and
createwalletdescriptor, and by the derivehdkey RPC added in a later commit.
The HDKeyFilter argument serves gethdkeys' active_only mode (Active vs All)
and createwalletdescriptor's active descriptor selection.

No behavior change, except the dynamic_cast now uses Assert() instead of
gethdkeys' CHECK_NONFATAL, since it is not a recoverable input.

833ec08908

wallet: add GetExtKey helper

Reconstruct a descriptor's extended private key from its xpub by looking
up the corresponding private key. This is the extended-key analog of
GetKey().

It is used here to simplify gethdkeys, and again by the derivehdkey RPC
introduced in a later commit.

f6cb402db9

rpc: add derivehdkey

Add an UnusedKey filter to GetHDPubKeys() so the new RPC can prefer
unused(KEY) descriptors before falling back to active descriptors.

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>

04f653f7f6

test: use derivehdkey in M-of-N multisig demo

Use derivehdkey instead of extracting each participant xpub (and
derivation info) from  the listdescriptors output.

Additionally use the new <0;1> descriptor syntax.

Finally this commits adds a few debug log lines, and expand the
explanation for why we use m/44h/1h/0h.

dbe9c2850b

doc: use derivehdkey in multisig tutorial

Use derivehdkey instead of extracting each participant xpub
from  the listdescriptors output.

Additionally use the new <0;1> descriptor syntax.

176213b93b

Sjors force-pushed on Jul 1, 2026

Sjors commented at 10:07 AM on July 1, 2026: member

Added two commits:

58f484d2dc8126b4b73f3418dbda8c8181cca945 test: move parse_hd_keypath test to bip32_tests : since this PR makes a bunch of changes to it, might as well move it to a more appropriate file
- ParseHDKeypath used to live in wallet/rpcwallet.cpp in #13557 / #13712
27a50f76ca571ff42094a203ef69f6a3887cb743 util: reject out-of-range BIP32 keypath indices : fixes #32784 (review)

fuzz: check ParseHDKeypath/WriteHDKeypath round-trip de552d1e03