From 6dc6aca3c4d72a5b6042722cce9cdf487e92aa54 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 14 Jul 2022 14:51:06 +0800 Subject: [PATCH 1/2] Make DerivedDescriptorKey first-class citizen Earlier we introduced DerivedDescriptorKey (an descriptor whose wildcards are known to have been replaced with derivation indexes) to eliminate wildcard derivation errors. This commit attempts to realise the full benefits of this idea. I tried to keep backwards compatibility. The naming of things no longer follows the logic of code so I'm going to fix that in a follow up. --- examples/xpub_descriptors.rs | 6 +- src/descriptor/key.rs | 105 +++++++++++++++++--------------- src/descriptor/mod.rs | 115 +++++++++++++++++++++++------------ src/lib.rs | 2 +- src/psbt/mod.rs | 30 ++++----- tests/test_desc.rs | 3 +- 6 files changed, 152 insertions(+), 109 deletions(-) diff --git a/examples/xpub_descriptors.rs b/examples/xpub_descriptors.rs index 54f68ba29..a055a554f 100644 --- a/examples/xpub_descriptors.rs +++ b/examples/xpub_descriptors.rs @@ -18,7 +18,7 @@ use std::str::FromStr; use miniscript::bitcoin::secp256k1::{Secp256k1, Verification}; use miniscript::bitcoin::{Address, Network}; -use miniscript::{Descriptor, DescriptorPublicKey}; +use miniscript::{DerivedDescriptorKey, Descriptor, DescriptorPublicKey}; const XPUB_1: &str = "xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB"; const XPUB_2: &str = "xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH"; @@ -40,9 +40,9 @@ fn p2wsh(secp: &Secp256k1) -> Address { let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_1, XPUB_2); // let s = format!("wsh(sortedmulti(1,{},{}))", XPUB_2, XPUB_1); - let address = Descriptor::::from_str(&s) + let address = Descriptor::::from_str(&s) .unwrap() - .derived_descriptor(&secp, 0) // dummy index value if it not a wildcard + .derived_descriptor(&secp) .unwrap() .address(Network::Bitcoin) .unwrap(); diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index c00114321..e9a1fac9a 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -70,14 +70,9 @@ pub enum SinglePubKey { XOnly(XOnlyPublicKey), } -/// A derived [`DescriptorPublicKey`] -/// -/// Derived keys are guaranteed to never contain wildcards +/// A [`DescriptorPublicKey`] without any wildcards. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -pub struct DerivedDescriptorKey { - key: DescriptorPublicKey, - index: u32, -} +pub struct DerivedDescriptorKey(DescriptorPublicKey); impl fmt::Display for DescriptorSecretKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -357,22 +352,14 @@ impl FromStr for DescriptorPublicKey { /// Descriptor key conversion error #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)] pub enum ConversionError { - /// Attempted to convert a key with a wildcard to a bitcoin public key - Wildcard, /// Attempted to convert a key with hardened derivations to a bitcoin public key HardenedChild, - /// Attempted to convert a key with a hardened wildcard to a bitcoin public key - HardenedWildcard, } impl fmt::Display for ConversionError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str(match *self { - ConversionError::Wildcard => "uninstantiated wildcard in bip32 path", ConversionError::HardenedChild => "hardened child step in bip32 path", - ConversionError::HardenedWildcard => { - "hardened and uninstantiated wildcard in bip32 path" - } }) } } @@ -383,7 +370,7 @@ impl error::Error for ConversionError { use self::ConversionError::*; match self { - Wildcard | HardenedChild | HardenedWildcard => None, + HardenedChild => None, } } } @@ -455,10 +442,7 @@ impl DescriptorPublicKey { /// /// - If this key is not an xpub, returns `self`. /// - If this key is an xpub but does not have a wildcard, returns `self`. - /// - Otherwise, returns the derived xpub at `index` (removing the wildcard). - /// - /// Since it's guaranteed that extended keys won't have wildcards, the key is returned as - /// [`DerivedDescriptorKey`]. + /// - Otherwise, returns the xpub at derivation `index` (removing the wildcard). /// /// # Panics /// @@ -469,12 +453,12 @@ impl DescriptorPublicKey { DescriptorPublicKey::XPub(xpub) => { let derivation_path = match xpub.wildcard { Wildcard::None => xpub.derivation_path, - Wildcard::Unhardened => xpub - .derivation_path - .into_child(bip32::ChildNumber::from_normal_idx(index).unwrap()), - Wildcard::Hardened => xpub - .derivation_path - .into_child(bip32::ChildNumber::from_hardened_idx(index).unwrap()), + Wildcard::Unhardened => xpub.derivation_path.into_child( + bip32::ChildNumber::from_normal_idx(index).expect("index must < 2^31"), + ), + Wildcard::Hardened => xpub.derivation_path.into_child( + bip32::ChildNumber::from_hardened_idx(index).expect("index must < 2^31"), + ), }; DescriptorPublicKey::XPub(DescriptorXKey { origin: xpub.origin, @@ -485,7 +469,7 @@ impl DescriptorPublicKey { } }; - DerivedDescriptorKey::new(derived, index) + DerivedDescriptorKey::new(derived) .expect("The key should not contain any wildcards at this point") } @@ -494,13 +478,10 @@ impl DescriptorPublicKey { /// and returns the obtained full [`bitcoin::PublicKey`]. All BIP32 derivations /// always return a compressed key /// - /// Will return an error if the descriptor key has any hardened - /// derivation steps in its path, or if the key has any wildcards. + /// Will return an error if the descriptor key has any hardened derivation steps in its path. To + /// avoid this error you should replace any such public keys first with [`translate_pk`]. /// - /// To ensure there are no wildcards, call `.derive(0)` or similar; - /// to avoid hardened derivation steps, start from a `DescriptorSecretKey` - /// and call `to_public`, or call `TranslatePk2::translate_pk2` with - /// some function which has access to secret key data. + /// [`translate_pk`]: crate::TranslatePk::translate_pk pub fn derive_public_key( &self, secp: &Secp256k1, @@ -511,8 +492,9 @@ impl DescriptorPublicKey { SinglePubKey::XOnly(xpk) => Ok(xpk.to_public_key()), }, DescriptorPublicKey::XPub(ref xpk) => match xpk.wildcard { - Wildcard::Unhardened => Err(ConversionError::Wildcard), - Wildcard::Hardened => Err(ConversionError::HardenedWildcard), + Wildcard::Unhardened | Wildcard::Hardened => { + unreachable!("we've excluded this error case") + } Wildcard::None => match xpk.xkey.derive_pub(secp, &xpk.derivation_path.as_ref()) { Ok(xpub) => Ok(bitcoin::PublicKey::new(xpub.public_key)), Err(bip32::Error::CannotDeriveFromHardenedKey) => { @@ -778,28 +760,47 @@ impl DerivedDescriptorKey { &self, secp: &Secp256k1, ) -> Result { - self.key.derive_public_key(secp) - } - - /// Return the derivation index of this key - pub fn index(&self) -> u32 { - self.index + self.0.derive_public_key(secp) } /// Construct an instance from a descriptor key and a derivation index /// /// Returns `None` if the key contains a wildcard - fn new(key: DescriptorPublicKey, index: u32) -> Option { - match key { - DescriptorPublicKey::XPub(ref xpk) if xpk.wildcard != Wildcard::None => None, - k => Some(DerivedDescriptorKey { key: k, index }), + fn new(key: DescriptorPublicKey) -> Option { + if key.is_deriveable() { + None + } else { + Some(Self(key)) } } + + /// The fingerprint of the master key associated with this key, `0x00000000` if none. + pub fn master_fingerprint(&self) -> bip32::Fingerprint { + self.0.master_fingerprint() + } + + /// Full path, from the master key + pub fn full_derivation_path(&self) -> bip32::DerivationPath { + self.0.full_derivation_path() + } +} + +impl FromStr for DerivedDescriptorKey { + type Err = DescriptorKeyParseError; + + fn from_str(s: &str) -> Result { + let inner = DescriptorPublicKey::from_str(s)?; + Ok( + DerivedDescriptorKey::new(inner).ok_or(DescriptorKeyParseError( + "cannot parse key with a wilcard as a DerivedDescriptorKey", + ))?, + ) + } } impl fmt::Display for DerivedDescriptorKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.key.fmt(f) + self.0.fmt(f) } } @@ -812,11 +813,11 @@ impl MiniscriptKey for DerivedDescriptorKey { type Hash160 = hash160::Hash; fn is_uncompressed(&self) -> bool { - self.key.is_uncompressed() + self.0.is_uncompressed() } fn is_x_only_key(&self) -> bool { - self.key.is_x_only_key() + self.0.is_x_only_key() } fn to_pubkeyhash(&self) -> Self { @@ -827,7 +828,7 @@ impl MiniscriptKey for DerivedDescriptorKey { impl ToPublicKey for DerivedDescriptorKey { fn to_public_key(&self) -> bitcoin::PublicKey { let secp = Secp256k1::verification_only(); - self.key.derive_public_key(&secp).unwrap() + self.0.derive_public_key(&secp).unwrap() } fn hash_to_hash160(hash: &Self) -> hash160::Hash { @@ -851,6 +852,12 @@ impl ToPublicKey for DerivedDescriptorKey { } } +impl From for DescriptorPublicKey { + fn from(d: DerivedDescriptorKey) -> Self { + d.0 + } +} + #[cfg(test)] mod test { use core::str::FromStr; diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 2b02dc165..9342950e5 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -539,27 +539,29 @@ impl Descriptor { .expect("BIP 32 key index substitution cannot fail") } - /// Derive a [`Descriptor`] with a concrete [`bitcoin::PublicKey`] at a given index - /// Removes all extended pubkeys and wildcards from the descriptor and only leaves - /// concrete [`bitcoin::PublicKey`]. All [`bitcoin::XOnlyPublicKey`]s are converted - /// to [`bitcoin::PublicKey`]s by adding a default(0x02) y-coordinate. For [`Tr`] - /// descriptor, spend info is also cached. + /// Convert all the public keys in the descriptor to [`bitcoin::PublicKey`] by deriving them or + /// otherwise converting them. All [`bitcoin::XOnlyPublicKey`]s are converted to by adding a + /// default(0x02) y-coordinate. /// - /// # Examples + /// This is a shorthand for: /// /// ``` - /// use miniscript::descriptor::{Descriptor, DescriptorPublicKey}; - /// use miniscript::bitcoin::secp256k1; - /// use std::str::FromStr; - /// - /// // test from bip 86 - /// let secp = secp256k1::Secp256k1::verification_only(); - /// let descriptor = Descriptor::::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)") + /// # use miniscript::{Descriptor, DescriptorPublicKey, bitcoin::secp256k1::Secp256k1}; + /// # use core::str::FromStr; + /// # let descriptor = Descriptor::::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)") /// .expect("Valid ranged descriptor"); - /// let result = descriptor.derived_descriptor(&secp, 0).expect("Non-hardened derivation"); - /// assert_eq!(result.to_string(), "tr(03cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115)#6qm9h8ym"); + /// # let index = 42; + /// # let secp = Secp256k1::verification_only(); + /// let derived_descriptor = descriptor.derive(index).derived_descriptor(&secp); + /// # assert_eq!(descriptor.derived_descriptor(&secp, index), derived_descriptor); /// ``` /// + /// and is only here really here for backwards compatbility. + /// See [`derive`] and `[derived_descriptor`] for more documentation. + /// + /// [`derive`]: Self::derive + /// [`derived_descriptor`]: crate::DerivedDescriptor::derived_descriptor + /// /// # Errors /// /// This function will return an error if hardened derivation is attempted. @@ -568,29 +570,7 @@ impl Descriptor { secp: &secp256k1::Secp256k1, index: u32, ) -> Result, ConversionError> { - struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1); - - impl<'a, C: secp256k1::Verification> - PkTranslator - for Derivator<'a, C> - { - fn pk( - &mut self, - pk: &DerivedDescriptorKey, - ) -> Result { - pk.derive_public_key(&self.0) - } - - fn pkh( - &mut self, - pkh: &DerivedDescriptorKey, - ) -> Result { - Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash()) - } - } - - let derived = self.derive(index).translate_pk(&mut Derivator(secp))?; - Ok(derived) + self.derive(index).derived_descriptor(&secp) } /// Parse a descriptor that may contain secret keys @@ -744,6 +724,59 @@ impl Descriptor { } } +impl Descriptor { + /// Convert all the public keys in the descriptor to [`bitcoin::PublicKey`] by deriving them or + /// otherwise converting them. All [`bitcoin::XOnlyPublicKey`]s are converted to by adding a + /// default(0x02) y-coordinate. + /// + /// # Examples + /// + /// ``` + /// use miniscript::descriptor::{Descriptor, DescriptorPublicKey}; + /// use miniscript::bitcoin::secp256k1; + /// use std::str::FromStr; + /// + /// // test from bip 86 + /// let secp = secp256k1::Secp256k1::verification_only(); + /// let descriptor = Descriptor::::from_str("tr(xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)") + /// .expect("Valid ranged descriptor"); + /// let result = descriptor.derive(0).derived_descriptor(&secp).expect("Non-hardened derivation"); + /// assert_eq!(result.to_string(), "tr(03cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115)#6qm9h8ym"); + /// ``` + /// + /// # Errors + /// + /// This function will return an error if hardened derivation is attempted. + pub fn derived_descriptor( + &self, + secp: &secp256k1::Secp256k1, + ) -> Result, ConversionError> { + struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1); + + impl<'a, C: secp256k1::Verification> + PkTranslator + for Derivator<'a, C> + { + fn pk( + &mut self, + pk: &DerivedDescriptorKey, + ) -> Result { + pk.derive_public_key(&self.0) + } + + fn pkh( + &mut self, + pkh: &DerivedDescriptorKey, + ) -> Result { + Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash()) + } + } + + let derived = self.translate_pk(&mut Derivator(secp))?; + Ok(derived) + } +} + impl_from_tree!( Descriptor, /// Parse an expression tree into a descriptor. @@ -1564,12 +1597,14 @@ mod tests { // Same address let addr_one = desc_one - .derived_descriptor(&secp_ctx, index) + .derive(index) + .derived_descriptor(&secp_ctx) .unwrap() .address(bitcoin::Network::Bitcoin) .unwrap(); let addr_two = desc_two - .derived_descriptor(&secp_ctx, index) + .derive(index) + .derived_descriptor(&secp_ctx) .unwrap() .address(bitcoin::Network::Bitcoin) .unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 69713728e..cc4f651b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,7 +135,7 @@ use std::error; use bitcoin::blockdata::{opcodes, script}; use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; -pub use crate::descriptor::{Descriptor, DescriptorPublicKey}; +pub use crate::descriptor::{DerivedDescriptorKey, Descriptor, DescriptorPublicKey}; pub use crate::interpreter::Interpreter; pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, Tap}; pub use crate::miniscript::decode::Terminal; diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 50a7144f7..a75833aa7 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -31,13 +31,14 @@ use bitcoin::util::sighash::SighashCache; use bitcoin::util::taproot::{self, ControlBlock, LeafVersion, TapLeafHash}; use bitcoin::{self, EcdsaSighashType, SchnorrSighashType, Script}; +use crate::descriptor::DerivedDescriptorKey; use crate::miniscript::iter::PkPkh; use crate::miniscript::limits::SEQUENCE_LOCKTIME_DISABLE_FLAG; use crate::miniscript::satisfy::{After, Older}; use crate::prelude::*; use crate::{ - descriptor, interpreter, Descriptor, DescriptorPublicKey, MiniscriptKey, PkTranslator, - Preimage32, Satisfier, ToPublicKey, TranslatePk, + descriptor, interpreter, Descriptor, MiniscriptKey, PkTranslator, Preimage32, Satisfier, + ToPublicKey, TranslatePk, }; mod finalizer; @@ -568,7 +569,7 @@ pub trait PsbtExt { fn update_input_with_descriptor( &mut self, input_index: usize, - descriptor: &Descriptor, + descriptor: &Descriptor, ) -> Result<(), UtxoUpdateError>; /// Get the sighash message(data to sign) at input index `idx` based on the sighash @@ -735,7 +736,7 @@ impl PsbtExt for Psbt { fn update_input_with_descriptor( &mut self, input_index: usize, - desc: &Descriptor, + desc: &Descriptor, ) -> Result<(), UtxoUpdateError> { let n_inputs = self.inputs.len(); let input = self @@ -916,14 +917,14 @@ pub trait PsbtInputExt { /// [`update_input_with_descriptor`]: PsbtExt::update_input_with_descriptor fn update_with_descriptor_unchecked( &mut self, - descriptor: &Descriptor, + descriptor: &Descriptor, ) -> Result, descriptor::ConversionError>; } impl PsbtInputExt for psbt::Input { fn update_with_descriptor_unchecked( &mut self, - descriptor: &Descriptor, + descriptor: &Descriptor, ) -> Result, descriptor::ConversionError> { let (derived, _) = update_input_with_descriptor_helper(self, descriptor, None)?; Ok(derived) @@ -937,19 +938,19 @@ struct XOnlyHashLookUp( pub secp256k1::Secp256k1, ); -impl PkTranslator +impl PkTranslator for XOnlyHashLookUp { fn pk( &mut self, - xpk: &DescriptorPublicKey, + xpk: &DerivedDescriptorKey, ) -> Result { xpk.derive_public_key(&self.1) } fn pkh( &mut self, - xpk: &DescriptorPublicKey, + xpk: &DerivedDescriptorKey, ) -> Result { let pk = xpk.derive_public_key(&self.1)?; let xonly = pk.to_x_only_pubkey(); @@ -966,12 +967,12 @@ struct KeySourceLookUp( pub secp256k1::Secp256k1, ); -impl PkTranslator +impl PkTranslator for KeySourceLookUp { fn pk( &mut self, - xpk: &DescriptorPublicKey, + xpk: &DerivedDescriptorKey, ) -> Result { let derived = xpk.derive_public_key(&self.1)?; self.0.insert( @@ -983,7 +984,7 @@ impl PkTranslator Result { Ok(self.pk(xpk)?.to_pubkeyhash()) } @@ -991,7 +992,7 @@ impl PkTranslator, + descriptor: &Descriptor, check_script: Option