rpc, wallet: addhdseed, infer seed when importing descriptor with xpub #23544

Sjors · 2021-11-18T16:40:53Z

Builds on top of #25907 and one commit from #22341.

First this PR introduces a descriptor wallet RPC addhdseed. Similar to its legacy wallet counterpart sethdseed it either generates a fresh random seed, for use on a blank wallet, or the user can provide a WIF. Unlike sethdseed this RPC call does not add keys / fill the keypool.

This allows a user to create a blank wallet (createwallet), provide it with a seed (addhdseed) and then craft custom descriptors. (An alternative approach would to add a nodescriptors argument to createwallet, but this RPC seems useful anyway)

An example use case of this is setting up a multisig between Core and a hardware wallet. After adding the seed to an otherwise empty wallet, the user would call getxpub m/87'/0'/0'. They would then combine this xpub and origin info with the one from their hardware wallet (e.g. using hwi --fingerprint .... getxpub m/87'/0'/0' and craft a descriptor like wsh(sorted_multi(2, [...../87'/0'/0']our_xpub, [..../87'/0'/0']their_xpub)), and import that. The resulting wallet will be able to sign its part of the transaction.

The second part of this PR enables to ability descriptors, such as in the multisig example above, in such a way that any fingerprint + xpub that is covered by our hd seed(s) can be signed for.

DrahtBot · 2021-11-18T23:20:23Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Rspigler

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin-core/gui/650 (Add Import to Wallet GUI by KolbyML)
#26627 (wallet: Migrate non-HD keys with single combo containing a list of keys by achow101)
#26596 (wallet: Migrate legacy wallets to descriptor wallets without requiring BDB by achow101)
#26573 (Wallet: don't underestimate the fees when spending a Taproot output by darosior)
#26567 (Wallet: estimate the size of signed inputs using descriptors by darosior)
#26462 (wallet: fix crash on loading descriptor wallet containing legacy key type entries by theStack)
#26008 (wallet: cache IsMine scriptPubKeys to improve performance of wallets with a lot of non-ranged descriptors by achow101)
#25991 (Wallet: Add foreign_outputs metadata to support CoinJoin transactions by luke-jr)
#25766 (wallet: Include a signature with encrypted keys to mitigate a wallet scam by achow101)
#25680 (rpc, docs: Add note for commands that supports only legacy wallets by yusufsahinhamza)
#24914 (wallet: Load database records in a particular order by achow101)
#24897 ([Draft / POC] Silent Payments by w0xlt)
#22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
#22341 (rpc: add getxpub by Sjors)

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.

Rspigler · 2021-11-25T20:15:01Z

Concept ACK! Thanks for continuing this work

Rspigler · 2021-12-22T15:08:49Z

So cool! 🚀

I tried testing this as far as I could.

First Node:

Created an empty wallet.

listdescriptors:

{
"wallet_name": "1",
"descriptors": [
]
}

✔️

addhdseed:

{
"fingerprint": "2aba6847"
}

getxpub "m/87'/0'/0'":

{
"xpub": "xpub6Bp9vBsEbMipghoCm7fnWbSvXc9gVyZS5Duzfc8WgrEEbCC8mDeg8YGHAtihrLg1XdjJT1qKYgM4Q7REz8B7zgoH6mWEHdMPDwMCihsbSgK",
"fingerprint": "2aba6847",
"origin": "[2aba6847/87'/0'/0']"
}

Now on to second node, since I don't have any HWW:

Created an empty wallet:

listdescriptors:

{
"wallet_name": "2",
"descriptors": [
]
}

✔️

addhdseed:

{
"fingerprint": "ad8a04e6"
}

getxpub "m/87'/0'/0'":

{
"xpub": "xpub6DR65ocDPDwTphuxNogrNSWWsmUnFEpBWjhZABGxupZ65st8kaSWVmFripasF8in6JELXPNPbjwyqE8s7dqiGSYyrE9k5UGDF4XQCSALkqi",
"fingerprint": "ad8a04e6",
"origin": "[ad8a04e6/87'/0'/0']"
}

Now back to first node, to combine into descriptor:

wsh(sorted_multi(2,[2aba6847/87'/0'/0']xpub6Bp9vBsEbMipghoCm7fnWbSvXc9gVyZS5Duzfc8WgrEEbCC8mDeg8YGHAtihrLg1XdjJT1qKYgM4Q7REz8B7zgoH6mWEHdMPDwMCihsbSgK,[ad8a04e6/87'/0'/0']xpub6DR65ocDPDwTphuxNogrNSWWsmUnFEpBWjhZABGxupZ65st8kaSWVmFripasF8in6JELXPNPbjwyqE8s7dqiGSYyrE9k5UGDF4XQCSALkqi))

I tried various formats/syntax for importdescriptors, but all failed. I'm sure it is something I did on my end. Can you help? For example:

importdescriptors '[{ "desc": "wsh(sorted_multi(2,[2aba6847/87'/0'/0']xpub6Bp9vBsEbMipghoCm7fnWbSvXc9gVyZS5Duzfc8WgrEEbCC8mDeg8YGHAtihrLg1XdjJT1qKYgM4Q7REz8B7zgoH6mWEHdMPDwMCihsbSgK,[ad8a04e6/87'/0'/0']xpub6DR65ocDPDwTphuxNogrNSWWsmUnFEpBWjhZABGxupZ65st8kaSWVmFripasF8in6JELXPNPbjwyqE8s7dqiGSYyrE9k5UGDF4XQCSALkqi))>", "timestamp":now, "internal": true }

Sjors · 2021-12-23T02:41:58Z

@Rspigler in your examples the descriptors you importing are missing a derivation rule after the xpub, e.g. xpub.../0/* and they're missing a checksum (add a wrong one and it will tell you the right answers, e.g. #00000000). Also the > seems out of place.

Btw, you could try running the Trezor or ColdCard emulator, it'll work with HWI.

Rspigler · 2021-12-26T18:53:54Z

Those were silly mistakes, sorry.

Tried this:

importdescriptors '[{ "desc": "wsh(sorted_multi(2,[3c9567fe/87'/0'/0'/0/*]xpub6DJT4N7LcSaLhKPSRkczg1nfD5z2wy1Kw48LAZHGTKDdWcjCc95wdRwQogYUX7M4fyzcXo3wQUrTH86RJ5w8cZ6T6nYdWe6guBN8SEBkPyD,[ad8a04e6/87'/0'/0'/0/*]xpub6DR65ocDPDwTphuxNogrNSWWsmUnFEpBWjhZABGxupZ65st8kaSWVmFripasF8in6JELXPNPbjwyqE8s7dqiGSYyrE9k5UGDF4XQCSALkqi))#00000000", "timestamp": now, "internal": "true" }]'

But still received Error: Error parsing JSON error code.

(The first descriptor is different because I re-compiled on Node 1)

Sjors · 2021-12-28T13:47:04Z

That's still the wrong format. The final /0/* needs to be after the xpub. And "now" is missing quotes, whereas true should not be in quotes, which is probably what triggers the JSON parsing error. (I know it's tedious). You probably also want to set internal to false if you want to generate receive addresses (true is used for change addresses)

Rspigler · 2022-01-07T01:19:09Z

Thanks for being patient with me.

Still getting errors but will continue to work on this myself rather than crowd the PR.

Key management will be done entirely by KeyManager, so DescriptorScriptPubKeyMan does not need key loading functions.

We will need access to a function that sets up a singular DescriptorSPKM, so refactor this out of the multiple DescriptorSPKM setup function.

Descriptor wallets store an HD master key that is used for new automatically generated descriptors. sethdseed is an existing RPC that can be repurposed to allow the users to set that HD key whenever they want. Also fixes the whitespace of sethdseed. Best to review this with --ignore-all-space

DrahtBot · 2022-12-05T22:51:57Z

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot · 2023-03-05T00:57:01Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

DrahtBot · 2023-06-03T01:19:56Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Sjors · 2023-08-01T14:15:20Z

This is very out of date and it's not on my priority list at the moment. Happy to review if someone else takes it over.

Sjors force-pushed the 2021/11/no_descriptors branch from bce34bd to bc3da28 Compare November 18, 2021 16:52

DrahtBot added Build system Descriptors RPC/REST/ZMQ Wallet labels Nov 18, 2021

Sjors force-pushed the 2021/11/no_descriptors branch from bc3da28 to d7f1068 Compare November 18, 2021 20:03

fanquake removed the Build system label Dec 10, 2021

DrahtBot added the Needs rebase label Dec 13, 2021

Sjors force-pushed the 2021/11/no_descriptors branch from d7f1068 to 96913f9 Compare December 30, 2021 08:56

DrahtBot removed the Needs rebase label Dec 30, 2021

DrahtBot mentioned this pull request Dec 30, 2021

docs: avoid C-style casts; use modern C++ casts #23810

Merged

Sjors mentioned this pull request Jan 6, 2022

[RFC] Stop automatically creating legacy and p2sh-segwit descriptors #23831

Open

DrahtBot added the Needs rebase label Jan 11, 2022

achow101 and others added 12 commits October 17, 2022 10:58

walletdb: Implement upgrading a wallet to use KeyManager

f7fbd0a

descspkm: Remove unneeded key loading

aee80e5

Key management will be done entirely by KeyManager, so DescriptorScriptPubKeyMan does not need key loading functions.

rpc: Add getxpub command

c9af030

wallet: Refactor function for single DescSPKM setup

e8d2afd

We will need access to a function that sets up a singular DescriptorSPKM, so refactor this out of the multiple DescriptorSPKM setup function.

wallet, descspkm: Move GetID into WalletDescriptor

60afd5b

wallet, descspkm: Refactor wallet descriptor generation to static func

b27e5d6

walletdb: Allow overwriting keyman active hd key

322db47

wallet, rpc: Add createwalletdescriptor RPC

3b42bd6

wallet: Test for descriptor wallet sethdseed and createwalletdescriptor

b2c3dee

wallet: GetExtPubKey()

cfb3af5

wallet: match xpub in imported descriptor to existing seed

7841583

Sjors force-pushed the 2021/11/no_descriptors branch from d097b70 to 7841583 Compare November 29, 2022 12:34

DrahtBot removed the Needs rebase label Nov 29, 2022

DrahtBot added the Needs rebase label Dec 5, 2022

S3RK mentioned this pull request Mar 29, 2023

wallet: add seeds argument to importdescriptors #27351

Closed

Sjors closed this Aug 1, 2023

ryanofsky mentioned this pull request Dec 20, 2023

wallet: Have the wallet store the key for automatically generated descriptors #26728

Closed

bitcoin locked and limited conversation to collaborators Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

rpc, wallet: addhdseed, infer seed when importing descriptor with xpub #23544

rpc, wallet: addhdseed, infer seed when importing descriptor with xpub #23544

Uh oh!

Sjors commented Nov 18, 2021 •

edited

Loading

Uh oh!

DrahtBot commented Nov 18, 2021 •

edited

Loading

Uh oh!

Rspigler commented Nov 25, 2021

Uh oh!

Rspigler commented Dec 22, 2021

Uh oh!

Sjors commented Dec 23, 2021 •

edited

Loading

Uh oh!

Rspigler commented Dec 26, 2021

Uh oh!

Sjors commented Dec 28, 2021 •

edited

Loading

Uh oh!

Rspigler commented Jan 7, 2022

Uh oh!

DrahtBot commented Dec 5, 2022

Uh oh!

DrahtBot commented Mar 5, 2023

Uh oh!

DrahtBot commented Jun 3, 2023

Uh oh!

Sjors commented Aug 1, 2023

Uh oh!

Uh oh!

rpc, wallet: addhdseed, infer seed when importing descriptor with xpub #23544

rpc, wallet: addhdseed, infer seed when importing descriptor with xpub #23544

Uh oh!

Conversation

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

Uh oh!

Uh oh!

DrahtBot commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

Rspigler commented Nov 25, 2021

Uh oh!

Rspigler commented Dec 22, 2021

Uh oh!

Sjors commented Dec 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rspigler commented Dec 26, 2021

Uh oh!

Sjors commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rspigler commented Jan 7, 2022

Uh oh!

DrahtBot commented Dec 5, 2022

Uh oh!

DrahtBot commented Mar 5, 2023

Uh oh!

DrahtBot commented Jun 3, 2023

Uh oh!

Sjors commented Aug 1, 2023

Uh oh!

Uh oh!

Sjors commented Nov 18, 2021 •

edited

Loading

DrahtBot commented Nov 18, 2021 •

edited

Loading

Sjors commented Dec 23, 2021 •

edited

Loading

Sjors commented Dec 28, 2021 •

edited

Loading