Have createwalletdescriptor auto-detect an unused(KEY) by Sjors · Pull Request #32861 · bitcoin/bitcoin

Sjors · 2025-07-03T08:55:40Z

The createwalletdescriptor was introduced in #29130 to let users add a tr() descriptor to an existing SegWit wallet. The new addhdkey method from #29136 introduces a new potential workflow: start from a blank wallet, generate an HD and then add only the descriptors you need, e.g.:

bitcoin rpc createwallet TaprootMaxi blank=true
bitcoin rpc addhdkey
bitcoin rpc createwalletdescriptor bech32m

Before this PR the last line would fail, requiring the user to call gethdkeys and copy-paste the xpub.

This PR makes createwalletdescriptor a bit smarter so it just finds the unused(KEY) generated by hdkey and uses that.

If multiple unused(KEY) descriptors are present the user still has to pick one.

A potential followup is to make our multisig tutorial slightly safer to use. Rather than creating a full wallet, the instruction could be changed to start with a blank wallet and only generate the default legacy descriptor. This avoids accidental use of single sig p2sh-segwit and bech32 addresses.

--

The first two commits are quick followups for #29136.

DrahtBot · 2025-07-03T08:55:45Z

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/32861.

Reviews

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

Type	Reviewers
Concept ACK	w0xlt, jeanpablojp
Approach ACK	adyshimony

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36133 (wallet: store multipath descriptor by Sjors)
#35436 (wallet: Add addHDkey interface by pseudoramdom)

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.

rkrux · 2025-07-18T09:38:11Z

This PR makes createwalletdescriptor a bit smarter so it just finds the unused(KEY) generated by hdkey and uses that.

Once the unused(KEY) descriptor is used, doesn't it become used? Keeping it as an unused descriptor later might come across as confusing.

rkrux · 2025-07-18T09:44:14Z

In e8b1440aa3ed7efe3a9c2d10afb05e7247fff1f6 "rpc: make createwalletdescriptor smarter"

Though this check here seems more thorough, but the presence of more than one spkm also seems sufficient to throw this error?

Sjors · 2025-07-18T09:50:28Z

Keeping it as an unused descriptor later might come across as confusing.

I agree, and it was also brought up here: #29136 (comment). It's orthogonal to this PR.

rkrux · 2025-07-18T10:19:13Z

Thanks, I recall reading this comment earlier but forgot about it later. If I am not missing anything, I don't suppose this point is orthogonal to this PR?

The linked comment also states that any RPC using the unused descriptor should delete it immediately afterwords.

add logic to importdescriptors and generatewalletdescriptor to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an new descriptor (taking into account both public and private parts of the key)

I don't believe generatewalletdescriptor is present as an RPC in the codebase, and createwalletdescriptor stores the newly created descriptor in the database as I verified from the below patch. I can see the unused descriptor present after this RPC as well. Shouldn't we add the deletion of the unused descriptor in this PR itself?

diff --git a/test/functional/wallet_createwalletdescriptor.py b/test/functional/wallet_createwalletdescriptor.py
index 6de0ca4782..a7086c5b9e 100755
--- a/test/functional/wallet_createwalletdescriptor.py
+++ b/test/functional/wallet_createwalletdescriptor.py
@@ -125,7 +125,11 @@ class WalletCreateDescriptorTest(BitcoinTestFramework):
 
         # Create unused(KEY) descriptor and try again
         w1.addhdkey()
+        print("listdescriptors: ", w1.listdescriptors())
+        print("gethdkeys: ", w1.gethdkeys())
         w1.createwalletdescriptor(type="bech32")
+        print("listdescriptors: ", w1.listdescriptors())
+        print("gethdkeys: ", w1.gethdkeys())
 
         self.nodes[0].createwallet("w2", blank=True)
         w2 = self.nodes[0].get_wallet_rpc("w2")
(END)

Sjors · 2025-07-25T07:12:28Z

The linked comment also states that any RPC using the unused descriptor should delete it immediately afterwords.

That should be done in #29136 which introduces createwalletdescriptor , or in a followup, but not in this PR (which consists of only the last two commits).

adyshimony · 2026-02-21T22:29:44Z

+    // Check both only have one pubkey
+    std::set<CPubKey> prv_pubkeys;
+    std::set<CExtPubKey> prv_extpubs;
+    parse_pub->GetPubKeys(prv_pubkeys, prv_extpubs);


Should this call use parse_priv instead of parse_pub?

Thanks, fixed in a new commit (even though it pertains to #29136).

adyshimony · 2026-02-21T22:44:38Z

+                }
+            }
+
+            LOCK(wallet->cs_wallet);


I think a duplicate check is needed. What if the wallet already has this xprv through an active descriptor and user calls addhdkey with the same xprv? The descriptor check would not catch that, because unused(xprv) is a different descriptor string. A check is needed here to prevent adding the same private key twice.

const CExtPubKey hd_xpub{hdkey.Nexuter()}; if (wallet->GetKey(hd_xpub.pubkey.GetID())) { throw JSONRPCError(RPC_WALLET_ERROR, "HD key already exists"); }

adyshimony · 2026-02-21T22:46:58Z

+        wallet = self.nodes[0].get_wallet_rpc("hdkey")
+
+        assert_equal(len(wallet.gethdkeys()), 1)
+


Cover the case where addhdkey is called with existing xprv. Expected behavior is to reject it as "HD key already exists"

existing_wallet_xprv = wallet.gethdkeys(private=True)[0]["xprv"] assert_raises_rpc_error(-4, "HD key already exists", wallet.addhdkey, existing_wallet_xprv)

adyshimony · 2026-02-21T22:57:40Z

Review and tested c8b6d4ee2abbd1271e44aa66da6079cee6323529

Concept ACK, Approach ACK.

I think I have found a small issue where a xprv already in wallet via active descriptors, and addhdkey(xprv) is called from RPC with the same xprv.

Suggested a fix in the review comments.

Sjors · 2026-05-29T08:49:44Z

Rebased after #28333.

w0xlt · 2026-06-08T21:12:55Z

Concept ACK

…tor' into HEAD Resolve the createwalletdescriptor conflict between bitcoin#32784 and bitcoin#32861 by keeping bitcoin#32861's fallback to unused KEY descriptors, but adapting the active descriptor lookup to bitcoin#32784's GetHDPubKeys(HDKeyFilter::Active) API and extracting the xpub from the map key.

Sjors · 2026-08-17T12:25:30Z

Rebased after #32784.

jeanpablojp

Concept ACK

I noticed that on a wallet with two active keys plus one from addhdkey, derivehdkey uses the unused one while createwalletdescriptor errors with "Unable to determine which HD key to use from active descriptors". Master does the same. But now that this RPC knows about unused(KEY), shouldn't that lookup also apply when there's more than one active key?

The hdkey hint is stale now too. It still says "The HD key used by all other active descriptors", where derivehdkey's mentions both cases.

jeanpablojp · 2026-08-24T22:38:13Z

                    throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unable to determine which HD key to use from active descriptors. Please specify with 'hdkey'");
+                } else {
+                    // Look for an unused(KEY) descriptor
+                    std::set<DescriptorScriptPubKeyMan*> spkms{pwallet->GetScriptlessSPKMs()};


GetHDPubKeys(HDKeyFilter::UnusedKey) already returns this set, and derivehdkey uses it further down in this same file. I swapped this block for a call to it, the wallet tests still pass and GetScriptlessSPKMs() goes away entirely. Is there a reason for the separate helper that I'm missing?

I dropped f336199, as it's indeed superseded by GetHDPubKeys(HDKeyFilter::UnusedKey) from #32784.

Co-authored-by: adyshimony <6388409+adyshimony@users.noreply.github.com>

When a wallet contains only an unused(KEY) descriptor, use it. Previously the user would have to call listdescriptors and manually specify it.

The participant wallets are only used to derive an xpub and sign PSBTs for the multisig wallet. Creating them blank and adding just the legacy descriptor avoids handing the reader singlesig addresses they are not supposed to use. The legacy descriptor can't be dropped entirely because the signing code only considers descriptors whose derivation paths cover the input. To further discourage use of the singlesig descriptors, the PSBT example now pays a separate recipient wallet rather than participant_1.

Sjors · 2026-08-25T08:12:47Z

The rule and help text is now:

The HD key used by all other active descriptors, or, if there are none, the HD key of an unused(KEY) descriptor.

In the ambiguous case @jeanpablojp describes, it's better to err on the side of failing, because descriptors can't be deleted. In that sense it's different from derivehdkey.

The hdkey hint is stale now too.

Fixed

I also added a commit to use improve the multisig tutorial.

This was referenced Jul 3, 2025

wallet: addhdkey RPC to add just keys to wallets via new unused(KEY) descriptor #29136

Merged

MuSig2 power Sjors/bitcoin#91

Open

DrahtBot mentioned this pull request Jul 14, 2025

Silent Payments: Receiving #32966

Draft

2 tasks

rkrux reviewed Jul 18, 2025

View reviewed changes

DrahtBot mentioned this pull request Jul 18, 2025

wallet: support bip388 policy with external signer #33008

Closed

4 tasks

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from e8b1440 to 6049553 Compare July 25, 2025 07:06

DrahtBot mentioned this pull request Jul 25, 2025

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

Merged

This was referenced Aug 5, 2025

wallet: warn against accidental unsafe older() import #33135

Merged

BIP360: Includes the following: #33163

Closed

DrahtBot mentioned this pull request Sep 12, 2025

Menu action to export a watchonly wallet bitcoin-core/gui#872

Merged

DrahtBot mentioned this pull request Sep 20, 2025

wallet, rpc: add UTXO set check and incremental rescan to importdescriptors #33392

Open

DrahtBot mentioned this pull request Nov 18, 2025

wallet: refactor ProcessDescriptorImport #33874

Closed

DrahtBot mentioned this pull request Dec 6, 2025

Added rescan option for import descriptors #31668

Open

DrahtBot added the Needs rebase label Jan 3, 2026

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from 6049553 to f2752ef Compare January 5, 2026 03:21

DrahtBot removed the Needs rebase label Jan 5, 2026

DrahtBot mentioned this pull request Jan 17, 2026

wallet: Fix MaxSatisfactionWeight calculation for Taproot descriptors #34314

Closed

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from f2752ef to c8b6d4e Compare February 2, 2026 15:53

adyshimony reviewed Feb 21, 2026

View reviewed changes

DrahtBot mentioned this pull request Mar 19, 2026

wallet: Add importdescriptors interface #34861

Open

DrahtBot added the Needs rebase label May 28, 2026

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from fb57795 to 13d807e Compare May 29, 2026 08:49

DrahtBot added CI failed and removed Needs rebase labels May 29, 2026

DrahtBot mentioned this pull request Jun 2, 2026

wallet: Add addHDkey interface #35436

Open

DrahtBot removed the CI failed label Jun 2, 2026

DrahtBot mentioned this pull request Jun 23, 2026

Remove boost as a unit test runner #35587

Closed

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from 13d807e to c4249c0 Compare July 3, 2026 18:17

sedited requested review from adyshimony and rkrux July 24, 2026 07:36

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from c4249c0 to fe1a296 Compare August 6, 2026 12:39

DrahtBot added the Needs rebase label Aug 15, 2026

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from fe1a296 to d46c58d Compare August 17, 2026 12:25

DrahtBot removed the Needs rebase label Aug 17, 2026

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from d46c58d to 9d8e530 Compare August 20, 2026 09:04

jeanpablojp reviewed Aug 24, 2026

View reviewed changes

Sjors and others added 5 commits August 25, 2026 09:33

test: check unused xprv descriptor pubkeys

331f876

Co-authored-by: adyshimony <6388409+adyshimony@users.noreply.github.com>

wallet: reject duplicate addhdkey xprvs

4daf376

Co-authored-by: adyshimony <6388409+adyshimony@users.noreply.github.com>

rpc: make createwalletdescriptor smarter

f36bf79

When a wallet contains only an unused(KEY) descriptor, use it. Previously the user would have to call listdescriptors and manually specify it.

doc: add release note for bitcoin#32861

9d4eb1f

Sjors force-pushed the 2025/07/smart-createwalletdescriptor branch from 9d8e530 to 9d4eb1f Compare August 25, 2026 08:12

DrahtBot mentioned this pull request Sep 1, 2026

wallet: store multipath descriptor #36133

Open

Sjors mentioned this pull request Sep 2, 2026

wallet: fill in the wallet's HD keys when importing a descriptor Sjors/bitcoin#127

Open

Sjors added a commit to Sjors/bitcoin that referenced this pull request Sep 3, 2026

Merge bitcoin#32861: wallet: smarter createwalletdescriptor

13fb992

		wallet = self.nodes[0].get_wallet_rpc("hdkey")

		assert_equal(len(wallet.gethdkeys()), 1)

Uh oh!

Conversation

Sjors commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

rkrux commented Jul 18, 2025

Uh oh!

rkrux Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Sjors commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkrux commented Jul 18, 2025

Uh oh!

Sjors commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adyshimony Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Sjors May 14, 2026

Choose a reason for hiding this comment

Uh oh!

adyshimony Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

adyshimony Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

adyshimony commented Feb 21, 2026

Uh oh!

Sjors commented May 29, 2026

Uh oh!

w0xlt commented Jun 8, 2026

Uh oh!

Sjors commented Aug 17, 2026

Uh oh!

jeanpablojp left a comment

Choose a reason for hiding this comment

Uh oh!

jeanpablojp Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Sjors Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Sjors commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Sjors commented Jul 3, 2025 •

edited

Loading

DrahtBot commented Jul 3, 2025 •

edited

Loading

Sjors commented Jul 18, 2025 •

edited

Loading

Sjors commented Jul 25, 2025 •

edited

Loading

Sjors commented Aug 25, 2026 •

edited

Loading