From 3bed0db6b79997f4c00d1065eb23c7dba83b4776 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Apr 2022 13:47:11 +1000 Subject: [PATCH 1/4] Re-order key structs Source files are easier to read if the most important stuff comes first. We have a bunch of key structs, order them pub/priv and in order of importance i.e., declare things below where they are first used. Refactor only, no logic changes. --- src/descriptor/key.rs | 58 +++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index b61c2e823..eec3c7c8a 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -12,15 +12,6 @@ use bitcoin::{ use {MiniscriptKey, ToPublicKey}; -/// Single public key without any origin or range information -#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -pub enum SinglePubKey { - /// FullKey (compressed or uncompressed) - FullKey(bitcoin::PublicKey), - /// XOnlyPublicKey - XOnly(XOnlyPublicKey), -} - /// The MiniscriptKey corresponding to Descriptors. This can /// either be Single public key or a Xpub #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] @@ -31,6 +22,15 @@ pub enum DescriptorPublicKey { XPub(DescriptorXKey), } +/// A Secret Key that can be either a single key or an Xprv +#[derive(Debug)] +pub enum DescriptorSecretKey { + /// Single Secret Key + SinglePriv(DescriptorSinglePriv), + /// Xprv + XPrv(DescriptorXKey), +} + /// A Single Descriptor Key with optional origin information #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub struct DescriptorSinglePub { @@ -49,13 +49,26 @@ pub struct DescriptorSinglePriv { pub key: bitcoin::PrivateKey, } -/// A Secret Key that can be either a single key or an Xprv -#[derive(Debug)] -pub enum DescriptorSecretKey { - /// Single Secret Key - SinglePriv(DescriptorSinglePriv), - /// Xprv - XPrv(DescriptorXKey), +/// Instance of an extended key with origin and derivation path +#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] +pub struct DescriptorXKey { + /// Origin information + pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, + /// The extended key + pub xkey: K, + /// The derivation path + pub derivation_path: bip32::DerivationPath, + /// Whether the descriptor is wildcard + pub wildcard: Wildcard, +} + +/// Single public key without any origin or range information +#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] +pub enum SinglePubKey { + /// FullKey (compressed or uncompressed) + FullKey(bitcoin::PublicKey), + /// XOnlyPublicKey + XOnly(XOnlyPublicKey), } impl fmt::Display for DescriptorSecretKey { @@ -124,19 +137,6 @@ pub enum Wildcard { Hardened, } -/// Instance of an extended key with origin and derivation path -#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -pub struct DescriptorXKey { - /// Origin information - pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, - /// The extended key - pub xkey: K, - /// The derivation path - pub derivation_path: bip32::DerivationPath, - /// Whether the descriptor is wildcard - pub wildcard: Wildcard, -} - impl DescriptorSinglePriv { /// Returns the public key of this key fn as_public( From 33af3ad24ee657f62198c28f65a9c363e3c26fa4 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Apr 2022 13:49:53 +1000 Subject: [PATCH 2/4] Improve docs on key structs Improve the documentation on the various key structs. --- src/descriptor/key.rs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index eec3c7c8a..09991eb18 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -12,44 +12,43 @@ use bitcoin::{ use {MiniscriptKey, ToPublicKey}; -/// The MiniscriptKey corresponding to Descriptors. This can -/// either be Single public key or a Xpub +/// The descriptor pubkey, either a single pubkey or an xpub. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub enum DescriptorPublicKey { - /// Single Public Key + /// Single public key. SinglePub(DescriptorSinglePub), - /// Xpub + /// Extended public key (xpub). XPub(DescriptorXKey), } -/// A Secret Key that can be either a single key or an Xprv +/// The descriptor secret key, either a single private key or an xprv. #[derive(Debug)] pub enum DescriptorSecretKey { - /// Single Secret Key + /// Single private key. SinglePriv(DescriptorSinglePriv), - /// Xprv + /// Extended private key (xpriv). XPrv(DescriptorXKey), } -/// A Single Descriptor Key with optional origin information +/// A descriptor [`SinglePubKey`] with optional origin information. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub struct DescriptorSinglePub { - /// Origin information + /// Origin information (fingerprint and derivation path). pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, - /// The key + /// The public key. pub key: SinglePubKey, } -/// A Single Descriptor Secret Key with optional origin information +/// A descriptor [`bitcoin::PrivateKey`] with optional origin information. #[derive(Debug)] pub struct DescriptorSinglePriv { - /// Origin information + /// Origin information (fingerprint and derivation path). pub origin: Option, - /// The key + /// The private key. pub key: bitcoin::PrivateKey, } -/// Instance of an extended key with origin and derivation path +/// An extended key with origin, derivation path, and wildcard. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub struct DescriptorXKey { /// Origin information @@ -62,12 +61,12 @@ pub struct DescriptorXKey { pub wildcard: Wildcard, } -/// Single public key without any origin or range information +/// Single public key without any origin or range information. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub enum SinglePubKey { - /// FullKey (compressed or uncompressed) + /// A bitcoin public key (compressed or uncompressed). FullKey(bitcoin::PublicKey), - /// XOnlyPublicKey + /// An xonly public key. XOnly(XOnlyPublicKey), } From 51e07db3d1c61001c7449c2577f2ad46a803f175 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Apr 2022 13:51:30 +1000 Subject: [PATCH 3/4] Use full tuple instead of KeySource The `KeySource` type is a type alias from `bitcoin::bip32` for a tuple (fingerprint, derivation_path). In other places in the code we use the full tuple, we should be uniform and use one or the other. Elect to use the tuple. Use full tuple `(FingerPrint, DerivationPath)` instead of `KeySource`. --- src/descriptor/key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index 09991eb18..481257b13 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -43,7 +43,7 @@ pub struct DescriptorSinglePub { #[derive(Debug)] pub struct DescriptorSinglePriv { /// Origin information (fingerprint and derivation path). - pub origin: Option, + pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, /// The private key. pub key: bitcoin::PrivateKey, } From 5a8514ab351bbd23b998a39a7b36d88976462512 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Apr 2022 14:14:47 +1000 Subject: [PATCH 4/4] Use more terse names for descriptor keys We have a plethora of key structs in the `key` module. Attempt to use more terse names without loosing any clarity. In particular: 1. Variants - Replace SinglePub with Single - Replace SinglePriv with Single The variants `SinglePub`, and `SinglePriv` stutter, we know what keys they are so can use `Single` with no loss of clarity. 2. Remove `Descriptor` from the nested descriptor types, users can use `descriptor::singlePub` if needed. --- integration_test/src/test_util.rs | 34 ++++++++++++------------ src/descriptor/key.rs | 43 ++++++++++++++----------------- src/descriptor/mod.rs | 19 ++++++-------- 3 files changed, 44 insertions(+), 52 deletions(-) diff --git a/integration_test/src/test_util.rs b/integration_test/src/test_util.rs index 9c320caf4..a4979aa60 100644 --- a/integration_test/src/test_util.rs +++ b/integration_test/src/test_util.rs @@ -21,10 +21,8 @@ extern crate rand; use self::rand::RngCore; use bitcoin::hashes::{hex::ToHex, Hash}; -use miniscript::{ - descriptor::{DescriptorSinglePub, SinglePubKey}, - Descriptor, DescriptorPublicKey, Miniscript, ScriptContext, TranslatePk, -}; +use miniscript::descriptor::{SinglePub, SinglePubKey}; +use miniscript::{Descriptor, DescriptorPublicKey, Miniscript, ScriptContext, TranslatePk}; use std::str::FromStr; use bitcoin; @@ -163,24 +161,24 @@ pub fn parse_insane_ms( if avail { i = i + 1; if pk_str.starts_with("K") { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(pubdata.pks[i]), }) } else if pk_str.starts_with("X") { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::XOnly(pubdata.x_only_pks[i]), }) } else { // Parse any other keys as known to allow compatibility with existing tests - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(pubdata.pks[i]), }) } } else { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(random_pk(59)), }) @@ -191,24 +189,24 @@ pub fn parse_insane_ms( if avail { j = j - 1; if pk_str.starts_with("K") { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(pubdata.pks[j]), }) } else if pk_str.starts_with("X") { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::XOnly(pubdata.x_only_pks[j]), }) } else { // Parse any other keys as known to allow compatibility with existing tests - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(pubdata.pks[j]), }) } } else { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { origin: None, key: SinglePubKey::FullKey(random_pk(59)), }) @@ -230,12 +228,12 @@ pub fn parse_test_desc(desc: &str, pubdata: &PubData) -> Descriptor Descriptor Descriptor Descriptor), } @@ -25,14 +25,14 @@ pub enum DescriptorPublicKey { #[derive(Debug)] pub enum DescriptorSecretKey { /// Single private key. - SinglePriv(DescriptorSinglePriv), + Single(SinglePriv), /// Extended private key (xpriv). XPrv(DescriptorXKey), } /// A descriptor [`SinglePubKey`] with optional origin information. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -pub struct DescriptorSinglePub { +pub struct SinglePub { /// Origin information (fingerprint and derivation path). pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, /// The public key. @@ -41,7 +41,7 @@ pub struct DescriptorSinglePub { /// A descriptor [`bitcoin::PrivateKey`] with optional origin information. #[derive(Debug)] -pub struct DescriptorSinglePriv { +pub struct SinglePriv { /// Origin information (fingerprint and derivation path). pub origin: Option<(bip32::Fingerprint, bip32::DerivationPath)>, /// The private key. @@ -73,7 +73,7 @@ pub enum SinglePubKey { impl fmt::Display for DescriptorSecretKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - &DescriptorSecretKey::SinglePriv(ref sk) => { + &DescriptorSecretKey::Single(ref sk) => { maybe_fmt_master_id(f, &sk.origin)?; sk.key.fmt(f)?; Ok(()) @@ -136,15 +136,15 @@ pub enum Wildcard { Hardened, } -impl DescriptorSinglePriv { +impl SinglePriv { /// Returns the public key of this key fn as_public( &self, secp: &Secp256k1, - ) -> Result { + ) -> Result { let pub_key = self.key.public_key(secp); - Ok(DescriptorSinglePub { + Ok(SinglePub { origin: self.origin.clone(), key: SinglePubKey::FullKey(pub_key), }) @@ -218,7 +218,7 @@ impl error::Error for DescriptorKeyParseError {} impl fmt::Display for DescriptorPublicKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - DescriptorPublicKey::SinglePub(ref pk) => { + DescriptorPublicKey::Single(ref pk) => { maybe_fmt_master_id(f, &pk.origin)?; match pk.key { SinglePubKey::FullKey(full_key) => full_key.fmt(f), @@ -243,7 +243,7 @@ impl fmt::Display for DescriptorPublicKey { impl DescriptorSecretKey { /// Return the public version of this key, by applying either - /// [`DescriptorSinglePriv::as_public`] or [`DescriptorXKey::as_public`] + /// [`SinglePriv::as_public`] or [`DescriptorXKey::as_public`] /// depending on the type of key. /// /// If the key is an "XPrv", the hardened derivation steps will be applied before converting it @@ -254,8 +254,8 @@ impl DescriptorSecretKey { secp: &Secp256k1, ) -> Result { Ok(match self { - &DescriptorSecretKey::SinglePriv(ref sk) => { - DescriptorPublicKey::SinglePub(sk.as_public(secp)?) + &DescriptorSecretKey::Single(ref sk) => { + DescriptorPublicKey::Single(sk.as_public(secp)?) } &DescriptorSecretKey::XPrv(ref xprv) => { DescriptorPublicKey::XPub(xprv.as_public(secp)?) @@ -340,10 +340,7 @@ impl FromStr for DescriptorPublicKey { )) } }; - Ok(DescriptorPublicKey::SinglePub(DescriptorSinglePub { - key, - origin, - })) + Ok(DescriptorPublicKey::Single(SinglePub { key, origin })) } } } @@ -384,7 +381,7 @@ impl DescriptorPublicKey { xpub.xkey.fingerprint() } } - DescriptorPublicKey::SinglePub(ref single) => { + DescriptorPublicKey::Single(ref single) => { if let Some((fingerprint, _)) = single.origin { fingerprint } else { @@ -416,7 +413,7 @@ impl DescriptorPublicKey { }; origin_path.extend(&xpub.derivation_path) } - DescriptorPublicKey::SinglePub(ref single) => { + DescriptorPublicKey::Single(ref single) => { if let Some((_, ref path)) = single.origin { path.clone() } else { @@ -429,7 +426,7 @@ impl DescriptorPublicKey { /// Whether or not the key has a wildcards pub fn is_deriveable(&self) -> bool { match *self { - DescriptorPublicKey::SinglePub(..) => false, + DescriptorPublicKey::Single(..) => false, DescriptorPublicKey::XPub(ref xpub) => xpub.wildcard != Wildcard::None, } } @@ -475,7 +472,7 @@ impl DescriptorPublicKey { secp: &Secp256k1, ) -> Result { match *self { - DescriptorPublicKey::SinglePub(ref pk) => match pk.key { + DescriptorPublicKey::Single(ref pk) => match pk.key { SinglePubKey::FullKey(pk) => Ok(pk), SinglePubKey::XOnly(xpk) => Ok(xpk.to_public_key()), }, @@ -503,7 +500,7 @@ impl FromStr for DescriptorSecretKey { if key_part.len() <= 52 { let sk = bitcoin::PrivateKey::from_str(key_part) .map_err(|_| DescriptorKeyParseError("Error while parsing a WIF private key"))?; - Ok(DescriptorSecretKey::SinglePriv(DescriptorSinglePriv { + Ok(DescriptorSecretKey::Single(SinglePriv { key: sk, origin: None, })) @@ -693,7 +690,7 @@ impl MiniscriptKey for DescriptorPublicKey { fn is_uncompressed(&self) -> bool { match self { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { key: SinglePubKey::FullKey(ref key), .. }) => key.is_uncompressed(), @@ -703,7 +700,7 @@ impl MiniscriptKey for DescriptorPublicKey { fn is_x_only_key(&self) -> bool { match self { - DescriptorPublicKey::SinglePub(DescriptorSinglePub { + DescriptorPublicKey::Single(SinglePub { key: SinglePubKey::XOnly(ref _key), .. }) => true, diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 36f2449fa..e778f01e8 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -62,7 +62,7 @@ mod key; pub use self::key::{ ConversionError, DescriptorKeyParseError, DescriptorPublicKey, DescriptorSecretKey, - DescriptorSinglePriv, DescriptorSinglePub, DescriptorXKey, InnerXKey, SinglePubKey, Wildcard, + DescriptorXKey, InnerXKey, SinglePriv, SinglePub, SinglePubKey, Wildcard, }; /// Alias type for a map of public key to secret key @@ -835,9 +835,7 @@ mod tests { use bitcoin::util::bip32; use bitcoin::{self, secp256k1, EcdsaSighashType, PublicKey}; use descriptor::key::Wildcard; - use descriptor::{ - DescriptorPublicKey, DescriptorSecretKey, DescriptorSinglePub, DescriptorXKey, - }; + use descriptor::{DescriptorPublicKey, DescriptorSecretKey, DescriptorXKey, SinglePub}; use hex_script; use std::cmp; use std::collections::HashMap; @@ -854,10 +852,9 @@ mod tests { impl cmp::PartialEq for DescriptorSecretKey { fn eq(&self, other: &Self) -> bool { match (self, other) { - ( - &DescriptorSecretKey::SinglePriv(ref a), - &DescriptorSecretKey::SinglePriv(ref b), - ) => a.origin == b.origin && a.key == b.key, + (&DescriptorSecretKey::Single(ref a), &DescriptorSecretKey::Single(ref b)) => { + a.origin == b.origin && a.key == b.key + } (&DescriptorSecretKey::XPrv(ref a), &DescriptorSecretKey::XPrv(ref b)) => { a.origin == b.origin && a.xkey == b.xkey @@ -1531,7 +1528,7 @@ mod tests { // Raw (compressed) pubkey let key = "03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8"; - let expected = DescriptorPublicKey::SinglePub(DescriptorSinglePub { + let expected = DescriptorPublicKey::Single(SinglePub { key: SinglePubKey::FullKey( bitcoin::PublicKey::from_str( "03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8", @@ -1545,7 +1542,7 @@ mod tests { // Raw (uncompressed) pubkey let key = "04f5eeb2b10c944c6b9fbcfff94c35bdeecd93df977882babc7f3a2cf7f5c81d3b09a68db7f0e04f21de5d4230e75e6dbe7ad16eefe0d4325a62067dc6f369446a"; - let expected = DescriptorPublicKey::SinglePub(DescriptorSinglePub { + let expected = DescriptorPublicKey::Single(SinglePub { key: SinglePubKey::FullKey(bitcoin::PublicKey::from_str( "04f5eeb2b10c944c6b9fbcfff94c35bdeecd93df977882babc7f3a2cf7f5c81d3b09a68db7f0e04f21de5d4230e75e6dbe7ad16eefe0d4325a62067dc6f369446a", ) @@ -1558,7 +1555,7 @@ mod tests { // Raw pubkey with origin let desc = "[78412e3a/0'/42/0']0231c7d3fc85c148717848033ce276ae2b464a4e2c367ed33886cc428b8af48ff8"; - let expected = DescriptorPublicKey::SinglePub(DescriptorSinglePub { + let expected = DescriptorPublicKey::Single(SinglePub { key: SinglePubKey::FullKey( bitcoin::PublicKey::from_str( "0231c7d3fc85c148717848033ce276ae2b464a4e2c367ed33886cc428b8af48ff8",