WIP: Add descriptor key by sgeisler · Pull Request #93 · rust-bitcoin/rust-miniscript

sgeisler · 2020-06-04T21:09:10Z

Keys in script descriptors can come in a wide variety of formats:

simple public or private keys
xpub, xpriv

all with or without origin information.

The xpub/xpriv variety can also allow further derivations by ending its derivation path with *. This PR allows all of theses keys to be represented and Descriptors to be derived. Deriving a descriptor leaves all keys as they are except wildcard keys. They are derived using a supplied path resulting in a Descriptor without wildcard keys/with the respective derived keys.

For example to derive the first 5 addresses of a 2-of-3 multisig wallet with one static key (for some reason) and two derived keys the resulting code would look like this:

et policy = "thresh(2,\
pk(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1/*),\
pk(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/2/*),\
pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";

let descriptor = Descriptor::Wsh(
    policy
        .parse::<Concrete<DescriptorKey>>()
        .unwrap()
        .compile()
        .unwrap(),
);

let base_deriv = DerivationPath::from(Vec::<ChildNumber>::new());

for child in base_deriv.normal_children().take(5) {
    println!(
        "child {}: {}",
        child,
        descriptor
            .derive(child.as_ref())
            .address(Network::Regtest)
            .unwrap()
    );
}

This would generate

child m/0: bcrt1qyaqn5jangsa8jt92n6evj7fzhwkttsh7prn6vaswhhekmr78a8vqedj0uw
child m/1: bcrt1q2m3lqldudthalkhukt595zv79awvdy2mmvkrnlck030fhccpyd5sqpt0cq
child m/2: bcrt1qjulpj2f0cfftvc3ae4jcfdrdfzvh58tf9vahj4uaqwgl2sfuc49ql0wc9h
child m/3: bcrt1qsvlq87een4pk76jq394j4tuc6058j93zy2nnjqj9m84am82ksw0qc5tjf7
child m/4: bcrt1qnz7346r9u7utq4srct06hrsqkcgy6paglx3qdcy9p8upzl30xt8qsex053

As in the title, this is still heavily WIP, I mainly want to get concept feedback.

sgeisler · 2020-06-04T21:10:51Z

src/descriptor/mod.rs

+
+#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
+pub struct DescriptorXPub {
+    source: Option<([u8; 4], DerivationPath)>,


All key types seem to allow a source description. Move this to a DescriptorKey struct.

Might be preferable to use use bitcoin::util::bip32::Fingerprint instead of [u8; 4]

afilini · 2020-06-30T10:05:16Z

src/descriptor/mod.rs

+    source: Option<([u8; 4], DerivationPath)>,
+    xpub: bitcoin::util::bip32::ExtendedPubKey,
+    derivation_path: DerivationPath,
+    is_wildcard: bool,


Do you think it would make sense to also support a "hardened wildcard"? Something like /*'.

Currently the parser in magical-bitcoin-wallet supports it, even tough I don't really see many use-cases that could use it.

In principle yes. I just didn't implement any private key types so far, so this didn't make any sense (no hardened derivations are currently supported).

I'm still not sure if we should even support anything priv-key related in rust-miniscript. The only use case would be recovering old funds automatically by searching a lot of possible (private) paths. But if you view miniscript more as a exchange format for policies/scripts private keys in it become somewhat hazardous. xpubs should be still seen as confidential, but to a way lesser degree than actual xprivs.

One use case I can imagine is to setup a new multisig wallet client for one of your cosigners you just copy the descriptor of your multisig wallet over. If it accidentally contains your xpriv instead of your xpub that would be really, really bad.

Maybe I'm just to paranoid. Maybe two different parses would help, one returns an error if the descriptor contains an xpriv.

Well if you are trying to build a wallet with rust-miniscript and not just a "monitor" then yeah I think it's pretty useful to have private keys in a descriptor, just like Core does.

But yeah, I agree with you that it should be an "explicit" parser/type to avoid the issues you described.

One thing that I have in Magical which I could probably upstream if this PR is merged is a way to transform a descriptor into its "public" version. It's very useful if you have a descriptor with some private keys inside and you just want to make a watch-only out of it, so instead of manually converting the keys you can just use that method.

I think core compatibility as a good reason to do it. But I'd still argue that it's better to have private keys separate from the descriptor. They are used in two completely different steps of tx creation and more often than not there might not even be a private key on the device the wallet is running on.

That's how I imagine wallet/miniscript/PSBT interaction:

Imo it's not a big advantage to have a config only consisting of a descriptor with private keys instead of a descriptor and a separate set of private keys.

I thought about adding private key support again and I have to be damn careful when implementing Ord (required by the MiniscriptKey trait) as it could easily lead to side channels that leak the key. So I'll probably implement inefficiently by first computing the public key (should be constant time in libsecp) and then using it for comparisons.

@apoelstra / @sanket1729 do you have any opinions on implementing MiniscriptKey for private key types?

cc @sipa

@sgeisler the way that Core works when parsing a descriptor with private keys is that it outputs a descriptor containing public keys, along with a map from pubkeys to privkeys. I think we should do something analogous, and have our map use the Satisfier trait to allow users to produce signatures etc with it.

...so I think we (a) should not impl Ord on any private key types; (b) not directly support private keys in descriptors. Instead we should write a function that parses a privkey-containing descriptor, converts all the privkeys to pubkeys, and outputs a mapping. I can offer guidance on how to do this.

…icKey

darosior · 2020-09-03T18:02:33Z

Hi @sgeisler, what's the status of this PR ? I'd happily help you polish it if this is not a priority for you right now :-)

sgeisler · 2020-09-03T18:55:50Z

Yeah, I'd actually like to get this moving again too to not rely on my custom patched version of rust-miniscript anymore. @afilini took over in #116 so I haven't been paying that much attention to this PR lately. I'll take a closer look again this evening/tomorrow.

sgeisler commented Jun 4, 2020

View reviewed changes

justinmoon mentioned this pull request Jun 25, 2020

Descriptor support bwt-dev/bwt#37

Closed

afilini reviewed Jun 30, 2020

View reviewed changes

sgeisler force-pushed the 2020-06-descriptor-key branch 2 times, most recently from 48a3512 to 50d0183 Compare July 18, 2020 21:50

afilini mentioned this pull request Jul 24, 2020

Add DescriptorSecretKey and Descriptor::parse_secret() #116

Merged

sgeisler and others added 9 commits August 5, 2020 14:55

Add descriptor compatible key struct

361ae7d

Impl FromStr for DescriptorKey

60ff452

Implement MiniscriptKey for DescriptorKey

5ada9ad

Implement ToPublicKey for DescriptorKey

621ec64

Make Descriptors derivable

7ba8be0

Abstract over extended keys

fdd31ed

[wip] Add DescriptorSecretKey, rename DescriptorKey to DescriptorPubl…

c8af618

…icKey

[wip] Add Descriptor::parse_secret()

e99eab7

Implement Satisfier<DescriptorPublicKey> on psbt::Input

dc4e0b5

darosior mentioned this pull request Aug 18, 2020

First implementation revault/revault_tx#1

Merged

Make DescriptorXKey<ExtendedPubKey> a standalone miniscript key

d9fa945

sgeisler force-pushed the 2020-06-descriptor-key branch from 50d0183 to d9fa945 Compare September 5, 2020 21:13

darosior mentioned this pull request Sep 11, 2020

DescriptorPublicKey, second pass #131

Merged

apoelstra closed this in #131 Oct 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add descriptor key#93

WIP: Add descriptor key#93
sgeisler wants to merge 10 commits intorust-bitcoin:masterfrom
sgeisler:2020-06-descriptor-key

sgeisler commented Jun 4, 2020 •

edited

Loading

Uh oh!

sgeisler Jun 4, 2020

Uh oh!

justinmoon Jun 9, 2020

Uh oh!

afilini Jun 30, 2020 •

edited

Loading

Uh oh!

sgeisler Jun 30, 2020

Uh oh!

afilini Jul 1, 2020

Uh oh!

sgeisler Jul 1, 2020

Uh oh!

sgeisler Jul 4, 2020

Uh oh!

apoelstra Jul 6, 2020

Uh oh!

apoelstra Jul 6, 2020

Uh oh!

darosior commented Sep 3, 2020

Uh oh!

sgeisler commented Sep 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

sgeisler commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afilini Jun 30, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darosior commented Sep 3, 2020

Uh oh!

sgeisler commented Sep 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sgeisler commented Jun 4, 2020 •

edited

Loading

afilini Jun 30, 2020 •

edited

Loading