Introduce ValidationParams structure; use in Miniscript constructors

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

examples/big.rs

@@ -75,7 +75,6 @@ fn use_descriptor<K: MiniscriptKey>(d: Descriptor<K>) {
     println!("{}", d);
     println!("{:?}", d);
     println!("{:?}", d.desc_type());
-    println!("{:?}", d.sanity_check());
 }
 
 struct StrPkTranslator {

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"

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!(

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::<bitcoin::PublicKey>::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!(

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);

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();
     }
 }
 

src/descriptor/bare.rs

@@ -47,12 +47,6 @@ impl<Pk: MiniscriptKey> Bare<Pk> {
     /// get the inner
     pub fn as_inner(&self) -> &Miniscript<Pk, BareCtx> { &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`
     ///

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<Pk: MiniscriptKey> Descriptor<Pk> {
         }
     }
 
-    /// 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<Pk: FromStrKey> FromStr for Descriptor<Pk> {
         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)

src/descriptor/segwitv0.rs

@@ -53,12 +53,6 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
     #[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<Pk: MiniscriptKey> Wpkh<Pk> {
     #[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`
     ///

src/descriptor/sh.rs

@@ -132,16 +132,6 @@ impl<Pk: MiniscriptKey> Sh<Pk> {
     /// Create a new p2sh wrapper for the given wsh descriptor
     pub fn new_with_wsh(wsh: Wsh<Pk>) -> 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(

src/descriptor/tr/mod.rs

@@ -139,14 +139,6 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
         }
     }
 
-    /// 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<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
             } 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();

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<BitcoinKey, NoChecks> {
         // Parsing should allow raw hashes in the interpreter
-        let elem: Miniscript<bitcoin::PublicKey, NoChecks> =
-            Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap();
+        let elem: Miniscript<bitcoin::PublicKey, NoChecks> = ms.parse().unwrap();
         elem.to_no_checks_ms()
     }
 
     fn x_only_no_checks_ms(ms: &str) -> Miniscript<BitcoinKey, NoChecks> {
-        let elem: Miniscript<bitcoin::key::XOnlyPublicKey, NoChecks> =
-            Miniscript::from_str_ext(ms, &ExtParams::allow_all()).unwrap();
+        let elem: Miniscript<bitcoin::XOnlyPublicKey, NoChecks> = ms.parse().unwrap();
         elem.to_no_checks_ms()
     }
 }

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 <OP_0 OP_32 <32-byte-hash>>
 //! // = (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<miniscript::context::ScriptContextError> for Error {
     fn from(e: miniscript::context::ScriptContextError) -> Error { Error::ContextError(e) }
 }
 
-#[doc(hidden)]
-impl From<miniscript::analyzable::AnalysisError> for Error {
-    fn from(e: miniscript::analyzable::AnalysisError) -> Error { Error::AnalysisError(e) }
-}
-
 #[doc(hidden)]
 impl From<bitcoin::secp256k1::Error> for Error {
     fn from(e: bitcoin::secp256k1::Error) -> Error { Error::Secp(e) }

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