Skip to content

Commit 709d641

Browse files
sanket1729sanket1729
committed
Merge #376: Increase terseness in descriptor module
5a8514a Use more terse names for descriptor keys (Tobin C. Harding) 51e07db Use full tuple instead of KeySource (Tobin C. Harding) 33af3ad Improve docs on key structs (Tobin C. Harding) 3bed0db Re-order key structs (Tobin C. Harding) Pull request description: In an effort to better understand the `descriptor` module and its associated types; do some refactoring and re-naming of types making the key struct identifiers more terse. - Patch 1: refactor, code layout - Patch 2: docs improvements - Patch 3: Minor refactor - Patch 4: Use more terse names, this is the bulk of the PR If this is too invasive for someone only recently making their way around the crate, feel free to say so :) ACKs for top commit: sanket1729: ACK 5a8514a. These changes are increase the code quality. Tree-SHA512: 5a140d3cc9273304873c0e94861f5f28506f47a9cfec17d9980676c09931b8fab96253bf26cbe3c5b4071823dfda0f65d498622111e0a8cd8a95301fda79a2c9
2 parents 965a136 + 5a8514a commit 709d641

File tree

3 files changed

+82
-91
lines changed

3 files changed

+82
-91
lines changed

‎integration_test/src/test_util.rs‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ extern crate rand;
2121

2222
use self::rand::RngCore;
2323
use bitcoin::hashes::{hex::ToHex, Hash};
24-
use miniscript::{
25-
descriptor::{DescriptorSinglePub, SinglePubKey},
26-
Descriptor, DescriptorPublicKey, Miniscript, ScriptContext, TranslatePk,
27-
};
24+
use miniscript::descriptor::{SinglePub, SinglePubKey};
25+
use miniscript::{Descriptor, DescriptorPublicKey, Miniscript, ScriptContext, TranslatePk};
2826
use std::str::FromStr;
2927

3028
use bitcoin;
@@ -163,24 +161,24 @@ pub fn parse_insane_ms<Ctx: ScriptContext>(
163161
if avail {
164162
i = i + 1;
165163
if pk_str.starts_with("K") {
166-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
164+
DescriptorPublicKey::Single(SinglePub {
167165
origin: None,
168166
key: SinglePubKey::FullKey(pubdata.pks[i]),
169167
})
170168
} else if pk_str.starts_with("X") {
171-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
169+
DescriptorPublicKey::Single(SinglePub {
172170
origin: None,
173171
key: SinglePubKey::XOnly(pubdata.x_only_pks[i]),
174172
})
175173
} else {
176174
// Parse any other keys as known to allow compatibility with existing tests
177-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
175+
DescriptorPublicKey::Single(SinglePub {
178176
origin: None,
179177
key: SinglePubKey::FullKey(pubdata.pks[i]),
180178
})
181179
}
182180
} else {
183-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
181+
DescriptorPublicKey::Single(SinglePub {
184182
origin: None,
185183
key: SinglePubKey::FullKey(random_pk(59)),
186184
})
@@ -191,24 +189,24 @@ pub fn parse_insane_ms<Ctx: ScriptContext>(
191189
if avail {
192190
j = j - 1;
193191
if pk_str.starts_with("K") {
194-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
192+
DescriptorPublicKey::Single(SinglePub {
195193
origin: None,
196194
key: SinglePubKey::FullKey(pubdata.pks[j]),
197195
})
198196
} else if pk_str.starts_with("X") {
199-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
197+
DescriptorPublicKey::Single(SinglePub {
200198
origin: None,
201199
key: SinglePubKey::XOnly(pubdata.x_only_pks[j]),
202200
})
203201
} else {
204202
// Parse any other keys as known to allow compatibility with existing tests
205-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
203+
DescriptorPublicKey::Single(SinglePub {
206204
origin: None,
207205
key: SinglePubKey::FullKey(pubdata.pks[j]),
208206
})
209207
}
210208
} else {
211-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
209+
DescriptorPublicKey::Single(SinglePub {
212210
origin: None,
213211
key: SinglePubKey::FullKey(random_pk(59)),
214212
})
@@ -230,20 +228,20 @@ pub fn parse_test_desc(desc: &str, pubdata: &PubData) -> Descriptor<DescriptorPu
230228
if avail {
231229
i = i + 1;
232230
if pk_str.starts_with("K") {
233-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
231+
Ok(DescriptorPublicKey::Single(SinglePub {
234232
origin: None,
235233
key: SinglePubKey::FullKey(pubdata.pks[i]),
236234
}))
237235
} else if pk_str.starts_with("X") {
238-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
236+
Ok(DescriptorPublicKey::Single(SinglePub {
239237
origin: None,
240238
key: SinglePubKey::XOnly(pubdata.x_only_pks[i]),
241239
}))
242240
} else {
243241
panic!("Key must start with either K or X")
244242
}
245243
} else {
246-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
244+
Ok(DescriptorPublicKey::Single(SinglePub {
247245
origin: None,
248246
key: SinglePubKey::FullKey(random_pk(59)),
249247
}))
@@ -254,20 +252,20 @@ pub fn parse_test_desc(desc: &str, pubdata: &PubData) -> Descriptor<DescriptorPu
254252
if avail {
255253
j = j - 1;
256254
if pkh_str.starts_with("K") {
257-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
255+
Ok(DescriptorPublicKey::Single(SinglePub {
258256
origin: None,
259257
key: SinglePubKey::FullKey(pubdata.pks[j]),
260258
}))
261259
} else if pkh_str.starts_with("X") {
262-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
260+
Ok(DescriptorPublicKey::Single(SinglePub {
263261
origin: None,
264262
key: SinglePubKey::XOnly(pubdata.x_only_pks[j]),
265263
}))
266264
} else {
267265
panic!("Key must start with either K or X")
268266
}
269267
} else {
270-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
268+
Ok(DescriptorPublicKey::Single(SinglePub {
271269
origin: None,
272270
key: SinglePubKey::FullKey(random_pk(61)),
273271
}))

‎src/descriptor/key.rs‎

Lines changed: 58 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -12,56 +12,68 @@ use bitcoin::{
1212

1313
use {MiniscriptKey, ToPublicKey};
1414

15-
/// Single public key without any origin or range information
16-
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
17-
pub enum SinglePubKey {
18-
/// FullKey (compressed or uncompressed)
19-
FullKey(bitcoin::PublicKey),
20-
/// XOnlyPublicKey
21-
XOnly(XOnlyPublicKey),
22-
}
23-
24-
/// The MiniscriptKey corresponding to Descriptors. This can
25-
/// either be Single public key or a Xpub
15+
/// The descriptor pubkey, either a single pubkey or an xpub.
2616
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
2717
pub enum DescriptorPublicKey {
28-
/// Single Public Key
29-
SinglePub(DescriptorSinglePub),
30-
/// Xpub
18+
/// Single public key.
19+
Single(SinglePub),
20+
/// Extended public key (xpub).
3121
XPub(DescriptorXKey<bip32::ExtendedPubKey>),
3222
}
3323

34-
/// A Single Descriptor Key with optional origin information
24+
/// The descriptor secret key, either a single private key or an xprv.
25+
#[derive(Debug)]
26+
pub enum DescriptorSecretKey {
27+
/// Single private key.
28+
Single(SinglePriv),
29+
/// Extended private key (xpriv).
30+
XPrv(DescriptorXKey<bip32::ExtendedPrivKey>),
31+
}
32+
33+
/// A descriptor [`SinglePubKey`] with optional origin information.
3534
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
36-
pub struct DescriptorSinglePub {
37-
/// Origin information
35+
pub struct SinglePub {
36+
/// Origin information (fingerprint and derivation path).
3837
pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>,
39-
/// The key
38+
/// The public key.
4039
pub key: SinglePubKey,
4140
}
4241

43-
/// A Single Descriptor Secret Key with optional origin information
42+
/// A descriptor [`bitcoin::PrivateKey`] with optional origin information.
4443
#[derive(Debug)]
45-
pub struct DescriptorSinglePriv {
46-
/// Origin information
47-
pub origin: Option<bip32::KeySource>,
48-
/// The key
44+
pub struct SinglePriv {
45+
/// Origin information (fingerprint and derivation path).
46+
pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>,
47+
/// The private key.
4948
pub key: bitcoin::PrivateKey,
5049
}
5150

52-
/// A Secret Key that can be either a single key or an Xprv
53-
#[derive(Debug)]
54-
pub enum DescriptorSecretKey {
55-
/// Single Secret Key
56-
SinglePriv(DescriptorSinglePriv),
57-
/// Xprv
58-
XPrv(DescriptorXKey<bip32::ExtendedPrivKey>),
51+
/// An extended key with origin, derivation path, and wildcard.
52+
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
53+
pub struct DescriptorXKey<K: InnerXKey> {
54+
/// Origin information
55+
pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>,
56+
/// The extended key
57+
pub xkey: K,
58+
/// The derivation path
59+
pub derivation_path: bip32::DerivationPath,
60+
/// Whether the descriptor is wildcard
61+
pub wildcard: Wildcard,
62+
}
63+
64+
/// Single public key without any origin or range information.
65+
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
66+
pub enum SinglePubKey {
67+
/// A bitcoin public key (compressed or uncompressed).
68+
FullKey(bitcoin::PublicKey),
69+
/// An xonly public key.
70+
XOnly(XOnlyPublicKey),
5971
}
6072

6173
impl fmt::Display for DescriptorSecretKey {
6274
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
6375
match self {
64-
&DescriptorSecretKey::SinglePriv(ref sk) => {
76+
&DescriptorSecretKey::Single(ref sk) => {
6577
maybe_fmt_master_id(f, &sk.origin)?;
6678
sk.key.fmt(f)?;
6779
Ok(())
@@ -124,28 +136,15 @@ pub enum Wildcard {
124136
Hardened,
125137
}
126138

127-
/// Instance of an extended key with origin and derivation path
128-
#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)]
129-
pub struct DescriptorXKey<K: InnerXKey> {
130-
/// Origin information
131-
pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>,
132-
/// The extended key
133-
pub xkey: K,
134-
/// The derivation path
135-
pub derivation_path: bip32::DerivationPath,
136-
/// Whether the descriptor is wildcard
137-
pub wildcard: Wildcard,
138-
}
139-
140-
impl DescriptorSinglePriv {
139+
impl SinglePriv {
141140
/// Returns the public key of this key
142141
fn as_public<C: Signing>(
143142
&self,
144143
secp: &Secp256k1<C>,
145-
) -> Result<DescriptorSinglePub, DescriptorKeyParseError> {
144+
) -> Result<SinglePub, DescriptorKeyParseError> {
146145
let pub_key = self.key.public_key(secp);
147146

148-
Ok(DescriptorSinglePub {
147+
Ok(SinglePub {
149148
origin: self.origin.clone(),
150149
key: SinglePubKey::FullKey(pub_key),
151150
})
@@ -219,7 +218,7 @@ impl error::Error for DescriptorKeyParseError {}
219218
impl fmt::Display for DescriptorPublicKey {
220219
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
221220
match *self {
222-
DescriptorPublicKey::SinglePub(ref pk) => {
221+
DescriptorPublicKey::Single(ref pk) => {
223222
maybe_fmt_master_id(f, &pk.origin)?;
224223
match pk.key {
225224
SinglePubKey::FullKey(full_key) => full_key.fmt(f),
@@ -244,7 +243,7 @@ impl fmt::Display for DescriptorPublicKey {
244243

245244
impl DescriptorSecretKey {
246245
/// Return the public version of this key, by applying either
247-
/// [`DescriptorSinglePriv::as_public`] or [`DescriptorXKey<bip32::ExtendedPrivKey>::as_public`]
246+
/// [`SinglePriv::as_public`] or [`DescriptorXKey<bip32::ExtendedPrivKey>::as_public`]
248247
/// depending on the type of key.
249248
///
250249
/// If the key is an "XPrv", the hardened derivation steps will be applied before converting it
@@ -255,8 +254,8 @@ impl DescriptorSecretKey {
255254
secp: &Secp256k1<C>,
256255
) -> Result<DescriptorPublicKey, DescriptorKeyParseError> {
257256
Ok(match self {
258-
&DescriptorSecretKey::SinglePriv(ref sk) => {
259-
DescriptorPublicKey::SinglePub(sk.as_public(secp)?)
257+
&DescriptorSecretKey::Single(ref sk) => {
258+
DescriptorPublicKey::Single(sk.as_public(secp)?)
260259
}
261260
&DescriptorSecretKey::XPrv(ref xprv) => {
262261
DescriptorPublicKey::XPub(xprv.as_public(secp)?)
@@ -341,10 +340,7 @@ impl FromStr for DescriptorPublicKey {
341340
))
342341
}
343342
};
344-
Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub {
345-
key,
346-
origin,
347-
}))
343+
Ok(DescriptorPublicKey::Single(SinglePub { key, origin }))
348344
}
349345
}
350346
}
@@ -385,7 +381,7 @@ impl DescriptorPublicKey {
385381
xpub.xkey.fingerprint()
386382
}
387383
}
388-
DescriptorPublicKey::SinglePub(ref single) => {
384+
DescriptorPublicKey::Single(ref single) => {
389385
if let Some((fingerprint, _)) = single.origin {
390386
fingerprint
391387
} else {
@@ -417,7 +413,7 @@ impl DescriptorPublicKey {
417413
};
418414
origin_path.extend(&xpub.derivation_path)
419415
}
420-
DescriptorPublicKey::SinglePub(ref single) => {
416+
DescriptorPublicKey::Single(ref single) => {
421417
if let Some((_, ref path)) = single.origin {
422418
path.clone()
423419
} else {
@@ -430,7 +426,7 @@ impl DescriptorPublicKey {
430426
/// Whether or not the key has a wildcards
431427
pub fn is_deriveable(&self) -> bool {
432428
match *self {
433-
DescriptorPublicKey::SinglePub(..) => false,
429+
DescriptorPublicKey::Single(..) => false,
434430
DescriptorPublicKey::XPub(ref xpub) => xpub.wildcard != Wildcard::None,
435431
}
436432
}
@@ -476,7 +472,7 @@ impl DescriptorPublicKey {
476472
secp: &Secp256k1<C>,
477473
) -> Result<bitcoin::PublicKey, ConversionError> {
478474
match *self {
479-
DescriptorPublicKey::SinglePub(ref pk) => match pk.key {
475+
DescriptorPublicKey::Single(ref pk) => match pk.key {
480476
SinglePubKey::FullKey(pk) => Ok(pk),
481477
SinglePubKey::XOnly(xpk) => Ok(xpk.to_public_key()),
482478
},
@@ -504,7 +500,7 @@ impl FromStr for DescriptorSecretKey {
504500
if key_part.len() <= 52 {
505501
let sk = bitcoin::PrivateKey::from_str(key_part)
506502
.map_err(|_| DescriptorKeyParseError("Error while parsing a WIF private key"))?;
507-
Ok(DescriptorSecretKey::SinglePriv(DescriptorSinglePriv {
503+
Ok(DescriptorSecretKey::Single(SinglePriv {
508504
key: sk,
509505
origin: None,
510506
}))
@@ -694,7 +690,7 @@ impl MiniscriptKey for DescriptorPublicKey {
694690

695691
fn is_uncompressed(&self) -> bool {
696692
match self {
697-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
693+
DescriptorPublicKey::Single(SinglePub {
698694
key: SinglePubKey::FullKey(ref key),
699695
..
700696
}) => key.is_uncompressed(),
@@ -704,7 +700,7 @@ impl MiniscriptKey for DescriptorPublicKey {
704700

705701
fn is_x_only_key(&self) -> bool {
706702
match self {
707-
DescriptorPublicKey::SinglePub(DescriptorSinglePub {
703+
DescriptorPublicKey::Single(SinglePub {
708704
key: SinglePubKey::XOnly(ref _key),
709705
..
710706
}) => true,

0 commit comments

Comments
 (0)