WIP: descriptor request using "getkeys"

[Sjors]

Todo:

• implement and discuss descriptor requests in Bitcoin Core
• rebase on Descriptor support for displayaddress #117

Implements getkeys as proposed here.

Update 2019/02/02: this used to include #117, which I've moved to a new PR

[achow101]

Needs rebase as #66 has been merged.

[Sjors]

@achow101 rebased, but I'm not happy with signtransaction just yet. Right now it requires a fingerprint argument. Ideally this should be inferred from the PSBT, but I have no idea how. Also it currently hardcodes Testnet, which I would also like to infer from the PSBT.

[Sjors]

I removed the code that inferred Testnet from the BIP32 derivation, instead relying on --testnet being set.

I'm just leaving the --fingerprint argument required for signtransaction for now.

[Sjors]

Regarding global variables, I'm not sure what the right approach is.

I guess if we translate this to gRPC we could use POST /:fingerprint/signtransaction with {psbt: ....} as the payload.

However some methods don't need the fingerprint argument because it's in the descriptor. Unless we just require this redundantly: POST /:fingerprint/displayaddress with {descriptor: "wpkh([fingerprint]/84'/1/'/0'....)", "testnet": true}

Do we also want testnet to be global?

[instagibbs]

Can we mark this as WIP if the Core branch is unmerged?

[Sjors]

I narrowed the scope a bit, so hopefully it doesn't have to stay in WIP limbo:

• no longer supporting redundant ability to put global params at the end
• no longer renaming signtransaction to signtx

The only backwards incompatible changes are:

• psbt is now a named argument. If necessary I can change that so the position argument is optional but still possible.
• displayaddress no longer uses a positional argument; use --path or --descriptor. Again, I could make this backwards compatible.

getkeys usage:

$ ./hwi.py --testnet --fingerprint 00000000 getkeys --desc "wpkh(00000000/84'/1'/0'/0/*)"
["wpkh([00000000/84'/1'/0']tpub.../0/*"]

displayaddress usage with descriptor:

./hwi.py --testnet --fingerprint 00000000 displayaddress --desc "wpkh(00000000/84'/1'/0'/0/0)"

[instagibbs]

what is "psuedo" about the option?

[Sjors]

I'm calling it a pseudo-descriptor, because there's no public key in it. The goal of a pseudo-descriptor like wpkh(00000000/84h/1h/0h/0/*) is to ask for an actual descriptor wpkh(00000000/84h/1h/0h/tpub..../0/*) with a public key inserted in the right place. That's not covered in @sipa's document: https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md

Ideally I would just call that a descriptor as well, but I don't know if that makes sense in the scheme of things.

[instagibbs]

On further thought, I think these are ok. Might want to call them something like "descriptor requests" or something, and say exactly what info must reside in the requests, and what's missing.

[achow101]

I don't follow why signtx needs psbt to be a named argument.

[Sjors]

@achow101 I made everything a named argument, because it's easier to translate to an RPC.

[achow101]

How does that make it easier to translate to an RPC?

I really dislike having command line switches for things that are required. It implies that the parameter is optional and has a default value, but it's actually required. It's an extra couple of characters that you now have to always type.

[Sjors]

I'm still trying to figure out how this mapping would work. I see it's still possible to use a special purpose value field in the JSON payload to deal with a positional argument: https://grpc.io/blog/coreos

So I'll change psbt back to positional, same with --desc for getkeys. That's not possible for displayaddress --path / --descriptor though, unless we try to guess based on the format of the positional argument.

[instagibbs]

just leaving this here since I went to the stale branch: bitcoin/bitcoin#14912

[Sjors]

Thanks, I closed that PR and updated the description above.

The branch wasn't stale by the way, but the PR discussion and description was out of date.

[Sjors]

• renamed "pseudo descriptor" to "descriptor request" (which now requires square brackets around the origin part, so it's exactly like a descriptor with origin info, but without the public key)
• cleaned op the descriptor.py and added tests
  (I also pushed the corresponding changes to bitcoind)

[Sjors]

I extracted the descriptor class and displayaddress changes into a new PR #117.

I'm no longer sure if descriptor requests are the right approach. Ultimately we're only asking the device for a key at a certain derivation path. That key is the same regardless of what descriptor it's used in. So it makes more sense for Bitcoin Core to just directly ask for the key using getxpub, and then it can construct the descriptor. It won't need getkeypool in any case.

It's still important for Bitcoin Core to know which kinds of things the device can sign, especially if we start adding more complicated stuff like multisig. Perhaps enumerate is the right place for that. That would still require some standard way of expressing these capabilities.

Something similar to descriptors, but perhaps with wildcards, e.g.: ["pkh("44'/1'/$'/{0,1}/*"), sh(wpkh("49'/1'/$'/{0,1}/*")), wpkh("84'/1'/$'/{0,1}/*")] would be returned by a device that supports legacy, wrapped segwit and segwit native, when called with --testnet.