From 372377a5a39389d0d8b6b9a0b13da5df5a3e780e Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 13 Jun 2022 09:58:27 -0700 Subject: [PATCH 1/6] Remove PkTranslator trait Create macros for writing Translator functions The trait has a general implementation of Translator like `impl Translator for PkTranslator where P::Sha256 ...`. However, this blanket implementation has constraints on associated types and makes it impossible to implement the trait for a generic type downstream. Rust compiler does not support trait specialization yet, so we should only provide a macro to ease implementation rather than a blanket implementation that causes duplicate conflicts downstream --- examples/taproot.rs | 23 ++------ src/descriptor/mod.rs | 12 ++-- src/interpreter/inner.rs | 9 ++- src/lib.rs | 62 ++------------------ src/policy/concrete.rs | 20 +------ src/policy/semantic.rs | 21 ++----- src/psbt/mod.rs | 21 +++++-- src/pub_macros.rs | 120 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 169 insertions(+), 119 deletions(-) create mode 100644 src/pub_macros.rs diff --git a/examples/taproot.rs b/examples/taproot.rs index 8af46f0c2..92feafec7 100644 --- a/examples/taproot.rs +++ b/examples/taproot.rs @@ -1,12 +1,12 @@ use std::collections::HashMap; use std::str::FromStr; -use bitcoin::hashes::{hash160, ripemd160, sha256}; +use bitcoin::hashes::hash160; use bitcoin::util::address::WitnessVersion; use bitcoin::Network; use miniscript::descriptor::DescriptorType; use miniscript::policy::Concrete; -use miniscript::{hash256, Descriptor, Miniscript, Tap, TranslatePk, Translator}; +use miniscript::{translate_hash_fail, Descriptor, Miniscript, Tap, TranslatePk, Translator}; use secp256k1::{rand, KeyPair}; // Refer to https://github.com/sanket1729/adv_btc_workshop/blob/master/workshop.md#creating-a-taproot-descriptor @@ -25,21 +25,10 @@ impl Translator for StrPkTranslator { unreachable!("Policy doesn't contain any pkh fragment"); } - fn sha256(&mut self, _sha256: &String) -> Result { - unreachable!("Policy does not contain any sha256 fragment"); - } - - fn hash256(&mut self, _sha256: &String) -> Result { - unreachable!("Policy does not contain any hash256 fragment"); - } - - fn ripemd160(&mut self, _ripemd160: &String) -> Result { - unreachable!("Policy does not contain any ripemd160 fragment"); - } - - fn hash160(&mut self, _hash160: &String) -> Result { - unreachable!("Policy does not contain any hash160 fragment"); - } + // We don't need to implement these methods as we are not using them in the policy + // Fail if we encounter any hash fragments. + // See also translate_hash_clone! macro + translate_hash_fail!(String, bitcoin::XOnlyPublicKey, ()); } fn main() { diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index fb7cd1687..11fd88cd8 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -37,8 +37,8 @@ use self::checksum::verify_checksum; use crate::miniscript::{Legacy, Miniscript, Segwitv0}; use crate::prelude::*; use crate::{ - expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, PkTranslator, - Satisfier, ToPublicKey, TranslatePk, Translator, + expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, Satisfier, + ToPublicKey, TranslatePk, Translator, }; mod bare; @@ -532,7 +532,7 @@ impl Descriptor { pub fn at_derivation_index(&self, index: u32) -> Descriptor { struct Derivator(u32); - impl PkTranslator for Derivator { + impl Translator for Derivator { fn pk(&mut self, pk: &DescriptorPublicKey) -> Result { Ok(pk.clone().at_derivation_index(self.0)) } @@ -540,6 +540,8 @@ impl Descriptor { fn pkh(&mut self, pkh: &DescriptorPublicKey) -> Result { Ok(pkh.clone().at_derivation_index(self.0)) } + + translate_hash_clone!(DescriptorPublicKey, DescriptorPublicKey, ()); } self.translate_pk(&mut Derivator(index)) .expect("BIP 32 key index substitution cannot fail") @@ -766,7 +768,7 @@ impl Descriptor { struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1); impl<'a, C: secp256k1::Verification> - PkTranslator + Translator for Derivator<'a, C> { fn pk( @@ -782,6 +784,8 @@ impl Descriptor { ) -> Result { Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash()) } + + translate_hash_clone!(DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError); } let derived = self.translate_pk(&mut Derivator(secp))?; diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs index efe6a44aa..0e1a257d5 100644 --- a/src/interpreter/inner.rs +++ b/src/interpreter/inner.rs @@ -20,7 +20,7 @@ use bitcoin::util::taproot::{ControlBlock, TAPROOT_ANNEX_PREFIX}; use super::{stack, BitcoinKey, Error, Stack, TypedHash160}; use crate::miniscript::context::{NoChecks, ScriptContext}; use crate::prelude::*; -use crate::{BareCtx, Legacy, Miniscript, MiniscriptKey, PkTranslator, Segwitv0, Tap}; +use crate::{BareCtx, Legacy, Miniscript, MiniscriptKey, Segwitv0, Tap, Translator}; /// Attempts to parse a slice as a Bitcoin public key, checking compressedness /// if asked to, but otherwise dropping it @@ -377,7 +377,7 @@ impl ToNoChecks for Miniscript { fn to_no_checks_ms(&self) -> Miniscript { struct TranslateFullPk; - impl PkTranslator for TranslateFullPk { + impl Translator for TranslateFullPk { fn pk(&mut self, pk: &bitcoin::PublicKey) -> Result { Ok(BitcoinKey::Fullkey(*pk)) } @@ -385,6 +385,8 @@ impl ToNoChecks for Miniscript { fn pkh(&mut self, pkh: &hash160::Hash) -> Result { Ok(TypedHash160::FullKey(*pkh)) } + + translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, ()); } self.real_translate_pk(&mut TranslateFullPk) @@ -397,7 +399,7 @@ impl ToNoChecks for Miniscript // specify the () error type as this cannot error struct TranslateXOnlyPk; - impl PkTranslator for TranslateXOnlyPk { + impl Translator for TranslateXOnlyPk { fn pk(&mut self, pk: &bitcoin::XOnlyPublicKey) -> Result { Ok(BitcoinKey::XOnlyPublicKey(*pk)) } @@ -405,6 +407,7 @@ impl ToNoChecks for Miniscript fn pkh(&mut self, pkh: &hash160::Hash) -> Result { Ok(TypedHash160::XonlyKey(*pkh)) } + translate_hash_clone!(bitcoin::XOnlyPublicKey, BitcoinKey, ()); } self.real_translate_pk(&mut TranslateXOnlyPk) .expect("Translation should succeed") diff --git a/src/lib.rs b/src/lib.rs index d735f91de..51b892704 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,6 +115,11 @@ extern crate test; #[macro_use] mod macros; +#[macro_use] +mod pub_macros; + +pub use pub_macros::*; + pub mod descriptor; pub mod expression; pub mod interpreter; @@ -586,63 +591,6 @@ where fn hash160(&mut self, hash160: &P::Hash160) -> Result; } -/// Provides the conversion information required in [`TranslatePk`]. -/// Same as [`Translator`], but useful when all the associated types apart -/// from Pk/Pkh don't change in translation -pub trait PkTranslator -where - P: MiniscriptKey, - Q: MiniscriptKey, -{ - /// Provides the translation public keys P -> Q - fn pk(&mut self, pk: &P) -> Result; - - /// Provides the translation public keys hashes P::Hash -> Q::Hash - fn pkh(&mut self, pkh: &P::RawPkHash) -> Result; -} - -impl Translator for T -where - T: PkTranslator, - P: MiniscriptKey, - Q: MiniscriptKey< - Sha256 = P::Sha256, - Hash256 = P::Hash256, - Ripemd160 = P::Ripemd160, - Hash160 = P::Hash160, - >, -{ - fn pk(&mut self, pk: &P) -> Result { - >::pk(self, pk) - } - - fn pkh( - &mut self, - pkh: &

::RawPkHash, - ) -> Result<::RawPkHash, E> { - >::pkh(self, pkh) - } - - fn sha256(&mut self, sha256: &

::Sha256) -> Result<::Sha256, E> { - Ok(sha256.clone()) - } - - fn hash256(&mut self, hash256: &

::Hash256) -> Result<::Hash256, E> { - Ok(hash256.clone()) - } - - fn ripemd160( - &mut self, - ripemd160: &

::Ripemd160, - ) -> Result<::Ripemd160, E> { - Ok(ripemd160.clone()) - } - - fn hash160(&mut self, hash160: &

::Hash160) -> Result<::Hash160, E> { - Ok(hash160.clone()) - } -} - /// Converts a descriptor using abstract keys to one using specific keys. Uses translator `t` to do /// the actual translation function calls. pub trait TranslatePk diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 7f24680ae..ae8875bab 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -684,6 +684,7 @@ impl Policy { /// ``` /// use miniscript::{bitcoin::PublicKey, policy::concrete::Policy, Translator, hash256}; /// use std::str::FromStr; + /// use miniscript::translate_hash_fail; /// use std::collections::HashMap; /// use miniscript::bitcoin::hashes::{sha256, hash160, ripemd160}; /// let alice_key = "0270cf3c71f65a3d93d285d9149fddeeb638f87a2d4d8cf16c525f71c417439777"; @@ -709,23 +710,8 @@ impl Policy { /// unreachable!("Policy does not contain any pkh fragment"); /// } /// - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn sha256(&mut self, sha256: &String) -> Result { - /// unreachable!("Policy does not contain any sha256 fragment"); - /// } - /// - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn hash256(&mut self, sha256: &String) -> Result { - /// unreachable!("Policy does not contain any sha256 fragment"); - /// } - /// - /// fn ripemd160(&mut self, ripemd160: &String) -> Result { - /// unreachable!("Policy does not contain any ripemd160 fragment"); - /// } - /// - /// fn hash160(&mut self, hash160: &String) -> Result { - /// unreachable!("Policy does not contain any hash160 fragment"); - /// } + /// // Fail for hash types + /// translate_hash_fail!(String, bitcoin::PublicKey, ()); /// } /// /// let mut pk_map = HashMap::new(); diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 7d0748ba5..5798366fa 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -99,6 +99,7 @@ impl Policy { /// /// ``` /// use miniscript::{bitcoin::{hashes::{hash160, ripemd160}, PublicKey}, policy::semantic::Policy, Translator, hash256}; + /// use miniscript::translate_hash_fail; /// use std::str::FromStr; /// use std::collections::HashMap; /// use miniscript::bitcoin::hashes::sha256; @@ -125,23 +126,9 @@ impl Policy { /// self.pk_map.get(pkh).copied().ok_or(()) // Dummy Err /// } /// - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn sha256(&mut self, sha256: &String) -> Result { - /// unreachable!("Policy does not contain any sha256 fragment"); - /// } - /// - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn hash256(&mut self, sha256: &String) -> Result { - /// unreachable!("Policy does not contain any sha256 fragment"); - /// } - /// - /// fn ripemd160(&mut self, ripemd160: &String) -> Result { - /// unreachable!("Policy does not contain any ripemd160 fragment"); - /// } - /// - /// fn hash160(&mut self, hash160: &String) -> Result { - /// unreachable!("Policy does not contain any hash160 fragment"); - /// } + /// // Handy macro for failing if we encounter any other fragment. + /// // also see translate_hash_clone! for cloning instead of failing + /// translate_hash_fail!(String, bitcoin::PublicKey, ()); /// } /// /// let mut pk_map = HashMap::new(); diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index f46eed174..8ead76869 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -36,8 +36,8 @@ use bitcoin::{self, EcdsaSighashType, LockTime, SchnorrSighashType, Script, Sequ use crate::miniscript::iter::PkPkh; use crate::prelude::*; use crate::{ - descriptor, interpreter, DefiniteDescriptorKey, Descriptor, MiniscriptKey, PkTranslator, - Preimage32, Satisfier, ToPublicKey, TranslatePk, + descriptor, interpreter, DefiniteDescriptorKey, Descriptor, DescriptorPublicKey, MiniscriptKey, + Preimage32, Satisfier, ToPublicKey, TranslatePk, Translator, }; mod finalizer; @@ -1005,7 +1005,7 @@ struct XOnlyHashLookUp( pub secp256k1::Secp256k1, ); -impl PkTranslator +impl Translator for XOnlyHashLookUp { fn pk( @@ -1025,6 +1025,13 @@ impl PkTranslator, ); -impl PkTranslator +impl Translator for KeySourceLookUp { fn pk( @@ -1055,6 +1062,12 @@ impl PkTranslator Result { Ok(self.pk(xpk)?.to_pubkeyhash()) } + + translate_hash_clone!( + DescriptorPublicKey, + bitcoin::PublicKey, + descriptor::ConversionError + ); } // Provides generalized access to PSBT fields common to inputs and outputs diff --git a/src/pub_macros.rs b/src/pub_macros.rs new file mode 100644 index 000000000..2e7bbd479 --- /dev/null +++ b/src/pub_macros.rs @@ -0,0 +1,120 @@ +//! Macros exported by the miniscript crate +//! + +/// Macro for failing translation for other associated types. +/// Handy for testing String -> concrete keys as we don't want to specify these +/// functions repeatedly. +/// +/// This macro is handy when dealing with scripts that are only contain keys. +/// See also [`crate::translate_hash_clone`] +/// ```rust +/// use miniscript::{bitcoin::PublicKey, policy::concrete::Policy, Translator, hash256}; +/// use std::str::FromStr; +/// use miniscript::translate_hash_fail; +/// use std::collections::HashMap; +/// use miniscript::bitcoin::hashes::{sha256, hash160, ripemd160}; +/// let alice_key = "0270cf3c71f65a3d93d285d9149fddeeb638f87a2d4d8cf16c525f71c417439777"; +/// let bob_key = "02f43b15c50a436f5335dbea8a64dd3b4e63e34c3b50c42598acb5f4f336b5d2fb"; +/// let placeholder_policy = Policy::::from_str("and(pk(alice_key),pk(bob_key))").unwrap(); +/// +/// // Information to translator abstract String type keys to concrete bitcoin::PublicKey. +/// // In practice, wallets would map from String key names to BIP32 keys +/// struct StrPkTranslator { +/// pk_map: HashMap +/// } +/// +/// // If we also wanted to provide mapping of other associated types(sha256, older etc), +/// // we would use the general Translator Trait. +/// impl Translator for StrPkTranslator { +/// // Provides the translation public keys P -> Q +/// fn pk(&mut self, pk: &String) -> Result { +/// self.pk_map.get(pk).copied().ok_or(()) // Dummy Err +/// } +/// +/// // If our policy also contained other fragments, we could provide the translation here. +/// fn pkh(&mut self, pkh: &String) -> Result { +/// unreachable!("Policy does not contain any pkh fragment"); +/// } +/// +/// // Fail for hash types +/// translate_hash_fail!(String, bitcoin::PublicKey, ()); +/// } +/// +/// let mut pk_map = HashMap::new(); +/// pk_map.insert(String::from("alice_key"), bitcoin::PublicKey::from_str(alice_key).unwrap()); +/// pk_map.insert(String::from("bob_key"), bitcoin::PublicKey::from_str(bob_key).unwrap()); +/// let mut t = StrPkTranslator { pk_map: pk_map }; +/// ``` +#[macro_export] +macro_rules! translate_hash_fail { + ($source: ty, $target:ty, $error_ty: ty) => { + fn sha256( + &mut self, + _sha256: &<$source as $crate::MiniscriptKey>::Sha256, + ) -> Result<<$target as $crate::MiniscriptKey>::Sha256, $error_ty> { + panic!("Called sha256 on translate_only_pk") + } + + fn hash256( + &mut self, + _hash256: &<$source as $crate::MiniscriptKey>::Hash256, + ) -> Result<<$target as $crate::MiniscriptKey>::Hash256, $error_ty> { + panic!("Called hash256 on translate_only_pk") + } + + fn hash160( + &mut self, + _hash160: &<$source as $crate::MiniscriptKey>::Hash160, + ) -> Result<<$target as $crate::MiniscriptKey>::Hash160, $error_ty> { + panic!("Called hash160 on translate_only_pk") + } + + fn ripemd160( + &mut self, + _ripemd160: &<$source as $crate::MiniscriptKey>::Ripemd160, + ) -> Result<<$target as $crate::MiniscriptKey>::Ripemd160, $error_ty> { + panic!("Called ripemd160 on translate_only_pk") + } + }; +} + +/// Macro for translation of associated types where the associated type is the same +/// Handy for Derived -> concrete keys where the associated types are the same. +/// +/// Writing the complete translator trait is tedious. This macro is handy when +/// we are not trying the associated types for hash160, ripemd160, hash256 and +/// sha256. +/// +/// See also [`crate::translate_hash_fail`] +#[macro_export] +macro_rules! translate_hash_clone { + ($source: ty, $target:ty, $error_ty: ty) => { + fn sha256( + &mut self, + sha256: &<$source as $crate::MiniscriptKey>::Sha256, + ) -> Result<<$target as $crate::MiniscriptKey>::Sha256, $error_ty> { + Ok((*sha256).into()) + } + + fn hash256( + &mut self, + hash256: &<$source as $crate::MiniscriptKey>::Hash256, + ) -> Result<<$target as $crate::MiniscriptKey>::Hash256, $error_ty> { + Ok((*hash256).into()) + } + + fn hash160( + &mut self, + hash160: &<$source as $crate::MiniscriptKey>::Hash160, + ) -> Result<<$target as $crate::MiniscriptKey>::Hash160, $error_ty> { + Ok((*hash160).into()) + } + + fn ripemd160( + &mut self, + ripemd160: &<$source as $crate::MiniscriptKey>::Ripemd160, + ) -> Result<<$target as $crate::MiniscriptKey>::Ripemd160, $error_ty> { + Ok((*ripemd160).into()) + } + }; +} From b1d64776eac4db25d0d08ec90ad7349a65b527e7 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Tue, 20 Sep 2022 07:55:43 -0700 Subject: [PATCH 2/6] Change Semantic KeyHash(Pk::Hash) to Key(Pk) This is one of the major breaking changes touching the parsing/display and logic of semantic policies. This should only affect the application/analysis layer of things, so it is okay to break somethings here for code clarity. My understanding is that will also help the simplicity bridge --- examples/htlc.rs | 2 +- src/descriptor/bare.rs | 2 +- src/descriptor/segwitv0.rs | 2 +- src/descriptor/sh.rs | 2 +- src/descriptor/sortedmulti.rs | 5 +- src/descriptor/tr.rs | 7 +-- src/policy/concrete.rs | 2 +- src/policy/mod.rs | 32 +++++----- src/policy/semantic.rs | 106 ++++++++++++++++------------------ 9 files changed, 76 insertions(+), 84 deletions(-) diff --git a/examples/htlc.rs b/examples/htlc.rs index 38c12a14f..46c80b2f0 100644 --- a/examples/htlc.rs +++ b/examples/htlc.rs @@ -48,7 +48,7 @@ fn main() { // Lift the descriptor into an abstract policy. assert_eq!( format!("{}", htlc_descriptor.lift().unwrap()), - "or(and(pkh(4377a5acd66dc5cb67148a24818d1e51fa183bd2),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pkh(51814f108670aced2d77c1805ddd6634bc9d4731),older(4444)))" +"or(and(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pk(020202020202020202020202020202020202020202020202020202020202020202),older(4444)))" ); // Get the scriptPpubkey for this Wsh descriptor. diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 9d5dbcc85..b99d96118 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -294,7 +294,7 @@ impl fmt::Display for Pkh { impl Liftable for Pkh { fn lift(&self) -> Result, Error> { - Ok(semantic::Policy::KeyHash(self.pk.to_pubkeyhash())) + Ok(semantic::Policy::Key(self.pk.clone())) } } diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 9f306ccbc..b78413752 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -407,7 +407,7 @@ impl fmt::Display for Wpkh { impl Liftable for Wpkh { fn lift(&self) -> Result, Error> { - Ok(semantic::Policy::KeyHash(self.pk.to_pubkeyhash())) + Ok(semantic::Policy::Key(self.pk.clone())) } } diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 3b387fc85..55de131f7 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -59,7 +59,7 @@ impl Liftable for Sh { fn lift(&self) -> Result, Error> { match self.inner { ShInner::Wsh(ref wsh) => wsh.lift(), - ShInner::Wpkh(ref pk) => Ok(semantic::Policy::KeyHash(pk.as_inner().to_pubkeyhash())), + ShInner::Wpkh(ref pk) => Ok(semantic::Policy::Key(pk.as_inner().clone())), ShInner::SortedMulti(ref smv) => smv.lift(), ShInner::Ms(ref ms) => ms.lift(), } diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index a79e84507..8d22cc811 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -216,9 +216,8 @@ impl policy::Liftable for SortedMulti let ret = policy::semantic::Policy::Threshold( self.k, self.pks - .clone() - .into_iter() - .map(|k| policy::semantic::Policy::KeyHash(k.to_pubkeyhash())) + .iter() + .map(|k| policy::semantic::Policy::Key(k.clone())) .collect(), ); Ok(ret) diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index f1ba69d01..60fce8708 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -548,12 +548,9 @@ impl Liftable for Tr { match &self.tree { Some(root) => Ok(Policy::Threshold( 1, - vec![ - Policy::KeyHash(self.internal_key.to_pubkeyhash()), - root.lift()?, - ], + vec![Policy::Key(self.internal_key.clone()), root.lift()?], )), - None => Ok(Policy::KeyHash(self.internal_key.to_pubkeyhash())), + None => Ok(Policy::Key(self.internal_key.clone())), } } } diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index ae8875bab..a74c2ced5 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -353,7 +353,7 @@ impl Policy { for key in concrete_keys.into_iter() { if semantic_policy .clone() - .satisfy_constraint(&Semantic::KeyHash(key.to_pubkeyhash()), true) + .satisfy_constraint(&Semantic::Key(key.clone()), true) == Semantic::Trivial { match key_prob_map.get(&Concrete::Key(key.clone())) { diff --git a/src/policy/mod.rs b/src/policy/mod.rs index e2891f089..c7cdcddf3 100644 --- a/src/policy/mod.rs +++ b/src/policy/mod.rs @@ -66,6 +66,8 @@ pub enum LiftError { HeightTimelockCombination, /// Duplicate Public Keys BranchExceedResourceLimits, + /// Cannot lift raw descriptors + RawDescriptorLift, } impl fmt::Display for LiftError { @@ -77,6 +79,7 @@ impl fmt::Display for LiftError { LiftError::BranchExceedResourceLimits => f.write_str( "Cannot lift policies containing one branch that exceeds resource limits", ), + LiftError::RawDescriptorLift => f.write_str("Cannot lift raw descriptors"), } } } @@ -87,7 +90,7 @@ impl error::Error for LiftError { use self::LiftError::*; match self { - HeightTimelockCombination | BranchExceedResourceLimits => None, + HeightTimelockCombination | BranchExceedResourceLimits | RawDescriptorLift => None, } } } @@ -124,8 +127,10 @@ impl Liftable for Miniscript impl Liftable for Terminal { fn lift(&self) -> Result, Error> { let ret = match *self { - Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => Semantic::KeyHash(pk.to_pubkeyhash()), - Terminal::RawPkH(ref pkh) => Semantic::KeyHash(pkh.clone()), + Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => Semantic::Key(pk.clone()), + Terminal::RawPkH(ref _pkh) => { + return Err(Error::LiftError(LiftError::RawDescriptorLift)) + } Terminal::After(t) => Semantic::After(t), Terminal::Older(t) => Semantic::Older(t), Terminal::Sha256(ref h) => Semantic::Sha256(h.clone()), @@ -161,12 +166,9 @@ impl Liftable for Terminal { let semantic_subs: Result<_, Error> = subs.iter().map(|s| s.node.lift()).collect(); Semantic::Threshold(k, semantic_subs?) } - Terminal::Multi(k, ref keys) | Terminal::MultiA(k, ref keys) => Semantic::Threshold( - k, - keys.iter() - .map(|k| Semantic::KeyHash(k.to_pubkeyhash())) - .collect(), - ), + Terminal::Multi(k, ref keys) | Terminal::MultiA(k, ref keys) => { + Semantic::Threshold(k, keys.iter().map(|k| Semantic::Key(k.clone())).collect()) + } } .normalized(); Ok(ret) @@ -200,7 +202,7 @@ impl Liftable for Concrete { let ret = match *self { Concrete::Unsatisfiable => Semantic::Unsatisfiable, Concrete::Trivial => Semantic::Trivial, - Concrete::Key(ref pk) => Semantic::KeyHash(pk.to_pubkeyhash()), + Concrete::Key(ref pk) => Semantic::Key(pk.clone()), Concrete::After(t) => Semantic::After(t), Concrete::Older(t) => Semantic::Older(t), Concrete::Sha256(ref h) => Semantic::Sha256(h.clone()), @@ -281,9 +283,9 @@ mod tests { concrete_policy_rtt("or(99@pk(),1@pk())"); concrete_policy_rtt("and(pk(),or(99@pk(),1@older(12960)))"); - semantic_policy_rtt("pkh()"); - semantic_policy_rtt("or(pkh(),pkh())"); - semantic_policy_rtt("and(pkh(),pkh())"); + semantic_policy_rtt("pk()"); + semantic_policy_rtt("or(pk(),pk())"); + semantic_policy_rtt("and(pk(),pk())"); //fuzzer crashes assert!(ConcretePol::from_str("thresh()").is_err()); @@ -362,11 +364,11 @@ mod tests { Semantic::Threshold( 2, vec![ - Semantic::KeyHash(key_a.pubkey_hash().as_hash()), + Semantic::Key(key_a), Semantic::Older(Sequence::from_height(42)) ] ), - Semantic::KeyHash(key_b.pubkey_hash().as_hash()) + Semantic::Key(key_b) ] ), ms_str.lift().unwrap() diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 5798366fa..a62d7c018 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -37,7 +37,7 @@ pub enum Policy { /// Trivially satisfiable Trivial, /// Signature and public key matching a given hash is required - KeyHash(Pk::RawPkHash), + Key(Pk), /// An absolute locktime restriction After(PackedLockTime), /// A relative locktime restriction @@ -79,7 +79,7 @@ impl ForEachKey for Policy { { match *self { Policy::Unsatisfiable | Policy::Trivial => true, - Policy::KeyHash(ref _pkh) => todo!("Semantic Policy KeyHash must store Pk"), + Policy::Key(ref _pkh) => todo!("Semantic Policy KeyHash must store Pk"), Policy::Sha256(..) | Policy::Hash256(..) | Policy::Ripemd160(..) @@ -98,32 +98,31 @@ impl Policy { /// # Example /// /// ``` - /// use miniscript::{bitcoin::{hashes::{hash160, ripemd160}, PublicKey}, policy::semantic::Policy, Translator, hash256}; + /// use miniscript::{bitcoin::{hashes::hash160, PublicKey}, policy::semantic::Policy, Translator}; /// use miniscript::translate_hash_fail; /// use std::str::FromStr; /// use std::collections::HashMap; - /// use miniscript::bitcoin::hashes::sha256; - /// let alice_pkh = "236ada020df3208d2517f4b0db03e16f92cd8cf1"; - /// let bob_pkh = "3e89b972416ae33870b4634d03b8cdc773200cac"; - /// let placeholder_policy = Policy::::from_str("and(pkh(alice_pkh),pkh(bob_pkh))").unwrap(); + /// let alice_pk = "02c79ef3ede6d14f72a00d0e49b4becfb152197b64c0707425c4f231df29500ee7"; + /// let bob_pk = "03d008a849fbf474bd17e9d2c1a827077a468150e58221582ec3410ab309f5afe4"; + /// let placeholder_policy = Policy::::from_str("and(pk(alice_pk),pk(bob_pk))").unwrap(); /// /// // Information to translator abstract String type keys to concrete bitcoin::PublicKey. /// // In practice, wallets would map from String key names to BIP32 keys /// struct StrPkTranslator { - /// pk_map: HashMap + /// pk_map: HashMap /// } /// /// // If we also wanted to provide mapping of other associated types(sha256, older etc), /// // we would use the general Translator Trait. /// impl Translator for StrPkTranslator { /// fn pk(&mut self, pk: &String) -> Result { - /// unreachable!("Policy does not contain any pk fragment"); + /// self.pk_map.get(pk).copied().ok_or(()) // Dummy Err /// } /// /// // Provides the translation public keys P::Hash -> Q::Hash /// // If our policy also contained other fragments, we could provide the translation here. /// fn pkh(&mut self, pkh: &String) -> Result { - /// self.pk_map.get(pkh).copied().ok_or(()) // Dummy Err + /// unreachable!("Policy does not contain any pk fragment"); /// } /// /// // Handy macro for failing if we encounter any other fragment. @@ -132,13 +131,13 @@ impl Policy { /// } /// /// let mut pk_map = HashMap::new(); - /// pk_map.insert(String::from("alice_pkh"), hash160::Hash::from_str(alice_pkh).unwrap()); - /// pk_map.insert(String::from("bob_pkh"), hash160::Hash::from_str(bob_pkh).unwrap()); + /// pk_map.insert(String::from("alice_pk"), bitcoin::PublicKey::from_str(alice_pk).unwrap()); + /// pk_map.insert(String::from("bob_pk"), bitcoin::PublicKey::from_str(bob_pk).unwrap()); /// let mut t = StrPkTranslator { pk_map: pk_map }; /// /// let real_policy = placeholder_policy.translate_pkh(&mut t).unwrap(); /// - /// let expected_policy = Policy::from_str(&format!("and(pkh({}),pkh({}))", alice_pkh, bob_pkh)).unwrap(); + /// let expected_policy = Policy::from_str(&format!("and(pk({}),pk({}))", alice_pk, bob_pk)).unwrap(); /// assert_eq!(real_policy, expected_policy); /// ``` pub fn translate_pkh(&self, t: &mut T) -> Result, E> @@ -157,7 +156,7 @@ impl Policy { match *self { Policy::Unsatisfiable => Ok(Policy::Unsatisfiable), Policy::Trivial => Ok(Policy::Trivial), - Policy::KeyHash(ref pkh) => t.pkh(pkh).map(Policy::KeyHash), + Policy::Key(ref pk) => t.pk(pk).map(Policy::Key), Policy::Sha256(ref h) => t.sha256(h).map(Policy::Sha256), Policy::Hash256(ref h) => t.hash256(h).map(Policy::Hash256), Policy::Ripemd160(ref h) => t.ripemd160(h).map(Policy::Ripemd160), @@ -262,7 +261,7 @@ impl fmt::Debug for Policy { match *self { Policy::Unsatisfiable => f.write_str("UNSATISFIABLE()"), Policy::Trivial => f.write_str("TRIVIAL()"), - Policy::KeyHash(ref pkh) => write!(f, "pkh({:?})", pkh), + Policy::Key(ref pkh) => write!(f, "pk({:?})", pkh), Policy::After(n) => write!(f, "after({})", n), Policy::Older(n) => write!(f, "older({})", n), Policy::Sha256(ref h) => write!(f, "sha256({})", h), @@ -295,7 +294,7 @@ impl fmt::Display for Policy { match *self { Policy::Unsatisfiable => f.write_str("UNSATISFIABLE"), Policy::Trivial => f.write_str("TRIVIAL"), - Policy::KeyHash(ref pkh) => write!(f, "pkh({})", pkh), + Policy::Key(ref pkh) => write!(f, "pk({})", pkh), Policy::After(n) => write!(f, "after({})", n), Policy::Older(n) => write!(f, "older({})", n), Policy::Sha256(ref h) => write!(f, "sha256({})", h), @@ -346,10 +345,7 @@ impl_from_tree!( match (top.name, top.args.len()) { ("UNSATISFIABLE", 0) => Ok(Policy::Unsatisfiable), ("TRIVIAL", 0) => Ok(Policy::Trivial), - ("pkh", 1) => expression::terminal(&top.args[0], |pk| { - // TODO: This will be fixed up in a later commit that changes semantic policy to Pk from Pk::Hash - Pk::RawPkHash::from_str(pk).map(Policy::KeyHash) - }), + ("pk", 1) => expression::terminal(&top.args[0], |pk| Pk::from_str(pk).map(Policy::Key)), ("after", 1) => expression::terminal(&top.args[0], |x| { expression::parse_num(x).map(|x| Policy::after(x)) }), @@ -503,7 +499,7 @@ impl Policy { match *self { Policy::Unsatisfiable | Policy::Trivial - | Policy::KeyHash(..) + | Policy::Key(..) | Policy::Sha256(..) | Policy::Hash256(..) | Policy::Ripemd160(..) @@ -531,7 +527,7 @@ impl Policy { match *self { Policy::Unsatisfiable | Policy::Trivial - | Policy::KeyHash(..) + | Policy::Key(..) | Policy::Sha256(..) | Policy::Hash256(..) | Policy::Ripemd160(..) @@ -609,7 +605,7 @@ impl Policy { pub fn n_keys(&self) -> usize { match *self { Policy::Unsatisfiable | Policy::Trivial => 0, - Policy::KeyHash(..) => 1, + Policy::Key(..) => 1, Policy::After(..) | Policy::Older(..) | Policy::Sha256(..) @@ -627,7 +623,7 @@ impl Policy { match *self { Policy::Unsatisfiable => None, Policy::Trivial => Some(0), - Policy::KeyHash(..) => Some(1), + Policy::Key(..) => Some(1), Policy::After(..) | Policy::Older(..) | Policy::Sha256(..) @@ -681,23 +677,29 @@ mod tests { assert!(StringPolicy::from_str("(").is_err()); assert!(StringPolicy::from_str("(x()").is_err()); assert!(StringPolicy::from_str("(\u{7f}()3").is_err()); - assert!(StringPolicy::from_str("pkh()").is_ok()); + assert!(StringPolicy::from_str("pk()").is_ok()); assert!(StringPolicy::from_str("or(or)").is_err()); - assert!(Policy::::from_str("pkh()").is_err()); + assert!(Policy::::from_str("pk()").is_err()); assert!(Policy::::from_str( - "pkh(\ + "pk(\ 0200000000000000000000000000000000000002\ )" ) + .is_err()); + assert!(Policy::::from_str( + "pk(\ + 02c79ef3ede6d14f72a00d0e49b4becfb152197b64c0707425c4f231df29500ee7\ + )" + ) .is_ok()); } #[test] fn semantic_analysis() { - let policy = StringPolicy::from_str("pkh()").unwrap(); - assert_eq!(policy, Policy::KeyHash("".to_owned())); + let policy = StringPolicy::from_str("pk()").unwrap(); + assert_eq!(policy, Policy::Key("".to_owned())); assert_eq!(policy.relative_timelocks(), vec![]); assert_eq!(policy.absolute_timelocks(), vec![]); assert_eq!(policy.clone().at_age(Sequence::ZERO), policy.clone()); @@ -728,13 +730,13 @@ mod tests { assert_eq!(policy.n_keys(), 0); assert_eq!(policy.minimum_n_keys(), Some(0)); - let policy = StringPolicy::from_str("or(pkh(),older(1000))").unwrap(); + let policy = StringPolicy::from_str("or(pk(),older(1000))").unwrap(); assert_eq!( policy, Policy::Threshold( 1, vec![ - Policy::KeyHash("".to_owned()), + Policy::Key("".to_owned()), Policy::Older(Sequence::from_height(1000)), ] ) @@ -743,11 +745,11 @@ mod tests { assert_eq!(policy.absolute_timelocks(), vec![]); assert_eq!( policy.clone().at_age(Sequence::ZERO), - Policy::KeyHash("".to_owned()) + Policy::Key("".to_owned()) ); assert_eq!( policy.clone().at_age(Sequence::from_height(999)), - Policy::KeyHash("".to_owned()) + Policy::Key("".to_owned()) ); assert_eq!( policy.clone().at_age(Sequence::from_height(1000)), @@ -760,26 +762,20 @@ mod tests { assert_eq!(policy.n_keys(), 1); assert_eq!(policy.minimum_n_keys(), Some(0)); - let policy = StringPolicy::from_str("or(pkh(),UNSATISFIABLE)").unwrap(); + let policy = StringPolicy::from_str("or(pk(),UNSATISFIABLE)").unwrap(); assert_eq!( policy, - Policy::Threshold( - 1, - vec![Policy::KeyHash("".to_owned()), Policy::Unsatisfiable,] - ) + Policy::Threshold(1, vec![Policy::Key("".to_owned()), Policy::Unsatisfiable,]) ); assert_eq!(policy.relative_timelocks(), vec![]); assert_eq!(policy.absolute_timelocks(), vec![]); assert_eq!(policy.n_keys(), 1); assert_eq!(policy.minimum_n_keys(), Some(1)); - let policy = StringPolicy::from_str("and(pkh(),UNSATISFIABLE)").unwrap(); + let policy = StringPolicy::from_str("and(pk(),UNSATISFIABLE)").unwrap(); assert_eq!( policy, - Policy::Threshold( - 2, - vec![Policy::KeyHash("".to_owned()), Policy::Unsatisfiable,] - ) + Policy::Threshold(2, vec![Policy::Key("".to_owned()), Policy::Unsatisfiable,]) ); assert_eq!(policy.relative_timelocks(), vec![]); assert_eq!(policy.absolute_timelocks(), vec![]); @@ -934,25 +930,25 @@ mod tests { fn entailment_liquid_test() { //liquid policy let liquid_pol = StringPolicy::from_str( - "or(and(older(4096),thresh(2,pkh(A),pkh(B),pkh(C))),thresh(11,pkh(F1),pkh(F2),pkh(F3),pkh(F4),pkh(F5),pkh(F6),pkh(F7),pkh(F8),pkh(F9),pkh(F10),pkh(F11),pkh(F12),pkh(F13),pkh(F14)))").unwrap(); + "or(and(older(4096),thresh(2,pk(A),pk(B),pk(C))),thresh(11,pk(F1),pk(F2),pk(F3),pk(F4),pk(F5),pk(F6),pk(F7),pk(F8),pk(F9),pk(F10),pk(F11),pk(F12),pk(F13),pk(F14)))").unwrap(); // Very bad idea to add master key,pk but let's have it have 50M blocks - let master_key = StringPolicy::from_str("and(older(50000000),pkh(master))").unwrap(); + let master_key = StringPolicy::from_str("and(older(50000000),pk(master))").unwrap(); let new_liquid_pol = Policy::Threshold(1, vec![liquid_pol.clone(), master_key]); assert!(liquid_pol.clone().entails(new_liquid_pol.clone()).unwrap()); assert!(!new_liquid_pol.entails(liquid_pol.clone()).unwrap()); // test liquid backup policy before the emergency timeout - let backup_policy = StringPolicy::from_str("thresh(2,pkh(A),pkh(B),pkh(C))").unwrap(); + let backup_policy = StringPolicy::from_str("thresh(2,pk(A),pk(B),pk(C))").unwrap(); assert!(!backup_policy .clone() .entails(liquid_pol.clone().at_age(Sequence::from_height(4095))) .unwrap()); // Finally test both spending paths - let fed_pol = StringPolicy::from_str("thresh(11,pkh(F1),pkh(F2),pkh(F3),pkh(F4),pkh(F5),pkh(F6),pkh(F7),pkh(F8),pkh(F9),pkh(F10),pkh(F11),pkh(F12),pkh(F13),pkh(F14))").unwrap(); + let fed_pol = StringPolicy::from_str("thresh(11,pk(F1),pk(F2),pk(F3),pk(F4),pk(F5),pk(F6),pk(F7),pk(F8),pk(F9),pk(F10),pk(F11),pk(F12),pk(F13),pk(F14))").unwrap(); let backup_policy_after_expiry = - StringPolicy::from_str("and(older(4096),thresh(2,pkh(A),pkh(B),pkh(C)))").unwrap(); + StringPolicy::from_str("and(older(4096),thresh(2,pk(A),pk(B),pk(C)))").unwrap(); assert!(fed_pol.entails(liquid_pol.clone()).unwrap()); assert!(backup_policy_after_expiry .entails(liquid_pol.clone()) @@ -962,19 +958,17 @@ mod tests { #[test] fn entailment_escrow() { // Escrow contract - let escrow_pol = - StringPolicy::from_str("thresh(2,pkh(Alice),pkh(Bob),pkh(Judge))").unwrap(); + let escrow_pol = StringPolicy::from_str("thresh(2,pk(Alice),pk(Bob),pk(Judge))").unwrap(); // Alice's authorization constraint // Authorization is a constraint that states the conditions under which one party must // be able to redeem the funds. - let auth_alice = StringPolicy::from_str("and(pkh(Alice),pkh(Judge))").unwrap(); + let auth_alice = StringPolicy::from_str("and(pk(Alice),pk(Judge))").unwrap(); //Alice's Control constraint // The control constraint states the conditions that one party requires // must be met if the funds are spent by anyone // Either Alice must authorize the funds or both Judge and Bob must control it - let control_alice = - StringPolicy::from_str("or(pkh(Alice),and(pkh(Judge),pkh(Bob)))").unwrap(); + let control_alice = StringPolicy::from_str("or(pk(Alice),and(pk(Judge),pk(Bob)))").unwrap(); // Entailment rules // Authorization entails |- policy |- control constraints @@ -985,7 +979,7 @@ mod tests { // Escrow contract let h = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; let htlc_pol = StringPolicy::from_str(&format!( - "or(and(pkh(Alice),older(100)),and(pkh(Bob),sha256({})))", + "or(and(pk(Alice),older(100)),and(pk(Bob),sha256({})))", h )) .unwrap(); @@ -993,14 +987,14 @@ mod tests { // Authorization is a constraint that states the conditions under which one party must // be able to redeem the funds. In HLTC, alice only cares that she can // authorize her funds with Pk and CSV 100. - let auth_alice = StringPolicy::from_str("and(pkh(Alice),older(100))").unwrap(); + let auth_alice = StringPolicy::from_str("and(pk(Alice),older(100))").unwrap(); //Alice's Control constraint // The control constraint states the conditions that one party requires // must be met if the funds are spent by anyone // Either Alice must authorize the funds or sha2 preimage must be revealed. let control_alice = - StringPolicy::from_str(&format!("or(pkh(Alice),sha256({}))", h)).unwrap(); + StringPolicy::from_str(&format!("or(pk(Alice),sha256({}))", h)).unwrap(); // Entailment rules // Authorization entails |- policy |- control constraints From 328d15fb92aa2791410815ff64a713b24057940b Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Tue, 20 Sep 2022 07:59:30 -0700 Subject: [PATCH 3/6] Rename the Semantic::translate_pkh to translate_pk --- src/policy/semantic.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index a62d7c018..a8e6b44eb 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -135,20 +135,20 @@ impl Policy { /// pk_map.insert(String::from("bob_pk"), bitcoin::PublicKey::from_str(bob_pk).unwrap()); /// let mut t = StrPkTranslator { pk_map: pk_map }; /// - /// let real_policy = placeholder_policy.translate_pkh(&mut t).unwrap(); + /// let real_policy = placeholder_policy.translate_pk(&mut t).unwrap(); /// /// let expected_policy = Policy::from_str(&format!("and(pk({}),pk({}))", alice_pk, bob_pk)).unwrap(); /// assert_eq!(real_policy, expected_policy); /// ``` - pub fn translate_pkh(&self, t: &mut T) -> Result, E> + pub fn translate_pk(&self, t: &mut T) -> Result, E> where T: Translator, Q: MiniscriptKey, { - self._translate_pkh(t) + self._translate_pk(t) } - fn _translate_pkh(&self, t: &mut T) -> Result, E> + fn _translate_pk(&self, t: &mut T) -> Result, E> where T: Translator, Q: MiniscriptKey, @@ -165,7 +165,7 @@ impl Policy { Policy::Older(n) => Ok(Policy::Older(n)), Policy::Threshold(k, ref subs) => { let new_subs: Result>, _> = - subs.iter().map(|sub| sub._translate_pkh(t)).collect(); + subs.iter().map(|sub| sub._translate_pk(t)).collect(); new_subs.map(|ok| Policy::Threshold(k, ok)) } } From fff727c5eb122cd8ff5978c64dad1660f8426e28 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Wed, 21 Sep 2022 12:18:57 -0700 Subject: [PATCH 4/6] Remove Hash associated type from RawPkh. Cleanup Translator trait by removing the translate_pkh method. RawPkh is not parsed from_str. It is only used in miniscript internally to parse scripts. --- examples/htlc.rs | 4 +- examples/taproot.rs | 5 - src/descriptor/bare.rs | 2 - src/descriptor/key.rs | 16 -- src/descriptor/mod.rs | 24 +-- src/descriptor/segwitv0.rs | 2 - src/descriptor/sh.rs | 1 - src/descriptor/sortedmulti.rs | 1 - src/descriptor/tr.rs | 1 - src/interpreter/inner.rs | 35 ++-- src/interpreter/mod.rs | 81 ++++----- src/interpreter/stack.rs | 24 +-- src/lib.rs | 89 ++-------- src/macros.rs | 12 -- src/miniscript/analyzable.rs | 12 +- src/miniscript/astelem.rs | 14 +- src/miniscript/context.rs | 2 +- src/miniscript/decode.rs | 14 +- src/miniscript/iter.rs | 312 +--------------------------------- src/miniscript/mod.rs | 14 +- src/miniscript/satisfy.rs | 95 ++++++----- src/policy/compiler.rs | 4 +- src/policy/concrete.rs | 6 - src/policy/semantic.rs | 7 - src/psbt/mod.rs | 48 ++---- src/pub_macros.rs | 5 - src/test_utils.rs | 21 +-- src/util.rs | 11 +- tests/setup/test_util.rs | 8 - tests/test_cpp.rs | 26 ++- tests/test_desc.rs | 33 +--- 31 files changed, 204 insertions(+), 725 deletions(-) diff --git a/examples/htlc.rs b/examples/htlc.rs index 46c80b2f0..4e7bf3572 100644 --- a/examples/htlc.rs +++ b/examples/htlc.rs @@ -42,13 +42,13 @@ fn main() { assert!(htlc_descriptor.sanity_check().is_ok()); assert_eq!( format!("{}", htlc_descriptor), - "wsh(andor(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111),and_v(v:pkh(51814f108670aced2d77c1805ddd6634bc9d4731),older(4444))))#s0qq76ng" + "wsh(andor(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111),and_v(v:pkh(020202020202020202020202020202020202020202020202020202020202020202),older(4444))))#lfytrjen" ); // Lift the descriptor into an abstract policy. assert_eq!( format!("{}", htlc_descriptor.lift().unwrap()), -"or(and(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pk(020202020202020202020202020202020202020202020202020202020202020202),older(4444)))" + "or(and(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pk(020202020202020202020202020202020202020202020202020202020202020202),older(4444)))" ); // Get the scriptPpubkey for this Wsh descriptor. diff --git a/examples/taproot.rs b/examples/taproot.rs index 92feafec7..3c9732817 100644 --- a/examples/taproot.rs +++ b/examples/taproot.rs @@ -1,7 +1,6 @@ use std::collections::HashMap; use std::str::FromStr; -use bitcoin::hashes::hash160; use bitcoin::util::address::WitnessVersion; use bitcoin::Network; use miniscript::descriptor::DescriptorType; @@ -21,10 +20,6 @@ impl Translator for StrPkTranslator { self.pk_map.get(pk).copied().ok_or(()) } - fn pkh(&mut self, _pkh: &String) -> Result { - unreachable!("Policy doesn't contain any pkh fragment"); - } - // We don't need to implement these methods as we are not using them in the policy // Fail if we encounter any hash fragments. // See also translate_hash_clone! macro diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index b99d96118..2495f16b1 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -167,7 +167,6 @@ impl ForEachKey for Bare { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { self.ms.for_each_key(pred) } @@ -329,7 +328,6 @@ impl ForEachKey for Pkh { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { pred(&self.pk) } diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index b12873e81..a35ac375e 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -730,8 +730,6 @@ impl DescriptorXKey { } impl MiniscriptKey for DescriptorPublicKey { - // This allows us to be able to derive public keys even for PkH s - type RawPkHash = Self; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; @@ -756,10 +754,6 @@ impl MiniscriptKey for DescriptorPublicKey { _ => false, } } - - fn to_pubkeyhash(&self) -> Self { - self.clone() - } } impl DefiniteDescriptorKey { @@ -818,8 +812,6 @@ impl fmt::Display for DefiniteDescriptorKey { } impl MiniscriptKey for DefiniteDescriptorKey { - // This allows us to be able to derive public keys even for PkH s - type RawPkHash = Self; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; @@ -832,10 +824,6 @@ impl MiniscriptKey for DefiniteDescriptorKey { fn is_x_only_key(&self) -> bool { self.0.is_x_only_key() } - - fn to_pubkeyhash(&self) -> Self { - self.clone() - } } impl ToPublicKey for DefiniteDescriptorKey { @@ -844,10 +832,6 @@ impl ToPublicKey for DefiniteDescriptorKey { self.0.derive_public_key(&secp).unwrap() } - fn hash_to_hash160(hash: &Self) -> hash160::Hash { - hash.to_public_key().to_pubkeyhash() - } - fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 11fd88cd8..6b8b2c87d 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -498,7 +498,6 @@ impl ForEachKey for Descriptor { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match *self { Descriptor::Bare(ref bare) => bare.for_each_key(pred), @@ -537,10 +536,6 @@ impl Descriptor { Ok(pk.clone().at_derivation_index(self.0)) } - fn pkh(&mut self, pkh: &DescriptorPublicKey) -> Result { - Ok(pkh.clone().at_derivation_index(self.0)) - } - translate_hash_clone!(DescriptorPublicKey, DescriptorPublicKey, ()); } self.translate_pk(&mut Derivator(index)) @@ -631,10 +626,6 @@ impl Descriptor { parse_key(pk, &mut self.0, self.1) } - fn pkh(&mut self, pkh: &String) -> Result { - parse_key(pkh, &mut self.0, self.1) - } - fn sha256(&mut self, sha256: &String) -> Result { let hash = sha256::Hash::from_str(sha256).map_err(|e| Error::Unexpected(e.to_string()))?; @@ -677,10 +668,6 @@ impl Descriptor { key_to_string(pk, self.0) } - fn pkh(&mut self, pkh: &DescriptorPublicKey) -> Result { - key_to_string(pkh, self.0) - } - fn sha256(&mut self, sha256: &sha256::Hash) -> Result { Ok(sha256.to_string()) } @@ -778,13 +765,6 @@ impl Descriptor { pk.derive_public_key(&self.0) } - fn pkh( - &mut self, - pkh: &DefiniteDescriptorKey, - ) -> Result { - Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash()) - } - translate_hash_clone!(DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError); } @@ -1344,8 +1324,8 @@ mod tests { assert_eq!( descriptor, format!( - "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh(516ca378e588a7ed71336147e2a72848b20aca1a))}}}})#xz8ny8ae", - p1, p2, p3, p4, + "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})#tvu28c0s", + p1, p2, p3, p4, p5 ) ) } diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index b78413752..118a859f9 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -249,7 +249,6 @@ impl ForEachKey for Wsh { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match self.inner { WshInner::SortedMulti(ref smv) => smv.for_each_key(pred), @@ -442,7 +441,6 @@ impl ForEachKey for Wpkh { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { pred(&self.pk) } diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 55de131f7..0c60c7b4b 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -380,7 +380,6 @@ impl ForEachKey for Sh { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match self.inner { ShInner::Wsh(ref wsh) => wsh.for_each_key(pred), diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index 8d22cc811..7857188b0 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -115,7 +115,6 @@ impl ForEachKey for SortedMultiVec bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { self.pks.iter().all(|key| pred(key)) } diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 60fce8708..d76bbd943 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -559,7 +559,6 @@ impl ForEachKey for Tr { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { let script_keys_res = self .iter_scripts() diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs index 0e1a257d5..44073c4f3 100644 --- a/src/interpreter/inner.rs +++ b/src/interpreter/inner.rs @@ -17,10 +17,10 @@ use bitcoin::blockdata::witness::Witness; use bitcoin::hashes::{hash160, sha256, Hash}; use bitcoin::util::taproot::{ControlBlock, TAPROOT_ANNEX_PREFIX}; -use super::{stack, BitcoinKey, Error, Stack, TypedHash160}; -use crate::miniscript::context::{NoChecks, ScriptContext}; +use super::{stack, BitcoinKey, Error, Stack}; +use crate::miniscript::context::{NoChecks, ScriptContext, SigType}; use crate::prelude::*; -use crate::{BareCtx, Legacy, Miniscript, MiniscriptKey, Segwitv0, Tap, Translator}; +use crate::{BareCtx, Legacy, Miniscript, Segwitv0, Tap, ToPublicKey, Translator}; /// Attempts to parse a slice as a Bitcoin public key, checking compressedness /// if asked to, but otherwise dropping it @@ -143,7 +143,8 @@ pub(super) fn from_txdata<'txin>( match ssig_stack.pop() { Some(elem) => { let pk = pk_from_stack_elem(&elem, false)?; - if *spk == bitcoin::Script::new_p2pkh(&pk.to_pubkeyhash().into()) { + if *spk == bitcoin::Script::new_p2pkh(&pk.to_pubkeyhash(SigType::Ecdsa).into()) + { Ok(( Inner::PublicKey(pk.into(), PubkeyType::Pkh), ssig_stack, @@ -164,11 +165,12 @@ pub(super) fn from_txdata<'txin>( match wit_stack.pop() { Some(elem) => { let pk = pk_from_stack_elem(&elem, true)?; - if *spk == bitcoin::Script::new_v0_p2wpkh(&pk.to_pubkeyhash().into()) { + let hash160 = pk.to_pubkeyhash(SigType::Ecdsa); + if *spk == bitcoin::Script::new_v0_p2wpkh(&hash160.into()) { Ok(( Inner::PublicKey(pk.into(), PubkeyType::Wpkh), wit_stack, - Some(bitcoin::Script::new_p2pkh(&pk.to_pubkeyhash().into())), // bip143, why.. + Some(bitcoin::Script::new_p2pkh(&hash160.into())), // bip143, why.. )) } else { Err(Error::IncorrectWPubkeyHash) @@ -274,17 +276,13 @@ pub(super) fn from_txdata<'txin>( Err(Error::NonEmptyScriptSig) } else { let pk = pk_from_stack_elem(&elem, true)?; - if slice - == &bitcoin::Script::new_v0_p2wpkh( - &pk.to_pubkeyhash().into(), - )[..] + let hash160 = pk.to_pubkeyhash(SigType::Ecdsa); + if slice == &bitcoin::Script::new_v0_p2wpkh(&hash160.into())[..] { Ok(( Inner::PublicKey(pk.into(), PubkeyType::ShWpkh), wit_stack, - Some(bitcoin::Script::new_p2pkh( - &pk.to_pubkeyhash().into(), - )), // bip143, why.. + Some(bitcoin::Script::new_p2pkh(&hash160.into())), // bip143, why.. )) } else { Err(Error::IncorrectWScriptHash) @@ -382,10 +380,6 @@ impl ToNoChecks for Miniscript { Ok(BitcoinKey::Fullkey(*pk)) } - fn pkh(&mut self, pkh: &hash160::Hash) -> Result { - Ok(TypedHash160::FullKey(*pkh)) - } - translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, ()); } @@ -404,9 +398,6 @@ impl ToNoChecks for Miniscript Ok(BitcoinKey::XOnlyPublicKey(*pk)) } - fn pkh(&mut self, pkh: &hash160::Hash) -> Result { - Ok(TypedHash160::XonlyKey(*pkh)) - } translate_hash_clone!(bitcoin::XOnlyPublicKey, BitcoinKey, ()); } self.real_translate_pk(&mut TranslateXOnlyPk) @@ -452,8 +443,8 @@ mod tests { ) .unwrap(); - let pkhash = key.to_pubkeyhash().into(); - let wpkhash = key.to_pubkeyhash().into(); + let pkhash = key.to_pubkeyhash(SigType::Ecdsa).into(); + let wpkhash = key.to_pubkeyhash(SigType::Ecdsa).into(); let wpkh_spk = bitcoin::Script::new_v0_p2wpkh(&wpkhash); let wpkh_scripthash = hash160::Hash::hash(&wpkh_spk[..]).into(); diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 24d6e9276..684782e33 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -27,7 +27,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256}; use bitcoin::util::{sighash, taproot}; use bitcoin::{self, secp256k1, LockTime, Sequence, TxOut}; -use crate::miniscript::context::NoChecks; +use crate::miniscript::context::{NoChecks, SigType}; use crate::miniscript::ScriptContext; use crate::prelude::*; use crate::{hash256, Descriptor, Miniscript, Terminal, ToPublicKey}; @@ -102,6 +102,15 @@ enum BitcoinKey { XOnlyPublicKey(bitcoin::XOnlyPublicKey), } +impl BitcoinKey { + fn to_pubkeyhash(&self, sig_type: SigType) -> hash160::Hash { + match self { + BitcoinKey::Fullkey(pk) => pk.to_pubkeyhash(sig_type), + BitcoinKey::XOnlyPublicKey(pk) => pk.to_pubkeyhash(sig_type), + } + } +} + // Displayed in full 33 byte representation. X-only keys are displayed with 0x02 prefix impl fmt::Display for BitcoinKey { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -124,42 +133,11 @@ impl From for BitcoinKey { } } -// While parsing we need to remember how to the hash was parsed so that we can -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -enum TypedHash160 { - XonlyKey(hash160::Hash), - FullKey(hash160::Hash), -} - -impl fmt::Display for TypedHash160 { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - TypedHash160::FullKey(pkh) | TypedHash160::XonlyKey(pkh) => pkh.fmt(f), - } - } -} - -impl TypedHash160 { - fn hash160(&self) -> hash160::Hash { - match self { - TypedHash160::XonlyKey(hash) | TypedHash160::FullKey(hash) => *hash, - } - } -} - impl MiniscriptKey for BitcoinKey { - type RawPkHash = TypedHash160; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; - - fn to_pubkeyhash(&self) -> Self::RawPkHash { - match self { - BitcoinKey::Fullkey(pk) => TypedHash160::FullKey(pk.to_pubkeyhash()), - BitcoinKey::XOnlyPublicKey(pk) => TypedHash160::XonlyKey(pk.to_pubkeyhash()), - } - } } impl<'txin> Interpreter<'txin> { @@ -215,6 +193,7 @@ impl<'txin> Interpreter<'txin> { age: self.age, lock_time: self.lock_time, has_errored: false, + sig_type: self.sig_type(), } } @@ -440,6 +419,22 @@ impl<'txin> Interpreter<'txin> { } } + /// Signature type of the spend + pub fn sig_type(&self) -> SigType { + match self.inner { + inner::Inner::PublicKey(_, inner::PubkeyType::Tr) => SigType::Schnorr, + inner::Inner::Script(_, inner::ScriptType::Tr) => SigType::Schnorr, + inner::Inner::PublicKey(_, inner::PubkeyType::Pk) + | inner::Inner::PublicKey(_, inner::PubkeyType::Pkh) + | inner::Inner::PublicKey(_, inner::PubkeyType::Wpkh) + | inner::Inner::PublicKey(_, inner::PubkeyType::ShWpkh) + | inner::Inner::Script(_, inner::ScriptType::Bare) + | inner::Inner::Script(_, inner::ScriptType::Sh) + | inner::Inner::Script(_, inner::ScriptType::Wsh) + | inner::Inner::Script(_, inner::ScriptType::ShWsh) => SigType::Ecdsa, + } + } + /// Outputs a "descriptor" which reproduces the spent coins /// /// This may not represent the original descriptor used to produce the transaction, @@ -534,6 +529,7 @@ pub struct Iter<'intp, 'txin: 'intp> { age: Sequence, lock_time: LockTime, has_errored: bool, + sig_type: SigType, } ///Iterator for Iter @@ -601,9 +597,11 @@ where Terminal::PkH(ref pk) => { debug_assert_eq!(node_state.n_evaluated, 0); debug_assert_eq!(node_state.n_satisfied, 0); - let res = self - .stack - .evaluate_pkh(&mut self.verify_sig, pk.to_pubkeyhash()); + let res = self.stack.evaluate_pkh( + &mut self.verify_sig, + pk.to_pubkeyhash(self.sig_type), + self.sig_type, + ); if res.is_some() { return res; } @@ -611,7 +609,9 @@ where Terminal::RawPkH(ref pkh) => { debug_assert_eq!(node_state.n_evaluated, 0); debug_assert_eq!(node_state.n_satisfied, 0); - let res = self.stack.evaluate_pkh(&mut self.verify_sig, *pkh); + let res = self + .stack + .evaluate_pkh(&mut self.verify_sig, *pkh, self.sig_type); if res.is_some() { return res; } @@ -1049,7 +1049,7 @@ mod tests { use super::inner::ToNoChecks; use super::*; use crate::miniscript::context::NoChecks; - use crate::{Miniscript, MiniscriptKey, ToPublicKey}; + use crate::{Miniscript, ToPublicKey}; fn setup_keys_sigs( n: usize, @@ -1148,6 +1148,7 @@ mod tests { age: Sequence::from_height(1002), lock_time: LockTime::from_height(1002).unwrap(), has_errored: false, + sig_type: SigType::Ecdsa, } } @@ -1197,7 +1198,7 @@ mod tests { assert_eq!( pkh_satisfied.unwrap(), vec![SatisfiedConstraint::PublicKeyHash { - keyhash: pks[1].to_pubkeyhash(), + keyhash: pks[1].to_pubkeyhash(SigType::Ecdsa), key_sig: KeySigPair::Ecdsa(pks[1], ecdsa_sigs[1]) }] ); @@ -1297,7 +1298,7 @@ mod tests { key_sig: KeySigPair::Ecdsa(pks[0], ecdsa_sigs[0]) }, SatisfiedConstraint::PublicKeyHash { - keyhash: pks[1].to_pubkeyhash(), + keyhash: pks[1].to_pubkeyhash(SigType::Ecdsa), key_sig: KeySigPair::Ecdsa(pks[1], ecdsa_sigs[1]) } ] @@ -1369,7 +1370,7 @@ mod tests { assert_eq!( and_or_satisfied.unwrap(), vec![SatisfiedConstraint::PublicKeyHash { - keyhash: pks[1].to_pubkeyhash(), + keyhash: pks[1].to_pubkeyhash(SigType::Ecdsa), key_sig: KeySigPair::Ecdsa(pks[1], ecdsa_sigs[1]) }] ); diff --git a/src/interpreter/stack.rs b/src/interpreter/stack.rs index 7fee93b4c..6405c0e8a 100644 --- a/src/interpreter/stack.rs +++ b/src/interpreter/stack.rs @@ -20,10 +20,9 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; use bitcoin::{LockTime, Sequence}; use super::error::PkEvalErrInner; -use super::{ - verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint, TypedHash160, -}; +use super::{verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint}; use crate::hash256; +use crate::miniscript::context::SigType; use crate::prelude::*; /// Definition of Stack Element of the Stack used for interpretation of Miniscript. @@ -170,26 +169,27 @@ impl<'txin> Stack<'txin> { pub(super) fn evaluate_pkh<'intp>( &mut self, verify_sig: &mut Box bool + 'intp>, - pkh: TypedHash160, + pkh: hash160::Hash, + sig_type: SigType, ) -> Option> { // Parse a bitcoin key from witness data slice depending on hash context // when we encounter a pkh(hash) // Depending on the tag of hash, we parse the as full key or x-only-key // TODO: All keys parse errors are currently captured in a single BadPubErr // We don't really store information about which key error. - fn bitcoin_key_from_slice(sl: &[u8], tag: TypedHash160) -> Option { - let key: BitcoinKey = match tag { - TypedHash160::XonlyKey(_) => bitcoin::XOnlyPublicKey::from_slice(sl).ok()?.into(), - TypedHash160::FullKey(_) => bitcoin::PublicKey::from_slice(sl).ok()?.into(), + fn bitcoin_key_from_slice(sl: &[u8], sig_type: SigType) -> Option { + let key: BitcoinKey = match sig_type { + SigType::Schnorr => bitcoin::XOnlyPublicKey::from_slice(sl).ok()?.into(), + SigType::Ecdsa => bitcoin::PublicKey::from_slice(sl).ok()?.into(), }; Some(key) } if let Some(Element::Push(pk)) = self.pop() { let pk_hash = hash160::Hash::hash(pk); - if pk_hash != pkh.hash160() { - return Some(Err(Error::PkHashVerifyFail(pkh.hash160()))); + if pk_hash != pkh { + return Some(Err(Error::PkHashVerifyFail(pkh))); } - match bitcoin_key_from_slice(pk, pkh) { + match bitcoin_key_from_slice(pk, sig_type) { Some(pk) => { if let Some(sigser) = self.pop() { match sigser { @@ -203,7 +203,7 @@ impl<'txin> Stack<'txin> { Ok(key_sig) => { self.push(Element::Satisfied); Some(Ok(SatisfiedConstraint::PublicKeyHash { - keyhash: pkh.hash160(), + keyhash: pkh, key_sig, })) } diff --git a/src/lib.rs b/src/lib.rs index 51b892704..d67e0ba92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,7 +141,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; pub use crate::descriptor::{DefiniteDescriptorKey, Descriptor, DescriptorPublicKey}; pub use crate::interpreter::Interpreter; -pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, Tap}; +pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, SigType, Tap}; pub use crate::miniscript::decode::Terminal; pub use crate::miniscript::satisfy::{Preimage32, Satisfier}; pub use crate::miniscript::{hash256, Miniscript}; @@ -161,12 +161,6 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha false } - /// The associated PublicKey Hash for this [`MiniscriptKey`], - /// used in the raw_pkh fragment - /// This fragment is only internally used for representing partial descriptors when parsing from script - /// The library does not support creating partial descriptors yet. - type RawPkHash: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - /// The associated [`sha256::Hash`] for this [`MiniscriptKey`], /// used in the hash256 fragment. type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; @@ -181,21 +175,13 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha /// The associated [`hash160::Hash`] for this [`MiniscriptKey`] type. /// used in the hash160 fragment type Hash160: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash; - - /// Converts this key to the associated pubkey hash. - fn to_pubkeyhash(&self) -> Self::RawPkHash; } impl MiniscriptKey for bitcoin::secp256k1::PublicKey { - type RawPkHash = hash160::Hash; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; - - fn to_pubkeyhash(&self) -> Self::RawPkHash { - hash160::Hash::hash(&self.serialize()) - } } impl MiniscriptKey for bitcoin::PublicKey { @@ -204,43 +190,28 @@ impl MiniscriptKey for bitcoin::PublicKey { !self.compressed } - type RawPkHash = hash160::Hash; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; - - fn to_pubkeyhash(&self) -> Self::RawPkHash { - hash160::Hash::hash(&self.to_bytes()) - } } impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey { - type RawPkHash = hash160::Hash; type Sha256 = sha256::Hash; type Hash256 = hash256::Hash; type Ripemd160 = ripemd160::Hash; type Hash160 = hash160::Hash; - fn to_pubkeyhash(&self) -> Self::RawPkHash { - hash160::Hash::hash(&self.serialize()) - } - fn is_x_only_key(&self) -> bool { true } } impl MiniscriptKey for String { - type RawPkHash = String; type Sha256 = String; // specify hashes as string type Hash256 = String; type Ripemd160 = String; type Hash160 = String; - - fn to_pubkeyhash(&self) -> Self::RawPkHash { - (&self).to_string() - } } /// Trait describing public key types which can be converted to bitcoin pubkeys @@ -254,13 +225,16 @@ pub trait ToPublicKey: MiniscriptKey { bitcoin::secp256k1::XOnlyPublicKey::from(pk.inner) } - /// Converts a hashed version of the public key to a `hash160` hash. - /// - /// This method must be consistent with `to_public_key`, in the sense - /// that calling `MiniscriptKey::to_pubkeyhash` followed by this function - /// should give the same result as calling `to_public_key` and hashing - /// the result directly. - fn hash_to_hash160(hash: &::RawPkHash) -> hash160::Hash; + /// Obtain the public key hash for this MiniscriptKey + /// Expects an argument to specify the signature type. + /// This would determine whether to serialize the key as 32 byte x-only pubkey + /// or regular public key when computing the hash160 + fn to_pubkeyhash(&self, sig_type: SigType) -> hash160::Hash { + match sig_type { + SigType::Ecdsa => hash160::Hash::hash(&self.to_public_key().to_bytes()), + SigType::Schnorr => hash160::Hash::hash(&self.to_x_only_pubkey().serialize()), + } + } /// Converts the generic associated [`MiniscriptKey::Sha256`] to [`sha256::Hash`] fn to_sha256(hash: &::Sha256) -> sha256::Hash; @@ -280,10 +254,6 @@ impl ToPublicKey for bitcoin::PublicKey { *self } - fn hash_to_hash160(hash: &hash160::Hash) -> hash160::Hash { - *hash - } - fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } @@ -306,10 +276,6 @@ impl ToPublicKey for bitcoin::secp256k1::PublicKey { bitcoin::PublicKey::new(*self) } - fn hash_to_hash160(hash: &hash160::Hash) -> hash160::Hash { - *hash - } - fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } @@ -341,10 +307,6 @@ impl ToPublicKey for bitcoin::secp256k1::XOnlyPublicKey { *self } - fn hash_to_hash160(hash: &hash160::Hash) -> hash160::Hash { - *hash - } - fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { *hash } @@ -378,15 +340,10 @@ impl str::FromStr for DummyKey { } impl MiniscriptKey for DummyKey { - type RawPkHash = DummyKeyHash; type Sha256 = DummySha256Hash; type Hash256 = DummyHash256Hash; type Ripemd160 = DummyRipemd160Hash; type Hash160 = DummyHash160Hash; - - fn to_pubkeyhash(&self) -> Self::RawPkHash { - DummyKeyHash - } } impl hash::Hash for DummyKey { @@ -409,10 +366,6 @@ impl ToPublicKey for DummyKey { .unwrap() } - fn hash_to_hash160(_: &DummyKeyHash) -> hash160::Hash { - hash160::Hash::from_str("f54a5851e9372b87810a8e60cdd2e7cfd80b6e31").unwrap() - } - fn to_sha256(_hash: &DummySha256Hash) -> sha256::Hash { sha256::Hash::from_str("50863ad64a87ae8a2fe83c1af1a8403cb53f53e486d8511dad8a04887e5b2352") .unwrap() @@ -575,9 +528,6 @@ where /// Translates public keys P -> Q. fn pk(&mut self, pk: &P) -> Result; - /// Translates public key hashes P::Hash -> Q::Hash. - fn pkh(&mut self, pkh: &P::RawPkHash) -> Result; - /// Provides the translation from P::Sha256 -> Q::Sha256 fn sha256(&mut self, sha256: &P::Sha256) -> Result; @@ -624,15 +574,13 @@ pub trait ForEachKey { /// the predicate returned true for every key fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool where - Pk: 'a, - Pk::RawPkHash: 'a; + Pk: 'a; /// Run a predicate on every key in the descriptor, returning whether /// the predicate returned true for any key fn for_any_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { !self.for_each_key(|key| !pred(key)) } @@ -971,7 +919,7 @@ mod tests { ).unwrap(); let want = hash160::Hash::from_str("ac2e7daf42d2c97418fd9f78af2de552bb9c6a7a").unwrap(); - let got = pk.to_pubkeyhash(); + let got = pk.to_pubkeyhash(SigType::Ecdsa); assert_eq!(got, want) } @@ -986,7 +934,7 @@ mod tests { .unwrap(); let want = hash160::Hash::from_str("9511aa27ef39bbfa4e4f3dd15f4d66ea57f475b4").unwrap(); - let got = pk.to_pubkeyhash(); + let got = pk.to_pubkeyhash(SigType::Ecdsa); assert_eq!(got, want) } @@ -1000,16 +948,9 @@ mod tests { .unwrap(); let want = hash160::Hash::from_str("eb8ac65f971ae688a94aeabf223506865e7e08f2").unwrap(); - let got = pk.to_pubkeyhash(); + let got = pk.to_pubkeyhash(SigType::Schnorr); assert_eq!(got, want) } - - #[test] - fn regression_string_key_hash() { - let pk = String::from("some-key-hash-string"); - let hash = pk.to_pubkeyhash(); - assert_eq!(hash, pk) - } } mod prelude { diff --git a/src/macros.rs b/src/macros.rs index cdad906ec..5b946e806 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -27,13 +27,11 @@ macro_rules! impl_from_tree { impl $crate::expression::FromTree for $name where Pk: MiniscriptKey + core::str::FromStr, - Pk::RawPkHash: core::str::FromStr, Pk::Sha256: core::str::FromStr, Pk::Hash256: core::str::FromStr, Pk::Ripemd160: core::str::FromStr, Pk::Hash160: core::str::FromStr, ::Err: $crate::prelude::ToString, - <::RawPkHash as core::str::FromStr>::Err: $crate::prelude::ToString, <::Sha256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Hash256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Ripemd160 as core::str::FromStr>::Err: $crate::prelude::ToString, @@ -61,13 +59,11 @@ macro_rules! impl_from_str { impl core::str::FromStr for $name where Pk: MiniscriptKey + core::str::FromStr, - Pk::RawPkHash: core::str::FromStr, Pk::Sha256: core::str::FromStr, Pk::Hash256: core::str::FromStr, Pk::Ripemd160: core::str::FromStr, Pk::Hash160: core::str::FromStr, ::Err: $crate::prelude::ToString, - <::RawPkHash as core::str::FromStr>::Err: $crate::prelude::ToString, <::Sha256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Hash256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Ripemd160 as core::str::FromStr>::Err: $crate::prelude::ToString, @@ -95,13 +91,11 @@ macro_rules! impl_block_str { impl $name where Pk: MiniscriptKey + core::str::FromStr, - Pk::RawPkHash: core::str::FromStr, Pk::Sha256: core::str::FromStr, Pk::Hash256: core::str::FromStr, Pk::Ripemd160: core::str::FromStr, Pk::Hash160: core::str::FromStr, ::Err: $crate::prelude::ToString, - <::RawPkHash as core::str::FromStr>::Err: $crate::prelude::ToString, <::Sha256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Hash256 as core::str::FromStr>::Err: $crate::prelude::ToString, <::Ripemd160 as core::str::FromStr>::Err: $crate::prelude::ToString, @@ -124,14 +118,11 @@ macro_rules! serde_string_impl_pk { impl<'de, Pk $(, $gen)*> $crate::serde::Deserialize<'de> for $name where Pk: $crate::MiniscriptKey + core::str::FromStr, - Pk::RawPkHash: core::str::FromStr, Pk::Sha256: core::str::FromStr, Pk::Hash256: core::str::FromStr, Pk::Ripemd160: core::str::FromStr, Pk::Hash160: core::str::FromStr, ::Err: core::fmt::Display, - <::RawPkHash as core::str::FromStr>::Err: - core::fmt::Display, <::Sha256 as core::str::FromStr>::Err: core::fmt::Display, <::Hash256 as core::str::FromStr>::Err: @@ -155,14 +146,11 @@ macro_rules! serde_string_impl_pk { impl<'de, Pk $(, $gen)*> $crate::serde::de::Visitor<'de> for Visitor where Pk: $crate::MiniscriptKey + core::str::FromStr, - Pk::RawPkHash: core::str::FromStr, Pk::Sha256: core::str::FromStr, Pk::Hash256: core::str::FromStr, Pk::Ripemd160: core::str::FromStr, Pk::Hash160: core::str::FromStr, ::Err: core::fmt::Display, - <::RawPkHash as core::str::FromStr>::Err: - core::fmt::Display, <::Sha256 as core::str::FromStr>::Err: core::fmt::Display, <::Hash256 as core::str::FromStr>::Err: diff --git a/src/miniscript/analyzable.rs b/src/miniscript/analyzable.rs index 7f2f2f11a..9e2d84a1b 100644 --- a/src/miniscript/analyzable.rs +++ b/src/miniscript/analyzable.rs @@ -21,7 +21,6 @@ use core::fmt; #[cfg(feature = "std")] use std::error; -use crate::miniscript::iter::PkPkh; use crate::prelude::*; use crate::{Miniscript, MiniscriptKey, ScriptContext}; @@ -110,16 +109,9 @@ impl Miniscript { pub fn has_repeated_keys(&self) -> bool { // Simple way to check whether all of these are correct is // to have an iterator - let all_pkhs_len = self.iter_pk_pkh().count(); + let all_pkhs_len = self.iter_pk().count(); - let unique_pkhs_len = self - .iter_pk_pkh() - .map(|pk_pkh| match pk_pkh { - PkPkh::PlainPubkey(pk) => pk.to_pubkeyhash(), - PkPkh::HashedPubkey(h) => h, - }) - .collect::>() - .len(); + let unique_pkhs_len = self.iter_pk().collect::>().len(); unique_pkhs_len != all_pkhs_len } diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index f60bcd92f..51f299496 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -78,7 +78,6 @@ impl Terminal { pub(super) fn real_for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: &mut F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match *self { Terminal::PkK(ref p) => pred(p), @@ -128,7 +127,7 @@ impl Terminal { let frag: Terminal = match *self { Terminal::PkK(ref p) => Terminal::PkK(t.pk(p)?), Terminal::PkH(ref p) => Terminal::PkH(t.pk(p)?), - Terminal::RawPkH(ref p) => Terminal::RawPkH(t.pkh(p)?), + Terminal::RawPkH(ref p) => Terminal::RawPkH(*p), Terminal::After(n) => Terminal::After(n), Terminal::Older(n) => Terminal::Older(n), Terminal::Sha256(ref x) => Terminal::Sha256(t.sha256(&x)?), @@ -199,7 +198,6 @@ impl ForEachKey for Terminal fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { self.real_for_each_key(&mut pred) } @@ -367,10 +365,14 @@ impl fmt::Display for Terminal { } else if let Terminal::RawPkH(ref pkh) = sub.node { // `RawPkH` is currently unsupported in the descriptor spec // alias: pkh(K) = c:pk_h(K) - return write!(f, "pkh({})", pkh); + // We temporarily display there using raw_pkh, but these descriptors + // are not defined in the spec yet. These are prefixed with `expr` + // in the descriptor string. + // We do not support parsing these descriptors yet. + return write!(f, "expr_raw_pkh({})", pkh); } else if let Terminal::PkH(ref pk) = sub.node { // alias: pkh(K) = c:pk_h(K) - return write!(f, "pkh({})", &pk.to_pubkeyhash()); + return write!(f, "pkh({})", pk); } } @@ -618,7 +620,7 @@ impl Terminal { Terminal::RawPkH(ref hash) => builder .push_opcode(opcodes::all::OP_DUP) .push_opcode(opcodes::all::OP_HASH160) - .push_slice(&Pk::hash_to_hash160(hash)[..]) + .push_slice(&hash) .push_opcode(opcodes::all::OP_EQUALVERIFY), Terminal::After(t) => builder .push_int(t.to_u32().into()) diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index d838ee4ba..fb4c79955 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -191,7 +191,6 @@ impl fmt::Display for ScriptContextError { pub trait ScriptContext: fmt::Debug + Clone + Ord + PartialOrd + Eq + PartialEq + hash::Hash + private::Sealed where - Self::Key: MiniscriptKey, Self::Key: MiniscriptKey, Self::Key: MiniscriptKey, Self::Key: MiniscriptKey, @@ -338,6 +337,7 @@ where fn name_str() -> &'static str; } +/// Signature algorithm type #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)] pub enum SigType { /// Ecdsa signature diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index c80b2a1a2..7715dd051 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -124,6 +124,12 @@ enum NonTerm { EndIfElse, } /// All AST elements +/// This variant is the inner Miniscript variant that allows the user to bypass +/// some of the miniscript rules. You should *never* construct Terminal directly. +/// This is only exposed to external user to allow matching on the [`crate::Miniscript`] +/// +/// The average user should always use the [`crate::Descriptor`] APIs. Advanced users +/// who want deal with Miniscript ASTs should use the [`crate::Miniscript`] APIs. #[allow(broken_intra_doc_links)] #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Terminal { @@ -136,8 +142,12 @@ pub enum Terminal { PkK(Pk), /// `DUP HASH160 EQUALVERIFY` PkH(Pk), - /// Only for parsing PkH for Script - RawPkH(Pk::RawPkHash), + /// Only for parsing PkH for Script. These raw descriptors are not yet specified in miniscript. + /// We only this variant internally for inferring miniscripts from raw Scripts. + /// It is not possible to construct this variant from any of the Miniscript APIs. + /// We don't have a generic over here because we don't want to user to have any abstract reasoning + /// over raw descriptors. + RawPkH(hash160::Hash), // timelocks /// `n CHECKLOCKTIMEVERIFY` After(PackedLockTime), diff --git a/src/miniscript/iter.rs b/src/miniscript/iter.rs index 1df400902..45f3ccbb3 100644 --- a/src/miniscript/iter.rs +++ b/src/miniscript/iter.rs @@ -40,20 +40,6 @@ impl Miniscript { PkIter::new(self) } - /// Creates a new [PkhIter] iterator that will iterate over all public keys hashes (and not - /// plain public keys) present in Miniscript items within AST by traversing all its branches. - /// For the specific algorithm please see [PkhIter::next] function. - pub fn iter_pkh(&self) -> PkhIter { - PkhIter::new(self) - } - - /// Creates a new [PkPkhIter] iterator that will iterate over all plain public keys and - /// key hash values present in Miniscript items within AST by traversing all its branches. - /// For the specific algorithm please see [PkPkhIter::next] function. - pub fn iter_pk_pkh(&self) -> PkPkhIter { - PkPkhIter::new(self) - } - /// Enumerates all child nodes of the current AST node (`self`) and returns a `Vec` referencing /// them. pub fn branches(&self) -> Vec<&Miniscript> { @@ -117,61 +103,6 @@ impl Miniscript { } } - /// Returns `Vec` with cloned version of all public keys from the current miniscript item, - /// if any. Otherwise returns an empty `Vec`. - /// - /// NB: The function analyzes only single miniscript item and not any of its descendants in AST. - /// To obtain a list of all public keys within AST use [Miniscript::iter_pk()] function, for example - /// `miniscript.iter_pubkeys().collect()`. - pub fn get_leapk(&self) -> Vec { - match self.node { - Terminal::PkK(ref key) | Terminal::PkH(ref key) => vec![key.clone()], - Terminal::Multi(_, ref keys) | Terminal::MultiA(_, ref keys) => keys.clone(), - _ => vec![], - } - } - - /// Returns `Vec` with hashes of all public keys from the current miniscript item, if any. - /// Otherwise returns an empty `Vec`. - /// - /// For each public key the function computes hash; for each hash of the public key the function - /// returns its cloned copy. - /// - /// NB: The function analyzes only single miniscript item and not any of its descendants in AST. - /// To obtain a list of all public key hashes within AST use [Miniscript::iter_pkh()] function, - /// for example `miniscript.iter_pubkey_hashes().collect()`. - pub fn get_leapkh(&self) -> Vec { - match self.node { - Terminal::RawPkH(ref hash) => vec![hash.clone()], - Terminal::PkK(ref key) | Terminal::PkH(ref key) => vec![key.to_pubkeyhash()], - Terminal::Multi(_, ref keys) | Terminal::MultiA(_, ref keys) => { - keys.iter().map(Pk::to_pubkeyhash).collect() - } - _ => vec![], - } - } - - /// Returns `Vec` of [PkPkh] entries, representing either public keys or public key - /// hashes, depending on the data from the current miniscript item. If there is no public - /// keys or hashes, the function returns an empty `Vec`. - /// - /// NB: The function analyzes only single miniscript item and not any of its descendants in AST. - /// To obtain a list of all public keys or hashes within AST use [Miniscript::iter_pk_pkh()] - /// function, for example `miniscript.iter_pubkeys_and_hashes().collect()`. - pub fn get_leapk_pkh(&self) -> Vec> { - match self.node { - Terminal::RawPkH(ref hash) => vec![PkPkh::HashedPubkey(hash.clone())], - Terminal::PkH(ref key) | Terminal::PkK(ref key) => { - vec![PkPkh::PlainPubkey(key.clone())] - } - Terminal::Multi(_, ref keys) | Terminal::MultiA(_, ref keys) => keys - .iter() - .map(|key| PkPkh::PlainPubkey(key.clone())) - .collect(), - _ => vec![], - } - } - /// Returns `Option::Some` with cloned n'th public key from the current miniscript item, /// if any. Otherwise returns `Option::None`. /// @@ -185,43 +116,6 @@ impl Miniscript { _ => None, } } - - /// Returns `Option::Some` with hash of n'th public key from the current miniscript item, - /// if any. Otherwise returns `Option::None`. - /// - /// For each public key the function computes hash; for each hash of the public key the function - /// returns it cloned copy. - /// - /// NB: The function analyzes only single miniscript item and not any of its descendants in AST. - pub fn get_nth_pkh(&self, n: usize) -> Option { - match (&self.node, n) { - (&Terminal::RawPkH(ref hash), 0) => Some(hash.clone()), - (&Terminal::PkK(ref key), 0) | (&Terminal::PkH(ref key), 0) => { - Some(key.to_pubkeyhash()) - } - (&Terminal::Multi(_, ref keys), _) | (&Terminal::MultiA(_, ref keys), _) => { - keys.get(n).map(Pk::to_pubkeyhash) - } - _ => None, - } - } - - /// Returns `Option::Some` with hash of n'th public key or hash from the current miniscript item, - /// if any. Otherwise returns `Option::None`. - /// - /// NB: The function analyzes only single miniscript item and not any of its descendants in AST. - pub fn get_nth_pk_pkh(&self, n: usize) -> Option> { - match (&self.node, n) { - (&Terminal::RawPkH(ref hash), 0) => Some(PkPkh::HashedPubkey(hash.clone())), - (&Terminal::PkH(ref key), 0) | (&Terminal::PkK(ref key), 0) => { - Some(PkPkh::PlainPubkey(key.clone())) - } - (&Terminal::Multi(_, ref keys), _) | (&Terminal::MultiA(_, ref keys), _) => { - keys.get(n).map(|key| PkPkh::PlainPubkey(key.clone())) - } - _ => None, - } - } } /// Iterator for traversing all [Miniscript] miniscript AST references starting from some specific @@ -328,134 +222,6 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkIter<'a, Pk, Ctx> } } -/// Iterator for traversing all [MiniscriptKey] hashes in AST starting from some specific node which -/// constructs the iterator via [Miniscript::iter_pkh] method. -pub struct PkhIter<'a, Pk: MiniscriptKey, Ctx: ScriptContext> { - node_iter: Iter<'a, Pk, Ctx>, - curr_node: Option<&'a Miniscript>, - key_index: usize, -} - -impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> PkhIter<'a, Pk, Ctx> { - fn new(miniscript: &'a Miniscript) -> Self { - let mut iter = Iter::new(miniscript); - PkhIter { - curr_node: iter.next(), - node_iter: iter, - key_index: 0, - } - } -} - -impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkhIter<'a, Pk, Ctx> { - type Item = Pk::RawPkHash; - - fn next(&mut self) -> Option { - loop { - match self.curr_node { - None => break None, - Some(node) => match node.get_nth_pkh(self.key_index) { - None => { - self.curr_node = self.node_iter.next(); - self.key_index = 0; - continue; - } - Some(pk) => { - self.key_index += 1; - break Some(pk); - } - }, - } - } - } -} - -/// Enum representing either key or a key hash value coming from a miniscript item inside AST -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum PkPkh { - /// Plain public key - PlainPubkey(Pk), - /// Hashed public key - HashedPubkey(Pk::RawPkHash), -} - -impl> PkPkh { - /// Convenience method to avoid distinguishing between keys and hashes when these are the same type - pub fn as_key(self) -> Pk { - match self { - PkPkh::PlainPubkey(pk) => pk, - PkPkh::HashedPubkey(pkh) => pkh, - } - } -} - -/// Iterator for traversing all [MiniscriptKey]'s and hashes, depending what data are present in AST, -/// starting from some specific node which constructs the iterator via -/// [Miniscript::iter_pk_pkh] method. -pub struct PkPkhIter<'a, Pk: MiniscriptKey, Ctx: ScriptContext> { - node_iter: Iter<'a, Pk, Ctx>, - curr_node: Option<&'a Miniscript>, - key_index: usize, -} - -impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> PkPkhIter<'a, Pk, Ctx> { - fn new(miniscript: &'a Miniscript) -> Self { - let mut iter = Iter::new(miniscript); - PkPkhIter { - curr_node: iter.next(), - node_iter: iter, - key_index: 0, - } - } - - /// Returns a `Option`, listing all public keys found in AST starting from this - /// `Miniscript` item, or `None` signifying that at least one key hash was found, making - /// impossible to enumerate all source public keys from the script. - /// - /// * Differs from `Miniscript::iter_pubkeys().collect()` in the way that this function fails on - /// the first met public key hash, while [PkIter] just ignores them. - /// * Differs from `Miniscript::iter_pubkeys_and_hashes().collect()` in the way that it lists - /// only public keys, and not their hashes - /// - /// Unlike these functions, [PkPkhIter::pk_only] returns an `Option` value with `Vec`, not an iterator, - /// and consumes the iterator object. - pub fn pk_only(self) -> Option> { - let mut keys = vec![]; - for item in self { - match item { - PkPkh::HashedPubkey(_) => return None, - PkPkh::PlainPubkey(key) => { - keys.push(key); - } - } - } - Some(keys) - } -} - -impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkPkhIter<'a, Pk, Ctx> { - type Item = PkPkh; - - fn next(&mut self) -> Option { - loop { - match self.curr_node { - None => break None, - Some(node) => match node.get_nth_pk_pkh(self.key_index) { - None => { - self.curr_node = self.node_iter.next(); - self.key_index = 0; - continue; - } - Some(pk) => { - self.key_index += 1; - break Some(pk); - } - }, - } - } - } -} - // Module is public since it export testcase generation which may be used in // dependent libraries for their own tasts based on Miniscript AST #[cfg(test)] @@ -464,7 +230,7 @@ pub mod test { use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d, Hash}; use bitcoin::secp256k1; - use super::{Miniscript, PkPkh}; + use super::Miniscript; use crate::miniscript::context::Segwitv0; pub type TestData = ( @@ -604,86 +370,10 @@ pub mod test { ] } - #[test] - fn get_keys() { - gen_testcases() - .into_iter() - .for_each(|(ms, k, _, test_top_level)| { - if !test_top_level { - return; - } - let ms = *ms.branches().first().unwrap_or(&&ms); - assert_eq!(ms.get_leapk(), k); - }) - } - - #[test] - fn get_hashes() { - gen_testcases() - .into_iter() - .for_each(|(ms, k, h, test_top_level)| { - if !test_top_level { - return; - } - let ms = *ms.branches().first().unwrap_or(&&ms); - let mut all: Vec = k - .iter() - .map(|p| hash160::Hash::hash(&p.to_bytes())) - .collect(); - // In our test cases we always have plain keys going first - all.extend(h); - assert_eq!(ms.get_leapkh(), all); - }) - } - - #[test] - fn get_pubkey_and_hashes() { - gen_testcases() - .into_iter() - .for_each(|(ms, k, h, test_top_level)| { - if !test_top_level { - return; - } - let ms = *ms.branches().first().unwrap_or(&&ms); - let r: Vec> = if k.is_empty() { - h.into_iter().map(|h| PkPkh::HashedPubkey(h)).collect() - } else { - k.into_iter().map(|k| PkPkh::PlainPubkey(k)).collect() - }; - assert_eq!(ms.get_leapk_pkh(), r); - }) - } - #[test] fn find_keys() { gen_testcases().into_iter().for_each(|(ms, k, _, _)| { assert_eq!(ms.iter_pk().collect::>(), k); }) } - - #[test] - fn find_hashes() { - gen_testcases().into_iter().for_each(|(ms, k, h, _)| { - let mut all: Vec = k - .iter() - .map(|p| hash160::Hash::hash(&p.to_bytes())) - .collect(); - // In our test cases we always have plain keys going first - all.extend(h); - assert_eq!(ms.iter_pkh().collect::>(), all); - }) - } - - #[test] - fn find_pubkeys_and_hashes() { - gen_testcases().into_iter().for_each(|(ms, k, h, _)| { - let mut all: Vec> = - k.into_iter().map(|k| PkPkh::PlainPubkey(k)).collect(); - all.extend(h.into_iter().map(|h| PkPkh::HashedPubkey(h))); - assert_eq!( - ms.iter_pk_pkh().collect::>>(), - all - ); - }) - } } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index c69179bf4..6d4368ed9 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -273,7 +273,6 @@ impl ForEachKey for Miniscript bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { self.real_for_each_key(&mut pred) } @@ -301,7 +300,6 @@ impl Miniscript { fn real_for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: &mut F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { self.node.real_for_each_key(pred) } @@ -462,7 +460,7 @@ mod tests { use crate::policy::Liftable; use crate::prelude::*; use crate::test_utils::{StrKeyTranslator, StrXOnlyKeyTranslator}; - use crate::{hex_script, DummyKey, DummyKeyHash, Satisfier, ToPublicKey, TranslatePk}; + use crate::{hex_script, DummyKey, Satisfier, ToPublicKey, TranslatePk}; type Segwitv0Script = Miniscript; type Tapscript = Miniscript; @@ -657,7 +655,7 @@ mod tests { let pkh_ms: Miniscript = Miniscript { node: Terminal::Check(Arc::new(Miniscript { - node: Terminal::RawPkH(DummyKeyHash), + node: Terminal::PkH(DummyKey), ty: Type::from_pk_h::(), ext: types::extra_props::ExtData::from_pk_h::(), phantom: PhantomData, @@ -667,7 +665,7 @@ mod tests { phantom: PhantomData, }; - let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(DummyKeyHash)"; + let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(DummyKey)"; let expected_display = "pkh()"; assert_eq!(pkh_ms.ty.corr.base, types::Base::B); @@ -782,7 +780,7 @@ mod tests { string_display_debug_test( script, "[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", - "pkh(60afcdec519698a263417ddfe7cea936737a0ee7)", + "pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); let script: Segwitv0Script = ms_str!("pkh({})", pubkey.to_string()); @@ -790,7 +788,7 @@ mod tests { string_display_debug_test( script, "[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", - "pkh(60afcdec519698a263417ddfe7cea936737a0ee7)", + "pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); let script: Segwitv0Script = ms_str!("tv:pkh({})", pubkey.to_string()); @@ -798,7 +796,7 @@ mod tests { string_display_debug_test( script, "[B/nufsm]t[V/nfsm]v[B/nduesm]c:[K/nduesm]pk_h(PublicKey { compressed: true, inner: PublicKey(aa4c32e50fb34a95a372940ae3654b692ea35294748c3dd2c08b29f87ba9288c8294efcb73dc719e45b91c45f084e77aebc07c1ff3ed8f37935130a36304a340) })", - "tv:pkh(60afcdec519698a263417ddfe7cea936737a0ee7)", + "tv:pkh(028c28a97bf8298bc0d23d8c749452a32e694b65e30a9472a3954ab30fe5324caa)", ); } diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index 85c2b2e93..a80faea40 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -20,11 +20,13 @@ use core::{cmp, i64, mem}; +use bitcoin::hashes::hash160; use bitcoin::secp256k1::XOnlyPublicKey; use bitcoin::util::taproot::{ControlBlock, LeafVersion, TapLeafHash}; use bitcoin::{LockTime, Sequence}; use sync::Arc; +use super::context::SigType; use crate::prelude::*; use crate::util::witness_size; use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal, ToPublicKey}; @@ -58,8 +60,8 @@ pub trait Satisfier { None } - /// Given a `Pkh`, lookup corresponding `Pk` - fn lookup_pkh_pk(&self, _: &Pk::RawPkHash) -> Option { + /// Given a raw `Pkh`, lookup corresponding `Pk` + fn lookup_raw_pkh_pk(&self, _: &hash160::Hash) -> Option { None } @@ -67,9 +69,9 @@ pub trait Satisfier { /// Even if signatures for public key Hashes are not available, the users /// can use this map to provide pkh -> pk mapping which can be useful /// for dissatisfying pkh. - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - _: &Pk::RawPkHash, + _: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { None } @@ -78,9 +80,9 @@ pub trait Satisfier { /// Even if signatures for public key Hashes are not available, the users /// can use this map to provide pkh -> pk mapping which can be useful /// for dissatisfying pkh. - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - _: &(Pk::RawPkHash, TapLeafHash), + _: &(hash160::Hash, TapLeafHash), ) -> Option<(XOnlyPublicKey, bitcoin::SchnorrSig)> { None } @@ -173,21 +175,21 @@ impl Satisfier } impl Satisfier - for HashMap + for HashMap where Pk: MiniscriptKey + ToPublicKey, { fn lookup_ecdsa_sig(&self, key: &Pk) -> Option { - self.get(&key.to_pubkeyhash()).map(|x| x.1) + self.get(&key.to_pubkeyhash(SigType::Ecdsa)).map(|x| x.1) } - fn lookup_pkh_pk(&self, pk_hash: &Pk::RawPkHash) -> Option { + fn lookup_raw_pkh_pk(&self, pk_hash: &hash160::Hash) -> Option { self.get(pk_hash).map(|x| x.0.clone()) } - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - pk_hash: &Pk::RawPkHash, + pk_hash: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { self.get(pk_hash) .map(|&(ref pk, sig)| (pk.to_public_key(), sig)) @@ -195,17 +197,18 @@ where } impl Satisfier - for HashMap<(Pk::RawPkHash, TapLeafHash), (Pk, bitcoin::SchnorrSig)> + for HashMap<(hash160::Hash, TapLeafHash), (Pk, bitcoin::SchnorrSig)> where Pk: MiniscriptKey + ToPublicKey, { fn lookup_tap_leaf_script_sig(&self, key: &Pk, h: &TapLeafHash) -> Option { - self.get(&(key.to_pubkeyhash(), *h)).map(|x| x.1) + self.get(&(key.to_pubkeyhash(SigType::Schnorr), *h)) + .map(|x| x.1) } - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - pk_hash: &(Pk::RawPkHash, TapLeafHash), + pk_hash: &(hash160::Hash, TapLeafHash), ) -> Option<(XOnlyPublicKey, bitcoin::SchnorrSig)> { self.get(pk_hash) .map(|&(ref pk, sig)| (pk.to_x_only_pubkey(), sig)) @@ -221,26 +224,26 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier> Satisfier for &' (**self).lookup_tap_leaf_script_sig(p, h) } - fn lookup_pkh_pk(&self, pkh: &Pk::RawPkHash) -> Option { - (**self).lookup_pkh_pk(pkh) + fn lookup_raw_pkh_pk(&self, pkh: &hash160::Hash) -> Option { + (**self).lookup_raw_pkh_pk(pkh) } - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - pkh: &Pk::RawPkHash, + pkh: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { - (**self).lookup_pkh_ecdsa_sig(pkh) + (**self).lookup_raw_pkh_ecdsa_sig(pkh) } fn lookup_tap_key_spend_sig(&self) -> Option { (**self).lookup_tap_key_spend_sig() } - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - pkh: &(Pk::RawPkHash, TapLeafHash), + pkh: &(hash160::Hash, TapLeafHash), ) -> Option<(XOnlyPublicKey, bitcoin::SchnorrSig)> { - (**self).lookup_pkh_tap_leaf_script_sig(pkh) + (**self).lookup_raw_pkh_tap_leaf_script_sig(pkh) } fn lookup_tap_control_block_map( @@ -287,22 +290,22 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier> Satisfier for &' (**self).lookup_tap_key_spend_sig() } - fn lookup_pkh_pk(&self, pkh: &Pk::RawPkHash) -> Option { - (**self).lookup_pkh_pk(pkh) + fn lookup_raw_pkh_pk(&self, pkh: &hash160::Hash) -> Option { + (**self).lookup_raw_pkh_pk(pkh) } - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - pkh: &Pk::RawPkHash, + pkh: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { - (**self).lookup_pkh_ecdsa_sig(pkh) + (**self).lookup_raw_pkh_ecdsa_sig(pkh) } - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - pkh: &(Pk::RawPkHash, TapLeafHash), + pkh: &(hash160::Hash, TapLeafHash), ) -> Option<(XOnlyPublicKey, bitcoin::SchnorrSig)> { - (**self).lookup_pkh_tap_leaf_script_sig(pkh) + (**self).lookup_raw_pkh_tap_leaf_script_sig(pkh) } fn lookup_tap_control_block_map( @@ -374,39 +377,39 @@ macro_rules! impl_tuple_satisfier { None } - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - key_hash: &Pk::RawPkHash, + key_hash: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { let &($(ref $ty,)*) = self; $( - if let Some(result) = $ty.lookup_pkh_ecdsa_sig(key_hash) { + if let Some(result) = $ty.lookup_raw_pkh_ecdsa_sig(key_hash) { return Some(result); } )* None } - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - key_hash: &(Pk::RawPkHash, TapLeafHash), + key_hash: &(hash160::Hash, TapLeafHash), ) -> Option<(XOnlyPublicKey, bitcoin::SchnorrSig)> { let &($(ref $ty,)*) = self; $( - if let Some(result) = $ty.lookup_pkh_tap_leaf_script_sig(key_hash) { + if let Some(result) = $ty.lookup_raw_pkh_tap_leaf_script_sig(key_hash) { return Some(result); } )* None } - fn lookup_pkh_pk( + fn lookup_raw_pkh_pk( &self, - key_hash: &Pk::RawPkHash, + key_hash: &hash160::Hash, ) -> Option { let &($(ref $ty,)*) = self; $( - if let Some(result) = $ty.lookup_pkh_pk(key_hash) { + if let Some(result) = $ty.lookup_raw_pkh_pk(key_hash) { return Some(result); } )* @@ -557,8 +560,8 @@ impl Witness { } /// Turn a public key related to a pkh into (part of) a satisfaction - fn pkh_public_key>(sat: S, pkh: &Pk::RawPkHash) -> Self { - match sat.lookup_pkh_pk(pkh) { + fn pkh_public_key>(sat: S, pkh: &hash160::Hash) -> Self { + match sat.lookup_raw_pkh_pk(pkh) { Some(pk) => Witness::Stack(vec![pk.to_public_key().to_bytes()]), // public key hashes are assumed to be unavailable // instead of impossible since it is the same as pub-key hashes @@ -567,8 +570,8 @@ impl Witness { } /// Turn a key/signature pair related to a pkh into (part of) a satisfaction - fn pkh_signature>(sat: S, pkh: &Pk::RawPkHash) -> Self { - match sat.lookup_pkh_ecdsa_sig(pkh) { + fn pkh_signature>(sat: S, pkh: &hash160::Hash) -> Self { + match sat.lookup_raw_pkh_ecdsa_sig(pkh) { Some((pk, sig)) => Witness::Stack(vec![sig.to_vec(), pk.to_public_key().to_bytes()]), None => Witness::Impossible, } @@ -931,7 +934,7 @@ impl Satisfaction { has_sig: true, }, Terminal::PkH(ref pk) => Satisfaction { - stack: Witness::pkh_signature(stfr, &pk.to_pubkeyhash()), + stack: Witness::pkh_signature(stfr, &pk.to_pubkeyhash(Ctx::sig_type())), has_sig: true, }, Terminal::RawPkH(ref pkh) => Satisfaction { @@ -1245,7 +1248,7 @@ impl Satisfaction { Terminal::PkH(ref pk) => Satisfaction { stack: Witness::combine( Witness::push_0(), - Witness::pkh_public_key(stfr, &pk.to_pubkeyhash()), + Witness::pkh_public_key(stfr, &pk.to_pubkeyhash(Ctx::sig_type())), ), has_sig: false, }, diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index c19e95745..e7269d170 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -1207,7 +1207,7 @@ mod tests { use super::*; use crate::miniscript::{Legacy, Segwitv0, Tap}; use crate::policy::Liftable; - use crate::script_num_size; + use crate::{script_num_size, ToPublicKey}; type SPolicy = Concrete; type BPolicy = Concrete; @@ -1419,7 +1419,7 @@ mod tests { left_sat.insert(keys[i], bitcoinsig); } for i in 5..8 { - right_sat.insert(keys[i].to_pubkeyhash(), (keys[i], bitcoinsig)); + right_sat.insert(keys[i].to_pubkeyhash(SigType::Ecdsa), (keys[i], bitcoinsig)); } assert!(ms.satisfy(no_sat).is_err()); diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index a74c2ced5..b664c67be 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -656,7 +656,6 @@ impl ForEachKey for Policy { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match *self { Policy::Unsatisfiable | Policy::Trivial => true, @@ -705,11 +704,6 @@ impl Policy { /// self.pk_map.get(pk).copied().ok_or(()) // Dummy Err /// } /// - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn pkh(&mut self, pkh: &String) -> Result { - /// unreachable!("Policy does not contain any pkh fragment"); - /// } - /// /// // Fail for hash types /// translate_hash_fail!(String, bitcoin::PublicKey, ()); /// } diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index a8e6b44eb..19855ace1 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -75,7 +75,6 @@ impl ForEachKey for Policy { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where Pk: 'a, - Pk::RawPkHash: 'a, { match *self { Policy::Unsatisfiable | Policy::Trivial => true, @@ -119,12 +118,6 @@ impl Policy { /// self.pk_map.get(pk).copied().ok_or(()) // Dummy Err /// } /// - /// // Provides the translation public keys P::Hash -> Q::Hash - /// // If our policy also contained other fragments, we could provide the translation here. - /// fn pkh(&mut self, pkh: &String) -> Result { - /// unreachable!("Policy does not contain any pk fragment"); - /// } - /// /// // Handy macro for failing if we encounter any other fragment. /// // also see translate_hash_clone! for cloning instead of failing /// translate_hash_fail!(String, bitcoin::PublicKey, ()); diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 8ead76869..ead519750 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -33,7 +33,7 @@ use bitcoin::util::sighash::SighashCache; use bitcoin::util::taproot::{self, ControlBlock, LeafVersion, TapLeafHash}; use bitcoin::{self, EcdsaSighashType, LockTime, SchnorrSighashType, Script, Sequence}; -use crate::miniscript::iter::PkPkh; +use crate::miniscript::context::SigType; use crate::prelude::*; use crate::{ descriptor, interpreter, DefiniteDescriptorKey, Descriptor, DescriptorPublicKey, MiniscriptKey, @@ -303,15 +303,15 @@ impl<'psbt, Pk: MiniscriptKey + ToPublicKey> Satisfier for PsbtInputSatisfie Some(&self.psbt.inputs[self.index].tap_scripts) } - fn lookup_pkh_tap_leaf_script_sig( + fn lookup_raw_pkh_tap_leaf_script_sig( &self, - pkh: &(Pk::RawPkHash, TapLeafHash), + pkh: &(hash160::Hash, TapLeafHash), ) -> Option<(bitcoin::secp256k1::XOnlyPublicKey, bitcoin::SchnorrSig)> { self.psbt.inputs[self.index] .tap_script_sigs .iter() .find(|&((pubkey, lh), _sig)| { - pubkey.to_pubkeyhash() == Pk::hash_to_hash160(&pkh.0) && *lh == pkh.1 + pubkey.to_pubkeyhash(SigType::Schnorr) == pkh.0 && *lh == pkh.1 }) .map(|((x_only_pk, _leaf_hash), sig)| (*x_only_pk, *sig)) } @@ -323,14 +323,14 @@ impl<'psbt, Pk: MiniscriptKey + ToPublicKey> Satisfier for PsbtInputSatisfie .copied() } - fn lookup_pkh_ecdsa_sig( + fn lookup_raw_pkh_ecdsa_sig( &self, - pkh: &Pk::RawPkHash, + pkh: &hash160::Hash, ) -> Option<(bitcoin::PublicKey, bitcoin::EcdsaSig)> { self.psbt.inputs[self.index] .partial_sigs .iter() - .find(|&(pubkey, _sig)| pubkey.to_pubkeyhash() == Pk::hash_to_hash160(pkh)) + .find(|&(pubkey, _sig)| pubkey.to_pubkeyhash(SigType::Ecdsa) == *pkh) .map(|(pk, sig)| (*pk, *sig)) } @@ -1015,16 +1015,7 @@ impl Translator Result { - let pk = xpk.derive_public_key(&self.1)?; - let xonly = pk.to_x_only_pubkey(); - let hash = xonly.to_pubkeyhash(); - self.0.insert(hash, xonly); - Ok(hash) - } + // TODO: cleanup this code in a later commit // Clone all the associated types in translation translate_hash_clone!( @@ -1056,13 +1047,6 @@ impl Translator Result { - Ok(self.pk(xpk)?.to_pubkeyhash()) - } - translate_hash_clone!( DescriptorPublicKey, bitcoin::PublicKey, @@ -1216,20 +1200,8 @@ fn update_item_with_descriptor_helper( tap_scripts.insert(control_block, leaf_script); } - for (pk_pkh_derived, pk_pkh_xpk) in ms_derived.iter_pk_pkh().zip(ms.iter_pk_pkh()) { - let (xonly, xpk) = match (pk_pkh_derived, pk_pkh_xpk) { - (PkPkh::PlainPubkey(pk), PkPkh::PlainPubkey(xpk)) => { - (pk.to_x_only_pubkey(), xpk) - } - (PkPkh::HashedPubkey(hash), PkPkh::HashedPubkey(xpk)) => ( - *hash_lookup - .0 - .get(&hash) - .expect("translate_pk inserted an entry for every hash"), - xpk, - ), - _ => unreachable!("the iterators work in the same order"), - }; + for (pk_pkh_derived, pk_pkh_xpk) in ms_derived.iter_pk().zip(ms.iter_pk()) { + let (xonly, xpk) = (pk_pkh_derived.to_x_only_pubkey(), pk_pkh_xpk); item.tap_key_origins() .entry(xonly) diff --git a/src/pub_macros.rs b/src/pub_macros.rs index 2e7bbd479..612e0b325 100644 --- a/src/pub_macros.rs +++ b/src/pub_macros.rs @@ -31,11 +31,6 @@ /// self.pk_map.get(pk).copied().ok_or(()) // Dummy Err /// } /// -/// // If our policy also contained other fragments, we could provide the translation here. -/// fn pkh(&mut self, pkh: &String) -> Result { -/// unreachable!("Policy does not contain any pkh fragment"); -/// } -/// /// // Fail for hash types /// translate_hash_fail!(String, bitcoin::PublicKey, ()); /// } diff --git a/src/test_utils.rs b/src/test_utils.rs index ccd5b55eb..6ea900d91 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -6,7 +6,8 @@ use std::str::FromStr; use bitcoin::hashes::{hash160, ripemd160, sha256}; use bitcoin::secp256k1; -use crate::{hash256, MiniscriptKey, Translator}; +use crate::miniscript::context::SigType; +use crate::{hash256, ToPublicKey, Translator}; /// Translate from a String MiniscriptKey type to bitcoin::PublicKey /// If the hashmap is populated, this will lookup for keys in HashMap @@ -31,13 +32,6 @@ impl Translator for StrKeyTranslator { Ok(key) } - fn pkh(&mut self, pkh: &String) -> Result { - let hash = self.pkh_map.get(pkh).copied().unwrap_or_else(|| { - hash160::Hash::from_str("be8f27af36217ba89c793c419f058cd4e2a54e26").unwrap() - }); - Ok(hash) - } - fn sha256(&mut self, _sha256: &String) -> Result { let hash = sha256::Hash::from_str( "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260", @@ -87,13 +81,6 @@ impl Translator for StrXOnlyKeyTranslator { Ok(key) } - fn pkh(&mut self, pkh: &String) -> Result { - let hash = self.pkh_map.get(pkh).copied().unwrap_or_else(|| { - hash160::Hash::from_str("be8f27af36217ba89c793c419f058cd4e2a54e26").unwrap() - }); - Ok(hash) - } - fn sha256(&mut self, _sha256: &String) -> Result { let hash = sha256::Hash::from_str( "4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260", @@ -150,7 +137,7 @@ impl StrKeyTranslator { for (i, c) in (b'A'..b'Z').enumerate() { let key = String::from_utf8(vec![c]).unwrap(); pk_map.insert(key.clone(), pks[i]); - pkh_map.insert(key, pks[i].to_pubkeyhash()); + pkh_map.insert(key, pks[i].to_pubkeyhash(SigType::Ecdsa)); } // We don't bother filling in sha256_map preimages in default implementation as it not unnecessary // for sane miniscripts @@ -181,7 +168,7 @@ impl StrXOnlyKeyTranslator { for (i, c) in (b'A'..b'Z').enumerate() { let key = String::from_utf8(vec![c]).unwrap(); pk_map.insert(key.clone(), pks[i]); - pkh_map.insert(key, pks[i].to_pubkeyhash()); + pkh_map.insert(key, pks[i].to_pubkeyhash(SigType::Schnorr)); } // We don't bother filling in sha256_map preimages in default implementation as it not unnecessary // for sane miniscripts diff --git a/src/util.rs b/src/util.rs index b59c2fde4..12936a848 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,9 +1,10 @@ use bitcoin::blockdata::script; -use bitcoin::Script; +use bitcoin::hashes::Hash; +use bitcoin::{PubkeyHash, Script}; use crate::miniscript::context; use crate::prelude::*; -use crate::{MiniscriptKey, ScriptContext, ToPublicKey}; +use crate::{ScriptContext, ToPublicKey}; pub(crate) fn varint_len(n: usize) -> usize { bitcoin::VarInt(n as u64).len() } @@ -58,11 +59,9 @@ impl MsKeyBuilder for script::Builder { Ctx: ScriptContext, { match Ctx::sig_type() { - context::SigType::Ecdsa => { - self.push_slice(&Pk::hash_to_hash160(&key.to_pubkeyhash())[..]) - } + context::SigType::Ecdsa => self.push_slice(&key.to_public_key().pubkey_hash()), context::SigType::Schnorr => { - self.push_slice(&key.to_x_only_pubkey().to_pubkeyhash()[..]) + self.push_slice(&PubkeyHash::hash(&key.to_x_only_pubkey().serialize())) } } } diff --git a/tests/setup/test_util.rs b/tests/setup/test_util.rs index f5043c7f5..fc8e079e2 100644 --- a/tests/setup/test_util.rs +++ b/tests/setup/test_util.rs @@ -190,10 +190,6 @@ impl<'a> Translator for StrDescPubKeyTranslator } } - fn pkh(&mut self, pkh: &String) -> Result { - self.pk(pkh) - } - fn sha256(&mut self, sha256: &String) -> Result { let sha = sha256::Hash::from_str(sha256).unwrap(); Ok(sha) @@ -251,10 +247,6 @@ impl<'a> Translator for StrTranslatorLoose<'a> } } - fn pkh(&mut self, pkh: &String) -> Result { - self.pk(pkh) - } - fn sha256(&mut self, sha256: &String) -> Result { let sha = sha256::Hash::from_str(sha256).unwrap(); Ok(sha) diff --git a/tests/test_cpp.rs b/tests/test_cpp.rs index af45a12b3..12d6c1add 100644 --- a/tests/test_cpp.rs +++ b/tests/test_cpp.rs @@ -15,9 +15,8 @@ use bitcoin::util::psbt; use bitcoin::util::psbt::PartiallySignedTransaction as Psbt; use bitcoin::{self, Amount, LockTime, OutPoint, Sequence, Transaction, TxIn, TxOut, Txid}; use bitcoind::bitcoincore_rpc::{json, Client, RpcApi}; -use miniscript::miniscript::iter; use miniscript::psbt::PsbtExt; -use miniscript::{Descriptor, Miniscript, MiniscriptKey, Segwitv0}; +use miniscript::Descriptor; mod setup; use setup::test_util::{self, PubData, TestData}; @@ -154,21 +153,18 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) { // Sign the transactions with all keys // AKA the signer role of psbt for i in 0..psbts.len() { - // Get all the pubkeys and the corresponding secret keys - let ms: Miniscript = - Miniscript::parse_insane(psbts[i].inputs[0].witness_script.as_ref().unwrap()).unwrap(); + let ms = if let Descriptor::Wsh(wsh) = &desc_vec[i] { + match wsh.as_inner() { + miniscript::descriptor::WshInner::Ms(ms) => ms, + _ => unreachable!(), + } + } else { + unreachable!("Only Wsh descriptors are supported"); + }; let sks_reqd: Vec<_> = ms - .iter_pk_pkh() - .map(|pk_pkh| match pk_pkh { - iter::PkPkh::PlainPubkey(pk) => sks[pks.iter().position(|&x| x == pk).unwrap()], - iter::PkPkh::HashedPubkey(hash) => { - sks[pks - .iter() - .position(|&pk| pk.to_pubkeyhash() == hash) - .unwrap()] - } - }) + .iter_pk() + .map(|pk| sks[pks.iter().position(|&x| x == pk).unwrap()]) .collect(); // Get the required sighash message let amt = btc(1).to_sat(); diff --git a/tests/test_desc.rs b/tests/test_desc.rs index f9a4c13d7..90a22a601 100644 --- a/tests/test_desc.rs +++ b/tests/test_desc.rs @@ -19,9 +19,8 @@ use bitcoin::{ TxOut, Txid, }; use bitcoind::bitcoincore_rpc::{json, Client, RpcApi}; -use miniscript::miniscript::iter; use miniscript::psbt::{PsbtExt, PsbtInputExt}; -use miniscript::{Descriptor, Miniscript, MiniscriptKey, ScriptContext, ToPublicKey}; +use miniscript::{Descriptor, Miniscript, ScriptContext, ToPublicKey}; mod setup; use rand::RngCore; @@ -192,17 +191,9 @@ pub fn test_desc_satisfy( .iter_scripts() .flat_map(|(_depth, ms)| { let leaf_hash = TapLeafHash::from_script(&ms.encode(), LeafVersion::TapScript); - ms.iter_pk_pkh().filter_map(move |pk_pkh| match pk_pkh { - iter::PkPkh::PlainPubkey(pk) => { - let i = x_only_pks.iter().position(|&x| x.to_public_key() == pk); - i.map(|idx| (xonly_keypairs[idx].clone(), leaf_hash)) - } - iter::PkPkh::HashedPubkey(hash) => { - let i = x_only_pks - .iter() - .position(|&x| x.to_public_key().to_pubkeyhash() == hash); - i.map(|idx| (xonly_keypairs[idx].clone(), leaf_hash)) - } + ms.iter_pk().filter_map(move |pk| { + let i = x_only_pks.iter().position(|&x| x.to_public_key() == pk); + i.map(|idx| (xonly_keypairs[idx].clone(), leaf_hash)) }) }) .collect(); @@ -335,18 +326,10 @@ fn find_sks_ms( let sks = &testdata.secretdata.sks; let pks = &testdata.pubdata.pks; let sks = ms - .iter_pk_pkh() - .filter_map(|pk_pkh| match pk_pkh { - iter::PkPkh::PlainPubkey(pk) => { - let i = pks.iter().position(|&x| x.to_public_key() == pk); - i.map(|idx| (sks[idx])) - } - iter::PkPkh::HashedPubkey(hash) => { - let i = pks - .iter() - .position(|&x| x.to_public_key().to_pubkeyhash() == hash); - i.map(|idx| (sks[idx])) - } + .iter_pk() + .filter_map(|pk| { + let i = pks.iter().position(|&x| x.to_public_key() == pk); + i.map(|idx| (sks[idx])) }) .collect(); sks From 94d5ebabaffe2c720a4879d1e10cda83c01c3191 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Wed, 21 Sep 2022 13:37:49 -0700 Subject: [PATCH 5/6] Cleanup psbt translator APIs We no longer need to maintain mappings from hash160::Hash as Pkh now stores the Pk instead of Pk::Hash --- src/psbt/mod.rs | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index ead519750..08c3efb1a 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -998,33 +998,6 @@ impl PsbtOutputExt for psbt::Output { } } -// Traverse the pkh lookup while maintaining a reverse map for storing the map -// hash160 -> (XonlyPublicKey)/PublicKey -struct XOnlyHashLookUp( - pub BTreeMap, - pub secp256k1::Secp256k1, -); - -impl Translator - for XOnlyHashLookUp -{ - fn pk( - &mut self, - xpk: &DefiniteDescriptorKey, - ) -> Result { - xpk.derive_public_key(&self.1) - } - - // TODO: cleanup this code in a later commit - - // Clone all the associated types in translation - translate_hash_clone!( - DescriptorPublicKey, - bitcoin::PublicKey, - descriptor::ConversionError - ); -} - // Traverse the pkh lookup while maintaining a reverse map for storing the map // hash160 -> (XonlyPublicKey)/PublicKey struct KeySourceLookUp( @@ -1155,9 +1128,7 @@ fn update_item_with_descriptor_helper( let secp = secp256k1::Secp256k1::verification_only(); let derived = if let Descriptor::Tr(_) = &descriptor { - let mut hash_lookup = XOnlyHashLookUp(BTreeMap::new(), secp); - // Feed in information about pkh -> pk mapping here - let derived = descriptor.translate_pk(&mut hash_lookup)?; + let derived = descriptor.derived_descriptor(&secp)?; if let Some(check_script) = check_script { if check_script != &derived.script_pubkey() { From 6a06a115978ef95e4eb53c0e9e7e12a94316b7d0 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Sun, 2 Oct 2022 17:16:50 -0700 Subject: [PATCH 6/6] Allow parsing expr fragments from string/script Standard/Sane scripts refer to miniscripts that are described in the spec. Insane scripts refer to miniscript that are valid but can contain timelock mixes, not all paths would require signature, etc. Experimental features with ext flag current only allow parsing hash160 expr_raw_pkh scripts from string and bitcoin::Script. All regular APIs fail while parsing scripts with raw_pkh. The interpreter module relies on partial information from the blockchian without the descriptor. All the parsing in the interpreter module in done with all exprimental features. Similarly, our psbt finalizer also uses the expr features as it relies on inferring the descriptor from witness script and then puts things into the correct place. --- src/interpreter/inner.rs | 20 +++-- src/interpreter/mod.rs | 6 +- src/lib.rs | 1 + src/macros.rs | 2 +- src/miniscript/analyzable.rs | 147 ++++++++++++++++++++++++++++++++++- src/miniscript/astelem.rs | 4 + src/miniscript/mod.rs | 63 +++++++++++++-- src/psbt/finalizer.rs | 28 +++++-- 8 files changed, 245 insertions(+), 26 deletions(-) diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs index 44073c4f3..59c8eb213 100644 --- a/src/interpreter/inner.rs +++ b/src/interpreter/inner.rs @@ -20,7 +20,7 @@ use bitcoin::util::taproot::{ControlBlock, TAPROOT_ANNEX_PREFIX}; use super::{stack, BitcoinKey, Error, Stack}; use crate::miniscript::context::{NoChecks, ScriptContext, SigType}; use crate::prelude::*; -use crate::{BareCtx, Legacy, Miniscript, Segwitv0, Tap, ToPublicKey, Translator}; +use crate::{BareCtx, ExtParams, Legacy, Miniscript, Segwitv0, Tap, ToPublicKey, Translator}; /// Attempts to parse a slice as a Bitcoin public key, checking compressedness /// if asked to, but otherwise dropping it @@ -54,9 +54,11 @@ fn script_from_stack_elem( elem: &stack::Element<'_>, ) -> Result, Error> { match *elem { - stack::Element::Push(sl) => { - Miniscript::parse_insane(&bitcoin::Script::from(sl.to_owned())).map_err(Error::from) - } + stack::Element::Push(sl) => Miniscript::parse_with_ext( + &bitcoin::Script::from(sl.to_owned()), + &ExtParams::allow_all(), + ) + .map_err(Error::from), stack::Element::Satisfied => { Miniscript::from_ast(crate::Terminal::True).map_err(Error::from) } @@ -345,7 +347,10 @@ pub(super) fn from_txdata<'txin>( } else { if wit_stack.is_empty() { // Bare script parsed in BareCtx - let miniscript = Miniscript::::parse_insane(spk)?; + let miniscript = Miniscript::::parse_with_ext( + spk, + &ExtParams::allow_all(), + )?; let miniscript = miniscript.to_no_checks_ms(); Ok(( Inner::Script(miniscript, ScriptType::Bare), @@ -416,6 +421,7 @@ mod tests { use bitcoin::{self, Script}; use super::*; + use crate::miniscript::analyzable::ExtParams; struct KeyTestData { pk_spk: bitcoin::Script, @@ -754,11 +760,13 @@ mod tests { } fn ms_inner_script(ms: &str) -> (Miniscript, bitcoin::Script) { - let ms = Miniscript::::from_str_insane(ms).unwrap(); + let ms = Miniscript::::from_str_ext(ms, &ExtParams::insane()) + .unwrap(); let spk = ms.encode(); let miniscript = ms.to_no_checks_ms(); (miniscript, spk) } + #[test] fn script_bare() { let preimage = b"12345678----____12345678----____"; diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 684782e33..293fd3273 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1048,6 +1048,7 @@ mod tests { use super::inner::ToNoChecks; use super::*; + use crate::miniscript::analyzable::ExtParams; use crate::miniscript::context::NoChecks; use crate::{Miniscript, ToPublicKey}; @@ -1608,14 +1609,15 @@ mod tests { // By design there is no support for parse a miniscript with BitcoinKey // because it does not implement FromStr fn no_checks_ms(ms: &str) -> Miniscript { + // Parsing should allow raw hashes in the interpreter let elem: Miniscript = - Miniscript::from_str_insane(ms).unwrap(); + Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap(); elem.to_no_checks_ms() } fn x_only_no_checks_ms(ms: &str) -> Miniscript { let elem: Miniscript = - Miniscript::from_str_insane(ms).unwrap(); + Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap(); elem.to_no_checks_ms() } } diff --git a/src/lib.rs b/src/lib.rs index d67e0ba92..713d66eb9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,6 +141,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; pub use crate::descriptor::{DefiniteDescriptorKey, Descriptor, DescriptorPublicKey}; pub use crate::interpreter::Interpreter; +pub use crate::miniscript::analyzable::{AnalysisError, ExtParams}; pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, SigType, Tap}; pub use crate::miniscript::decode::Terminal; pub use crate::miniscript::satisfy::{Preimage32, Satisfier}; diff --git a/src/macros.rs b/src/macros.rs index 5b946e806..c7ef9ebd3 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -6,7 +6,7 @@ /// `ms_str!("c:or_i(pk({}),pk({}))", pk1, pk2)` #[cfg(test)] macro_rules! ms_str { - ($($arg:tt)*) => (Miniscript::from_str_insane(&format!($($arg)*)).unwrap()) + ($($arg:tt)*) => (Miniscript::from_str_ext(&format!($($arg)*), &$crate::ExtParams::allow_all()).unwrap()) } /// Allows tests to create a concrete policy directly from string as diff --git a/src/miniscript/analyzable.rs b/src/miniscript/analyzable.rs index 9e2d84a1b..a98d2acbf 100644 --- a/src/miniscript/analyzable.rs +++ b/src/miniscript/analyzable.rs @@ -22,7 +22,117 @@ use core::fmt; use std::error; use crate::prelude::*; -use crate::{Miniscript, MiniscriptKey, ScriptContext}; +use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal}; + +/// Params for parsing miniscripts that either non-sane or non-specified(experimental) in the spec. +/// Used as a parameter [`Miniscript::from_str_ext`] and [`Miniscript::parse_with_ext`]. +/// +/// This allows parsing miniscripts if +/// 1. It is unsafe(does not require a digital signature to spend it) +/// 2. It contains a unspendable path because of either +/// a. Resource limitations +/// b. Timelock Mixing +/// 3. The script is malleable and thereby some of satisfaction weight +/// guarantees are not satisfied. +/// 4. It has repeated public keys +/// 5. raw pkh fragments without the pk. This could be obtained when parsing miniscript from script +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default)] +pub struct ExtParams { + /// Allow parsing of non-safe miniscripts + pub top_unsafe: bool, + /// Allow parsing of miniscripts with unspendable paths + pub resource_limitations: bool, + /// Allow parsing of miniscripts with timelock mixing + pub timelock_mixing: bool, + /// Allow parsing of malleable miniscripts + pub malleability: bool, + /// Allow parsing of miniscripts with repeated public keys + pub repeated_pk: bool, + /// Allow parsing of miniscripts with raw pkh fragments without the pk. + /// This could be obtained when parsing miniscript from script + pub raw_pkh: bool, +} + +impl ExtParams { + /// Create a new ExtParams that with all the sanity rules + pub fn new() -> ExtParams { + ExtParams { + top_unsafe: false, + resource_limitations: false, + timelock_mixing: false, + malleability: false, + repeated_pk: false, + raw_pkh: false, + } + } + + /// Create a new ExtParams that allows all the sanity rules + pub fn sane() -> ExtParams { + ExtParams::new() + } + + /// Create a new ExtParams that insanity rules + /// This enables parsing well specified but "insane" miniscripts. + /// Refer to the [`ExtParams`] documentation for more details on "insane" miniscripts. + pub fn insane() -> ExtParams { + ExtParams { + top_unsafe: true, + resource_limitations: true, + timelock_mixing: true, + malleability: true, + repeated_pk: true, + raw_pkh: false, + } + } + + /// Enable all non-sane rules and experimental rules + pub fn allow_all() -> ExtParams { + ExtParams { + top_unsafe: true, + resource_limitations: true, + timelock_mixing: true, + malleability: true, + repeated_pk: true, + raw_pkh: true, + } + } + + /// Builder that allows non-safe miniscripts. + pub fn top_unsafe(mut self) -> ExtParams { + self.top_unsafe = true; + self + } + + /// Builder that allows miniscripts with exceed resource limitations. + pub fn exceed_resource_limitations(mut self) -> ExtParams { + self.resource_limitations = true; + self + } + + /// Builder that allows miniscripts with timelock mixing. + pub fn timelock_mixing(mut self) -> ExtParams { + self.timelock_mixing = true; + self + } + + /// Builder that allows malleable miniscripts. + pub fn malleability(mut self) -> ExtParams { + self.malleability = true; + self + } + + /// Builder that allows miniscripts with repeated public keys. + pub fn repeated_pk(mut self) -> ExtParams { + self.repeated_pk = true; + self + } + + /// Builder that allows miniscripts with raw pkh fragments. + pub fn raw_pkh(mut self) -> ExtParams { + self.raw_pkh = true; + self + } +} /// Possible reasons Miniscript guarantees can fail /// We currently mark Miniscript as Non-Analyzable if @@ -45,6 +155,8 @@ pub enum AnalysisError { HeightTimelockCombination, /// Malleable script Malleable, + /// Contains partial descriptor raw pkh + ContainsRawPkh, } impl fmt::Display for AnalysisError { @@ -62,7 +174,8 @@ impl fmt::Display for AnalysisError { AnalysisError::HeightTimelockCombination => { f.write_str("Contains a combination of heightlock and timelock") } - AnalysisError::Malleable => f.write_str("Miniscript is malleable") + AnalysisError::Malleable => f.write_str("Miniscript is malleable"), + AnalysisError::ContainsRawPkh => f.write_str("Miniscript contains raw pkh"), } } } @@ -77,7 +190,8 @@ impl error::Error for AnalysisError { | RepeatedPubkeys | BranchExceedResouceLimits | HeightTimelockCombination - | Malleable => None, + | Malleable + | ContainsRawPkh => None, } } } @@ -116,6 +230,14 @@ impl Miniscript { unique_pkhs_len != all_pkhs_len } + /// Whether the given miniscript contains a raw pkh fragment + pub fn contains_raw_pkh(&self) -> bool { + self.iter().any(|ms| match ms.node { + Terminal::RawPkH(_) => true, + _ => false, + }) + } + /// Check whether the underlying Miniscript is safe under the current context /// Lifting these polices would create a semantic representation that does /// not represent the underlying semantics when miniscript is spent. @@ -140,4 +262,23 @@ impl Miniscript { Ok(()) } } + + /// Check whether the miniscript follows the given Extra policy [`ExtParams`] + pub fn ext_check(&self, ext: &ExtParams) -> Result<(), AnalysisError> { + if !ext.top_unsafe && !self.requires_sig() { + Err(AnalysisError::SiglessBranch) + } else if !ext.malleability && !self.is_non_malleable() { + Err(AnalysisError::Malleable) + } else if !ext.resource_limitations && !self.within_resource_limits() { + Err(AnalysisError::BranchExceedResouceLimits) + } else if !ext.repeated_pk && self.has_repeated_keys() { + Err(AnalysisError::RepeatedPubkeys) + } else if !ext.timelock_mixing && self.has_mixed_timelocks() { + Err(AnalysisError::HeightTimelockCombination) + } else if !ext.raw_pkh && self.contains_raw_pkh() { + Err(AnalysisError::ContainsRawPkh) + } else { + Ok(()) + } + } } diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index 51f299496..9ebf41210 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -23,6 +23,7 @@ use core::fmt; use core::str::FromStr; use bitcoin::blockdata::{opcodes, script}; +use bitcoin::hashes::hash160; use bitcoin::{LockTime, Sequence}; use sync::Arc; @@ -457,6 +458,9 @@ impl_from_tree!( } } let mut unwrapped = match (frag_name, top.args.len()) { + ("expr_raw_pkh", 1) => expression::terminal(&top.args[0], |x| { + hash160::Hash::from_str(x).map(Terminal::RawPkH) + }), ("pk_k", 1) => { expression::terminal(&top.args[0], |x| Pk::from_str(x).map(Terminal::PkK)) } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 6d4368ed9..7c864adf2 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -30,6 +30,7 @@ use core::{fmt, hash, str}; use bitcoin::blockdata::script; use bitcoin::util::taproot::{LeafVersion, TapLeafHash}; +use self::analyzable::ExtParams; pub use self::context::{BareCtx, Legacy, Segwitv0, Tap}; use crate::prelude::*; @@ -156,6 +157,20 @@ impl Miniscript { /// insane scripts. In general, in a multi-party setting users should only /// accept sane scripts. pub fn parse_insane(script: &script::Script) -> Result, Error> { + Miniscript::parse_with_ext(script, &ExtParams::insane()) + } + + /// Attempt to parse an miniscript with extra features that not yet specified in the spec. + /// Users should not use this function unless they scripts can/will change in the future. + /// Currently, this function supports the following features: + /// - Parsing all insane scripts + /// - Parsing miniscripts with raw pubkey hashes + /// + /// Allowed extra features can be specified by the ext [`ExtParams`] argument. + pub fn parse_with_ext( + script: &script::Script, + ext: &ExtParams, + ) -> Result, Error> { let tokens = lex(script)?; let mut iter = TokenIter::new(tokens); @@ -168,6 +183,7 @@ impl Miniscript { if let Some(leading) = iter.next() { Err(Error::Trailing(leading.to_string())) } else { + top.ext_check(ext)?; Ok(top) } } @@ -206,8 +222,7 @@ impl Miniscript { /// /// ``` pub fn parse(script: &script::Script) -> Result, Error> { - let ms = Self::parse_insane(script)?; - ms.sanity_check()?; + let ms = Self::parse_with_ext(script, &ExtParams::sane())?; Ok(ms) } } @@ -337,10 +352,25 @@ impl_block_str!( /// insane scripts. In general, in a multi-party setting users should only /// accept sane scripts. pub fn from_str_insane(s: &str,) -> Result, Error> + { + Miniscript::from_str_ext(s, &ExtParams::insane()) + } +); + +impl_block_str!( + ;Ctx; ScriptContext, + Miniscript, + /// Attempt to parse an Miniscripts that don't follow the spec. + /// Use this to parse scripts with repeated pubkeys, timelock mixing, malleable + /// scripts, raw pubkey hashes without sig or scripts that can exceed resource limits. + /// + /// Use [`ExtParams`] builder to specify the types of non-sane rules to allow while parsing. + pub fn from_str_ext(s: &str, ext: &ExtParams,) -> Result, Error> { // This checks for invalid ASCII chars let top = expression::Tree::from_str(s)?; let ms: Miniscript = expression::FromTree::from_tree(&top)?; + ms.ext_check(ext)?; if ms.ty.corr.base != types::Base::B { Err(Error::NonTopLevel(format!("{:?}", ms))) @@ -433,8 +463,7 @@ impl_from_str!( /// See [Miniscript::from_str_insane] to parse scripts from string that /// do not clear the [Miniscript::sanity_check] checks. fn from_str(s: &str) -> Result, Error> { - let ms = Self::from_str_insane(s)?; - ms.sanity_check()?; + let ms = Self::from_str_ext(s, &ExtParams::sane())?; Ok(ms) } ); @@ -460,7 +489,7 @@ mod tests { use crate::policy::Liftable; use crate::prelude::*; use crate::test_utils::{StrKeyTranslator, StrXOnlyKeyTranslator}; - use crate::{hex_script, DummyKey, Satisfier, ToPublicKey, TranslatePk}; + use crate::{hex_script, DummyKey, ExtParams, Satisfier, ToPublicKey, TranslatePk}; type Segwitv0Script = Miniscript; type Tapscript = Miniscript; @@ -545,8 +574,9 @@ mod tests { if let Some(expected) = expected_hex.into() { assert_eq!(format!("{:x}", bitcoin_script), expected); } - let roundtrip = - Segwitv0Script::parse_insane(&bitcoin_script).expect("parse string serialization"); + // Parse scripts with all extensions + let roundtrip = Segwitv0Script::parse_with_ext(&bitcoin_script, &ExtParams::allow_all()) + .expect("parse string serialization"); assert_eq!(roundtrip, script); } @@ -1091,4 +1121,23 @@ mod tests { let ms = Miniscript::::parse_insane(&enc).unwrap(); assert_eq!(ms_trans.encode(), ms.encode()); } + + #[test] + fn expr_features() { + // test that parsing raw hash160 does not work with + let hash160_str = "e9f171df53e04b270fa6271b42f66b0f4a99c5a2"; + let ms_str = &format!("c:expr_raw_pkh({})", hash160_str); + type SegwitMs = Miniscript; + + // Test that parsing raw hash160 from string does not work without extra features + SegwitMs::from_str(ms_str).unwrap_err(); + SegwitMs::from_str_insane(ms_str).unwrap_err(); + let ms = SegwitMs::from_str_ext(ms_str, &ExtParams::allow_all()).unwrap(); + + let script = ms.encode(); + // The same test, but parsing from script + SegwitMs::parse(&script).unwrap_err(); + SegwitMs::parse_insane(&script).unwrap_err(); + SegwitMs::parse_with_ext(&script, &ExtParams::allow_all()).unwrap(); + } } diff --git a/src/psbt/finalizer.rs b/src/psbt/finalizer.rs index 7bc507f5f..8f7e921cb 100644 --- a/src/psbt/finalizer.rs +++ b/src/psbt/finalizer.rs @@ -29,7 +29,9 @@ use bitcoin::{self, PublicKey, Script, TxOut}; use super::{sanity_check, Error, InputError, Psbt, PsbtInputSatisfier}; use crate::prelude::*; use crate::util::witness_size; -use crate::{interpreter, BareCtx, Descriptor, Legacy, Miniscript, Satisfier, Segwitv0, Tap}; +use crate::{ + interpreter, BareCtx, Descriptor, ExtParams, Legacy, Miniscript, Satisfier, Segwitv0, Tap, +}; // Satisfy the taproot descriptor. It is not possible to infer the complete // descriptor from psbt because the information about all the scripts might not @@ -58,7 +60,10 @@ fn construct_tap_witness( // We don't know how to satisfy non default version scripts yet continue; } - let ms = match Miniscript::::parse_insane(script) { + let ms = match Miniscript::::parse_with_ext( + script, + &ExtParams::allow_all(), + ) { Ok(ms) => ms, Err(..) => continue, // try another script }; @@ -181,7 +186,10 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In p2wsh_expected: script_pubkey.clone(), }); } - let ms = Miniscript::::parse_insane(witness_script)?; + let ms = Miniscript::::parse_with_ext( + witness_script, + &ExtParams::allow_all(), + )?; Ok(Descriptor::new_wsh(ms)?) } else { Err(InputError::MissingWitnessScript) @@ -205,8 +213,9 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In p2wsh_expected: redeem_script.clone(), }); } - let ms = Miniscript::::parse_insane( + let ms = Miniscript::::parse_with_ext( witness_script, + &ExtParams::allow_all(), )?; Ok(Descriptor::new_sh_wsh(ms)?) } else { @@ -229,8 +238,10 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In return Err(InputError::NonEmptyWitnessScript); } if let Some(ref redeem_script) = inp.redeem_script { - let ms = - Miniscript::::parse_insane(redeem_script)?; + let ms = Miniscript::::parse_with_ext( + redeem_script, + &ExtParams::allow_all(), + )?; Ok(Descriptor::new_sh(ms)?) } else { Err(InputError::MissingWitnessScript) @@ -246,7 +257,10 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In if inp.redeem_script.is_some() { return Err(InputError::NonEmptyRedeemScript); } - let ms = Miniscript::::parse_insane(script_pubkey)?; + let ms = Miniscript::::parse_with_ext( + script_pubkey, + &ExtParams::allow_all(), + )?; Ok(Descriptor::new_bare(ms)?) } }