wallet: derivehdkey RPC to get xpub at arbitrary path - #32784
Conversation
DrahtBot
changed the title
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32784. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
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:
Possible places where comparison-specific test macros should replace generic comparisons:
2026-08-07 13:13:06 |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
de7f5c6 to
3f35b02
Compare
There was a problem hiding this comment.
In util.h: “@params[in] path” → “@param[in] path” [Doxygen tag typo]
There was a problem hiding this comment.
addressesd -> addresses [extra “d” makes “addresses” misspelled]
|
Very nice, Concept ACK. |
380a57f to
017fb68
Compare
pseudoramdom
left a comment
pseudoramdom
left a comment
There was a problem hiding this comment.
Since #35606 has now merged, we can simplify the fingerprint handling using the new type-safe helpers.
| CExtKey descendant = ext_key; | ||
| KeyOriginInfo origin; | ||
| const CKeyID id = ext_key.key.GetPubKey().GetID(); | ||
| std::copy(id.begin(), id.begin() + sizeof(origin.fingerprint), origin.fingerprint); |
|
|
||
| KeyOriginInfo expected_origin; | ||
| const CKeyID id{master.key.GetPubKey().GetID()}; | ||
| std::copy(id.begin(), id.begin() + sizeof(expected_origin.fingerprint), expected_origin.fingerprint); |
There was a problem hiding this comment.
Ditto. We can use
expected_origin.fingerprint = master.id_key_fingerprint();| #include <test/util/setup_common.h> | ||
| #include <util/strencodings.h> | ||
|
|
||
| #include <algorithm> |
There was a problem hiding this comment.
This can be dropped if we use the id_key_fingerprint() suggestion
|
|
||
| UniValue res{UniValue::VOBJ}; | ||
|
|
||
| std::string fingerprint{HexStr(std::span<unsigned char>(child->second.fingerprint, child->second.fingerprint + 4))}; |
|
Rebased and switched to #35606 |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
The ci failure is unrelated, you can just add wno-error=... to the ci config. |
|
I think I'll just wait for #35896 to land (tested locally that it helps). |
| if (std::optional<CKey> key = priv ? desc_spkm->GetKey(xpub.pubkey.GetID()) : std::nullopt) { | ||
| wallet_xprvs[xpub] = CExtKey(xpub, *key); | ||
| wallet_xpubs[xpub].emplace(desc_str, wallet->IsActiveScriptPubKeyMan(*desc_spkm), desc_spkm->HasPrivKey(xpub.pubkey.GetID())); | ||
| } |
There was a problem hiding this comment.
Can we look up the private key directly from each associated desc_spkm inside the existing loop in RPCMethod gethdkeys()?
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index cfba900d22..aa9dae8eb7 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -704,10 +704,8 @@ RPCMethod gethdkeys()
bool ok = desc_spkm->GetDescriptorString(desc_str, /*priv=*/false);
CHECK_NONFATAL(ok);
wallet_xpubs[xpub].emplace(desc_str, wallet->IsActiveScriptPubKeyMan(*desc_spkm), desc_spkm->HasPrivKey(xpub.pubkey.GetID()));
- }
- if (priv) {
- if (std::optional<CExtKey> xprv = wallet->GetExtKey(xpub)) {
- wallet_xprvs[xpub] = *xprv;
+ if (std::optional<CKey> key = priv ? desc_spkm->GetKey(xpub.pubkey.GetID()) : std::nullopt) {
+ wallet_xprvs[xpub] = CExtKey{xpub, *key};
}
}
}There was a problem hiding this comment.
I dropped the rpc/wallet.cpp change in 99bea69.
| }, | ||
| }, | ||
| RPCExamples{ | ||
| HelpExampleCli("derivehdkey", "m/87h/0h/0h") + HelpExampleRpc("derivehdkey", "") |
There was a problem hiding this comment.
The parameter is required.
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index cfba900d22..33450695e1 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -979,7 +979,7 @@ RPCMethod derivehdkey()
},
},
RPCExamples{
- HelpExampleCli("derivehdkey", "m/87h/0h/0h") + HelpExampleRpc("derivehdkey", "")
+ HelpExampleCli("derivehdkey", "m/87h/0h/0h") + HelpExampleRpc("derivehdkey", "\"m/87h/0h/0h\"")
+ HelpExampleCliNamed("derivehdkey", {{"path", "m/87h/0h/0h"}, {"private", "true"}})
+ HelpExampleRpcNamed("derivehdkey", {{"path", "m/87h/0h/0h"}, {"private", "true"}})
},
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
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.
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.
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.
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() and is used by the derivehdkey RPC in the following commit.
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>
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.
Use derivehdkey instead of extracting each participant xpub from the listdescriptors output. Additionally use the new <0;1> descriptor syntax.
6 participants
Adds a
derivehdkeyRPC 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.mdand (updated) functional test to see how that workflow improves.The first commits are some helpful helpers:
psbt_wallet_testsParseHDKeypathwould previously map overflowing values withouthto hardened.HasHardenedDerivation(), to enforce the "at least one hardened step" rulegethdkeyswhichderivehdkeyneedsGetKey()); behavior-preserving prep, also simplifiesgethdkeys.Meat and potatoes:
UnusedKeyfilter onGetHDPubKeysthat drives key selection.<0;1>syntax.