From 6a891562be49718afb8b7774e21bcb6e52c5b604 Mon Sep 17 00:00:00 2001 From: Jeremy Rubin Date: Tue, 26 Oct 2021 16:37:39 -0700 Subject: [PATCH] Fix over-generic parameterization of ScriptContext --- src/miniscript/context.rs | 172 ++++++++++++++++---------------------- src/miniscript/mod.rs | 4 +- 2 files changed, 74 insertions(+), 102 deletions(-) diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index 169df46af..7dd5c9760 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -155,16 +155,14 @@ where /// 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, + fn check_terminal_non_malleable( + _frag: &Terminal, ) -> Result<(), ScriptContextError>; /// Check whether the given satisfaction is valid under the ScriptContext /// For example, segwit satisfactions may fail if the witness len is more /// 3600 or number of stack elements are more than 100. - fn check_witness( - _witness: &[Vec], - ) -> Result<(), ScriptContextError> { + fn check_witness(_witness: &[Vec]) -> Result<(), ScriptContextError> { // Only really need to do this for segwitv0 and legacy // Bare is already restrcited by standardness rules // and would reach these limits. @@ -172,9 +170,7 @@ where } /// Depending on script context, the size of a satifaction witness may slightly differ. - fn max_satisfaction_size( - ms: &Miniscript, - ) -> Option; + 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. @@ -185,8 +181,8 @@ where /// 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, + fn check_global_consensus_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } @@ -199,8 +195,8 @@ where /// 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, + fn check_global_policy_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } @@ -209,8 +205,8 @@ where /// 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, + fn check_local_consensus_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } @@ -220,16 +216,16 @@ where /// 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, + 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, + fn check_global_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { Self::check_global_consensus_validity(ms)?; Self::check_global_policy_validity(ms)?; @@ -238,8 +234,8 @@ where /// Check the consensus + policy(if not disabled) rules including the /// ones for satisfaction - fn check_local_validity( - ms: &Miniscript, + fn check_local_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { Self::check_global_consensus_validity(ms)?; Self::check_global_policy_validity(ms)?; @@ -249,9 +245,7 @@ where } /// Check whether the top-level is type B - fn top_level_type_check( - ms: &Miniscript, - ) -> Result<(), Error> { + fn top_level_type_check(ms: &Miniscript) -> Result<(), Error> { if ms.ty.corr.base != types::Base::B { return Err(Error::NonTopLevel(format!("{:?}", ms))); } @@ -259,9 +253,7 @@ where } /// Other top level checks that are context specific - fn other_top_level_checks( - _ms: &Miniscript, - ) -> Result<(), Error> { + fn other_top_level_checks(_ms: &Miniscript) -> Result<(), Error> { Ok(()) } @@ -274,9 +266,7 @@ where // 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> { + fn top_level_checks(ms: &Miniscript) -> Result<(), Error> { Self::top_level_type_check(ms)?; Self::other_top_level_checks(ms) } @@ -307,8 +297,8 @@ pub enum Legacy {} impl ScriptContext for Legacy { type Key = bitcoin::PublicKey; - fn check_terminal_non_malleable( - frag: &Terminal, + fn check_terminal_non_malleable( + frag: &Terminal, ) -> Result<(), ScriptContextError> { match *frag { Terminal::PkH(ref _pkh) => Err(ScriptContextError::MalleablePkH), @@ -318,9 +308,7 @@ impl ScriptContext for Legacy { } } - fn check_witness( - witness: &[Vec], - ) -> Result<(), ScriptContextError> { + fn check_witness(witness: &[Vec]) -> Result<(), ScriptContextError> { // In future, we could avoid by having a function to count only // len of script instead of converting it. if witness_to_scriptsig(witness).len() > MAX_SCRIPTSIG_SIZE { @@ -329,8 +317,8 @@ impl ScriptContext for Legacy { Ok(()) } - fn check_global_consensus_validity( - ms: &Miniscript, + fn check_global_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { if ms.ext.pk_cost > MAX_SCRIPT_ELEMENT_SIZE { return Err(ScriptContextError::MaxRedeemScriptSizeExceeded); @@ -338,8 +326,8 @@ impl ScriptContext for Legacy { Ok(()) } - fn check_local_consensus_validity( - ms: &Miniscript, + fn check_local_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { match ms.ext.ops_count_sat { None => Err(ScriptContextError::MaxOpCountExceeded), @@ -350,8 +338,8 @@ impl ScriptContext for Legacy { } } - fn check_local_policy_validity( - ms: &Miniscript, + 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 @@ -365,9 +353,7 @@ impl ScriptContext for Legacy { } } - fn max_satisfaction_size( - ms: &Miniscript, - ) -> Option { + fn max_satisfaction_size(ms: &Miniscript) -> Option { // The scriptSig cost is the second element of the tuple ms.ext.max_sat_size.map(|x| x.1) } @@ -391,15 +377,13 @@ pub enum Segwitv0 {} impl ScriptContext for Segwitv0 { type Key = bitcoin::PublicKey; - fn check_terminal_non_malleable( - _frag: &Terminal, + fn check_terminal_non_malleable( + _frag: &Terminal, ) -> Result<(), ScriptContextError> { Ok(()) } - fn check_witness( - witness: &[Vec], - ) -> Result<(), ScriptContextError> { + fn check_witness(witness: &[Vec]) -> Result<(), ScriptContextError> { if witness.len() > MAX_STANDARD_P2WSH_STACK_ITEMS { return Err(ScriptContextError::MaxWitnessItemssExceeded { actual: witness.len(), @@ -409,8 +393,8 @@ impl ScriptContext for Segwitv0 { Ok(()) } - fn check_global_consensus_validity( - ms: &Miniscript, + fn check_global_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { if ms.ext.pk_cost > MAX_SCRIPT_SIZE { return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); @@ -435,8 +419,8 @@ impl ScriptContext for Segwitv0 { } } - fn check_local_consensus_validity( - ms: &Miniscript, + fn check_local_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { match ms.ext.ops_count_sat { None => Err(ScriptContextError::MaxOpCountExceeded), @@ -447,8 +431,8 @@ impl ScriptContext for Segwitv0 { } } - fn check_global_policy_validity( - ms: &Miniscript, + fn check_global_policy_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { if ms.ext.pk_cost > MAX_STANDARD_P2WSH_SCRIPT_SIZE { return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); @@ -456,8 +440,8 @@ impl ScriptContext for Segwitv0 { Ok(()) } - fn check_local_policy_validity( - ms: &Miniscript, + 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. @@ -475,9 +459,7 @@ impl ScriptContext for Segwitv0 { } } - fn max_satisfaction_size( - ms: &Miniscript, - ) -> Option { + fn max_satisfaction_size(ms: &Miniscript) -> Option { // The witness stack cost is the first element of the tuple ms.ext.max_sat_size.map(|x| x.0) } @@ -497,17 +479,15 @@ pub enum Tap {} impl ScriptContext for Tap { type Key = bitcoin::schnorr::PublicKey; - fn check_terminal_non_malleable( - _frag: &Terminal, + 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(()) } - fn check_witness( - witness: &[Vec], - ) -> Result<(), ScriptContextError> { + fn check_witness(witness: &[Vec]) -> Result<(), ScriptContextError> { // Note that tapscript has a 1000 limit compared to 100 of segwitv0 if witness.len() > MAX_STACK_SIZE { return Err(ScriptContextError::MaxWitnessItemssExceeded { @@ -518,8 +498,8 @@ impl ScriptContext for Tap { Ok(()) } - fn check_global_consensus_validity( - ms: &Miniscript, + fn check_global_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { // No script size checks for global consensus rules // Should we really check for block limits here. @@ -544,8 +524,8 @@ impl ScriptContext for Tap { } } - fn check_local_consensus_validity( - ms: &Miniscript, + fn check_local_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { // Taproot introduces the concept of sigops budget. // All valid miniscripts satisfy the sigops constraint @@ -570,22 +550,20 @@ impl ScriptContext for Tap { Ok(()) } - fn check_global_policy_validity( - _ms: &Miniscript, + 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, + fn check_local_policy_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } - fn max_satisfaction_size( - ms: &Miniscript, - ) -> Option { + fn max_satisfaction_size(ms: &Miniscript) -> Option { // The witness stack cost is the first element of the tuple ms.ext.max_sat_size.map(|x| x.0) } @@ -612,8 +590,8 @@ pub enum BareCtx {} impl ScriptContext for BareCtx { type Key = bitcoin::PublicKey; - fn check_terminal_non_malleable( - _frag: &Terminal, + 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 @@ -622,8 +600,8 @@ impl ScriptContext for BareCtx { Ok(()) } - fn check_global_consensus_validity( - ms: &Miniscript, + fn check_global_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { if ms.ext.pk_cost > MAX_SCRIPT_SIZE { return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); @@ -631,8 +609,8 @@ impl ScriptContext for BareCtx { Ok(()) } - fn check_local_consensus_validity( - ms: &Miniscript, + fn check_local_consensus_validity( + ms: &Miniscript, ) -> Result<(), ScriptContextError> { match ms.ext.ops_count_sat { None => Err(ScriptContextError::MaxOpCountExceeded), @@ -643,9 +621,7 @@ impl ScriptContext for BareCtx { } } - fn other_top_level_checks( - ms: &Miniscript, - ) -> Result<(), Error> { + fn other_top_level_checks(ms: &Miniscript) -> Result<(), Error> { match &ms.node { Terminal::Check(ref ms) => match &ms.node { Terminal::PkH(_pkh) => Ok(()), @@ -657,9 +633,7 @@ impl ScriptContext for BareCtx { } } - fn max_satisfaction_size( - ms: &Miniscript, - ) -> Option { + fn max_satisfaction_size(ms: &Miniscript) -> Option { // The witness stack cost is the first element of the tuple ms.ext.max_sat_size.map(|x| x.1) } @@ -687,39 +661,37 @@ pub enum NoChecks {} impl ScriptContext for NoChecks { // todo: When adding support for interpreter, we need a enum with all supported keys here type Key = bitcoin::PublicKey; - fn check_terminal_non_malleable( - _frag: &Terminal, + fn check_terminal_non_malleable( + _frag: &Terminal, ) -> Result<(), ScriptContextError> { Ok(()) } - fn check_global_policy_validity( - _ms: &Miniscript, + fn check_global_policy_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } - fn check_global_consensus_validity( - _ms: &Miniscript, + fn check_global_consensus_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } - fn check_local_policy_validity( - _ms: &Miniscript, + fn check_local_policy_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } - fn check_local_consensus_validity( - _ms: &Miniscript, + fn check_local_consensus_validity( + _ms: &Miniscript, ) -> Result<(), ScriptContextError> { Ok(()) } - fn max_satisfaction_size( - _ms: &Miniscript, - ) -> Option { + fn max_satisfaction_size(_ms: &Miniscript) -> Option { panic!("Tried to compute a satisfaction size bound on a no-checks miniscript") } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 0a0f7ae4a..72956a274 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -360,7 +360,7 @@ impl Miniscript { { match satisfy::Satisfaction::satisfy(&self.node, &satisfier, self.ty.mall.safe).stack { satisfy::Witness::Stack(stack) => { - Ctx::check_witness::(&stack)?; + Ctx::check_witness::(&stack)?; Ok(stack) } satisfy::Witness::Unavailable | satisfy::Witness::Impossible => { @@ -380,7 +380,7 @@ impl Miniscript { { match satisfy::Satisfaction::satisfy_mall(&self.node, &satisfier, self.ty.mall.safe).stack { satisfy::Witness::Stack(stack) => { - Ctx::check_witness::(&stack)?; + Ctx::check_witness::(&stack)?; Ok(stack) } satisfy::Witness::Unavailable | satisfy::Witness::Impossible => {