diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index ea3da15c9..bb53ed67a 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -394,11 +394,8 @@ impl ScriptContext for Legacy { fn check_global_consensus_validity( ms: &Miniscript, ) -> Result<(), ScriptContextError> { - if ms.ext.pk_cost > MAX_SCRIPT_ELEMENT_SIZE { - return Err(ScriptContextError::MaxRedeemScriptSizeExceeded); - } - - match ms.node { + // 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) => { for pk in thresh.iter() { @@ -408,6 +405,17 @@ impl ScriptContext for Legacy { } Terminal::MultiA(..) => 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) + } else { + Ok(()) + } + } + Err(_) => node_checked, } } @@ -492,11 +500,8 @@ impl ScriptContext for Segwitv0 { fn check_global_consensus_validity( ms: &Miniscript, ) -> Result<(), ScriptContextError> { - if ms.ext.pk_cost > MAX_SCRIPT_SIZE { - return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); - } - - match ms.node { + // 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) => { for pk in thresh.iter() { @@ -506,6 +511,17 @@ impl ScriptContext for Segwitv0 { } Terminal::MultiA(..) => 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) + } else { + Ok(()) + } + } + Err(_) => node_checked, } } @@ -598,16 +614,8 @@ impl ScriptContext for Tap { 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. - // 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() { - return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); - } - - match ms.node { + // 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) => { for pk in thresh.iter() { @@ -617,6 +625,22 @@ impl ScriptContext for Tap { } Terminal::Multi(..) => 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) + } else { + Ok(()) + } + } + Err(_) => node_checked, } } @@ -700,10 +724,8 @@ impl ScriptContext for BareCtx { fn check_global_consensus_validity( ms: &Miniscript, ) -> Result<(), ScriptContextError> { - if ms.ext.pk_cost > MAX_SCRIPT_SIZE { - return Err(ScriptContextError::MaxWitnessScriptSizeExceeded); - } - match ms.node { + // 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) => { for pk in thresh.iter() { @@ -713,6 +735,17 @@ impl ScriptContext for BareCtx { } Terminal::MultiA(..) => 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) + } else { + Ok(()) + } + } + Err(_) => node_checked, } } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index f5ca05fdb..02829ac5f 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -829,7 +829,9 @@ mod tests { use crate::policy::Liftable; use crate::prelude::*; use crate::test_utils::{StrKeyTranslator, StrXOnlyKeyTranslator}; - use crate::{hex_script, Error, ExtParams, RelLockTime, Satisfier, ToPublicKey}; + use crate::{ + hex_script, BareCtx, Error, ExtParams, Legacy, RelLockTime, Satisfier, ToPublicKey, + }; type Segwitv0Script = Miniscript; type Tapscript = Miniscript; @@ -1634,6 +1636,67 @@ mod tests { } Tapscript::parse_insane(&script.into_script()).unwrap_err(); } + + #[test] + fn test_context_global_consensus() { + // Test from string tests + type LegacyMs = Miniscript; + type Segwitv0Ms = Miniscript; + type BareMs = Miniscript; + + // multisig script of 20 pubkeys exceeds 520 bytes + let pubkey_vec_20: Vec = (0..20).map(|x| x.to_string()).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(","))); + let segwit_multi_a_ms = + Segwitv0Ms::from_str(&format!("multi_a(300,{})", pubkey_vec_300.join(","))); + let bare_multi_a_ms = + BareMs::from_str(&format!("multi_a(300,{})", pubkey_vec_300.join(","))); + + // 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" + ); + assert_eq!( + segwit_multi_a_ms.unwrap_err().to_string(), + "Multi a(CHECKSIGADD) only allowed post tapscript" + ); + assert_eq!( + bare_multi_a_ms.unwrap_err().to_string(), + "Multi a(CHECKSIGADD) only allowed post tapscript" + ); + + // multisig script of 20 pubkeys exceeds 520 bytes + let multi_ms = format!("multi(20,{})", pubkey_vec_20.join(",")); + // 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 + "))))))))))))))"; + + // correct multi for non-tapscript, but exceeding consensus size limit + let legacy_multi_ms = LegacyMs::from_str(&multi_ms); + 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 would be larger than MAX_SCRIPT_ELEMENT_SIZE bytes." + ); + assert_eq!( + segwit_multi_ms.unwrap_err().to_string(), + "The Miniscript corresponding Script would be larger than MAX_STANDARD_P2WSH_SCRIPT_SIZE bytes." + ); + assert_eq!( + bare_multi_ms.unwrap_err().to_string(), + "The Miniscript corresponding Script would be larger than MAX_STANDARD_P2WSH_SCRIPT_SIZE bytes." + ); + } } #[cfg(bench)]