diff --git a/embedded/src/main.rs b/embedded/src/main.rs index df91f4c17..fde0b8e03 100644 --- a/embedded/src/main.rs +++ b/embedded/src/main.rs @@ -44,12 +44,6 @@ fn main() -> ! { hprintln!("p2sh address {}", p2sh_addr).unwrap(); assert_eq!(p2sh_addr, "3CJxbQBfWAe1ZkKiGQNEYrioV73ZwvBWns"); - // Check whether the descriptor is safe - // This checks whether all spend paths are accessible in bitcoin network. - // It maybe possible that some of the spend require more than 100 elements in Wsh scripts - // Or they contain a combination of timelock and heightlock. - assert!(desc.sanity_check().is_ok()); - // Estimate the satisfaction cost assert_eq!(desc.max_weight_to_satisfy().unwrap().to_wu(), 288); // end miniscript test diff --git a/examples/big.rs b/examples/big.rs index a5057937b..f8ea1d991 100644 --- a/examples/big.rs +++ b/examples/big.rs @@ -75,7 +75,6 @@ fn use_descriptor(d: Descriptor) { println!("{}", d); println!("{:?}", d); println!("{:?}", d.desc_type()); - println!("{:?}", d.sanity_check()); } struct StrPkTranslator { diff --git a/examples/htlc.rs b/examples/htlc.rs index 8f864d639..edc662592 100644 --- a/examples/htlc.rs +++ b/examples/htlc.rs @@ -25,10 +25,6 @@ fn main() { ) .expect("Resource limits"); - // Check whether the descriptor is safe. This checks whether all spend paths are accessible in - // the Bitcoin network. It may be possible that some of the spend paths require more than 100 - // elements in Wsh scripts or they contain a combination of timelock and heightlock. - assert!(htlc_descriptor.sanity_check().is_ok()); assert_eq!( format!("{}", htlc_descriptor), "wsh(andor(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111),and_v(v:pkh(020202020202020202020202020202020202020202020202020202020202020202),older(4444))))#lfytrjen" diff --git a/examples/parse.rs b/examples/parse.rs index 04c2fd837..80f16c4e5 100644 --- a/examples/parse.rs +++ b/examples/parse.rs @@ -13,11 +13,6 @@ fn main() { ) .unwrap(); - // Check whether the descriptor is safe. This checks whether all spend paths are accessible in - // the Bitcoin network. It may be possible that some of the spend paths require more than 100 - // elements in Wsh scripts or they contain a combination of timelock and heightlock. - assert!(desc.sanity_check().is_ok()); - // Compute the script pubkey. As mentioned in the documentation, script_pubkey only fails // for Tr descriptors that don't have some pre-computed data. assert_eq!( diff --git a/examples/psbt_sign_finalize.rs b/examples/psbt_sign_finalize.rs index 29bf8bc4e..c5c1fe6fb 100644 --- a/examples/psbt_sign_finalize.rs +++ b/examples/psbt_sign_finalize.rs @@ -20,7 +20,6 @@ fn main() { let s = "wsh(t:or_c(pk(027a3565454fe1b749bccaef22aff72843a9c3efefd7b16ac54537a0c23f0ec0de),v:thresh(1,pkh(032d672a1a91cc39d154d366cd231983661b0785c7f27bc338447565844f4a6813),a:pkh(03417129311ed34c242c012cd0a3e0b9bca0065f742d0dfb63c78083ea6a02d4d9),a:pkh(025a687659658baeabdfc415164528065be7bcaade19342241941e556557f01e28))))#7hut9ukn"; let bridge_descriptor = Descriptor::from_str(s).unwrap(); //let bridge_descriptor = Descriptor::::from_str(&s).expect("parse descriptor string"); - assert!(bridge_descriptor.sanity_check().is_ok()); println!("Bridge pubkey script: {}", bridge_descriptor.script_pubkey()); println!("Bridge address: {}", bridge_descriptor.address(Network::Regtest).unwrap()); println!( diff --git a/examples/taproot.rs b/examples/taproot.rs index aea13f6ce..e05f9dfdd 100644 --- a/examples/taproot.rs +++ b/examples/taproot.rs @@ -52,9 +52,6 @@ fn main() { .unwrap(); assert_eq!(desc, expected_desc); - // Check whether the descriptors are safe. - assert!(desc.sanity_check().is_ok()); - // Descriptor type and version should match respectively for taproot let desc_type = desc.desc_type(); assert_eq!(desc_type, DescriptorType::Tr); diff --git a/fuzz/fuzz_targets/parse_descriptor_priv.rs b/fuzz/fuzz_targets/parse_descriptor_priv.rs index 98bfb992c..2de53ff87 100644 --- a/fuzz/fuzz_targets/parse_descriptor_priv.rs +++ b/fuzz/fuzz_targets/parse_descriptor_priv.rs @@ -10,7 +10,6 @@ fn do_test(data: &[u8]) { if let Ok((desc, _)) = Descriptor::parse_descriptor(secp, &data_str) { let _output = desc.to_string(); - let _sanity_check = desc.sanity_check(); } } diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 844516779..14cb4f535 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -47,12 +47,6 @@ impl Bare { /// get the inner pub fn as_inner(&self) -> &Miniscript { &self.ms } - /// Checks whether the descriptor is safe. - pub fn sanity_check(&self) -> Result<(), Error> { - self.ms.sanity_check()?; - Ok(()) - } - /// Computes an upper bound on the difference between a non-satisfied /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` /// diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 995276c4b..b2a12391e 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -24,7 +24,7 @@ use sync::Arc; use crate::expression::FromTree as _; use crate::miniscript::decode::Terminal; use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; -use crate::miniscript::{satisfy, Legacy, Miniscript, Segwitv0}; +use crate::miniscript::{satisfy, Legacy, Miniscript, ScriptContext as _, Segwitv0, Tap}; use crate::plan::{AssetProvider, Plan}; use crate::prelude::*; use crate::{ @@ -305,26 +305,6 @@ impl Descriptor { } } - /// Checks whether the descriptor is safe. - /// - /// Checks whether all the spend paths in the descriptor are possible on the - /// bitcoin network under the current standardness and consensus rules. Also - /// checks whether the descriptor requires signatures on all spend paths and - /// whether the script is malleable. - /// - /// In general, all the guarantees of miniscript hold only for safe scripts. - /// The signer may not be able to find satisfactions even if one exists. - pub fn sanity_check(&self) -> Result<(), Error> { - match *self { - Descriptor::Bare(ref bare) => bare.sanity_check(), - Descriptor::Pkh(_) => Ok(()), - Descriptor::Wpkh(ref wpkh) => wpkh.sanity_check(), - Descriptor::Wsh(ref wsh) => wsh.sanity_check(), - Descriptor::Sh(ref sh) => sh.sanity_check(), - Descriptor::Tr(ref tr) => tr.sanity_check(), - } - } - /// Computes an upper bound on the difference between a non-satisfied /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` /// @@ -1139,12 +1119,10 @@ impl FromStr for Descriptor { let top = expression::Tree::from_str(s)?; let ret = Self::from_tree(top.root())?; if let Descriptor::Tr(ref inner) = ret { - // FIXME preserve weird/broken behavior from 12.x. - // See https://github.com/rust-bitcoin/rust-miniscript/issues/734 - ret.sanity_check()?; for item in inner.leaves() { item.miniscript() - .ext_check(&crate::miniscript::analyzable::ExtParams::sane())?; + .validate(&Tap::SANE) + .map_err(Error::Validation)?; } } Ok(ret) diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 883ac7b1b..8282f4458 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -53,12 +53,6 @@ impl Wsh { #[deprecated(since = "8.0.0", note = "use format!(\"{:#}\") instead")] pub fn to_string_no_checksum(&self) -> String { format!("{:#}", self) } - /// Checks whether the descriptor is safe. - pub fn sanity_check(&self) -> Result<(), Error> { - self.ms.sanity_check()?; - Ok(()) - } - /// Computes an upper bound on the difference between a non-satisfied /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` /// @@ -249,15 +243,6 @@ impl Wpkh { #[deprecated(since = "8.0.0", note = "use format!(\"{:#}\") instead")] pub fn to_string_no_checksum(&self) -> String { format!("{:#}", self) } - /// Checks whether the descriptor is safe. - pub fn sanity_check(&self) -> Result<(), Error> { - if self.pk.is_uncompressed() { - Err(Error::ContextError(ScriptContextError::CompressedOnly(self.pk.to_string()))) - } else { - Ok(()) - } - } - /// Computes an upper bound on the difference between a non-satisfied /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` /// diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index c824c7bda..a9b68d52e 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -132,16 +132,6 @@ impl Sh { /// Create a new p2sh wrapper for the given wsh descriptor pub fn new_with_wsh(wsh: Wsh) -> Self { Self { inner: ShInner::Wsh(wsh) } } - /// Checks whether the descriptor is safe. - pub fn sanity_check(&self) -> Result<(), Error> { - match self.inner { - ShInner::Wsh(ref wsh) => wsh.sanity_check()?, - ShInner::Wpkh(ref wpkh) => wpkh.sanity_check()?, - ShInner::Ms(ref ms) => ms.sanity_check()?, - } - Ok(()) - } - /// Create a new p2sh wrapped wsh sortedmulti descriptor from threshold /// `k` and Vec of `pks` pub fn new_wsh_sortedmulti( diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index 47a72cfff..77a882045 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -139,14 +139,6 @@ impl Tr { } } - /// Checks whether the descriptor is safe. - pub fn sanity_check(&self) -> Result<(), Error> { - for leaf in self.leaves() { - leaf.miniscript().sanity_check()?; - } - Ok(()) - } - /// Computes an upper bound on the difference between a non-satisfied /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` /// @@ -383,9 +375,9 @@ impl crate::expression::FromTree for Tr { } else { let script = Miniscript::from_tree(node)?; // FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734 - if script.ty.corr.base != crate::miniscript::types::Base::B { - return Err(Error::NonTopLevel(format!("{:?}", script))); - }; + script + .validate(&Tap::CONSENSUS) + .map_err(Error::Validation)?; tree_builder.push_leaf(script); tap_tree_iter.skip_descendants(); diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 5aed49fb0..775141536 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1059,7 +1059,6 @@ mod tests { use super::inner::ToNoChecks; use super::*; - use crate::miniscript::analyzable::ExtParams; #[allow(clippy::type_complexity)] fn setup_keys_sigs( @@ -1562,14 +1561,12 @@ mod tests { // because it does not implement FromStr fn no_checks_ms(ms: &str) -> Miniscript { // Parsing should allow raw hashes in the interpreter - let elem: Miniscript = - Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap(); + let elem: Miniscript = ms.parse().unwrap(); elem.to_no_checks_ms() } fn x_only_no_checks_ms(ms: &str) -> Miniscript { - let elem: Miniscript = - Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap(); + let elem: Miniscript = ms.parse().unwrap(); elem.to_no_checks_ms() } } diff --git a/src/lib.rs b/src/lib.rs index cbed64e51..d783a70d3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,11 +56,6 @@ //! "3CJxbQBfWAe1ZkKiGQNEYrioV73ZwvBWns" //! ); //! -//! // Check whether the descriptor is safe. This checks whether all spend paths are accessible in -//! // the Bitcoin network. It may be possible that some of the spend paths require more than 100 -//! // elements in Wsh scripts or they contain a combination of timelock and heightlock. -//! assert!(desc.sanity_check().is_ok()); -//! //! // Estimate the satisfaction cost. //! // scriptSig: OP_PUSH34 > //! // = (1 + 1 + 1 + 32) * 4 = 140 WU @@ -124,6 +119,7 @@ pub mod plan; pub mod policy; mod primitives; pub mod psbt; +mod validation; #[cfg(test)] mod test_utils; @@ -138,7 +134,6 @@ pub use crate::descriptor::{DefiniteDescriptorKey, Descriptor, DescriptorPublicK pub use crate::error::ParseError; pub use crate::expression::{ParseNumError, ParseThresholdError, ParseTreeError}; pub use crate::interpreter::Interpreter; -pub use crate::miniscript::analyzable::{AnalysisError, ExtParams}; pub use crate::miniscript::context::{BareCtx, Legacy, ScriptContext, Segwitv0, SigType, Tap}; pub use crate::miniscript::decode::Terminal; pub use crate::miniscript::satisfy::{Preimage32, Satisfier}; @@ -147,6 +142,7 @@ use crate::prelude::*; pub use crate::primitives::absolute_locktime::{AbsLockTime, AbsLockTimeError}; pub use crate::primitives::relative_locktime::{RelLockTime, RelLockTimeError}; pub use crate::primitives::threshold::{Threshold, ThresholdError}; +pub use crate::validation::{Error as ValidationError, ValidationParams}; /// Trait representing a key which can be converted to a hash type. pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash { @@ -441,8 +437,6 @@ pub enum Error { Unexpected(String), /// Encountered a wrapping character that we don't recognize UnknownWrapper(char), - /// Parsed a miniscript and the result was not of type T - NonTopLevel(String), /// Parsed a miniscript but there were more script opcodes after it Trailing(String), /// Could not satisfy a script (fragment) because of a missing signature @@ -469,8 +463,6 @@ pub enum Error { /// 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, - /// Analysis Error - AnalysisError(miniscript::analyzable::AnalysisError), /// Miniscript is equivalent to false. No possible satisfaction ImpossibleSatisfaction, /// Bare descriptors don't have any addresses @@ -492,6 +484,8 @@ pub enum Error { ParseThreshold(ParseThresholdError), /// Invalid expression tree. Parse(ParseError), + /// Validation of a script failed. + Validation(ValidationError), } #[doc(hidden)] // will be removed when we remove Error @@ -511,7 +505,6 @@ impl fmt::Display for Error { Error::UnexpectedStart => f.write_str("unexpected start of script"), Error::Unexpected(ref s) => write!(f, "unexpected «{}»", s), Error::UnknownWrapper(ch) => write!(f, "unknown wrapper «{}:»", ch), - Error::NonTopLevel(ref s) => write!(f, "non-T miniscript: {}", s), Error::Trailing(ref s) => write!(f, "trailing tokens: {}", s), Error::MissingSig(ref pk) => write!(f, "missing signature for key {:?}", pk), Error::CouldNotSatisfy => f.write_str("could not satisfy"), @@ -534,7 +527,6 @@ impl fmt::Display for Error { up to n=3 is invalid by standardness (bare). " ), - Error::AnalysisError(ref e) => e.fmt(f), Error::ImpossibleSatisfaction => write!(f, "Impossible to satisfy Miniscript"), Error::BareDescriptorAddr => write!(f, "Bare descriptors don't have address"), Error::PubKeyCtxError(ref pk, ref ctx) => { @@ -547,6 +539,7 @@ impl fmt::Display for Error { Error::Threshold(ref e) => e.fmt(f), Error::ParseThreshold(ref e) => e.fmt(f), Error::Parse(ref e) => e.fmt(f), + Error::Validation(ref e) => e.fmt(f), } } } @@ -560,7 +553,6 @@ impl std::error::Error for Error { UnexpectedStart | Unexpected(_) | UnknownWrapper(_) - | NonTopLevel(_) | Trailing(_) | MissingSig(_) | CouldNotSatisfy @@ -581,13 +573,13 @@ impl std::error::Error for Error { LiftError(e) => Some(e), ContextError(e) => Some(e), TapTreeDepthError(e) => Some(e), - AnalysisError(e) => Some(e), PubKeyCtxError(e, _) => Some(e), AbsoluteLockTime(e) => Some(e), RelativeLockTime(e) => Some(e), Threshold(e) => Some(e), ParseThreshold(e) => Some(e), Parse(e) => Some(e), + Validation(e) => Some(e), } } } @@ -617,11 +609,6 @@ impl From for Error { fn from(e: miniscript::context::ScriptContextError) -> Error { Error::ContextError(e) } } -#[doc(hidden)] -impl From for Error { - fn from(e: miniscript::analyzable::AnalysisError) -> Error { Error::AnalysisError(e) } -} - #[doc(hidden)] impl From for Error { fn from(e: bitcoin::secp256k1::Error) -> Error { Error::Secp(e) } diff --git a/src/macros.rs b/src/macros.rs index 6e4631f62..f4898b88a 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -8,7 +8,7 @@ /// `ms_str!("c:or_i(pk({}),pk({}))", pk1, pk2)` #[cfg(test)] macro_rules! ms_str { - ($($arg:tt)*) => (Miniscript::from_str_ext(&format!($($arg)*), &$crate::ExtParams::allow_all()).unwrap()) + ($($arg:tt)*) => (Miniscript::from_str_with_validation_params(&format!($($arg)*), &$crate::ValidationParams::MAX).unwrap()) } /// Allows tests to create a concrete policy directly from string as diff --git a/src/miniscript/analyzable.rs b/src/miniscript/analyzable.rs index 13ccb5e65..b058c84f2 100644 --- a/src/miniscript/analyzable.rs +++ b/src/miniscript/analyzable.rs @@ -5,183 +5,9 @@ //! Tools for determining whether the guarantees offered by the library //! actually hold. -use core::fmt; -#[cfg(feature = "std")] -use std::error; - use crate::prelude::*; use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal}; -/// Params for parsing miniscripts that either non-sane or non-specified(experimental) in the spec. -/// Used as a parameter [`Miniscript::from_str_ext`] and [`Miniscript::decode_with_ext`]. -/// -/// This allows parsing miniscripts if -/// 1. It is unsafe(does not require a digital signature to spend it) -/// 2. It contains a unspendable path because of either -/// 1. Resource limitations -/// 2. Timelock Mixing -/// 3. The script is malleable and thereby some of satisfaction weight -/// guarantees are not satisfied. -/// 4. It has repeated public keys -/// 5. raw pkh fragments without the pk. This could be obtained when parsing miniscript from script -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Hash)] -pub struct ExtParams { - /// Allow parsing of non-safe miniscripts - pub top_unsafe: bool, - /// Allow parsing of miniscripts with unspendable paths - pub resource_limitations: bool, - /// Allow parsing of miniscripts with timelock mixing - pub timelock_mixing: bool, - /// Allow parsing of malleable miniscripts - pub malleability: bool, - /// Allow parsing of miniscripts with repeated public keys - pub repeated_pk: bool, - /// Allow parsing of miniscripts with raw pkh fragments without the pk. - /// This could be obtained when parsing miniscript from script - pub raw_pkh: bool, -} - -impl ExtParams { - /// Create a new ExtParams that with all the sanity rules - pub fn new() -> ExtParams { - ExtParams { - top_unsafe: false, - resource_limitations: false, - timelock_mixing: false, - malleability: false, - repeated_pk: false, - raw_pkh: false, - } - } - - /// Create a new ExtParams that allows all the sanity rules - pub fn sane() -> ExtParams { ExtParams::new() } - - /// Create a new ExtParams that insanity rules - /// This enables parsing well specified but "insane" miniscripts. - /// Refer to the [`ExtParams`] documentation for more details on "insane" miniscripts. - pub fn insane() -> ExtParams { - ExtParams { - top_unsafe: true, - resource_limitations: true, - timelock_mixing: true, - malleability: true, - repeated_pk: true, - raw_pkh: false, - } - } - - /// Enable all non-sane rules and experimental rules - pub fn allow_all() -> ExtParams { - ExtParams { - top_unsafe: true, - resource_limitations: true, - timelock_mixing: true, - malleability: true, - repeated_pk: true, - raw_pkh: true, - } - } - - /// Builder that allows non-safe miniscripts. - pub fn top_unsafe(mut self) -> ExtParams { - self.top_unsafe = true; - self - } - - /// Builder that allows miniscripts with exceed resource limitations. - pub fn exceed_resource_limitations(mut self) -> ExtParams { - self.resource_limitations = true; - self - } - - /// Builder that allows miniscripts with timelock mixing. - pub fn timelock_mixing(mut self) -> ExtParams { - self.timelock_mixing = true; - self - } - - /// Builder that allows malleable miniscripts. - pub fn malleability(mut self) -> ExtParams { - self.malleability = true; - self - } - - /// Builder that allows miniscripts with repeated public keys. - pub fn repeated_pk(mut self) -> ExtParams { - self.repeated_pk = true; - self - } - - /// Builder that allows miniscripts with raw pkh fragments. - pub fn raw_pkh(mut self) -> ExtParams { - self.raw_pkh = true; - self - } -} - -/// Possible reasons Miniscript guarantees can fail -/// We currently mark Miniscript as Non-Analyzable if -/// 1. It is unsafe(does not require a digital signature to spend it) -/// 2. It contains a unspendable path because of either -/// 1. Resource limitations -/// 2. Timelock Mixing -/// 3. The script is malleable and thereby some of satisfaction weight -/// guarantees are not satisfied. -/// 4. It has repeated publickeys -#[derive(Debug, PartialEq)] -pub enum AnalysisError { - /// Top level is not safe. - SiglessBranch, - /// Repeated Pubkeys - RepeatedPubkeys, - /// Miniscript contains at least one path that exceeds resource limits - BranchExceedResouceLimits, - /// Contains a combination of heightlock and timelock - HeightTimelockCombination, - /// Malleable script - Malleable, - /// Contains partial descriptor raw pkh - ContainsRawPkh, -} - -impl fmt::Display for AnalysisError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - AnalysisError::SiglessBranch => { - f.write_str("All spend paths must require a signature") - } - AnalysisError::RepeatedPubkeys => { - f.write_str("Miniscript contains repeated pubkeys or pubkeyhashes") - } - AnalysisError::BranchExceedResouceLimits => { - f.write_str("At least one spend path exceeds the resource limits(stack depth/satisfaction size..)") - } - AnalysisError::HeightTimelockCombination => { - f.write_str("Contains a combination of heightlock and timelock") - } - AnalysisError::Malleable => f.write_str("Miniscript is malleable"), - AnalysisError::ContainsRawPkh => f.write_str("Miniscript contains raw pkh"), - } - } -} - -#[cfg(feature = "std")] -impl error::Error for AnalysisError { - fn cause(&self) -> Option<&dyn error::Error> { - use self::AnalysisError::*; - - match self { - SiglessBranch - | RepeatedPubkeys - | BranchExceedResouceLimits - | HeightTimelockCombination - | Malleable - | ContainsRawPkh => None, - } - } -} - impl Miniscript { /// Whether all spend paths of miniscript require a signature pub fn requires_sig(&self) -> bool { self.ty.mall.safe } @@ -212,48 +38,4 @@ impl Miniscript { pub fn contains_raw_pkh(&self) -> bool { self.iter().any(|ms| matches!(ms.node, Terminal::RawPkH(_))) } - - /// Check whether the underlying Miniscript is safe under the current context - /// Lifting these polices would create a semantic representation that does - /// not represent the underlying semantics when miniscript is spent. - /// Signing logic may not find satisfaction even if one exists. - /// - /// For most cases, users should be dealing with safe scripts. - /// Use this function to check whether the guarantees of library hold. - /// Most functions of the library like would still - /// work, but results cannot be relied upon - pub fn sanity_check(&self) -> Result<(), AnalysisError> { - if !self.requires_sig() { - Err(AnalysisError::SiglessBranch) - } else if !self.is_non_malleable() { - Err(AnalysisError::Malleable) - } else if !self.within_resource_limits() { - Err(AnalysisError::BranchExceedResouceLimits) - } else if self.has_repeated_keys() { - Err(AnalysisError::RepeatedPubkeys) - } else if self.has_mixed_timelocks() { - Err(AnalysisError::HeightTimelockCombination) - } else { - Ok(()) - } - } - - /// Check whether the miniscript follows the given Extra policy [`ExtParams`] - pub fn ext_check(&self, ext: &ExtParams) -> Result<(), AnalysisError> { - if !ext.top_unsafe && !self.requires_sig() { - Err(AnalysisError::SiglessBranch) - } else if !ext.malleability && !self.is_non_malleable() { - Err(AnalysisError::Malleable) - } else if !ext.resource_limitations && !self.within_resource_limits() { - Err(AnalysisError::BranchExceedResouceLimits) - } else if !ext.repeated_pk && self.has_repeated_keys() { - Err(AnalysisError::RepeatedPubkeys) - } else if !ext.timelock_mixing && self.has_mixed_timelocks() { - Err(AnalysisError::HeightTimelockCombination) - } else if !ext.raw_pkh && self.contains_raw_pkh() { - Err(AnalysisError::ContainsRawPkh) - } else { - Ok(()) - } - } } diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index bc2d6bb6f..38c330af2 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -13,9 +13,8 @@ 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, }; -use crate::miniscript::types; use crate::prelude::*; -use crate::{hash256, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal}; +use crate::{hash256, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal, ValidationParams}; /// Error for Script Context #[derive(Clone, PartialEq, Eq, Debug)] @@ -176,6 +175,18 @@ where { /// The consensus key associated with the type. Must be a parseable key type Key: ParseableKey; + + /// The validation parameters enforcing consensus limits in this context, and + /// nothing further. + const CONSENSUS: ValidationParams; + + /// Sensible validation parameters for this context. Unless you have a good reason + /// to choose otherwise, these are the validation parameters you want. + /// + /// They are also the validation parameters used throughout this library when no + /// 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 @@ -271,9 +282,6 @@ where /// Check whether the top-level is type B fn top_level_type_check(ms: &Miniscript) -> Result<(), Error> { - if ms.ty.corr.base != types::Base::B { - return Err(Error::NonTopLevel(format!("{:?}", ms))); - } // (Ab)use `for_each_key` to record the number of derivation paths a multipath key has. #[derive(PartialEq)] enum MultipathLenChecker { @@ -359,6 +367,20 @@ pub enum Legacy {} impl ScriptContext for Legacy { type Key = bitcoin::PublicKey; + + const CONSENSUS: ValidationParams = ValidationParams { + allow_compressed_keys: true, + allow_dup_if: false, + allow_uncompressed_keys: true, + allow_multi_a: false, + allow_or_i: false, + allow_x_only_keys: false, + max_opcode_count: MAX_OPS_PER_SCRIPT, + max_script_size: MAX_SCRIPT_ELEMENT_SIZE, + ..ValidationParams::CONSENSUS + }; + const SANE: ValidationParams = Self::CONSENSUS.intersect(&ValidationParams::SANE); + fn check_terminal_non_malleable( frag: &Terminal, ) -> Result<(), ScriptContextError> { @@ -467,6 +489,22 @@ pub enum Segwitv0 {} impl ScriptContext for Segwitv0 { type Key = bitcoin::PublicKey; + + const CONSENSUS: ValidationParams = ValidationParams { + allow_compressed_keys: true, + allow_uncompressed_keys: false, + allow_multi_a: false, + allow_x_only_keys: false, + max_opcode_count: MAX_OPS_PER_SCRIPT, + max_exec_stack_size: MAX_STACK_SIZE, + ..ValidationParams::CONSENSUS + }; + const SANE: ValidationParams = ValidationParams { + max_script_size: MAX_STANDARD_P2WSH_SCRIPT_SIZE, + max_witness_items: MAX_STANDARD_P2WSH_STACK_ITEMS, + ..Self::CONSENSUS.intersect(&ValidationParams::SANE) + }; + fn check_terminal_non_malleable( _frag: &Terminal, ) -> Result<(), ScriptContextError> { @@ -580,6 +618,22 @@ pub enum Tap {} impl ScriptContext for Tap { type Key = bitcoin::secp256k1::XOnlyPublicKey; + + const CONSENSUS: ValidationParams = ValidationParams { + allow_compressed_keys: false, + allow_uncompressed_keys: false, + allow_multi: false, + allow_x_only_keys: true, + ..ValidationParams::CONSENSUS + }; + const SANE: ValidationParams = ValidationParams { + // Segwit runtime stack item number applies, but no script size limit (though maybe we should + // enforce a 4mb limit?) and no policy limit on number of initial stack items. + // https://github.com/bitcoin/bips/blob/master/bip-0342.mediawiki#user-content-Resource_limits + max_exec_stack_size: MAX_STACK_SIZE, + ..Self::CONSENSUS.intersect(&ValidationParams::SANE) + }; + fn check_terminal_non_malleable( _frag: &Terminal, ) -> Result<(), ScriptContextError> { @@ -691,6 +745,20 @@ pub enum BareCtx {} impl ScriptContext for BareCtx { type Key = bitcoin::PublicKey; + + const CONSENSUS: ValidationParams = ValidationParams { + allow_compressed_keys: true, + allow_dup_if: false, + allow_uncompressed_keys: true, + allow_multi_a: false, + allow_or_i: false, + allow_x_only_keys: false, + max_opcode_count: MAX_OPS_PER_SCRIPT, + max_script_size: MAX_SCRIPT_SIZE, + ..ValidationParams::CONSENSUS + }; + const SANE: ValidationParams = Self::CONSENSUS.intersect(&ValidationParams::SANE); + fn check_terminal_non_malleable( _frag: &Terminal, ) -> Result<(), ScriptContextError> { @@ -801,6 +869,10 @@ 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; + + const CONSENSUS: ValidationParams = ValidationParams::MAX; + const SANE: ValidationParams = ValidationParams::MAX; + fn check_terminal_non_malleable( _frag: &Terminal, ) -> Result<(), ScriptContextError> { @@ -865,10 +937,7 @@ impl ScriptContext for NoChecks { Ok(()) } - fn top_level_type_check(ms: &Miniscript) -> Result<(), Error> { - if ms.ty.corr.base != types::Base::B { - return Err(Error::NonTopLevel(format!("{:?}", ms))); - } + fn top_level_type_check(_: &Miniscript) -> Result<(), Error> { Ok(()) } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index cb944b585..885b22dc6 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -18,7 +18,6 @@ use bitcoin::hashes::hash160; use bitcoin::script; use bitcoin::taproot::{LeafVersion, TapLeafHash}; -use self::analyzable::ExtParams; pub use self::context::{BareCtx, Legacy, Segwitv0, Tap}; use crate::iter::TreeLike; use crate::prelude::*; @@ -39,12 +38,13 @@ use core::cmp; use sync::Arc; +pub use self::context::ScriptContext; use self::lex::{lex, TokenIter}; use crate::expression::{FromTree, TreeIterItem}; -pub use crate::miniscript::context::ScriptContext; use crate::miniscript::decode::Terminal; use crate::{ expression, plan, Error, ForEachKey, FromStrKey, MiniscriptKey, ToPublicKey, Translator, + ValidationParams, }; #[cfg(test)] mod ms_tests; @@ -54,10 +54,13 @@ mod private { use super::limits::{MAX_PUBKEYS_IN_CHECKSIGADD, MAX_PUBKEYS_PER_MULTISIG}; use super::types::{self, ExtData, Type}; + use super::ScriptContext; use crate::iter::TreeLike as _; - pub use crate::miniscript::context::ScriptContext; use crate::prelude::sync::Arc; - use crate::{AbsLockTime, Error, MiniscriptKey, RelLockTime, Terminal, MAX_RECURSION_DEPTH}; + use crate::{ + AbsLockTime, Error, MiniscriptKey, RelLockTime, Terminal, ValidationError, + ValidationParams, MAX_RECURSION_DEPTH, + }; /// The top-level miniscript abstract syntax tree (AST). pub struct Miniscript { @@ -326,6 +329,7 @@ mod private { node: t, phantom: PhantomData, }; + // TODO: This recursion depth is based on segwitv0. // We can relax this in tapscript, but this should be good for almost // all practical cases and we can revisit this if needed. @@ -349,6 +353,170 @@ mod private { ) -> Miniscript { Miniscript { node, ty, ext, phantom: PhantomData } } + + /// Validates whether a given fragment meets the given set of + /// validation parameters. + pub fn validate(&self, params: &ValidationParams) -> Result<(), ValidationError> + where + Pk: MiniscriptKey, + Ctx: ScriptContext, + { + self.validate_non_top_level(params)?; + + // 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() { + return Err(ValidationError::Malleable); + } + + // The B check is inherently top-level. + if !params.allow_non_b && self.ty.corr.base != types::Base::B { + return Err(ValidationError::NonBase(self.ty.corr.base)); + } + + // Sigless branches can be fixed by adding a conjunction with a signature. + if !params.allow_sigless_branch && !self.requires_sig() { + return Err(ValidationError::SiglessBranch); + } + + // Unsatisifiable scripts can be fixed by adding a disjunction with something + // satisfiable. + if !params.allow_unsatisfiable && self.ext.sat_data.is_none() { + return Err(ValidationError::Unsatisfiable); + } + + // All checks passed. + Ok(()) + } + + /// Validates a miniscript, doing only the checks that are applicable to all nodes, + /// not just top-level ones. + /// + /// In particular this excludes the "must be B" and "no sigless branches" checks. + /// To get these, run [`Self::validate`] which also calls through to this method. + pub fn validate_non_top_level( + &self, + params: &ValidationParams, + ) -> Result<(), ValidationError> { + if self.ext.tree_height > params.max_recursive_depth { + return Err(ValidationError::MaxRecursiveDepthExceeded { + limit: params.max_recursive_depth, + }); + } + if !params.allow_duplicate_keys && self.has_repeated_keys() { + return Err(ValidationError::DuplicateKeys); + } + if !params.allow_mixed_time_locks && self.has_mixed_timelocks() { + return Err(ValidationError::MixedTimeLocks); + } + + let mut multipath_len = None; + let mut multipath_check = |pk: &Pk| { + if params.allow_inconsistent_multipath_keys { + return Ok(()); + } + match (multipath_len, pk.num_der_paths()) { + (_, 0) | (_, 1) => {} + (None, n) => multipath_len = Some(n), + (Some(x), y) if x == y => { /* ok */ } + (Some(x), y) => { + return Err(ValidationError::MultipathKeyLenMismatch { len1: x, len2: y }) + } + } + Ok(()) + }; + for ms in self.iter() { + match ms.node { + Terminal::DupIf(..) if !params.allow_dup_if => { + return Err(ValidationError::IllegalDupIf) + } + Terminal::Multi(ref thresh) | Terminal::SortedMulti(ref thresh) => { + if !params.allow_multi { + return Err(ValidationError::IllegalMulti); + } + for key in thresh.iter() { + params.validate_pk(key).map_err(ValidationError::Key)?; + multipath_check(key)?; + } + } + Terminal::MultiA(ref thresh) | Terminal::SortedMultiA(ref thresh) => { + if !params.allow_multi_a { + return Err(ValidationError::IllegalMultiA); + } + for key in thresh.iter() { + params.validate_pk(key).map_err(ValidationError::Key)?; + multipath_check(key)?; + } + } + Terminal::OrI(..) if !params.allow_or_i => { + return Err(ValidationError::IllegalOrI) + } + Terminal::RawPkH(..) if !params.allow_raw_pkh => { + return Err(ValidationError::IllegalRawPkh) + } + Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => { + params.validate_pk(pk).map_err(ValidationError::Key)?; + multipath_check(pk)?; + } + _ => {} + } + } + + // FIXME we have to gate this check on params.max_script_size being finite + // because otherwise we'll attempt to call self.script_size(), which calls + // Ctx::pk_len, when validating NoChecks scripts. But NoChecks::pk_len just + // panics because we've forgotten the length of keys once we're in the + // NoChecks context. + // + // This gate will be removed in a later PR which removes Ctx::pk_len and + // replaces it with a less-fragile solution. + if params.max_script_size < usize::MAX && self.script_size() > params.max_script_size { + return Err(ValidationError::MaxScriptSizeExceeded { + actual: self.script_size(), + limit: params.max_script_size, + }); + } + // Satisfiability checks -- if there is no satisfaciton data set then we will + // early-return here, so all other checks should be done before this. + match self.max_satisfaction_witness_elements() { + // No possible satisfactions -- we are doing non-toplevel checks here + // so fail gracefully. + Err(..) => return Ok(()), + Ok(max_n) if max_n > params.max_witness_items => { + return Err(ValidationError::MaxWitnessItemsExceeded { + actual: max_n, + limit: params.max_witness_items, + }) + } + Ok(..) => {} + } + + let sat_op_count = self + .ext + .sat_op_count() + .expect("checked that satisfaction was possible above"); + if sat_op_count > params.max_opcode_count { + return Err(ValidationError::MaxOpCountExceeded { + actual: sat_op_count, + limit: params.max_opcode_count, + }); + } + + let sat_data = self + .ext + .sat_data + .expect("checked that satisfaction was possible above"); + if sat_data.max_witness_stack_count + sat_data.max_exec_stack_count + > params.max_exec_stack_size + { + return Err(ValidationError::MaxExecStackSizeExceeded { + actual: sat_data.max_witness_stack_size + sat_data.max_exec_stack_count, + limit: params.max_exec_stack_size, + }); + } + + Ok(()) + } } } @@ -564,36 +732,33 @@ impl Miniscript { /// embedded in the chain. While it is inadvisable to use insane Miniscripts, /// once it's on the chain you don't have much choice anymore. pub fn decode_consensus(script: &script::Script) -> Result, Error> { - Miniscript::decode_with_ext(script, &ExtParams::allow_all()) + Miniscript::decode_with_validation_params(script, &Ctx::CONSENSUS) } /// Attempt to decode a Miniscript from Script, specifying which validation parameters to apply. - pub fn decode_with_ext( + pub fn decode_with_validation_params( script: &script::Script, - ext: &ExtParams, + params: &ValidationParams, ) -> Result, Error> { let tokens = lex(script)?; let mut iter = TokenIter::new(tokens); let top = decode::decode(&mut iter)?; Ctx::check_global_validity(&top)?; - let type_check = types::Type::type_check(&top.node)?; - if type_check.corr.base != types::Base::B { - return Err(Error::NonTopLevel(format!("{:?}", top))); - }; + types::Type::type_check(&top.node)?; if let Some(leading) = iter.next() { Err(Error::Trailing(leading.to_string())) } else { - top.ext_check(ext)?; + top.validate(params).map_err(Error::Validation)?; Ok(top) } } - /// Attempt to parse a Script into Miniscript representation. + /// Attempt to decode a Script as a Miniscript. /// - /// This function will fail parsing for scripts that do not clear the - /// [`Miniscript::sanity_check`] checks. Use [`Miniscript::decode_consensus`] to - /// parse such scripts. + /// This will enforce sanity and standardness rules and fail to decode if they + /// are validated. You may want [`Self::decode_consensus`] if you want to decode + /// anyway. /// /// ## Decode/Parse a miniscript from script hex /// @@ -622,7 +787,7 @@ impl Miniscript { /// /// ``` pub fn decode(script: &script::Script) -> Result, Error> { - let ms = Self::decode_with_ext(script, &ExtParams::sane())?; + let ms = Self::decode_with_validation_params(script, &Ctx::SANE)?; Ok(ms) } } @@ -837,33 +1002,26 @@ impl Miniscript { } impl Miniscript { - /// Attempt to parse an insane(scripts don't clear sanity checks) - /// from string into a Miniscript representation. - /// Use this to parse scripts with repeated pubkeys, timelock mixing, malleable - /// scripts without sig or scripts that can exceed resource limits. - /// Some of the analysis guarantees of miniscript are lost when dealing with - /// insane scripts. In general, in a multi-party setting users should only - /// accept sane scripts. - pub fn from_str_insane(s: &str) -> Result, Error> { - Miniscript::from_str_ext(s, &ExtParams::insane()) + /// Attempt to parse a Miniscript, checking only for consensus compatibility, + /// lack of raw pubkeyhashes, and no other checks. + /// + /// It is not recommended to use scripts which require this function in order + /// to parse, especially in a multiparty setting. + pub fn from_str_insane(s: &str) -> Result { + let params = ValidationParams { allow_raw_pkh: false, ..Ctx::CONSENSUS }; + Miniscript::from_str_with_validation_params(s, ¶ms) } - /// Attempt to parse an Miniscripts that don't follow the spec. - /// Use this to parse scripts with repeated pubkeys, timelock mixing, malleable - /// scripts, raw pubkey hashes without sig or scripts that can exceed resource limits. - /// - /// Use [`ExtParams`] builder to specify the types of non-sane rules to allow while parsing. - pub fn from_str_ext(s: &str, ext: &ExtParams) -> Result, Error> { + /// Attempt to parse a Miniscript, specifying which validation parameters to apply. + pub fn from_str_with_validation_params( + s: &str, + params: &ValidationParams, + ) -> Result { // This checks for invalid ASCII chars let top = expression::Tree::from_str(s)?; let ms: Miniscript = expression::FromTree::from_tree(top.root())?; - ms.ext_check(ext)?; - - if ms.ty.corr.base != types::Base::B { - Err(Error::NonTopLevel(format!("{:?}", ms))) - } else { - Ok(ms) - } + ms.validate(params).map_err(Error::Validation)?; + Ok(ms) } } @@ -1077,11 +1235,8 @@ impl FromTree for Miniscript { impl str::FromStr for Miniscript { type Err = Error; - /// Parse a Miniscript from string and perform sanity checks - /// See [Miniscript::from_str_insane] to parse scripts from string that - /// do not clear the [Miniscript::sanity_check] checks. fn from_str(s: &str) -> Result, Error> { - let ms = Self::from_str_ext(s, &ExtParams::sane())?; + let ms = Self::from_str_with_validation_params(s, &Ctx::SANE)?; Ok(ms) } } @@ -1110,13 +1265,12 @@ mod tests { use bitcoin::taproot::TapLeafHash; use sync::Arc; - use super::{Miniscript, ScriptContext, Segwitv0, Tap}; + use super::*; use crate::miniscript::{types, Terminal}; use crate::policy::Liftable; - use crate::prelude::*; use crate::test_utils::{StrKeyTranslator, StrXOnlyKeyTranslator}; use crate::{ - hex_script, BareCtx, Error, ExtParams, Legacy, RelLockTime, Satisfier, ToPublicKey, + hex_script, BareCtx, Error, Legacy, RelLockTime, Satisfier, ToPublicKey, ValidationError, }; type Segwitv0Script = Miniscript; @@ -1746,12 +1900,13 @@ mod tests { // Test that parsing raw hash160 from string does not work without extra features SegwitMs::from_str(ms_str).unwrap_err(); SegwitMs::from_str_insane(ms_str).unwrap_err(); - let ms = SegwitMs::from_str_ext(ms_str, &ExtParams::allow_all()).unwrap(); + let ms = SegwitMs::from_str_with_validation_params(ms_str, &ValidationParams::MAX).unwrap(); let script = ms.encode(); - // The same test, but parsing from script + // The same test, but parsing from script. Notice that unlike the previous "insane" + // extparams, the Ctx::CONSENSUS constant allows raw pubkeyhashes. SegwitMs::decode(&script).unwrap_err(); - SegwitMs::decode_with_ext(&script, &ExtParams::insane()).unwrap_err(); + SegwitMs::decode_with_validation_params(&script, &Segwitv0::CONSENSUS).unwrap(); SegwitMs::decode_consensus(&script).unwrap(); // Try replacing the raw_pkh with a pkh @@ -1784,16 +1939,11 @@ mod tests { fn duplicate_keys() { // You cannot parse a Miniscript that has duplicate keys let err = Miniscript::::from_str("and_v(v:pk(A),pk(A))").unwrap_err(); - assert!(matches!(err, Error::AnalysisError(crate::AnalysisError::RepeatedPubkeys))); + assert!(matches!(err, Error::Validation(ValidationError::DuplicateKeys))); // ...though you can parse one with from_str_insane let ok_insane = Miniscript::::from_str_insane("and_v(v:pk(A),pk(A))").unwrap(); - // ...but this cannot be sanity checked. - assert!(matches!( - ok_insane.sanity_check().unwrap_err(), - crate::AnalysisError::RepeatedPubkeys - )); // ...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. @@ -1807,17 +1957,13 @@ mod tests { "and_v(v:and_v(v:older(4194304),pk(A)),and_v(v:older(1),pk(B)))", ) .unwrap_err(); - assert!(matches!( - err, - Error::AnalysisError(crate::AnalysisError::HeightTimelockCombination) - )); + assert!(matches!(err, Error::Validation(ValidationError::MixedTimeLocks))); // Though you can in an or() rather than and() let ok_or = Miniscript::::from_str( "or_i(and_v(v:older(4194304),pk(A)),and_v(v:older(1),pk(B)))", ) .unwrap(); - ok_or.sanity_check().unwrap(); ok_or.lift().unwrap(); // ...and you can parse one with from_str_insane @@ -1825,15 +1971,20 @@ mod tests { "and_v(v:and_v(v:older(4194304),pk(A)),and_v(v:older(1),pk(B)))", ) .unwrap(); - // ...but this cannot be sanity checked or lifted - assert_eq!( - ok_insane.sanity_check().unwrap_err(), - crate::AnalysisError::HeightTimelockCombination - ); + // ...but this cannot lifted assert!(matches!( ok_insane.lift().unwrap_err(), Error::LiftError(crate::policy::LiftError::HeightTimelockCombination) )); + // nor can it have sane rules applied to it + assert_eq!( + ok_insane.validate(&ValidationParams::SANE).unwrap_err(), + ValidationError::MixedTimeLocks, + ); + assert_eq!( + ok_insane.validate(&ValidationParams::SANE).unwrap_err(), + ValidationError::MixedTimeLocks, + ); } #[test] diff --git a/src/miniscript/types/correctness.rs b/src/miniscript/types/correctness.rs index 5db001567..06c149def 100644 --- a/src/miniscript/types/correctness.rs +++ b/src/miniscript/types/correctness.rs @@ -115,7 +115,7 @@ impl Correctness { } /// Confirm invariants of the correctness checker. - pub fn sanity_checks(&self) { + pub(crate) fn sanity_checks(&self) { match self.base { Base::B => {} Base::K => { diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 204b4bddf..b5ea9fbbb 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -198,9 +198,6 @@ impl ExtData { } impl ExtData { - /// Confirm invariants of the extra property checker. - pub fn sanity_checks(&self) {} - /// Extra properties for the `pk_k` fragment. /// /// The key must be provided to determine its size. @@ -1012,7 +1009,6 @@ impl ExtData { Self::threshold(thresh.k(), thresh.n(), |n| thresh.data()[n].ext) } }; - ret.sanity_checks(); ret } diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 674603edf..472e909c9 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -211,11 +211,12 @@ impl Type { } /// Confirm invariants of the type checker. - pub fn sanity_checks(&self) { + fn sanity_checks(&self) { debug_assert!(!self.corr.dissatisfiable || self.mall.dissat != Dissat::None); debug_assert!(self.mall.dissat == Dissat::None || self.corr.base != Base::V); debug_assert!(self.mall.safe || self.corr.base != Base::K); debug_assert!(self.mall.non_malleable || self.corr.input != Input::Zero); + self.corr.sanity_checks(); } /// Constructor for the type of the `pk_k` fragment. diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 34873e548..5a18ba990 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -251,9 +251,6 @@ impl Policy { continue; } let compilation = compiler::best_compilation::(pol)?; - compilation - .sanity_check() - .expect("compiler produces sane output"); leaf_compilations.push((OrdF64(prob), compilation)); } if !leaf_compilations.is_empty() { @@ -312,9 +309,6 @@ impl Policy { continue; } let compilation = compiler::best_compilation::(pol.as_ref())?; - compilation - .sanity_check() - .expect("compiler produces sane output"); if has_if_fragment(&compilation) { return Err(CompilerError::IfFragmentInNativeLeaf { leaf_index: leaf_idx, diff --git a/src/validation.rs b/src/validation.rs new file mode 100644 index 000000000..e88e1d80e --- /dev/null +++ b/src/validation.rs @@ -0,0 +1,415 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Validation Parameters +//! +//! Settings for determining what a "valid" Miniscript is. + +use core::fmt; +#[cfg(feature = "std")] +use std::error; + +use crate::prelude::{String, ToString as _}; + +/// Validation Parameters +#[non_exhaustive] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct ValidationParams { + /// Allow compressed 33-byte keys. + pub allow_compressed_keys: bool, + /// Allow duplicate public keys in the script. + pub allow_duplicate_keys: bool, + /// Allow the `d:` fragment. (Disallowed pre-segwit because minimality is not enforced.) + pub allow_dup_if: bool, + /// Allow scripts with 3rd-party malleability vectors. + pub allow_malleability: bool, + /// Allow the `multi` fragment. (This allowed only pre-Taproot.) + pub allow_multi: bool, + /// Allow the `multi_a` fragment. (This allowed only post-Taproot.) + pub allow_multi_a: bool, + /// Allow mixing of height-based and time-based timelocks in a given script. + pub allow_mixed_time_locks: bool, + /// Allow the `or_i` fragment. (Disallowed pre-segwit because minimality is not enforced.) + pub allow_or_i: bool, + /// Allow the `expr_raw_pkh` fragment. + pub allow_raw_pkh: bool, + /// Allow branches without any signatures (this property is referred to as `hassig` + /// in the Miniscript specification, and in very old documentation as "safety"). + pub allow_sigless_branch: bool, + /// Allow scripts that don't have the "B" type, i.e. which are not valid scripts + /// on their own. + pub allow_non_b: bool, + /// Allow 65-byte uncompressed and hybrid keys. + pub allow_uncompressed_keys: bool, + /// Allow unsatisfiable programs. + pub allow_unsatisfiable: bool, + /// Allow 32-byte x-only keys. + pub allow_x_only_keys: bool, + /// Allow multipath keys with inconsistent lengths. + pub allow_inconsistent_multipath_keys: bool, + /// Maximum number of non-push opcodes executed in any branch. + pub max_opcode_count: usize, + /// Maximum size of the encoded script. + /// + /// Depending on context, a Miniscript may be embedded in a Taproot leaf, a segwit + /// witness script, a P2SH redeem script, a bare `scriptPubKey`, or possibly some + /// future thing. This limit describes how many bytes it may be encoded as, *not* + /// including any length prefix bytes. + pub max_script_size: usize, + /// Maximum number of witness stack items. + pub max_witness_items: usize, + /// Maximum number of stack elements during script execution. + pub max_exec_stack_size: usize, + /// Maximum recursive depth allowed in the Miniscript tree. + /// + /// This value is a limitation of the library; it is not related to the maximum depth + /// of Taproot trees (which is enforced elsewhere) or anything else. We hope to relax + /// it in the future once we have eliminated all recursive structures from the library. + /// + /// See for some history of this limit. + pub max_recursive_depth: usize, +} + +impl ValidationParams { + /// A set of parameters representing "anything goes". + pub const MAX: Self = Self { + allow_compressed_keys: true, + allow_duplicate_keys: true, + allow_dup_if: true, + allow_malleability: true, + allow_mixed_time_locks: true, + allow_multi: true, + allow_multi_a: true, + allow_or_i: true, + allow_raw_pkh: true, + allow_sigless_branch: true, + allow_non_b: true, + allow_uncompressed_keys: true, + allow_unsatisfiable: true, + allow_x_only_keys: true, + allow_inconsistent_multipath_keys: true, + max_opcode_count: usize::MAX, + max_script_size: usize::MAX, + max_witness_items: usize::MAX, + max_exec_stack_size: usize::MAX, + max_recursive_depth: 402, // https://github.com/sipa/miniscript/pull/5 + }; + + /// Enforce a basic set of sanity checks. + /// + /// These limitations are those that apply to _all_ contexts. So, for example, `multi_a` + /// is allowed by this constant, and no size/opcode limits are enforced. You likely want + /// to use a context-specific constant such as `crate::Segwitv0::SANE` rather than + /// this constant. + pub const SANE: Self = Self { + allow_compressed_keys: true, + allow_duplicate_keys: false, + allow_dup_if: true, + allow_malleability: false, + allow_mixed_time_locks: false, + allow_multi: true, + allow_multi_a: true, + allow_or_i: true, + allow_raw_pkh: false, + allow_sigless_branch: false, + allow_non_b: false, + allow_uncompressed_keys: true, + // FIXME should make allow_unsatisfiable false, but for compatibility + // with existing code we leave it as true even for "sane" scripts. + allow_unsatisfiable: true, + allow_x_only_keys: true, + allow_inconsistent_multipath_keys: false, + max_opcode_count: usize::MAX, + max_script_size: usize::MAX, + max_witness_items: usize::MAX, + max_exec_stack_size: usize::MAX, + max_recursive_depth: Self::MAX.max_recursive_depth, + }; + + /// Enforce a bare minimum set of sanity checks. + /// + /// This includes any consensus limits that Miniscript understands, and + /// forbids unsatisfiable programs. If you really want to parse anything + /// that can be parsed, use [`Self::MAX`] instead. + pub const CONSENSUS: Self = Self { + allow_compressed_keys: true, + allow_duplicate_keys: true, + allow_dup_if: true, + allow_malleability: true, + allow_mixed_time_locks: true, + allow_multi: true, + allow_multi_a: true, + allow_or_i: true, + allow_raw_pkh: true, + allow_sigless_branch: true, + allow_non_b: false, + allow_uncompressed_keys: true, + // FIXME see above; we should forbid unsatisfiable programs + allow_unsatisfiable: true, + allow_x_only_keys: true, + allow_inconsistent_multipath_keys: true, + max_opcode_count: usize::MAX, + max_script_size: usize::MAX, + max_witness_items: usize::MAX, + max_exec_stack_size: usize::MAX, + max_recursive_depth: Self::MAX.max_recursive_depth, + }; + + /// Whether this set of parameters is equal to another. + /// + /// Defined as an explicit method to get `const`. + pub const fn eq(&self, other: &Self) -> bool { + self.allow_compressed_keys == other.allow_compressed_keys + && self.allow_duplicate_keys == other.allow_duplicate_keys + && self.allow_dup_if == other.allow_dup_if + && self.allow_malleability == other.allow_malleability + && self.allow_mixed_time_locks == other.allow_mixed_time_locks + && self.allow_multi == other.allow_multi + && self.allow_multi_a == other.allow_multi_a + && self.allow_or_i == other.allow_or_i + && self.allow_raw_pkh == other.allow_raw_pkh + && self.allow_sigless_branch == other.allow_sigless_branch + && self.allow_non_b == other.allow_non_b + && self.allow_uncompressed_keys == other.allow_uncompressed_keys + && self.allow_unsatisfiable == other.allow_unsatisfiable + && self.allow_x_only_keys == other.allow_x_only_keys + && self.allow_inconsistent_multipath_keys == other.allow_inconsistent_multipath_keys + && self.max_opcode_count == other.max_opcode_count + && self.max_script_size == other.max_script_size + && self.max_witness_items == other.max_witness_items + && self.max_exec_stack_size == other.max_exec_stack_size + && self.max_recursive_depth == other.max_recursive_depth + } + + /// Whether a script satisfying this set of parameters must also satisfy the + /// other set of parameters. + pub const fn entails(&self, other: &Self) -> bool { self.intersect(other).eq(self) } + + /// Computes the intersection of two sets of validation parameters. + pub const fn intersect(&self, other: &Self) -> Self { + ValidationParams { + allow_compressed_keys: self.allow_compressed_keys && other.allow_compressed_keys, + allow_duplicate_keys: self.allow_duplicate_keys && other.allow_duplicate_keys, + allow_dup_if: self.allow_dup_if && other.allow_dup_if, + allow_malleability: self.allow_malleability && other.allow_malleability, + allow_mixed_time_locks: self.allow_mixed_time_locks && other.allow_mixed_time_locks, + allow_multi: self.allow_multi && other.allow_multi, + allow_multi_a: self.allow_multi_a && other.allow_multi_a, + allow_or_i: self.allow_or_i && other.allow_or_i, + allow_raw_pkh: self.allow_raw_pkh && other.allow_raw_pkh, + allow_sigless_branch: self.allow_sigless_branch && other.allow_sigless_branch, + allow_non_b: self.allow_non_b && other.allow_non_b, + allow_uncompressed_keys: self.allow_uncompressed_keys && other.allow_uncompressed_keys, + allow_unsatisfiable: self.allow_unsatisfiable && other.allow_unsatisfiable, + allow_x_only_keys: self.allow_x_only_keys && other.allow_x_only_keys, + allow_inconsistent_multipath_keys: self.allow_inconsistent_multipath_keys + && other.allow_inconsistent_multipath_keys, + // cannot use cmp::min in const ctx + max_opcode_count: if self.max_opcode_count < other.max_opcode_count { + self.max_opcode_count + } else { + other.max_opcode_count + }, + max_script_size: if self.max_script_size < other.max_script_size { + self.max_script_size + } else { + other.max_script_size + }, + max_witness_items: if self.max_witness_items < other.max_witness_items { + self.max_witness_items + } else { + other.max_witness_items + }, + max_exec_stack_size: if self.max_exec_stack_size < other.max_exec_stack_size { + self.max_exec_stack_size + } else { + other.max_exec_stack_size + }, + max_recursive_depth: if self.max_recursive_depth < other.max_recursive_depth { + self.max_recursive_depth + } else { + other.max_recursive_depth + }, + } + } + + /// Validates a single key against the validation parameters. + pub fn validate_pk(&self, key: &Pk) -> Result<(), KeyError> + where + Pk: crate::MiniscriptKey, + { + // Compressed keys are allowed if -either- compressed keys or x-only keys are allowed: + // for purposes of validating a constructed Miniscript, we treat compressed keys as + // x-only ones, and when encoding them we will just drop their parity. (When decoding + // a Miniscript from Script, we are strict and do not allow compressed keys in place + // of x-only ones. But there we have explicit logic, rather than using this function + // for rejection.) + if !self.allow_compressed_keys + && !self.allow_x_only_keys + && !key.is_uncompressed() + && !key.is_x_only_key() + { + return Err(KeyError::IllegalCompressedKey(key.to_string())); + } + if !self.allow_uncompressed_keys && key.is_uncompressed() { + return Err(KeyError::IllegalUncompressedKey(key.to_string())); + } + if !self.allow_x_only_keys && key.is_x_only_key() { + return Err(KeyError::IllegalXOnlyKey(key.to_string())); + } + Ok(()) + } +} + +/// A validation error +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum Error { + /// Script had duplicate public keys. + DuplicateKeys, + /// Script had a `d:` fragment in a context it was not allowed. + IllegalDupIf, + /// Taproot script had a `multi` fragment + IllegalMulti, + /// Non-Taproot script had a `multi_a` fragment + IllegalMultiA, + /// Script had a `or_i` fragment in a context it was not allowed. + IllegalOrI, + /// Script contains an `expr_raw_pkh` fragment. + IllegalRawPkh, + /// Malleable script + Malleable, + /// At least one satisfaction path executes more than the allowable number of + /// non-push opcodes. + #[allow(missing_docs)] + MaxOpCountExceeded { actual: usize, limit: usize }, + /// The script exceeds the maximum allowable script size. + #[allow(missing_docs)] + MaxScriptSizeExceeded { actual: usize, limit: usize }, + /// At least one satisfaction path has more than the allowable number of initial + /// witness stack elements. + #[allow(missing_docs)] + MaxWitnessItemsExceeded { actual: usize, limit: usize }, + /// At least one satisfaction path requires more than the allowable number of elements + /// to be on the stack. + #[allow(missing_docs)] + MaxExecStackSizeExceeded { actual: usize, limit: usize }, + /// A script exceeded the maximum allowable recursive depth for a Miniscript tree. + /// + /// This error triggers as soon as the limit is exceeded, so we do not know the actual + /// depth of the script, only that it is in excess of the limit. + #[allow(missing_docs)] + MaxRecursiveDepthExceeded { limit: usize }, + /// Script had a branch which required a height-based and time-based timelock simultaneously. + MixedTimeLocks, + /// Two multipath keys were present, which had a different number of lengths. + #[allow(missing_docs)] + MultipathKeyLenMismatch { len1: usize, len2: usize }, + /// Top-level script did not have the "B" type, i.e. it is not valid as a standalone script. + NonBase(crate::miniscript::types::Base), + /// Script contains some branch that does not require any signatures. + /// + /// This is both directly dangerous (it likely indicates an "anyone-can-spend" output) + /// and presents a malleability vector, since it allows 3rd parties to modify parts of + /// the transaction such as locktimes or sequence numbers. + SiglessBranch, + /// Illegal key type (e.g. x-only key outside of Taproot). + Key(KeyError), + /// The Miniscript cannot be satisfied. + Unsatisfiable, +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Error::DuplicateKeys => f.write_str("duplicate public keys"), + Error::IllegalDupIf => f.write_str("legacy script with a `dup_if` fragment"), + Error::IllegalMulti => f.write_str("non-Taproot script with a `multi_a` fragment"), + Error::IllegalMultiA => f.write_str("non-Taproot script with a `multi_a` fragment"), + Error::IllegalOrI => f.write_str("legacy script with a `or_i` fragment"), + Error::IllegalRawPkh => f.write_str("script with a `expr_raw_pkh` fragment"), + Error::Malleable => f.write_str("script has a 3rd-party malleability vector"), + Error::MaxOpCountExceeded { actual, limit } => write!( + f, + "a satisfaction path executes at least {} non-push opcodes (limit: {}).", + actual, limit + ), + Error::MaxScriptSizeExceeded { actual, limit } => write!( + f, + "script has size at least {} (limit: {}).", + actual, limit + ), + Error::MaxWitnessItemsExceeded { actual, limit } => write!( + f, + "a satisfaction path requires at least {} witness items (limit: {}).", + actual, limit + ), + Error::MaxExecStackSizeExceeded { actual, limit } => write!( + f, + "a satisfaction path requires at least {} items on the stack at once (limit: {}).", + actual, limit + ), + Error::MaxRecursiveDepthExceeded { limit } => write!( + f, + "script exceeded the maximum recursive depth limit of {}; if you have a use case for deeper scripts, please file an issue against rust-miniscript", + limit + ), + Error::MixedTimeLocks => f.write_str("mixed a height-based and time-based timelock"), + Error::MultipathKeyLenMismatch { len1, len2 } => write!(f, "found multipath keys with lengths {} and {}; lengths must match", len1, len2), + Error::NonBase(base) => write!(f, "script has type {:?}, which is not allowed for a top-level Miniscript", base), + Error::SiglessBranch => f.write_str("all spending paths must require a signature"), + Error::Key(ref e) => e.fmt(f), + Error::Unsatisfiable => f.write_str("no satisfaction exists for script"), + } + } +} + +#[cfg(feature = "std")] +impl error::Error for Error { + fn cause(&self) -> Option<&dyn error::Error> { + match *self { + Self::Key(ref e) => Some(e), + _ => None, + } + } +} + +impl From for Error { + fn from(i: core::convert::Infallible) -> Self { match i {} } +} + +/// A validation error +#[allow(clippy::enum_variant_names)] // clippy doesn't like the Illegal* prefix +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum KeyError { + /// A compressed key appeared in a context it was not allowed. + IllegalCompressedKey(String), + /// An uncompressed key appeared in a context it was not allowed. + IllegalUncompressedKey(String), + /// An x-only key appeared in a context it was not allowed. + IllegalXOnlyKey(String), +} + +impl fmt::Display for KeyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Self::IllegalCompressedKey(ref s) => write!(f, "compressed key `{}` not allowed", s), + Self::IllegalUncompressedKey(ref s) => write!(f, "compressed key `{}` not allowed", s), + Self::IllegalXOnlyKey(ref s) => write!(f, "compressed key `{}` not allowed", s), + } + } +} + +#[cfg(feature = "std")] +impl error::Error for KeyError { + fn cause(&self) -> Option<&dyn error::Error> { None } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn entailment() { + assert!(ValidationParams::SANE.entails(&ValidationParams::CONSENSUS)); + assert!(!ValidationParams::CONSENSUS.entails(&ValidationParams::SANE)); + } +}