btc: descriptor wallet support #1659

chappjc · 2022-06-11T04:14:27Z

This works toward supporting Bitcoin descriptor wallets. Resolves #1601.

primitives for decoding descriptors and the keys they contain
RPC types and method updates for listdescriptors private=true and getaddressinfo
matching the 'hdkeypath' and key origin fingerprint from getaddressinfo with the extended private key from listdescriptors
updating (*rpcClient).privKeyForAddress to switch between either dumpprivkey or above approach depending on config
update BTC simnet harness - beta and gamma are now descriptor wallets

See https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md

chappjc · 2022-06-14T22:00:03Z

NOTE:

./alpha -named createwallet wallet_name=desc descriptors=true
cp alpha desc
# edit desc with "-rpcwallet=desc"

I'm contemplating either: (a) regenerating one of the alpha/beta/gamma/delta wallets to be a descriptor wallet, but I'm not excited about a big binary update of the chain data tarball, or (b) epsilon, yet another wallet. Wasn't another PR adding more?

dex/networks/btc/descriptors.go

client/asset/btc/rpcclient.go

buck54321

Can we create a descriptor wallet in the harness?

chappjc · 2022-06-18T13:43:22Z

Can we create a descriptor wallet in the harness?

Yes, question is this: #1659 (comment)

New "epsilon" wallet or regenerate harnesschain tarbal with say, beta, turned into a descriptor wallet? I wanted opinions on that choice.

chappjc · 2022-06-18T13:47:12Z

On second thought, regenerating the harness chain archive with v23 is likely to make it unusable for say v0.21. I think making another wallet in the shell script is best.

buck54321 · 2022-06-18T14:24:23Z

Can we create a descriptor wallet in the harness?

Yes, question is this: #1659 (comment)

New "epsilon" wallet or regenerate harnesschain tarbal with say, beta, turned into a descriptor wallet? I wanted opinions on that choice.

Oops. I should have seen that. I don't have a strong preference either way. Do we even need to commit the tarball? It's about 5 MB.

chappjc · 2022-06-18T14:28:31Z

Time to start harness with tarball: ~13 sec (not counting the history command at the end)
Time to start harness without tarball: ~22 sec
Not a big difference.
Think we should delete BTC's harnesschain.tar.gz?

buck54321 · 2022-06-18T14:39:26Z

Time to start harness with tarball: ~13 sec (not counting the history command at the end) Time to start harness without tarball: ~22 sec Not a big difference. Think we should delete BTC's harnesschain.tar.gz?

🤣 yeah.

chappjc · 2022-06-20T21:23:13Z

Not surprisingly, sethdseed does not work with descriptor wallets, so using importdescriptors with beta and gamma, which are now descriptor wallets.

The BTC harnesschain.tar.gz is now deleted. The harness directory has a version file to detect if it was extracted from out out-of-date archive.

Tested the BTC, LTC, and BCH harnesses. DOGE is independent, so unaffected.

client/asset/btc/btc.go

client/asset/btc/rpcclient.go

dex/testing/btc/harness.sh

JoeGruffins · 2022-06-22T06:32:59Z

I made this comment on another recent pr, but if someone were to make an internal descriptor wallet with this version of the client and then tried to go back to an earlier version of dexc, they could no longer use the wallet. I think there could be some version saved in the database that is upped to stop the user at start-up and tell them that they must use a newer dexc version.

chappjc · 2022-06-22T17:01:15Z

Review edits: https://github.com/decred/dcrdex/compare/41ab7777e1350aca04d079badd20dcc8abb736a9..34ae4ddc7255149b744f2ad513ef0ad35871e269

chappjc · 2022-06-22T17:04:01Z

I made this comment on another recent pr, but if someone were to make an internal descriptor wallet with this version of the client and then tried to go back to an earlier version of dexc, they could no longer use the wallet. I think there could be some version saved in the database that is upped to stop the user at start-up and tell them that they must use a newer dexc version.

Personally don't think that's needed. The check for descriptor wallets in older dexc also stops the user at start-up. I think the README can warn that if you use a descriptor wallet with 0.5, you won't be able to use it with 0.4.

buck54321

I've got my head around the basics now. This is looking good. I'll swing back on this within a day or two.

buck54321 · 2022-06-23T11:05:24Z

dex/networks/btc/descriptors.go

+	// Fingerprint is "Exactly 8 hex characters for the fingerprint of the key
+	// where the derivation starts".
+	Fingerprint string


Notably,

unsigned char fingerprint[4]; //!< First 32 bits of the Hash160 of the public key at the root of the path

https://github.com/bitcoin/bitcoin/blob/01e9e2d1ca7fc08d65663b398c71ecec6a01f686/src/script/keyorigin.h#L13

So one could conceivably devise a scheme where we don't do path examination of the listdescriptors true result, but instead just match a key to the fingerprint. I only mention this because when I run listdescriptors true on the beta wallet, I notice that the same private key is listed for all functions.

That's generally correct in practice. The search loop I have in the RPC code really could stop after finding the xprv with the matching fingerprint, and checking that it is indeed a private key instead of proceeding to match the path too.
I decided to match the paths since we are required to tolerate fingerprint collisions (although it's fairly unlikely), and the different address types are allowed to use descriptors with different fingerprints (you can import one of your choice just for say wpkh and make it 'active' even though bitcoind uses the same master for all by default)

Namely, right after IsPrivate passes here we could just go straight to DeepChild to get the private key and verify the public key matches. If the keys match, they match.

For reference, the fingerprint is computed below:

dcrdex/dex/networks/btc/descriptors.go

Lines 90 to 92 in b19f258

func keyFingerprint(key *hdkeychain.ExtendedKey) []byte {

return btcutil.Hash160(pubKeyBytes(key))[:4]

}

buck54321

Beautiful!

buck54321 · 2022-06-25T20:27:50Z

client/asset/btc/wallettypes.go

+	ParentDesc          string `json:"parent_desc"`         // e.g. "wpkh([b940190e/84'/1'/0']tpubDCo.../0/*)#xn4kr3dw" meaning range of external addresses
+	HDKeyPath           string `json:"hdkeypath"`           // e.g. "m/84'/1'/0'/0/0"
+	HDMasterFingerprint string `json:"hdmasterfingerprint"` // e.g. "b940190e"


Three fields aren't actually used.

buck54321 · 2022-06-25T20:28:51Z

client/asset/btc/wallettypes.go

+type listDescriptorsResult struct {
+	WalletName  string `json:"wallet_name"`
+	Descriptors []*struct {
+		Descriptor string `json:"desc"`      // public or private depending on private RPC arg


This is the only field actually used, I think.

buck54321 · 2022-06-25T21:42:33Z

dex/testing/btc/harness.sh

+# descriptors and no passphrase. $1 is the node to create the wallet on.
+# Possible arguments to createwallet are:
+# "wallet_name" ( disable_private_keys blank(boolean) "passphrase" avoid_reuse descriptors load_on_startup external_signer )
+export NEW_WALLET_CMD="./\$1 -named createwallet wallet_name=\$2 descriptors=false load_on_startup=true"


Would be good to have a descriptors=true alternative so we can test plumb into e2e and loadbot tests.

Added an optional arg

dex/networks/btc: descriptor parsing primitives This adds descriptor parsing functions and types. See https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md client/asset/btc: listdescriptors and get priv key This adds the RPC code to find the matching private key descriptor from the listdescriptors response to the address public key in the getaddressinfo response. Double check private key accessibility if wallet is unlocked. harness: make btc descriptor wallets This makes the beta and gamma wallets into descriptor wallets. This also removes the BTC test chain archive. The new-wallet script has an optional $3 arg for the descriptors bool.

chappjc mentioned this pull request Jun 13, 2022

dex/testing/btc: do not create descriptor wallets #1658

Closed

chappjc added this to the 0.5 milestone Jun 13, 2022

chappjc changed the title ~~dex/networks/btc: descriptor parsing~~ btc: descriptor wallet support Jun 13, 2022

chappjc force-pushed the descriptors branch 2 times, most recently from 39a40a8 to ed93468 Compare June 14, 2022 20:59

chappjc marked this pull request as ready for review June 14, 2022 21:12

chappjc force-pushed the descriptors branch from ed93468 to a3f4cb6 Compare June 16, 2022 02:26

martonp reviewed Jun 16, 2022

View reviewed changes

dex/networks/btc/descriptors.go Show resolved Hide resolved

dex/networks/btc/descriptors.go Show resolved Hide resolved

dex/networks/btc/descriptors.go Outdated Show resolved Hide resolved

client/asset/btc/rpcclient.go Outdated Show resolved Hide resolved

chappjc force-pushed the descriptors branch from a3f4cb6 to 9744b84 Compare June 16, 2022 18:57

buck54321 reviewed Jun 18, 2022

View reviewed changes

chappjc force-pushed the descriptors branch from 9744b84 to 92bed94 Compare June 20, 2022 21:18

chappjc commented Jun 20, 2022

View reviewed changes

client/asset/btc/btc.go Outdated Show resolved Hide resolved

JoeGruffins approved these changes Jun 22, 2022

View reviewed changes

client/asset/btc/rpcclient.go Outdated Show resolved Hide resolved

dex/testing/btc/harness.sh Outdated Show resolved Hide resolved

chappjc force-pushed the descriptors branch 2 times, most recently from 34ae4dd to 1147489 Compare June 22, 2022 17:01

buck54321 reviewed Jun 23, 2022

View reviewed changes

buck54321 approved these changes Jun 25, 2022

View reviewed changes

chappjc force-pushed the descriptors branch 2 times, most recently from 8c31e24 to 6d862ef Compare June 27, 2022 17:24

chappjc force-pushed the descriptors branch from 6d862ef to 9b1b68f Compare June 27, 2022 17:29

chappjc merged commit 5fb9657 into decred:master Jun 27, 2022

chappjc deleted the descriptors branch June 27, 2022 17:35

	func keyFingerprint(key *hdkeychain.ExtendedKey) []byte {
	return btcutil.Hash160(pubKeyBytes(key))[:4]
	}

btc: descriptor wallet support #1659

btc: descriptor wallet support #1659

Uh oh!

Conversation

chappjc commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chappjc commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buck54321 left a comment

Choose a reason for hiding this comment

Uh oh!

chappjc commented Jun 18, 2022

Uh oh!

chappjc commented Jun 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buck54321 commented Jun 18, 2022

Uh oh!

chappjc commented Jun 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buck54321 commented Jun 18, 2022

Uh oh!

chappjc commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoeGruffins commented Jun 22, 2022

Uh oh!

chappjc commented Jun 22, 2022

Uh oh!

chappjc commented Jun 22, 2022

Uh oh!

buck54321 left a comment

Choose a reason for hiding this comment

Uh oh!

buck54321 Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chappjc Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buck54321 left a comment

Choose a reason for hiding this comment

Uh oh!

buck54321 Jun 25, 2022

Choose a reason for hiding this comment

Uh oh!

buck54321 Jun 25, 2022

Choose a reason for hiding this comment

Uh oh!

buck54321 Jun 25, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chappjc commented Jun 11, 2022 •

edited

Loading

chappjc commented Jun 14, 2022 •

edited

Loading

chappjc commented Jun 18, 2022 •

edited

Loading

chappjc commented Jun 18, 2022 •

edited

Loading

chappjc commented Jun 20, 2022 •

edited

Loading

buck54321 Jun 23, 2022 •

edited

Loading

chappjc Jun 23, 2022 •

edited

Loading