diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 18e40116b..f276199a3 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -14,7 +14,7 @@ use bitcoin::{Address, Network, ScriptBuf, Weight}; use crate::descriptor::write_descriptor; use crate::expression::{self, FromTree}; -use crate::miniscript::context::{ScriptContext, ScriptContextError}; +use crate::miniscript::context::ScriptContext; use crate::miniscript::satisfy::{Placeholder, Satisfaction, Witness}; use crate::plan::AssetProvider; use crate::policy::{semantic, Liftable}; @@ -22,7 +22,7 @@ use crate::prelude::*; use crate::util::{varint_len, witness_to_scriptsig}; use crate::{ BareCtx, Error, ForEachKey, FromStrKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, - TranslateErr, Translator, + TranslateErr, Translator, ValidationError, }; /// Create a Bare Descriptor. That is descriptor that is @@ -35,9 +35,8 @@ pub struct Bare { impl Bare { /// Create a new raw descriptor - pub fn new(ms: Miniscript) -> Result { - // do the top-level checks - BareCtx::top_level_checks(&ms)?; + pub fn new(ms: Miniscript) -> Result { + ms.validate(&BareCtx::SANE)?; Ok(Self { ms }) } @@ -91,7 +90,9 @@ impl Bare { where T: Translator, { - Bare::new(self.ms.translate_pk(t)?).map_err(TranslateErr::OuterError) + Bare::new(self.ms.translate_pk(t)?) + .map_err(Error::Validation) + .map_err(TranslateErr::OuterError) } } @@ -159,14 +160,13 @@ impl fmt::Display for Bare { } impl Liftable for Bare { - fn lift(&self) -> Result, Error> { self.ms.lift() } + fn lift(&self) -> Result, ValidationError> { self.ms.lift() } } impl FromTree for Bare { fn from_tree(root: expression::TreeIterItem) -> Result { let sub = Miniscript::::from_tree(root)?; - BareCtx::top_level_checks(&sub)?; - Self::new(sub) + Self::new(sub).map_err(Error::Validation) } } @@ -193,12 +193,11 @@ pub struct Pkh { impl Pkh { /// Create a new Pkh descriptor - pub fn new(pk: Pk) -> Result { - // do the top-level checks - match BareCtx::check_pk(&pk) { - Ok(()) => Ok(Self { pk }), - Err(e) => Err(e), - } + pub fn new(pk: Pk) -> Result { + BareCtx::SANE + .validate_pk(&pk) + .map_err(ValidationError::Key)?; + Ok(Self { pk }) } /// Get a reference to the inner key @@ -248,7 +247,7 @@ impl Pkh { let res = Pkh::new(t.pk(&self.pk)?); match res { Ok(pk) => Ok(pk), - Err(e) => Err(TranslateErr::OuterError(Error::from(e))), + Err(e) => Err(TranslateErr::OuterError(Error::Validation(e))), } } } @@ -343,7 +342,7 @@ impl fmt::Display for Pkh { } impl Liftable for Pkh { - fn lift(&self) -> Result, Error> { + fn lift(&self) -> Result, ValidationError> { Ok(semantic::Policy::Key(self.pk.clone())) } } @@ -353,7 +352,7 @@ impl FromTree for Pkh { let pk = root .verify_terminal_parent("pkh", "public key") .map_err(Error::Parse)?; - Self::new(pk).map_err(Error::ContextError) + Self::new(pk).map_err(Error::Validation) } } diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index a2e48bcf2..b15e52a4f 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -29,7 +29,7 @@ use crate::plan::{AssetProvider, Plan}; use crate::prelude::*; use crate::{ expression, hash256, BareCtx, Error, ForEachKey, FromStrKey, MiniscriptKey, ParseError, - Satisfier, Threshold, ToPublicKey, TranslateErr, Translator, + Satisfier, Threshold, ToPublicKey, TranslateErr, Translator, ValidationError, }; mod bare; @@ -157,41 +157,43 @@ impl Descriptor { } /// Create a new PkH descriptor - pub fn new_pkh(pk: Pk) -> Result { Ok(Self::Pkh(Pkh::new(pk)?)) } + pub fn new_pkh(pk: Pk) -> Result { Ok(Self::Pkh(Pkh::new(pk)?)) } /// Create a new Wpkh descriptor /// Will return Err if uncompressed key is used - pub fn new_wpkh(pk: Pk) -> Result { Ok(Self::Wpkh(Wpkh::new(pk)?)) } + pub fn new_wpkh(pk: Pk) -> Result { Ok(Self::Wpkh(Wpkh::new(pk)?)) } /// Create a new sh wrapped wpkh from `Pk`. /// Errors when uncompressed keys are supplied - pub fn new_sh_wpkh(pk: Pk) -> Result { Ok(Self::Sh(Sh::new_wpkh(pk)?)) } + pub fn new_sh_wpkh(pk: Pk) -> Result { Ok(Self::Sh(Sh::new_wpkh(pk)?)) } // Miniscripts /// Create a new sh for a given redeem script /// Errors when miniscript exceeds resource limits under p2sh context /// or does not type check at the top level - pub fn new_sh(ms: Miniscript) -> Result { Ok(Self::Sh(Sh::new(ms)?)) } + pub fn new_sh(ms: Miniscript) -> Result { + Ok(Self::Sh(Sh::new(ms)?)) + } /// Create a new wsh descriptor from witness script /// Errors when miniscript exceeds resource limits under p2sh context /// or does not type check at the top level - pub fn new_wsh(ms: Miniscript) -> Result { + pub fn new_wsh(ms: Miniscript) -> Result { Ok(Self::Wsh(Wsh::new(ms)?)) } /// Create a new sh wrapped wsh descriptor with witness script /// Errors when miniscript exceeds resource limits under wsh context /// or does not type check at the top level - pub fn new_sh_wsh(ms: Miniscript) -> Result { + pub fn new_sh_wsh(ms: Miniscript) -> Result { Ok(Self::Sh(Sh::new_wsh(ms)?)) } /// Create a new bare descriptor from witness script /// Errors when miniscript exceeds resource limits under bare context /// or does not type check at the top level - pub fn new_bare(ms: Miniscript) -> Result { + pub fn new_bare(ms: Miniscript) -> Result { Ok(Self::Bare(Bare::new(ms)?)) } @@ -210,7 +212,7 @@ impl Descriptor { /// Errors when miniscript exceeds resource limits under p2sh context pub fn new_sh_sortedmulti( thresh: Threshold, - ) -> Result { + ) -> Result { Ok(Self::Sh(Sh::new_sortedmulti(thresh)?)) } @@ -219,7 +221,7 @@ impl Descriptor { /// Errors when miniscript exceeds resource limits under segwit context pub fn new_sh_wsh_sortedmulti( thresh: Threshold, - ) -> Result { + ) -> Result { Ok(Self::Sh(Sh::new_wsh_sortedmulti(thresh)?)) } @@ -227,13 +229,13 @@ impl Descriptor { /// Errors when miniscript exceeds resource limits under p2sh context pub fn new_wsh_sortedmulti( thresh: Threshold, - ) -> Result { + ) -> Result { Ok(Self::Wsh(Wsh::new_sortedmulti(thresh)?)) } /// Create new tr descriptor /// Errors when miniscript exceeds resource limits under Tap context - pub fn new_tr(key: Pk, script: Option>) -> Result { + pub fn new_tr(key: Pk, script: Option>) -> Result { Ok(Self::Tr(Tr::new(key, script)?)) } @@ -967,6 +969,12 @@ impl Descriptor { /// /// For multipath descriptors it will return as many descriptors as there is /// "parallel" paths. For regular descriptors it will just return itself. + /// + /// # Panics + /// + /// May panic if given a descriptor which has multiple multi-path keys with different + /// numbers of paths. Such descriptors are rejected by the ordinary constructors, + /// which enforce "sanity" rules, so most users do not need to worry about this. #[allow(clippy::blocks_in_conditions)] pub fn into_single_descriptors(self) -> Result, Error> { // All single-path descriptors contained in this descriptor. @@ -998,19 +1006,16 @@ impl Descriptor { struct IndexChoser(usize); impl Translator for IndexChoser { type TargetPk = DescriptorPublicKey; - type Error = Error; + type Error = core::convert::Infallible; - fn pk(&mut self, pk: &DescriptorPublicKey) -> Result { + fn pk(&mut self, pk: &DescriptorPublicKey) -> Result { match pk { DescriptorPublicKey::Single(..) | DescriptorPublicKey::XPub(..) => { Ok(pk.clone()) } - DescriptorPublicKey::MultiXPub(_) => pk - .clone() - .into_single_keys() - .get(self.0) - .cloned() - .ok_or(Error::MultipathDescLenMismatch), + DescriptorPublicKey::MultiXPub(_) => { + Ok(pk.clone().into_single_keys()[self.0].clone()) + } } } translate_hash_clone!(DescriptorPublicKey); @@ -1018,9 +1023,13 @@ impl Descriptor { for (i, desc) in descriptors.iter_mut().enumerate() { let mut index_choser = IndexChoser(i); + // This ? could trigger if e.g. there are two multipath keys (a,b) and (a,c); + // these are distinct and will not trigger "duplicate pubkey" checks but once + // we fork the descriptor, 'c' will appear twice in one of them, resulting in + // an error. *desc = desc .translate_pk(&mut index_choser) - .map_err(|e| e.expect_translator_err("No Context errors possible"))?; + .map_err(TranslateErr::into_outer_err)?; } Ok(descriptors) @@ -1231,7 +1240,6 @@ mod tests { use super::{checksum, *}; use crate::hex_script; - use crate::miniscript::context::ScriptContextError; #[cfg(feature = "compiler")] use crate::policy; @@ -1254,13 +1262,15 @@ mod tests { // in terms of the in-memory representation -- OrI(False, False). // Test that the way we display the ambiguous fragment doesn't // change, in case somebody somehow is depending on it. - let desc = StdDescriptor::from_str("sh(u:0)").unwrap(); - assert_eq!("sh(u:0)#ncq3yf9h", desc.to_string()); + let desc = StdDescriptor::from_str("wsh(u:0)").unwrap(); + assert_eq!("wsh(u:0)#6mwq40tt", desc.to_string()); // This is a regression test for https://github.com/rust-bitcoin/rust-miniscript/pull/735 - // which was found at the same time. It's just a bug plain and simple. - let desc = StdDescriptor::from_str("sh(and_n(u:0,1))").unwrap(); - assert_eq!("sh(and_n(u:0,1))#5j5tw8nm", desc.to_string()); + // which was found at the same time. It's just a bug plain and simple. (We were + // encoding and_n as "and_b".) + let desc_str = format!("wsh(and_n({TEST_PK},1))#a23r7pua"); + let desc = desc_str.parse::().unwrap(); + assert_eq!(desc_str, desc.to_string()); } #[test] @@ -1648,7 +1658,9 @@ mod tests { #[test] fn after_is_cltv() { - let descriptor = Descriptor::::from_str("wsh(after(1000))").unwrap(); + let descriptor = format!("wsh(and_v(v:{TEST_PK},after(1000)))") + .parse::>() + .unwrap(); let script = descriptor.explicit_script().unwrap(); let actual_instructions: Vec<_> = script.instructions().collect(); @@ -1659,7 +1671,9 @@ mod tests { #[test] fn older_is_csv() { - let descriptor = Descriptor::::from_str("wsh(older(1000))").unwrap(); + let descriptor = format!("wsh(and_v(v:{TEST_PK},older(1000)))") + .parse::>() + .unwrap(); let script = descriptor.explicit_script().unwrap(); let actual_instructions: Vec<_> = script.instructions().collect(); @@ -2252,17 +2266,17 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))"; "sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))", ); - let wsh = StdDescriptor::from_str("wsh(1)").unwrap(); - assert_eq!(format!("{}", wsh), "wsh(1)#mrg7xj7p"); - assert_eq!(format!("{:#}", wsh), "wsh(1)"); + let wsh = StdDescriptor::from_str("wsh(0)").unwrap(); + assert_eq!(format!("{}", wsh), "wsh(0)#a0qjqdfq"); + assert_eq!(format!("{:#}", wsh), "wsh(0)"); - let sh = StdDescriptor::from_str("sh(1)").unwrap(); - assert_eq!(format!("{}", sh), "sh(1)#l8r75ggs"); - assert_eq!(format!("{:#}", sh), "sh(1)"); + let sh = StdDescriptor::from_str("sh(0)").unwrap(); + assert_eq!(format!("{}", sh), "sh(0)#ettjjhl3"); + assert_eq!(format!("{:#}", sh), "sh(0)"); - let shwsh = StdDescriptor::from_str("sh(wsh(1))").unwrap(); - assert_eq!(format!("{}", shwsh), "sh(wsh(1))#hcyfl07f"); - assert_eq!(format!("{:#}", shwsh), "sh(wsh(1))"); + let shwsh = StdDescriptor::from_str("sh(wsh(0))").unwrap(); + assert_eq!(format!("{}", shwsh), "sh(wsh(0))#f45zwpau"); + assert_eq!(format!("{:#}", shwsh), "sh(wsh(0))"); let tr = StdDescriptor::from_str( "tr(020000000000000000000000000000000000000000000000000000000000000002)", @@ -2320,11 +2334,11 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))"; Descriptor::::from_str( "wsh(or_i(pk(0202baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a66a),1))", ) - .unwrap(); + .unwrap_err(); Descriptor::::from_str( "sh(or_i(pk(0202baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a66a),1))", ) - .unwrap(); + .unwrap_err(); Descriptor::::from_str( "tr(02baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a66a,1)", ) @@ -2934,22 +2948,29 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))"; #[test] fn too_many_pubkeys_for_p2sh() { - // Arbitrary 65-byte public key (66 with length prefix). - let pk = PublicKey::from_str( - "0400232a2acfc9b43fa89f1b4f608fde335d330d7114f70ea42bfb4a41db368a3e3be6934a4097dd25728438ef73debb1f2ffdb07fec0f18049df13bdc5285dc5b", - ) - .unwrap(); + // Arbitrary 65-byte public keys (66 with length prefix). We need distinct ones + // to avoid getting DuplicateKey errors. + let pks: Vec = vec![ + "0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798b7c52588d95c3b9aa25b0403f1eef75702e84bb7597aabe663b82f6f04ef2777".parse().unwrap(), + "04c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5e51e970159c23cc65c3a7be6b99315110809cd9acd992f1edc9bce55af301705".parse().unwrap(), + "04f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9c77084f09cd217ebf01cc819d5c80ca99aff5666cb3ddce4934602897b4715bd".parse().unwrap(), + "04e493dbf1c10d80f3581e4904930b1404cc6c13900ee0758474fa94abe8c4cd13ae1266c15f2baa48a9bd1df6715aebb7269851cc404201bf30168422b88c630d".parse().unwrap(), + "042f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe42753ddd9c91a1c292b24562259363bd90877d8e454f297bf235782c459539959".parse().unwrap(), + "04fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a146029755651ed8885530449df0c4169fe80ba3a9f217f0f09ae701b5fc378f3c84f8a0998".parse().unwrap(), + "045cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc951435bf45daa69f5ce8729279e5ab2457ec2f47ec02184a5af7d9d6f78d9755".parse().unwrap(), + "042f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01a3b25758beac66b6d6c2f7d5ecd2ec4b3d1dec2945a489e84a25d3479342132b".parse().unwrap(), + ]; // This is legal for CHECKMULTISIG, but the 8 keys consume the whole 520 bytes // allowed by P2SH, meaning that the full script goes over the limit. - let thresh = Threshold::new(2, vec![pk; 8]).expect("the thresh is ok.."); + let thresh = Threshold::new(2, pks).expect("the thresh is ok.."); let script = Miniscript::<_, Legacy>::sortedmulti(thresh).encode(); let res = Miniscript::<_, Legacy>::decode(&script); let error = res.expect_err("decoding should err"); match error { - Error::ContextError(ScriptContextError::MaxRedeemScriptSizeExceeded { .. }) => {} // ok + Error::Validation(ValidationError::MaxScriptSizeExceeded { .. }) => {} // ok other => panic!("unexpected error: {:?}", other), } } diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 84e3ceb05..b1755b146 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -12,7 +12,7 @@ use bitcoin::{Address, Network, ScriptBuf, Weight}; use crate::descriptor::write_descriptor; use crate::expression::{self, FromTree}; -use crate::miniscript::context::{ScriptContext, ScriptContextError}; +use crate::miniscript::context::ScriptContext; use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; use crate::miniscript::satisfy::{Placeholder, Satisfaction, Witness}; use crate::plan::AssetProvider; @@ -21,7 +21,7 @@ use crate::prelude::*; use crate::util::varint_len; use crate::{ Error, ForEachKey, FromStrKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, Terminal, - Threshold, ToPublicKey, TranslateErr, Translator, + Threshold, ToPublicKey, TranslateErr, Translator, ValidationError, }; /// A Segwitv0 wsh descriptor #[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] @@ -38,14 +38,17 @@ impl Wsh { pub fn as_inner(&self) -> &Miniscript { &self.ms } /// Create a new wsh descriptor - pub fn new(ms: Miniscript) -> Result { - // do the top-level checks - Segwitv0::top_level_checks(&ms)?; + pub fn new(ms: Miniscript) -> Result { + ms.validate(&Segwitv0::SANE)?; Ok(Self { ms }) } /// Create a new sortedmulti wsh descriptor - pub fn new_sortedmulti(thresh: Threshold) -> Result { + pub fn new_sortedmulti( + thresh: Threshold, + ) -> Result { + // The context checks will be carried out inside new function for + // sortedMultiVec Ok(Self { ms: Miniscript::sortedmulti(thresh) }) } @@ -176,7 +179,7 @@ impl Wsh { } impl Liftable for Wsh { - fn lift(&self) -> Result, Error> { self.ms.lift() } + fn lift(&self) -> Result, ValidationError> { self.ms.lift() } } impl crate::expression::FromTree for Wsh { @@ -187,7 +190,7 @@ impl crate::expression::FromTree for Wsh { .map_err(Error::Parse)?; let sub = Miniscript::from_tree(top)?; - Segwitv0::top_level_checks(&sub)?; + sub.validate(&Segwitv0::SANE).map_err(Error::Validation)?; Ok(Self { ms: sub }) } } @@ -225,12 +228,11 @@ pub struct Wpkh { impl Wpkh { /// Create a new Wpkh descriptor - pub fn new(pk: Pk) -> Result { - // do the top-level checks - match Segwitv0::check_pk(&pk) { - Ok(_) => Ok(Self { pk }), - Err(e) => Err(e), - } + pub fn new(pk: Pk) -> Result { + Segwitv0::SANE + .validate_pk(&pk) + .map_err(ValidationError::Key)?; + Ok(Self { pk }) } /// Get the inner key @@ -276,7 +278,7 @@ impl Wpkh { let res = Wpkh::new(t.pk(&self.pk)?); match res { Ok(pk) => Ok(pk), - Err(e) => Err(TranslateErr::OuterError(Error::from(e))), + Err(e) => Err(TranslateErr::OuterError(Error::Validation(e))), } } } @@ -379,7 +381,7 @@ impl fmt::Display for Wpkh { } impl Liftable for Wpkh { - fn lift(&self) -> Result, Error> { + fn lift(&self) -> Result, ValidationError> { Ok(semantic::Policy::Key(self.pk.clone())) } } @@ -389,7 +391,7 @@ impl crate::expression::FromTree for Wpkh { let pk = top .verify_terminal_parent("wpkh", "public key") .map_err(Error::Parse)?; - Self::new(pk).map_err(Error::ContextError) + Self::new(pk).map_err(Error::Validation) } } diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index d926bb381..384d44c31 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -25,7 +25,7 @@ use crate::prelude::*; use crate::util::{varint_len, witness_to_scriptsig}; use crate::{ push_opcode_size, Error, ForEachKey, FromStrKey, Legacy, Miniscript, MiniscriptKey, Satisfier, - Segwitv0, Threshold, ToPublicKey, TranslateErr, Translator, + Segwitv0, Threshold, ToPublicKey, TranslateErr, Translator, ValidationError, }; /// A Legacy p2sh Descriptor @@ -47,7 +47,7 @@ pub enum ShInner { } impl Liftable for Sh { - fn lift(&self) -> Result, Error> { + fn lift(&self) -> Result, ValidationError> { match self.inner { ShInner::Wsh(ref wsh) => wsh.lift(), ShInner::Wpkh(ref pk) => Ok(semantic::Policy::Key(pk.as_inner().clone())), @@ -88,7 +88,7 @@ impl crate::expression::FromTree for Sh { "wpkh" => ShInner::Wpkh(Wpkh::from_tree(top)?), _ => { let sub = Miniscript::from_tree(top)?; - Legacy::top_level_checks(&sub)?; + sub.validate(&Legacy::SANE).map_err(Error::Validation)?; ShInner::Ms(sub) } }; @@ -112,20 +112,23 @@ impl Sh { pub fn as_inner(&self) -> &ShInner { &self.inner } /// Create a new p2sh descriptor with the raw miniscript - pub fn new(ms: Miniscript) -> Result { - // do the top-level checks - Legacy::top_level_checks(&ms)?; + pub fn new(ms: Miniscript) -> Result { + ms.validate(&Legacy::SANE)?; Ok(Self { inner: ShInner::Ms(ms) }) } /// Create a new p2sh sortedmulti descriptor with threshold `k` /// and Vec of `pks`. - pub fn new_sortedmulti(thresh: Threshold) -> Result { + pub fn new_sortedmulti( + thresh: Threshold, + ) -> Result { + // The context checks will be carried out inside new function for + // sortedMultiVec Ok(Self { inner: ShInner::Ms(Miniscript::sortedmulti(thresh)) }) } /// Create a new p2sh wrapped wsh descriptor with the raw miniscript - pub fn new_wsh(ms: Miniscript) -> Result { + pub fn new_wsh(ms: Miniscript) -> Result { Ok(Self { inner: ShInner::Wsh(Wsh::new(ms)?) }) } @@ -136,14 +139,14 @@ impl Sh { /// `k` and Vec of `pks` pub fn new_wsh_sortedmulti( thresh: Threshold, - ) -> Result { + ) -> Result { // The context checks will be carried out inside new function for // sortedMultiVec Ok(Self { inner: ShInner::Wsh(Wsh::new_sortedmulti(thresh)?) }) } /// Create a new p2sh wrapped wpkh from `Pk` - pub fn new_wpkh(pk: Pk) -> Result { + pub fn new_wpkh(pk: Pk) -> Result { Ok(Self { inner: ShInner::Wpkh(Wpkh::new(pk)?) }) } diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index 6a1b1893f..7b83ec15a 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -9,15 +9,14 @@ use sync::Arc; use super::checksum; use crate::expression::{self, FromTree}; use crate::miniscript::satisfy::{Placeholder, Satisfaction, SchnorrSigType, Witness}; -use crate::miniscript::Miniscript; use crate::plan::AssetProvider; use crate::policy::semantic::Policy; use crate::policy::Liftable; use crate::prelude::*; use crate::util::{varint_len, witness_size}; use crate::{ - Error, ForEachKey, FromStrKey, MiniscriptKey, ParseError, Satisfier, ScriptContext, Tap, - Threshold, ToPublicKey, TranslateErr, Translator, + Error, ForEachKey, FromStrKey, Miniscript, MiniscriptKey, ParseError, Satisfier, + ScriptContext as _, Tap, Threshold, ToPublicKey, TranslateErr, Translator, ValidationError, }; mod spend_info; @@ -92,8 +91,10 @@ impl hash::Hash for Tr { impl Tr { /// Create a new [`Tr`] descriptor from internal key and [`TapTree`] - pub fn new(internal_key: Pk, tree: Option>) -> Result { - Tap::check_pk(&internal_key)?; + pub fn new(internal_key: Pk, tree: Option>) -> Result { + Tap::SANE + .validate_pk(&internal_key) + .map_err(ValidationError::Key)?; Ok(Self { internal_key, tree, spend_info: Mutex::new(None) }) } @@ -250,8 +251,9 @@ impl Tr { Some(tree) => Some(tree.translate_pk(translate)?), None => None, }; - let translate_desc = - Tr::new(translate.pk(&self.internal_key)?, tree).map_err(TranslateErr::OuterError)?; + let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree) + .map_err(Error::Validation) + .map_err(TranslateErr::OuterError)?; Ok(translate_desc) } } @@ -352,7 +354,7 @@ impl crate::expression::FromTree for Tr { .map_err(Error::Parse)?; let tap_tree = match root_children.next() { - None => return Self::new(internal_key, None), + None => return Self::new(internal_key, None).map_err(Error::Validation), Some(tree) => tree, }; @@ -383,7 +385,7 @@ impl crate::expression::FromTree for Tr { tap_tree_iter.skip_descendants(); } } - Self::new(internal_key, Some(tree_builder.finalize())) + Self::new(internal_key, Some(tree_builder.finalize())).map_err(Error::Validation) } } @@ -410,7 +412,7 @@ impl fmt::Display for Tr { } impl Liftable for Tr { - fn lift(&self) -> Result, Error> { + fn lift(&self) -> Result, ValidationError> { match &self.tree { Some(root) => Ok(Policy::Thresh(Threshold::or( Arc::new(Policy::Key(self.internal_key.clone())), diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index 8d8d0e03f..13fe58f2b 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -8,7 +8,7 @@ use crate::miniscript::context::Tap; use crate::policy::{Liftable, Semantic}; use crate::prelude::Vec; use crate::sync::Arc; -use crate::{Miniscript, MiniscriptKey, Threshold, ToPublicKey}; +use crate::{Miniscript, MiniscriptKey, Threshold, ToPublicKey, ValidationError}; /// Tried to construct Taproot tree which was too deep. #[derive(PartialEq, Eq, Debug)] @@ -74,7 +74,7 @@ impl TapTree { } impl Liftable for TapTree { - fn lift(&self) -> Result, crate::Error> { + fn lift(&self) -> Result, ValidationError> { let thresh_vec = self .leaves() .map(|item| item.miniscript().lift().map(Arc::new)) diff --git a/src/lib.rs b/src/lib.rs index 2d9beaf78..f504adce3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -450,19 +450,10 @@ pub enum Error { #[cfg(feature = "compiler")] /// Compiler related errors CompilerError(crate::policy::compiler::CompilerError), - /// Errors related to policy - ConcretePolicy(policy::concrete::PolicyError), - /// Errors related to lifting - LiftError(policy::LiftError), - /// Forward script context related errors - ContextError(miniscript::context::ScriptContextError), /// Tried to construct a Taproot tree which was too deep. TapTreeDepthError(crate::descriptor::TapTreeDepthError), /// Recursion depth exceeded when parsing policy/miniscript from string MaxRecursiveDepthExceeded, - /// Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) - /// up to n=3 is invalid by standardness (bare) - NonStandardBareScript, /// Miniscript is equivalent to false. No possible satisfaction ImpossibleSatisfaction, /// Bare descriptors don't have any addresses @@ -471,9 +462,6 @@ pub enum Error { PubKeyCtxError(miniscript::decode::KeyError, &'static str), /// No script code for Tr descriptors TrNoScriptCode, - /// At least two BIP389 key expressions in the descriptor contain tuples of - /// derivation indexes of different lengths. - MultipathDescLenMismatch, /// Invalid absolute locktime AbsoluteLockTime(AbsLockTimeError), /// Invalid absolute locktime @@ -510,30 +498,18 @@ impl fmt::Display for Error { Self::CouldNotSatisfy => f.write_str("could not satisfy"), Self::TypeCheck(ref e) => write!(f, "typecheck: {}", e), Self::Secp(ref e) => fmt::Display::fmt(e, f), - Self::ContextError(ref e) => fmt::Display::fmt(e, f), Self::TapTreeDepthError(ref e) => fmt::Display::fmt(e, f), #[cfg(feature = "compiler")] Self::CompilerError(ref e) => fmt::Display::fmt(e, f), - Self::ConcretePolicy(ref e) => fmt::Display::fmt(e, f), - Self::LiftError(ref e) => fmt::Display::fmt(e, f), - Self::MaxRecursiveDepthExceeded => write!( - f, - "Recursive depth over {} not permitted", - MAX_RECURSION_DEPTH - ), - Self::NonStandardBareScript => write!( - f, - "Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) \ - up to n=3 is invalid by standardness (bare). - " - ), + Self::MaxRecursiveDepthExceeded => { + write!(f, "Recursive depth over {} not permitted", MAX_RECURSION_DEPTH) + } Self::ImpossibleSatisfaction => write!(f, "Impossible to satisfy Miniscript"), Self::BareDescriptorAddr => write!(f, "Bare descriptors don't have address"), Self::PubKeyCtxError(ref pk, ref ctx) => { write!(f, "Pubkey error: {} under {} scriptcontext", pk, ctx) } Self::TrNoScriptCode => write!(f, "No script code for Tr descriptors"), - Self::MultipathDescLenMismatch => write!(f, "At least two BIP389 key expressions in the descriptor contain tuples of derivation indexes of different lengths"), Self::AbsoluteLockTime(ref e) => e.fmt(f), Self::RelativeLockTime(ref e) => e.fmt(f), Self::Threshold(ref e) => e.fmt(f), @@ -558,20 +534,15 @@ impl std::error::Error for Error { | CouldNotSatisfy | TypeCheck(_) | MaxRecursiveDepthExceeded - | NonStandardBareScript | ImpossibleSatisfaction | BareDescriptorAddr - | TrNoScriptCode - | MultipathDescLenMismatch => None, + | TrNoScriptCode => None, ScriptLexer(e) => Some(e), AddrError(e) => Some(e), AddrP2shError(e) => Some(e), Secp(e) => Some(e), #[cfg(feature = "compiler")] CompilerError(e) => Some(e), - ConcretePolicy(e) => Some(e), - LiftError(e) => Some(e), - ContextError(e) => Some(e), TapTreeDepthError(e) => Some(e), PubKeyCtxError(e, _) => Some(e), AbsoluteLockTime(e) => Some(e), @@ -594,22 +565,11 @@ impl From for Error { fn from(e: miniscript::types::Error) -> Self { Self::TypeCheck(e.to_string()) } } -#[doc(hidden)] -impl From for Error { - fn from(e: policy::LiftError) -> Self { Self::LiftError(e) } -} - #[doc(hidden)] impl From for Error { fn from(e: crate::descriptor::TapTreeDepthError) -> Self { Self::TapTreeDepthError(e) } } -#[doc(hidden)] -impl From for Error { - fn from(e: miniscript::context::ScriptContextError) -> Self { Self::ContextError(e) } -} - -#[doc(hidden)] impl From for Error { fn from(e: bitcoin::secp256k1::Error) -> Self { Self::Secp(e) } } diff --git a/src/miniscript/analyzable.rs b/src/miniscript/analyzable.rs index d58a62038..ef65a1ab8 100644 --- a/src/miniscript/analyzable.rs +++ b/src/miniscript/analyzable.rs @@ -15,11 +15,6 @@ impl Miniscript { /// Whether the miniscript is malleable pub fn is_non_malleable(&self) -> bool { self.ty.mall.non_malleable } - /// Whether the miniscript can exceed the resource limits(Opcodes, Stack limit etc) - // It maybe possible to return a detail error type containing why the miniscript - // failed. But doing so may require returning a collection of errors - pub fn within_resource_limits(&self) -> bool { Ctx::check_local_validity(self).is_ok() } - /// Whether the miniscript contains a combination of timelocks pub fn has_mixed_timelocks(&self) -> bool { self.ext.timelock_info.contains_unspendable_path() } diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index e0c889d0f..92eadb24e 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -2,162 +2,15 @@ // SPDX-License-Identifier: CC0-1.0 use core::{fmt, hash}; -#[cfg(feature = "std")] -use std::error; use bitcoin::hashes::{hash160, ripemd160, sha256}; -use bitcoin::Weight; use super::decode::ParseableKey; use crate::miniscript::limits::{ - MAX_OPS_PER_SCRIPT, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE, - MAX_STACK_SIZE, MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS, + MAX_OPS_PER_SCRIPT, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE, MAX_STACK_SIZE, + MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS, }; -use crate::prelude::*; -use crate::{hash256, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal, ValidationParams}; - -/// Error for Script Context -#[derive(Clone, PartialEq, Eq, Debug)] -pub enum ScriptContextError { - /// Script Context does not permit OrI for non-malleability - /// Legacy fragments allow non-minimal IF which results in malleability - MalleableOrI, - /// Script Context does not permit DupIf for non-malleability - /// Legacy fragments allow non-minimal IF which results in malleability - MalleableDupIf, - /// Only Compressed keys allowed under current descriptor - /// Segwitv0 fragments do not allow uncompressed pubkeys - CompressedOnly(String), - /// XOnly keys are only allowed in Tap context - /// The first element is key, and second element is current script context - XOnlyKeysNotAllowed(String, &'static str), - /// Tapscript descriptors cannot contain uncompressed keys - /// Tap context can contain compressed or xonly - UncompressedKeysNotAllowed, - /// At least one satisfaction path in the Miniscript fragment has more than - /// `MAX_STANDARD_P2WSH_STACK_ITEMS` (100) witness elements. - MaxWitnessItemsExceeded { actual: usize, limit: usize }, - /// At least one satisfaction path in the Miniscript fragment contains more - /// than `MAX_OPS_PER_SCRIPT`(201) opcodes. - MaxOpCountExceeded { actual: usize, limit: usize }, - /// The Miniscript(under segwit context) corresponding - /// Script would be larger than `MAX_STANDARD_P2WSH_SCRIPT_SIZE`, - /// `MAX_SCRIPT_SIZE` or `MAX_BLOCK`(`Tap`) bytes. - MaxWitnessScriptSizeExceeded { max: usize, got: usize }, - /// The Miniscript (under p2sh context) corresponding Script would be - /// larger than `MAX_SCRIPT_ELEMENT_SIZE` bytes. - MaxRedeemScriptSizeExceeded { max: usize, got: usize }, - /// The Miniscript(under bare context) corresponding - /// Script would be larger than `MAX_SCRIPT_SIZE` bytes. - MaxBareScriptSizeExceeded { max: usize, got: usize }, - /// The policy rules of bitcoin core only permit Script size upto 1650 bytes - MaxScriptSigSizeExceeded { actual: usize, limit: usize }, - /// Impossible to satisfy the miniscript under the current context - ImpossibleSatisfaction, - /// No Multi Node in Taproot context - TaprootMultiDisabled, - /// Stack size exceeded in script execution - StackSizeLimitExceeded { actual: usize, limit: usize }, - /// MultiA is only allowed in post tapscript - MultiANotAllowed, -} - -#[cfg(feature = "std")] -impl error::Error for ScriptContextError { - fn cause(&self) -> Option<&dyn error::Error> { - use self::ScriptContextError::*; - - match self { - MalleableOrI - | MalleableDupIf - | CompressedOnly(_) - | XOnlyKeysNotAllowed(_, _) - | UncompressedKeysNotAllowed - | MaxWitnessItemsExceeded { .. } - | MaxOpCountExceeded { .. } - | MaxWitnessScriptSizeExceeded { .. } - | MaxRedeemScriptSizeExceeded { .. } - | MaxBareScriptSizeExceeded { .. } - | MaxScriptSigSizeExceeded { .. } - | ImpossibleSatisfaction - | TaprootMultiDisabled - | StackSizeLimitExceeded { .. } - | MultiANotAllowed => None, - } - } -} - -impl fmt::Display for ScriptContextError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - Self::MalleableOrI => write!(f, "OrI is malleable under Legacy rules"), - Self::MalleableDupIf => { - write!(f, "DupIf is malleable under Legacy rules") - } - Self::CompressedOnly(ref pk) => { - write!(f, "Only Compressed pubkeys are allowed in segwit context. Found {}", pk) - } - Self::XOnlyKeysNotAllowed(ref pk, ref ctx) => { - write!(f, "x-only key {} not allowed in {}", pk, ctx) - } - Self::UncompressedKeysNotAllowed => { - write!(f, "uncompressed keys cannot be used in Taproot descriptors.") - } - Self::MaxWitnessItemsExceeded { actual, limit } => write!( - f, - "At least one satisfaction path in the Miniscript fragment has {} witness items \ - (limit: {}).", - actual, limit - ), - Self::MaxOpCountExceeded { actual, limit } => write!( - f, - "At least one satisfaction path in the Miniscript fragment contains {} opcodes \ - (limit: {}).", - actual, limit - ), - Self::MaxWitnessScriptSizeExceeded { max, got } => write!( - f, - "The Miniscript corresponding Script cannot be larger than \ - {} bytes, but got {} bytes.", - max, got - ), - Self::MaxRedeemScriptSizeExceeded { max, got } => write!( - f, - "The Miniscript corresponding Script cannot be larger than \ - {} bytes, but got {} bytes.", - max, got - ), - Self::MaxBareScriptSizeExceeded { max, got } => write!( - f, - "The Miniscript corresponding Script cannot be larger than \ - {} bytes, but got {} bytes.", - max, got - ), - Self::MaxScriptSigSizeExceeded { actual, limit } => write!( - f, - "At least one satisfaction path in the Miniscript fragment has {} bytes \ - (limit: {}).", - actual, limit - ), - Self::ImpossibleSatisfaction => { - write!(f, "Impossible to satisfy Miniscript under the current context") - } - Self::TaprootMultiDisabled => { - write!(f, "Invalid use of Multi node in taproot context") - } - Self::StackSizeLimitExceeded { actual, limit } => { - write!( - f, - "Stack limit {} can exceed the allowed limit {} in at least one script path during script execution", - actual, limit - ) - } - Self::MultiANotAllowed => { - write!(f, "Multi a(CHECKSIGADD) only allowed post tapscript") - } - } - } -} +use crate::{hash256, Miniscript, MiniscriptKey, ValidationParams}; /// The ScriptContext for Miniscript. /// @@ -187,152 +40,8 @@ where /// explicit choice of parameters is made. const SANE: ValidationParams; - /// Depending on ScriptContext, fragments can be malleable. For Example, - /// under Legacy context, PkH is malleable because it is possible to - /// estimate the cost of satisfaction because of compressed keys - /// This is currently only used in compiler code for removing malleable - /// compilations. - /// This does NOT recursively check if the children of the fragment are - /// valid or not. Since the compilation proceeds in a leaf to root fashion, - /// a recursive check is unnecessary. - fn check_terminal_non_malleable( - _frag: &Terminal, - ) -> Result<(), ScriptContextError>; - - /// Each context has slightly different rules on what Pks are allowed in descriptors - /// Legacy/Bare does not allow x_only keys - /// Segwit does not allow uncompressed keys and x_only keys - /// Tapscript does not allow uncompressed keys - fn check_pk(pk: &Pk) -> Result<(), ScriptContextError>; - /// Depending on script context, the size of a satifaction witness may slightly differ. fn max_satisfaction_size(ms: &Miniscript) -> Option; - /// Depending on script Context, some of the Terminals might not - /// be valid under the current consensus rules. - /// Or some of the script resource limits may have been exceeded. - /// These miniscripts would never be accepted by the Bitcoin network and hence - /// it is safe to discard them - /// For example, in Segwit Context with MiniscriptKey as bitcoin::PublicKey - /// uncompressed public keys are non-standard and thus invalid. - /// In LegacyP2SH context, scripts above 520 bytes are invalid. - /// Post Tapscript upgrade, this would have to consider other nodes. - /// This does *NOT* recursively check the miniscript fragments. - fn check_global_consensus_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - /// Depending on script Context, some of the script resource limits - /// may have been exceeded under the current bitcoin core policy rules - /// These miniscripts would never be accepted by the Bitcoin network and hence - /// it is safe to discard them. (unless explicitly disabled by non-standard flag) - /// For example, in Segwit Context with MiniscriptKey as bitcoin::PublicKey - /// scripts over 3600 bytes are invalid. - /// Post Tapscript upgrade, this would have to consider other nodes. - /// This does *NOT* recursively check the miniscript fragments. - fn check_global_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - /// Consensus rules at the Miniscript satisfaction time. - /// It is possible that some paths of miniscript may exceed resource limits - /// and our current satisfier and lifting analysis would not work correctly. - /// For example, satisfaction path(Legacy/Segwitv0) may require more than 201 opcodes. - fn check_local_consensus_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - /// Policy rules at the Miniscript satisfaction time. - /// It is possible that some paths of miniscript may exceed resource limits - /// and our current satisfier and lifting analysis would not work correctly. - /// For example, satisfaction path in Legacy context scriptSig more - /// than 1650 bytes - fn check_local_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - /// Check the consensus + policy(if not disabled) rules that are not based - /// satisfaction - fn check_global_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Self::check_global_consensus_validity(ms)?; - Self::check_global_policy_validity(ms)?; - Ok(()) - } - - /// Check the consensus + policy(if not disabled) rules including the - /// ones for satisfaction - fn check_local_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Self::check_global_consensus_validity(ms)?; - Self::check_global_policy_validity(ms)?; - Self::check_local_consensus_validity(ms)?; - Self::check_local_policy_validity(ms)?; - Ok(()) - } - - /// Check whether the top-level is type B - fn top_level_type_check(ms: &Miniscript) -> Result<(), Error> { - // (Ab)use `for_each_key` to record the number of derivation paths a multipath key has. - #[derive(PartialEq)] - enum MultipathLenChecker { - SinglePath, - MultipathLen(usize), - LenMismatch, - } - - let mut checker = MultipathLenChecker::SinglePath; - ms.for_each_key(|key| { - match key.num_der_paths() { - 0 | 1 => {} - n => match checker { - MultipathLenChecker::SinglePath => { - checker = MultipathLenChecker::MultipathLen(n); - } - MultipathLenChecker::MultipathLen(len) => { - if len != n { - checker = MultipathLenChecker::LenMismatch; - } - } - MultipathLenChecker::LenMismatch => {} - }, - } - true - }); - - if checker == MultipathLenChecker::LenMismatch { - return Err(Error::MultipathDescLenMismatch); - } - Ok(()) - } - - /// Other top level checks that are context specific - fn other_top_level_checks(_ms: &Miniscript) -> Result<(), Error> { - Ok(()) - } - - /// Check top level consensus rules. - // All the previous check_ were applied at each fragment while parsing script - // Because if any of sub-miniscripts failed the resource level check, the entire - // miniscript would also be invalid. However, there are certain checks like - // in Bare context, only c:pk(key) (P2PK), - // c:pk_h(key) (P2PKH), and thresh_m(k,...) up to n=3 are allowed - // that are only applicable at the top-level - // We can also combine the top-level check for Base::B here - // even though it does not depend on context, but helps in cleaner code - fn top_level_checks(ms: &Miniscript) -> Result<(), Error> { - Self::top_level_type_check(ms)?; - Self::other_top_level_checks(ms) - } /// The type of signature required for satisfaction // We need to context decide whether the serialize pk to 33 byte or 32 bytes. @@ -381,91 +90,6 @@ impl ScriptContext for Legacy { }; const SANE: ValidationParams = Self::CONSENSUS.intersect(&ValidationParams::SANE); - fn check_terminal_non_malleable( - frag: &Terminal, - ) -> Result<(), ScriptContextError> { - match *frag { - Terminal::OrI(ref _a, ref _b) => Err(ScriptContextError::MalleableOrI), - Terminal::DupIf(ref _ms) => Err(ScriptContextError::MalleableDupIf), - _ => Ok(()), - } - } - - // Only compressed and uncompressed public keys are allowed in Legacy context - fn check_pk(pk: &Pk) -> Result<(), ScriptContextError> { - if pk.is_x_only_key() { - Err(ScriptContextError::XOnlyKeysNotAllowed(pk.to_string(), Self::name_str())) - } else { - Ok(()) - } - } - - fn check_global_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // 1. Check the node first, throw an error on the language itself - let node_checked = match ms.node { - Terminal::PkK(ref pk) => Self::check_pk(pk), - Terminal::Multi(ref thresh) | Terminal::SortedMulti(ref thresh) => { - for pk in thresh.iter() { - Self::check_pk(pk)?; - } - Ok(()) - } - Terminal::MultiA(..) | Terminal::SortedMultiA(..) => { - Err(ScriptContextError::MultiANotAllowed) - } - _ => Ok(()), - }; - // 2. After fragment and param check, validate the script size finally - match node_checked { - Ok(_) => { - if ms.ext.pk_cost > MAX_SCRIPT_ELEMENT_SIZE { - Err(ScriptContextError::MaxRedeemScriptSizeExceeded { - max: MAX_SCRIPT_ELEMENT_SIZE, - got: ms.ext.pk_cost, - }) - } else { - Ok(()) - } - } - Err(_) => node_checked, - } - } - - fn check_local_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - match ms.ext.sat_op_count() { - None => Err(ScriptContextError::ImpossibleSatisfaction), - Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => { - Err(ScriptContextError::MaxOpCountExceeded { - actual: op_count, - limit: MAX_OPS_PER_SCRIPT, - }) - } - _ => Ok(()), - } - } - - fn check_local_policy_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // Legacy scripts permit upto 1000 stack elements, 520 bytes consensus limits - // on P2SH size, it is not possible to reach the 1000 elements limit and hence - // we do not check it. - match ms.max_satisfaction_size() { - Err(_e) => Err(ScriptContextError::ImpossibleSatisfaction), - Ok(size) if size > MAX_SCRIPTSIG_SIZE => { - Err(ScriptContextError::MaxScriptSigSizeExceeded { - actual: size, - limit: MAX_SCRIPTSIG_SIZE, - }) - } - _ => Ok(()), - } - } - fn max_satisfaction_size(ms: &Miniscript) -> Option { ms.ext.sat_data.map(|data| data.max_script_sig_size) } @@ -505,102 +129,6 @@ impl ScriptContext for Segwitv0 { ..Self::CONSENSUS.intersect(&ValidationParams::SANE) }; - fn check_terminal_non_malleable( - _frag: &Terminal, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - // No x-only keys or uncompressed keys in Segwitv0 context - fn check_pk(pk: &Pk) -> Result<(), ScriptContextError> { - if pk.is_uncompressed() { - Err(ScriptContextError::UncompressedKeysNotAllowed) - } else if pk.is_x_only_key() { - Err(ScriptContextError::XOnlyKeysNotAllowed(pk.to_string(), Self::name_str())) - } else { - Ok(()) - } - } - - fn check_global_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // 1. Check the node first, throw an error on the language itself - let node_checked = match ms.node { - Terminal::PkK(ref pk) => Self::check_pk(pk), - Terminal::Multi(ref thresh) | Terminal::SortedMulti(ref thresh) => { - for pk in thresh.iter() { - Self::check_pk(pk)?; - } - Ok(()) - } - Terminal::MultiA(..) | Terminal::SortedMultiA(..) => { - Err(ScriptContextError::MultiANotAllowed) - } - _ => Ok(()), - }; - // 2. After fragment and param check, validate the script size finally - match node_checked { - Ok(_) => { - if ms.ext.pk_cost > MAX_SCRIPT_SIZE { - Err(ScriptContextError::MaxWitnessScriptSizeExceeded { - max: MAX_SCRIPT_SIZE, - got: ms.ext.pk_cost, - }) - } else { - Ok(()) - } - } - Err(_) => node_checked, - } - } - - fn check_local_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - match ms.ext.sat_op_count() { - None => Err(ScriptContextError::ImpossibleSatisfaction), - Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => { - Err(ScriptContextError::MaxOpCountExceeded { - actual: op_count, - limit: MAX_OPS_PER_SCRIPT, - }) - } - _ => Ok(()), - } - } - - fn check_global_policy_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - if ms.ext.pk_cost > MAX_STANDARD_P2WSH_SCRIPT_SIZE { - return Err(ScriptContextError::MaxWitnessScriptSizeExceeded { - max: MAX_STANDARD_P2WSH_SCRIPT_SIZE, - got: ms.ext.pk_cost, - }); - } - Ok(()) - } - - fn check_local_policy_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // We don't need to know if this is actually a p2wsh as the standard satisfaction for - // other Segwitv0 defined programs all require (much) less than 100 elements. - // The witness script item is accounted for in max_satisfaction_witness_elements(). - match ms.max_satisfaction_witness_elements() { - // No possible satisfactions - Err(_e) => Err(ScriptContextError::ImpossibleSatisfaction), - Ok(max_witness_items) if max_witness_items > MAX_STANDARD_P2WSH_STACK_ITEMS => { - Err(ScriptContextError::MaxWitnessItemsExceeded { - actual: max_witness_items, - limit: MAX_STANDARD_P2WSH_STACK_ITEMS, - }) - } - _ => Ok(()), - } - } - fn max_satisfaction_size(ms: &Miniscript) -> Option { ms.ext.sat_data.map(|data| data.max_witness_stack_size) } @@ -634,97 +162,6 @@ impl ScriptContext for Tap { ..Self::CONSENSUS.intersect(&ValidationParams::SANE) }; - fn check_terminal_non_malleable( - _frag: &Terminal, - ) -> Result<(), ScriptContextError> { - // No fragment is malleable in tapscript context. - // Certain fragments like Multi are invalid, but are not malleable - Ok(()) - } - - // No uncompressed keys in Tap context - fn check_pk(pk: &Pk) -> Result<(), ScriptContextError> { - if pk.is_uncompressed() { - Err(ScriptContextError::UncompressedKeysNotAllowed) - } else { - Ok(()) - } - } - - fn check_global_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // 1. Check the node first, throw an error on the language itself - let node_checked = match ms.node { - Terminal::PkK(ref pk) => Self::check_pk(pk), - Terminal::MultiA(ref thresh) | Terminal::SortedMultiA(ref thresh) => { - for pk in thresh.iter() { - Self::check_pk(pk)?; - } - Ok(()) - } - Terminal::Multi(..) | Terminal::SortedMulti(..) => { - Err(ScriptContextError::TaprootMultiDisabled) - } - _ => Ok(()), - }; - // 2. After fragment and param check, validate the script size finally - match node_checked { - Ok(_) => { - // No script size checks for global consensus rules - // Should we really check for block limits here. - // When the transaction sizes get close to block limits, - // some guarantees are not easy to satisfy because of knapsack - // constraints - if ms.ext.pk_cost as u64 > Weight::MAX_BLOCK.to_wu() { - Err(ScriptContextError::MaxWitnessScriptSizeExceeded { - max: Weight::MAX_BLOCK.to_wu() as usize, - got: ms.ext.pk_cost, - }) - } else { - Ok(()) - } - } - Err(_) => node_checked, - } - } - - fn check_local_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // Taproot introduces the concept of sigops budget. - // All valid miniscripts satisfy the sigops constraint - // Whenever we add new fragment that uses pk(pk() or multi based on checksigadd) - // miniscript typing rules ensure that pk when executed successfully has it's - // own unique signature. That is, there is no way to re-use signatures from one CHECKSIG - // to another checksig. In other words, for each successfully executed checksig - // will have it's corresponding 64 bytes signature. - // sigops budget = witness_script.len() + witness.size() + 50 - // Each signature will cover it's own cost(64 > 50) and thus will will never exceed the budget - if let Some(data) = ms.ext.sat_data { - if data.max_witness_stack_count + data.max_exec_stack_count > MAX_STACK_SIZE { - return Err(ScriptContextError::StackSizeLimitExceeded { - actual: data.max_witness_stack_count + data.max_exec_stack_count, - limit: MAX_STACK_SIZE, - }); - } - } - Ok(()) - } - - fn check_global_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // No script rules, rules are subject to entire tx rules - Ok(()) - } - - fn check_local_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - fn max_satisfaction_size(ms: &Miniscript) -> Option { ms.ext.sat_data.map(|data| data.max_witness_stack_size) } @@ -759,87 +196,6 @@ impl ScriptContext for BareCtx { }; const SANE: ValidationParams = Self::CONSENSUS.intersect(&ValidationParams::SANE); - fn check_terminal_non_malleable( - _frag: &Terminal, - ) -> Result<(), ScriptContextError> { - // Bare fragments can't contain miniscript because of standardness rules - // This function is only used in compiler which already checks the standardness - // and consensus rules, and because of the limited allowance of bare scripts - // we need check for malleable scripts - Ok(()) - } - - // No x-only keys in Bare context - fn check_pk(pk: &Pk) -> Result<(), ScriptContextError> { - if pk.is_x_only_key() { - Err(ScriptContextError::XOnlyKeysNotAllowed(pk.to_string(), Self::name_str())) - } else { - Ok(()) - } - } - - fn check_global_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - // 1. Check the node first, throw an error on the language itself - let node_checked = match ms.node { - Terminal::PkK(ref key) => Self::check_pk(key), - Terminal::Multi(ref thresh) | Terminal::SortedMulti(ref thresh) => { - for pk in thresh.iter() { - Self::check_pk(pk)?; - } - Ok(()) - } - Terminal::MultiA(..) | Terminal::SortedMultiA(..) => { - Err(ScriptContextError::MultiANotAllowed) - } - _ => Ok(()), - }; - // 2. After fragment and param check, validate the script size finally - match node_checked { - Ok(_) => { - if ms.ext.pk_cost > MAX_SCRIPT_SIZE { - Err(ScriptContextError::MaxBareScriptSizeExceeded { - max: MAX_SCRIPT_SIZE, - got: ms.ext.pk_cost, - }) - } else { - Ok(()) - } - } - Err(_) => node_checked, - } - } - - fn check_local_consensus_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - match ms.ext.sat_op_count() { - None => Err(ScriptContextError::ImpossibleSatisfaction), - Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => { - Err(ScriptContextError::MaxOpCountExceeded { - actual: op_count, - limit: MAX_OPS_PER_SCRIPT, - }) - } - _ => Ok(()), - } - } - - fn other_top_level_checks(ms: &Miniscript) -> Result<(), Error> { - match &ms.node { - Terminal::Check(ref ms) => match &ms.node { - Terminal::RawPkH(_pkh) => Ok(()), - Terminal::PkK(_pk) | Terminal::PkH(_pk) => Ok(()), - _ => Err(Error::NonStandardBareScript), - }, - Terminal::Multi(ref thresh) | Terminal::SortedMulti(ref thresh) if thresh.n() <= 3 => { - Ok(()) - } - _ => Err(Error::NonStandardBareScript), - } - } - fn max_satisfaction_size(ms: &Miniscript) -> Option { // For bare outputs the script appears in the scriptpubkey; its cost // is the same as for a legacy scriptsig. @@ -873,39 +229,6 @@ impl ScriptContext for NoChecks { const CONSENSUS: ValidationParams = ValidationParams::MAX; const SANE: ValidationParams = ValidationParams::MAX; - fn check_terminal_non_malleable( - _frag: &Terminal, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - // No checks in NoChecks - fn check_pk(_pk: &Pk) -> Result<(), ScriptContextError> { Ok(()) } - - fn check_global_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - fn check_global_consensus_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - fn check_local_policy_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - - fn check_local_consensus_validity( - _ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Ok(()) - } - fn max_satisfaction_size(_ms: &Miniscript) -> Option { panic!("Tried to compute a satisfaction size bound on a no-checks ecdsa miniscript") } @@ -919,37 +242,6 @@ impl ScriptContext for NoChecks { "NochecksEcdsa" } - fn check_global_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Self::check_global_consensus_validity(ms)?; - Self::check_global_policy_validity(ms)?; - Ok(()) - } - - fn check_local_validity( - ms: &Miniscript, - ) -> Result<(), ScriptContextError> { - Self::check_global_consensus_validity(ms)?; - Self::check_global_policy_validity(ms)?; - Self::check_local_consensus_validity(ms)?; - Self::check_local_policy_validity(ms)?; - Ok(()) - } - - fn top_level_type_check(_: &Miniscript) -> Result<(), Error> { - Ok(()) - } - - fn other_top_level_checks(_ms: &Miniscript) -> Result<(), Error> { - Ok(()) - } - - fn top_level_checks(ms: &Miniscript) -> Result<(), Error> { - Self::top_level_type_check(ms)?; - Self::other_top_level_checks(ms) - } - fn sig_type() -> SigType { SigType::Ecdsa } } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 74883e7bd..cd2fc5e0b 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -23,7 +23,7 @@ use crate::iter::TreeLike; use crate::prelude::*; use crate::{script_num_size, TranslateErr}; -pub mod analyzable; +mod analyzable; pub mod astelem; pub(crate) mod context; pub mod decode; @@ -337,7 +337,7 @@ mod private { if (res.ext.tree_height as u32) > MAX_RECURSION_DEPTH { return Err(Error::MaxRecursiveDepthExceeded); } - Ctx::check_global_validity(&res)?; + Ok(res) } @@ -366,6 +366,7 @@ mod private { // Malleability is only a top-level check since you can fix malleability // in some cases by adding wrappers. if !params.allow_malleability && !self.is_non_malleable() { + println!("{:?}", self); return Err(ValidationError::Malleable); } @@ -744,7 +745,6 @@ impl Miniscript { let mut iter = TokenIter::new(tokens); let top = decode::decode(&mut iter)?; - Ctx::check_global_validity(&top)?; types::Type::type_check(&top.node)?; if let Some(leading) = iter.next() { Err(Error::Trailing(leading.to_string())) @@ -937,7 +937,11 @@ impl Miniscript { translated.push(Arc::new(new_ms)); } - Ok(Arc::try_unwrap(translated.pop().unwrap()).unwrap()) + let ret = translated.pop().unwrap(); + ret.validate(&CtxQ::SANE) + .map_err(Error::Validation) + .map_err(TranslateErr::OuterError)?; + Ok(Arc::try_unwrap(ret).unwrap()) } /// Substitutes raw public keys hashes with the public keys as provided by map. @@ -1214,13 +1218,6 @@ impl FromTree for Miniscript { assert_eq!(stack.len(), 1); let ret = stack.pop().unwrap(); - // Iterate through every node to check global validity. It is definitely not sufficient - // to check only the root, since this will fail to notice illegal xonly keys at the - // leaves. But probably checking every single node is overkill. This may be worth - // optimizing. - for node in ret.pre_order_iter() { - Ctx::check_global_validity(node)?; - } Ok(Arc::try_unwrap(ret).unwrap()) } } @@ -1801,11 +1798,11 @@ mod tests { let segwit_sortedmulti_a_ms = Segwitv0Ms::from_str_insane("sortedmulti_a(1,A,B,C)"); assert_eq!( segwit_multi_a_ms.unwrap_err().to_string(), - "Multi a(CHECKSIGADD) only allowed post tapscript" + "non-Taproot script with a `multi_a` fragment", ); assert_eq!( segwit_sortedmulti_a_ms.unwrap_err().to_string(), - "Multi a(CHECKSIGADD) only allowed post tapscript" + "non-Taproot script with a `multi_a` fragment" ); let tap_multi_a_ms = TapMs::from_str_insane("multi_a(1,A,B,C)").unwrap(); let tap_sortedmulti_a_ms = TapMs::from_str_insane("sortedmulti_a(1,A,B,C)").unwrap(); @@ -1937,10 +1934,8 @@ mod tests { // ...though you can parse one with from_str_insane let ok_insane = Miniscript::::from_str_insane("and_v(v:pk(A),pk(A))").unwrap(); - // ...it can be lifted, though it's unclear whether this is a deliberate - // choice or just an accident. It seems weird given that duplicate public - // keys are forbidden in several other places. - ok_insane.lift().unwrap(); + // ...but you cannot then lift it. + ok_insane.lift().unwrap_err(); } #[test] @@ -1965,10 +1960,7 @@ mod tests { ) .unwrap(); // ...but this cannot lifted - assert!(matches!( - ok_insane.lift().unwrap_err(), - Error::LiftError(crate::policy::LiftError::HeightTimelockCombination) - )); + assert!(matches!(ok_insane.lift().unwrap_err(), ValidationError::MixedTimeLocks,)); // nor can it have sane rules applied to it assert_eq!( ok_insane.validate(&ValidationParams::SANE).unwrap_err(), @@ -2154,13 +2146,15 @@ mod tests { type BareMs = Miniscript; // multisig script of 20 pubkeys exceeds 520 bytes - let pubkey_vec_20: Vec = (0..20).map(|x| x.to_string()).collect(); + let pubkey_vec_20: Vec> = (0..15) + .map(|n| (n * 20..(n + 1) * 20).map(|x| x.to_string()).collect()) + .collect(); // multisig script of 300 pubkeys exceeds 10,000 bytes let pubkey_vec_300: Vec = (0..300).map(|x| x.to_string()).collect(); // wrong multi_a for non-tapscript, while exceeding consensus size limit let legacy_multi_a_ms = - LegacyMs::from_str(&format!("multi_a(20,{})", pubkey_vec_20.join(","))); + LegacyMs::from_str(&format!("multi_a(20,{})", pubkey_vec_20[0].join(","))); let segwit_multi_a_ms = Segwitv0Ms::from_str(&format!("multi_a(300,{})", pubkey_vec_300.join(","))); let bare_multi_a_ms = @@ -2169,41 +2163,45 @@ mod tests { // Should panic for wrong multi_a, even if it exceeds the max consensus size assert_eq!( legacy_multi_a_ms.unwrap_err().to_string(), - "Multi a(CHECKSIGADD) only allowed post tapscript" + "non-Taproot script with a `multi_a` fragment", ); assert_eq!( segwit_multi_a_ms.unwrap_err().to_string(), - "Multi a(CHECKSIGADD) only allowed post tapscript" + "non-Taproot script with a `multi_a` fragment", ); assert_eq!( bare_multi_a_ms.unwrap_err().to_string(), - "Multi a(CHECKSIGADD) only allowed post tapscript" + "non-Taproot script with a `multi_a` fragment", ); // multisig script of 20 pubkeys exceeds 520 bytes - let multi_ms = format!("multi(20,{})", pubkey_vec_20.join(",")); + let multi_ms: Vec = pubkey_vec_20 + .iter() + .map(|vec_20| format!("multi(20,{})", vec_20.join(","))) + .collect(); // other than legacy, and_v to build 15 nested 20-of-20 multisig script // to exceed 10,000 bytes without violation of threshold limit(max: 20) let and_v_nested_multi_ms = - format!("and_v(v:{},", multi_ms).repeat(14) + &multi_ms + "))))))))))))))"; + format!("and_v(v:{},{}", multi_ms[..14].join(",and_v(v:"), multi_ms[14]) + + "))))))))))))))"; // correct multi for non-tapscript, but exceeding consensus size limit - let legacy_multi_ms = LegacyMs::from_str(&multi_ms); + let legacy_multi_ms = LegacyMs::from_str(&multi_ms[0]); let segwit_multi_ms = Segwitv0Ms::from_str(&and_v_nested_multi_ms); let bare_multi_ms = BareMs::from_str(&and_v_nested_multi_ms); // Should panic for exceeding the max consensus size, as multi properly used assert_eq!( legacy_multi_ms.unwrap_err().to_string(), - "The Miniscript corresponding Script cannot be larger than 520 bytes, but got 685 bytes." + "script has size at least 685 (limit: 520).", ); assert_eq!( segwit_multi_ms.unwrap_err().to_string(), - "The Miniscript corresponding Script cannot be larger than 3600 bytes, but got 4110 bytes." + "script has size at least 10275 (limit: 3600).", ); assert_eq!( bare_multi_ms.unwrap_err().to_string(), - "The Miniscript corresponding Script cannot be larger than 10000 bytes, but got 10275 bytes." + "script has size at least 10275 (limit: 10000).", ); } } diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 9ad89c349..15e729d17 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -73,7 +73,7 @@ impl fmt::Display for Error { match self.error { ErrorKind::NonZeroDupIf => write!( f, - "fragment «{}» represents needs to be `z`, needs to consume zero elements from the stack", + "fragment «{}» needs to be `z` (consumes zero stack elements) to use `d`", self.fragment_string, ), ErrorKind::LeftNotDissatisfiable => write!( diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index 4ffadb804..0a7d02479 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -16,7 +16,7 @@ use crate::miniscript::types::{self, ErrorKind, ExtData, Type}; use crate::miniscript::ScriptContext; use crate::policy::Concrete; use crate::prelude::*; -use crate::{policy, Miniscript, MiniscriptKey, Terminal}; +use crate::{Miniscript, MiniscriptKey, Terminal}; type PolicyCache = BTreeMap<(Concrete, OrdF64, Option), BTreeMap>>; @@ -39,7 +39,7 @@ impl Ord for OrdF64 { } /// Detailed error type for compiler. -#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] +#[derive(Clone, PartialEq, Eq, Debug)] pub enum CompilerError { /// `And` fragments only support two args. NonBinaryArgAnd, @@ -69,8 +69,6 @@ pub enum CompilerError { /// Index of the leaf that contains branching fragments. leaf_index: usize, }, - ///Policy related errors - PolicyError(policy::concrete::PolicyError), } impl fmt::Display for CompilerError { @@ -99,7 +97,6 @@ impl fmt::Display for CompilerError { leaf_index ) } - Self::PolicyError(ref e) => fmt::Display::fmt(e, f), } } } @@ -118,16 +115,10 @@ impl error::Error for CompilerError { | NoInternalKey | TooManyTapleaves { .. } | IfFragmentInNativeLeaf { .. } => None, - PolicyError(e) => Some(e), } } } -#[doc(hidden)] -impl From for CompilerError { - fn from(e: policy::concrete::PolicyError) -> Self { Self::PolicyError(e) } -} - /// Hash required for using OrdF64 as key for hashmap impl hash::Hash for OrdF64 { fn hash(&self, state: &mut H) { self.0.to_bits().hash(state); } @@ -671,14 +662,16 @@ fn insert_elem( sat_prob: f64, dissat_prob: Option, ) -> bool { - // We check before compiling that non-malleable satisfactions exist, and it appears that - // there are no cases when malleable satisfactions beat non-malleable ones (and if there - // are, we don't want to use them). Anyway, detect these and early return. - if !elem.ms.ty.mall.non_malleable { - return false; - } - - if Ctx::check_local_validity(&elem.ms).is_err() { + // Disable checks which were done on the policy before compilation. + if elem + .ms + .validate_non_top_level(&crate::ValidationParams { + allow_duplicate_keys: true, + allow_mixed_time_locks: true, + ..Ctx::SANE + }) + .is_err() + { return false; } @@ -1220,8 +1213,8 @@ mod tests { use super::*; use crate::miniscript::{Legacy, Segwitv0, Tap}; - use crate::policy::Liftable; - use crate::{script_num_size, AbsLockTime, RelLockTime, Threshold, ToPublicKey}; + use crate::policy::{self, Liftable}; + use crate::{script_num_size, RelLockTime, Threshold, ToPublicKey}; type SPolicy = Concrete; type BPolicy = Concrete; @@ -1263,16 +1256,6 @@ mod tests { #[test] fn compile_timelocks() { - // artificially create a policy that is problematic and try to compile - let pol: SPolicy = Concrete::And(vec![ - Arc::new(Concrete::Key("A".to_string())), - Arc::new(Concrete::And(vec![ - Arc::new(Concrete::After(AbsLockTime::from_consensus(9).unwrap())), - Arc::new(Concrete::After(AbsLockTime::from_consensus(1_000_000_000).unwrap())), - ])), - ]); - assert!(pol.compile::().is_err()); - // This should compile let pol: SPolicy = SPolicy::from_str("and(pk(A),or(and(after(9),pk(B)),and(after(1000000000),pk(C))))") @@ -1316,7 +1299,7 @@ mod tests { // compile into taproot context to avoid limit errors let policy = SPolicy::from_str( - "and(and(and(or(127@thresh(2,pk(A),pk(B),thresh(2,or(127@pk(A),1@pk(B)),after(100),or(and(pk(C),after(200)),and(pk(D),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925))),pk(E))),1@pk(F)),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925)),or(127@pk(G),1@after(300))),or(127@after(400),pk(H)))" + "and(and(and(or(127@thresh(2,pk(A1),pk(B1),thresh(2,or(127@pk(A),1@pk(B)),after(100),or(and(pk(C),after(200)),and(pk(D),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925))),pk(E))),1@pk(F)),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925)),or(127@pk(G),1@after(300))),or(127@after(400),pk(H)))" ).expect("parsing"); let compilation: TapAstElemExt = best_t(&mut BTreeMap::new(), &policy, 1.0, None).unwrap(); @@ -1608,22 +1591,6 @@ mod tests { "Compilation succeeded with '{:?}' OP count (sat)", ops_count, ); - - // Test that we refuse to compile policies with duplicated keys - let (keys, _) = pubkeys_and_a_sig(1); - let key = Arc::new(Concrete::Key(keys[0])); - let res = - Concrete::Or(vec![(1, Arc::clone(&key)), (1, Arc::clone(&key))]).compile::(); - assert_eq!( - res, - Err(CompilerError::PolicyError(policy::concrete::PolicyError::DuplicatePubKeys)) - ); - // Same for legacy - let res = Concrete::Or(vec![(1, key.clone()), (1, key)]).compile::(); - assert_eq!( - res, - Err(CompilerError::PolicyError(policy::concrete::PolicyError::DuplicatePubKeys)) - ); } #[test] diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 3e2220df5..b99169b82 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -4,8 +4,6 @@ //! use core::{cmp, fmt, str}; -#[cfg(feature = "std")] -use std::error; use bitcoin::absolute; #[cfg(feature = "compiler")] @@ -29,6 +27,7 @@ use crate::sync::Arc; use crate::Descriptor; use crate::{ AbsLockTime, Error, ForEachKey, FromStrKey, MiniscriptKey, RelLockTime, Threshold, Translator, + ValidationError, ValidationParams, }; /// Maximum `TapLeaf`s allowed in a compiled TapTree @@ -117,15 +116,6 @@ impl Ord for Policy { } } -/// Detailed error type for concrete policies. -#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] -pub enum PolicyError { - /// Cannot lift policies that have a combination of height and timelocks. - HeightTimelockCombination, - /// Duplicate Public Keys. - DuplicatePubKeys, -} - /// Descriptor context for [`Policy`] compilation into a [`Descriptor`]. pub enum DescriptorCtx { /// See docs for [`Descriptor::Bare`]. @@ -141,28 +131,6 @@ pub enum DescriptorCtx { Tr(Option), } -impl fmt::Display for PolicyError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - Self::HeightTimelockCombination => { - f.write_str("Cannot lift policies that have a heightlock and timelock combination") - } - Self::DuplicatePubKeys => f.write_str("Policy contains duplicate keys"), - } - } -} - -#[cfg(feature = "std")] -impl error::Error for PolicyError { - fn cause(&self) -> Option<&dyn error::Error> { - use self::PolicyError::*; - - match self { - HeightTimelockCombination | DuplicatePubKeys => None, - } - } -} - #[cfg(feature = "compiler")] struct TapleafProbabilityIter<'p, Pk: MiniscriptKey> { stack: Vec<(f64, &'p Policy)>, @@ -278,7 +246,6 @@ impl Policy { // TODO: We might require other compile errors for Taproot. #[cfg(feature = "compiler")] pub fn compile_tr(&self, unspendable_key: Option) -> Result, CompilerError> { - self.is_valid().map_err(CompilerError::PolicyError)?; self.check_binary_ops()?; match self.is_safe_nonmalleable() { (false, _) => Err(CompilerError::TopLevelSigless), @@ -334,7 +301,6 @@ impl Policy { return Err(CompilerError::TooManyTapleaves { n: 1, max: 0 }); } let max_leaves = max_leaves.min(MAX_COMPILATION_LEAVES); - self.is_valid().map_err(CompilerError::PolicyError)?; self.check_binary_ops()?; match self.is_safe_nonmalleable() { (false, _) => Err(CompilerError::TopLevelSigless), @@ -400,7 +366,6 @@ impl Policy { &self, unspendable_key: Option, ) -> Result, Error> { - self.is_valid().map_err(Error::ConcretePolicy)?; self.check_binary_ops()?; match self.is_safe_nonmalleable() { (false, _) => Err(Error::from(CompilerError::TopLevelSigless)), @@ -433,7 +398,8 @@ impl Policy { } } }, - )?; + ) + .map_err(Error::Validation)?; Ok(tree) } } @@ -454,16 +420,20 @@ impl Policy { &self, desc_ctx: DescriptorCtx, ) -> Result, Error> { - self.is_valid().map_err(Error::ConcretePolicy)?; self.check_binary_ops()?; match self.is_safe_nonmalleable() { (false, _) => Err(Error::from(CompilerError::TopLevelSigless)), (_, false) => Err(Error::from(CompilerError::ImpossibleNonMalleableCompilation)), _ => match desc_ctx { - DescriptorCtx::Bare => Descriptor::new_bare(compiler::best_compilation(self)?), - DescriptorCtx::Sh => Descriptor::new_sh(compiler::best_compilation(self)?), - DescriptorCtx::Wsh => Descriptor::new_wsh(compiler::best_compilation(self)?), - DescriptorCtx::ShWsh => Descriptor::new_sh_wsh(compiler::best_compilation(self)?), + DescriptorCtx::Bare => Descriptor::new_bare(compiler::best_compilation(self)?) + .map_err(Error::Validation), + DescriptorCtx::Sh => { + Descriptor::new_sh(compiler::best_compilation(self)?).map_err(Error::Validation) + } + DescriptorCtx::Wsh => Descriptor::new_wsh(compiler::best_compilation(self)?) + .map_err(Error::Validation), + DescriptorCtx::ShWsh => Descriptor::new_sh_wsh(compiler::best_compilation(self)?) + .map_err(Error::Validation), DescriptorCtx::Tr(unspendable_key) => self .compile_tr(unspendable_key) .map_err(Error::CompilerError), @@ -480,7 +450,6 @@ impl Policy { /// the compiler document in doc/compiler.md for more details. #[cfg(feature = "compiler")] pub fn compile(&self) -> Result, CompilerError> { - self.is_valid()?; self.check_binary_ops()?; match self.is_safe_nonmalleable() { (false, _) => Err(CompilerError::TopLevelSigless), @@ -786,33 +755,29 @@ impl Policy { Ok(()) } - /// Checks whether the policy contains duplicate public keys. - pub fn check_duplicate_keys(&self) -> Result<(), PolicyError> { - let pks = self.keys(); - let pks_len = pks.len(); - let unique_pks_len = pks.into_iter().collect::>().len(); - - if pks_len > unique_pks_len { - Err(PolicyError::DuplicatePubKeys) - } else { - Ok(()) + /// Validates the concrete policy against a set of validation parameters. + /// + /// Because policies do not correspond directly to on-chain objects, none of the consensus + /// limits are checked. + pub fn validate(&self, params: &ValidationParams) -> Result<(), ValidationError> { + if !params.allow_duplicate_keys { + let pks = self.keys(); + let pks_len = pks.len(); + let unique_pks_len = pks.into_iter().collect::>().len(); + + if pks_len > unique_pks_len { + return Err(ValidationError::DuplicateKeys); + } } - } - /// Checks whether the given concrete policy contains a combination of - /// timelocks and heightlocks. - /// - /// # Returns - /// - /// Returns an error if there is at least one satisfaction that contains - /// a combination of heightlock and timelock. - pub fn check_timelocks(&self) -> Result<(), PolicyError> { - let aggregated_timelock_info = self.timelock_info(); - if aggregated_timelock_info.contains_combination { - Err(PolicyError::HeightTimelockCombination) - } else { - Ok(()) + if !params.allow_mixed_time_locks { + let aggregated_timelock_info = self.timelock_info(); + if aggregated_timelock_info.contains_combination { + return Err(ValidationError::MixedTimeLocks); + } } + + Ok(()) } /// Processes `Policy` using `post_order_iter`, creates a `TimelockInfo` for each `Nullary` node @@ -861,16 +826,6 @@ impl Policy { infos.pop().unwrap() } - /// This returns whether the given policy is valid or not. It maybe possible that the policy - /// contains Non-two argument `and`, `or` or a `0` arg thresh. - /// Validity condition also checks whether there is a possible satisfaction - /// combination of timelocks and heightlocks - pub fn is_valid(&self) -> Result<(), PolicyError> { - self.check_timelocks()?; - self.check_duplicate_keys()?; - Ok(()) - } - /// Checks if any possible compilation of the policy could be compiled /// as non-malleable and requiring signatures. /// @@ -1000,7 +955,9 @@ impl str::FromStr for Policy { fn from_str(s: &str) -> Result { let tree = expression::Tree::from_str(s)?; let policy: Self = FromTree::from_tree(tree.root())?; - policy.check_timelocks().map_err(Error::ConcretePolicy)?; + policy + .validate(&ValidationParams::SANE) + .map_err(Error::Validation)?; Ok(policy) } } @@ -1430,6 +1387,17 @@ mod tests { let _ = Policy::::from_str("and(after(10),after(500000000))").unwrap(); } + #[test] + fn parse_validation_error() { + let res = "or(pk(A),pk(A))".parse::>(); + assert!(matches!(res.unwrap_err(), Error::Validation(ValidationError::DuplicateKeys))); + let res = "and(after(100),after(1000000000))".parse::>(); + assert!(matches!(res.unwrap_err(), Error::Validation(ValidationError::MixedTimeLocks))); + // Same policy with `or` is not "mixing timelocks" because the timelocks are exclusive. + let res = "or(after(100),after(1000000000))".parse::>(); + assert!(res.is_ok()); + } + #[test] fn parse_zero_disjunction() { "or(0@pk(09),0@TRIVIAL)" diff --git a/src/policy/mod.rs b/src/policy/mod.rs index 068bddc4e..cebd3e8b5 100644 --- a/src/policy/mod.rs +++ b/src/policy/mod.rs @@ -9,9 +9,6 @@ //! The format represents EC public keys abstractly to allow wallets to replace //! these with BIP32 paths, pay-to-contract instructions, etc. //! -use core::fmt; -#[cfg(feature = "std")] -use std::error; #[cfg(feature = "compiler")] pub mod compiler; @@ -26,7 +23,7 @@ use crate::miniscript::{Miniscript, ScriptContext}; use crate::sync::Arc; #[cfg(all(not(feature = "std"), not(test)))] use crate::Vec; -use crate::{Error, MiniscriptKey, Terminal, Threshold}; +use crate::{MiniscriptKey, Terminal, Threshold, ValidationError, ValidationParams}; /// Policy entailment algorithm maximum number of terminals allowed. const ENTAILMENT_MAX_TERMINALS: usize = 20; @@ -49,69 +46,14 @@ const ENTAILMENT_MAX_TERMINALS: usize = 20; /// policies. pub trait Liftable { /// Converts this object into an abstract policy. - fn lift(&self) -> Result, Error>; -} - -/// Error occurring during lifting. -#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] -pub enum LiftError { - /// Cannot lift policies that have a combination of height and timelocks. - HeightTimelockCombination, - /// Duplicate public keys. - BranchExceedResourceLimits, - /// Cannot lift raw descriptors. - RawDescriptorLift, -} - -impl fmt::Display for LiftError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - Self::HeightTimelockCombination => { - f.write_str("Cannot lift policies that have a heightlock and timelock combination") - } - Self::BranchExceedResourceLimits => f.write_str( - "Cannot lift policies containing one branch that exceeds resource limits", - ), - Self::RawDescriptorLift => f.write_str("Cannot lift raw descriptors"), - } - } -} - -#[cfg(feature = "std")] -impl error::Error for LiftError { - fn cause(&self) -> Option<&dyn error::Error> { - match self { - Self::HeightTimelockCombination - | Self::BranchExceedResourceLimits - | Self::RawDescriptorLift => None, - } - } -} - -impl Miniscript { - /// Lifting corresponds to conversion of a miniscript into a [`Semantic`] - /// policy for human readable or machine analysis. However, naively lifting - /// miniscripts can result in incorrect interpretations that don't - /// correspond to the underlying semantics when we try to spend them on - /// bitcoin network. This can occur if the miniscript contains: - /// 1. A combination of timelocks - /// 2. A spend that exceeds resource limits - pub fn lift_check(&self) -> Result<(), LiftError> { - if !self.within_resource_limits() { - Err(LiftError::BranchExceedResourceLimits) - } else if self.has_mixed_timelocks() { - Err(LiftError::HeightTimelockCombination) - } else { - Ok(()) - } - } + fn lift(&self) -> Result, ValidationError>; } impl Liftable for Miniscript { - fn lift(&self) -> Result, Error> { - // check whether the root miniscript can have a spending path that is - // a combination of heightlock and timelock - self.lift_check()?; + fn lift(&self) -> Result, ValidationError> { + // The "sane" rules are pretty-much defined as "the rules that must be followed + // for lifting to work and analysis to make sense" + self.validate(&ValidationParams::SANE)?; let mut stack = vec![]; for item in self.rtl_post_order_iter() { @@ -119,9 +61,7 @@ impl Liftable for Miniscript Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => { Arc::new(Semantic::Key(pk.clone())) } - Terminal::RawPkH(ref _pkh) => { - return Err(Error::LiftError(LiftError::RawDescriptorLift)) - } + Terminal::RawPkH(ref _pkh) => unreachable!("checked by self.validate above"), Terminal::After(t) => Arc::new(Semantic::After(t)), Terminal::Older(t) => Arc::new(Semantic::Older(t)), Terminal::Sha256(ref h) => Arc::new(Semantic::Sha256(h.clone())), @@ -178,7 +118,7 @@ impl Liftable for Miniscript } impl Liftable for Descriptor { - fn lift(&self) -> Result, Error> { + fn lift(&self) -> Result, ValidationError> { match *self { Self::Bare(ref bare) => bare.lift(), Self::Pkh(ref pkh) => pkh.lift(), @@ -191,14 +131,12 @@ impl Liftable for Descriptor { } impl Liftable for Semantic { - fn lift(&self) -> Result { Ok(self.clone()) } + fn lift(&self) -> Result { Ok(self.clone()) } } impl Liftable for Concrete { - fn lift(&self) -> Result, Error> { - // do not lift if there is a possible satisfaction - // involving combination of timelocks and heightlocks - self.check_timelocks().map_err(Error::ConcretePolicy)?; + fn lift(&self) -> Result, ValidationError> { + self.validate(&ValidationParams::SANE)?; let ret = match *self { Self::Unsatisfiable => Semantic::Unsatisfiable, Self::Trivial => Semantic::Trivial, @@ -210,13 +148,13 @@ impl Liftable for Concrete { Self::Ripemd160(ref h) => Semantic::Ripemd160(h.clone()), Self::Hash160(ref h) => Semantic::Hash160(h.clone()), Self::And(ref subs) => { - let semantic_subs: Result>, Error> = + let semantic_subs: Result>, ValidationError> = subs.iter().map(Liftable::lift).collect(); let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect(); Semantic::Thresh(Threshold::new(2, semantic_subs).unwrap()) } Self::Or(ref subs) => { - let semantic_subs: Result>, Error> = + let semantic_subs: Result>, ValidationError> = subs.iter().map(|(_p, sub)| sub.lift()).collect(); let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect(); Semantic::Thresh(Threshold::new(1, semantic_subs).unwrap()) @@ -230,7 +168,7 @@ impl Liftable for Concrete { } } impl Liftable for Arc> { - fn lift(&self) -> Result, Error> { self.as_ref().lift() } + fn lift(&self) -> Result, ValidationError> { self.as_ref().lift() } } #[cfg(test)] @@ -276,6 +214,12 @@ mod tests { // thresh with k = 2 assert!(ConcretePol::from_str("thresh(2,after(1000000000),after(100),pk())").is_err()); } + + #[test] + fn parse_duplicate_pubkeys() { + assert!(ConcretePol::from_str("or(and(pk(A),pk(B)),and(pk(A),pk(D)))").is_err()); + } + #[test] fn policy_rtt_tests() { concrete_policy_rtt("pk()"); @@ -402,14 +346,6 @@ mod tests { assert_eq!(descriptor, expected_descriptor); } - { - // Invalid policy compilation (Duplicate PubKeys) - let policy: Concrete = policy_str!("or(and(pk(A),pk(B)),and(pk(A),pk(D)))"); - let descriptor = policy.compile_tr(Some(unspendable_key.clone())); - - assert_eq!(descriptor.unwrap_err().to_string(), "Policy contains duplicate keys"); - } - // Non-trivial multi-node compilation { let node_policies = [ diff --git a/src/psbt/finalizer.rs b/src/psbt/finalizer.rs index 79cd50ce5..f81d1ea16 100644 --- a/src/psbt/finalizer.rs +++ b/src/psbt/finalizer.rs @@ -178,7 +178,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In *script_pubkey == addr.script_pubkey() }); match partial_sig_contains_pk { - Some((pk, _sig)) => Descriptor::new_pkh(*pk).map_err(InputError::from), + Some((pk, _sig)) => Descriptor::new_pkh(*pk).map_err(InputError::Validation), None => Err(InputError::MissingPubkey), } } else if script_pubkey.is_p2wpkh() { @@ -195,7 +195,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In } }); match partial_sig_contains_pk { - Some((pk, _sig)) => Ok(Descriptor::new_wpkh(*pk)?), + Some((pk, _sig)) => Descriptor::new_wpkh(*pk).map_err(InputError::Validation), None => Err(InputError::MissingPubkey), } } else if script_pubkey.is_p2wsh() { @@ -211,7 +211,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In }); } let ms = Miniscript::::decode_consensus(witness_script)?; - Ok(Descriptor::new_wsh(ms.substitute_raw_pkh(&map))?) + Ok(Descriptor::new_wsh(ms.substitute_raw_pkh(&map)).map_err(InputError::Validation)?) } else { Err(InputError::MissingWitnessScript) } @@ -237,7 +237,8 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In let ms = Miniscript::::decode_consensus( witness_script, )?; - Ok(Descriptor::new_sh_wsh(ms.substitute_raw_pkh(&map))?) + Ok(Descriptor::new_sh_wsh(ms.substitute_raw_pkh(&map)) + .map_err(InputError::Validation)?) } else { Err(InputError::MissingWitnessScript) } @@ -256,7 +257,9 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In } }); match partial_sig_contains_pk { - Some((pk, _sig)) => Ok(Descriptor::new_sh_wpkh(*pk)?), + Some((pk, _sig)) => { + Descriptor::new_sh_wpkh(*pk).map_err(InputError::Validation) + } None => Err(InputError::MissingPubkey), } } else { @@ -268,7 +271,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In let ms = Miniscript::::decode_consensus( redeem_script, )?; - Ok(Descriptor::new_sh(ms)?) + Ok(Descriptor::new_sh(ms).map_err(InputError::Validation)?) } else { Err(InputError::MissingWitnessScript) } @@ -284,7 +287,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In return Err(InputError::NonEmptyRedeemScript); } let ms = Miniscript::::decode_consensus(&script_pubkey)?; - Ok(Descriptor::new_bare(ms.substitute_raw_pkh(&map))?) + Ok(Descriptor::new_bare(ms.substitute_raw_pkh(&map)).map_err(InputError::Validation)?) } } diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 07860217e..c5dcada4e 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -144,6 +144,8 @@ pub enum InputError { /// the corresponding publickey pubkey: bitcoin::PublicKey, }, + /// Pass through the underlying errors in miniscript. + Validation(crate::ValidationError), } #[cfg(feature = "std")] @@ -169,6 +171,7 @@ impl error::Error for InputError { KeyErr(e) => Some(e), Interpreter(e) => Some(e), MiniscriptError(e) => Some(e), + Validation(e) => Some(e), } } } @@ -216,6 +219,7 @@ impl fmt::Display for InputError { Self::NonStandardSighashType(ref e) => { write!(f, "Non-standard sighash type {}", e) } + Self::Validation(ref e) => e.fmt(f), } } }