Updated Bare descriptor to use Bare Ctx

Updated Bare descriptor to user Bare context
for standardness checks instead of previously
incorrect Legacy context

Combine top level checks

Author: sanket1729

src/descriptor/create_descriptor.rs

@@ -9,7 +9,7 @@ use bitcoin::blockdata::script::Instruction;
 use descriptor::satisfied_constraints::Error as IntError;
 use descriptor::satisfied_constraints::{Stack, StackElement};
 use descriptor::Descriptor;
-use miniscript::{Legacy, Miniscript, Segwitv0};
+use miniscript::{Bare, Legacy, Miniscript, Segwitv0};
 use Error;
 use ToPublicKey;
 
@@ -233,7 +233,7 @@ pub fn from_txin_with_witness_stack<'txin>(
         if !witness.is_empty() {
             return Err(Error::NonEmptyWitness);
         }
-        let ms = Miniscript::<bitcoin::PublicKey, Legacy>::parse(script_pubkey)?;
+        let ms = Miniscript::<bitcoin::PublicKey, Bare>::parse(script_pubkey)?;
         Ok((Descriptor::Bare(ms), Stack(stack?)))
     }
 }

src/descriptor/mod.rs

@@ -37,13 +37,12 @@ use bitcoin::{self, Script};
 use serde::{de, ser};
 
 use expression;
-use miniscript;
-use miniscript::context::ScriptContextError;
-use miniscript::{Legacy, Miniscript, Segwitv0};
+use miniscript::context::{ScriptContext, ScriptContextError};
 use Error;
 use MiniscriptKey;
 use Satisfier;
 use ToPublicKey;
+use {Bare, Legacy, Miniscript, Segwitv0};
 
 mod create_descriptor;
 mod satisfied_constraints;
@@ -58,7 +57,7 @@ pub use self::satisfied_constraints::Stack;
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum Descriptor<Pk: MiniscriptKey> {
     /// A raw scriptpubkey (including pay-to-pubkey) under Legacy context
-    Bare(Miniscript<Pk, Legacy>),
+    Bare(Miniscript<Pk, Bare>),
     /// Pay-to-Pubkey
     Pk(Pk),
     /// Pay-to-PubKey-Hash
@@ -747,11 +746,8 @@ where
                 match (newtop.name, newtop.args.len()) {
                     ("wsh", 1) => {
                         let sub = Miniscript::from_tree(&newtop.args[0])?;
-                        if sub.ty.corr.base != miniscript::types::Base::B {
-                            Err(Error::NonTopLevel(format!("{:?}", sub)))
-                        } else {
-                            Ok(Descriptor::ShWsh(sub))
-                        }
+                        Segwitv0::top_level_checks(&sub)?;
+                        Ok(Descriptor::ShWsh(sub))
                     }
                     ("wpkh", 1) => {
                         let wpkh = expression::terminal(&newtop.args[0], |pk| Pk::from_str(pk))?;
@@ -763,29 +759,20 @@ where
                     }
                     _ => {
                         let sub = Miniscript::from_tree(&top.args[0])?;
-                        if sub.ty.corr.base != miniscript::types::Base::B {
-                            Err(Error::NonTopLevel(format!("{:?}", sub)))
-                        } else {
-                            Ok(Descriptor::Sh(sub))
-                        }
+                        Legacy::top_level_checks(&sub)?;
+                        Ok(Descriptor::Sh(sub))
                     }
                 }
             }
             ("wsh", 1) => {
                 let sub = Miniscript::from_tree(&top.args[0])?;
-                if sub.ty.corr.base != miniscript::types::Base::B {
-                    Err(Error::NonTopLevel(format!("{:?}", sub)))
-                } else {
-                    Ok(Descriptor::Wsh(sub))
-                }
+                Segwitv0::top_level_checks(&sub)?;
+                Ok(Descriptor::Wsh(sub))
             }
             _ => {
                 let sub = Miniscript::from_tree(&top)?;
-                if sub.ty.corr.base != miniscript::types::Base::B {
-                    Err(Error::NonTopLevel(format!("{:?}", sub)))
-                } else {
-                    Ok(Descriptor::Bare(sub))
-                }
+                Bare::top_level_checks(&sub)?;
+                Ok(Descriptor::Bare(sub))
             }
         }
     }
@@ -855,6 +842,7 @@ mod tests {
     use bitcoin::util::bip32;
     use bitcoin::{self, secp256k1, PublicKey};
     use descriptor::{DescriptorPublicKey, DescriptorSinglePub, DescriptorXPub};
+    use hex_script;
     use miniscript::satisfy::BitcoinSig;
     use std::collections::HashMap;
     use std::str::FromStr;
@@ -902,14 +890,25 @@ mod tests {
         StdDescriptor::from_str(&format!("sh(wpkh({}))", uncompressed_pk)).unwrap_err();
         StdDescriptor::from_str(&format!("wsh(pk{})", uncompressed_pk)).unwrap_err();
         StdDescriptor::from_str(&format!("sh(wsh(pk{}))", uncompressed_pk)).unwrap_err();
+        StdDescriptor::from_str(&format!(
+            "or_i(pk({}),pk({}))",
+            uncompressed_pk, uncompressed_pk
+        ))
+        .unwrap_err();
     }
 
     #[test]
     pub fn script_pubkey() {
-        let bare = StdDescriptor::from_str("older(1000)").unwrap();
+        let bare = StdDescriptor::from_str(&format!(
+            "multi(1,020000000000000000000000000000000000000000000000000000000000000002)"
+        ))
+        .unwrap();
+        println!("{:x}", bare.script_pubkey());
         assert_eq!(
             bare.script_pubkey(),
-            bitcoin::Script::from(vec![0x02, 0xe8, 0x03, 0xb2])
+            hex_script(
+                "512102000000000000000000000000000000000000000000000000000000000000000251ae"
+            )
         );
         assert_eq!(bare.address(bitcoin::Network::Bitcoin), None);
 
@@ -1175,6 +1174,7 @@ mod tests {
                 .into_script()
         );
 
+        let ms = ms_str!("c:pk_k({})", pk);
         let sh = Descriptor::Sh(ms.clone());
         sh.satisfy(&mut txin, &satisfier).expect("satisfaction");
         assert_eq!(

src/descriptor/satisfied_constraints.rs

@@ -309,21 +309,32 @@ where
                     has_errored: false,
                 }
             }
-            &Descriptor::Sh(ref miniscript) | &Descriptor::Bare(ref miniscript) => {
-                SatisfiedConstraints {
-                    verify_sig: verify_sig,
-                    public_key: None,
-                    state: vec![NodeEvaluationState {
-                        node: Any::from_legacy(miniscript),
-                        n_evaluated: 0,
-                        n_satisfied: 0,
-                    }],
-                    stack: stack,
-                    age,
-                    height,
-                    has_errored: false,
-                }
-            }
+            &Descriptor::Sh(ref miniscript) => SatisfiedConstraints {
+                verify_sig: verify_sig,
+                public_key: None,
+                state: vec![NodeEvaluationState {
+                    node: Any::from_legacy(miniscript),
+                    n_evaluated: 0,
+                    n_satisfied: 0,
+                }],
+                stack: stack,
+                age,
+                height,
+                has_errored: false,
+            },
+            &Descriptor::Bare(ref miniscript) => SatisfiedConstraints {
+                verify_sig: verify_sig,
+                public_key: None,
+                state: vec![NodeEvaluationState {
+                    node: Any::from_bare(miniscript),
+                    n_evaluated: 0,
+                    n_satisfied: 0,
+                }],
+                stack: stack,
+                age,
+                height,
+                has_errored: false,
+            },
         }
     }
 }

src/lib.rs

@@ -347,6 +347,9 @@ pub enum Error {
     MaxRecursiveDepthExceeded,
     /// Script size too large
     ScriptSizeTooLarge,
+    /// 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,
 }
 
 #[doc(hidden)]
@@ -458,6 +461,12 @@ impl fmt::Display for Error {
                 "Standardness rules imply bitcoin than {} bytes",
                 MAX_SCRIPT_SIZE
             ),
+            Error::NonStandardBareScript => write!(
+                f,
+                "Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) \
+                up to n=3 is invalid by standardness (bare).
+                "
+            ),
         }
     }
 }

src/miniscript/context.rs

@@ -12,13 +12,14 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
+use miniscript::types;
 use miniscript::types::extra_props::{
     MAX_OPS_PER_SCRIPT, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE,
     MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS,
 };
 use std::fmt;
+use Error;
 use {Miniscript, MiniscriptKey, Terminal};
-
 /// Error for Script Context
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum ScriptContextError {
@@ -49,9 +50,6 @@ pub enum ScriptContextError {
     MaxRedeemScriptSizeExceeded,
     /// The policy rules of bitcoin core only permit Script size upto 1650 bytes
     MaxScriptSigSizeExceeded,
-    /// 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,
 }
 
 impl fmt::Display for ScriptContextError {
@@ -90,12 +88,6 @@ impl fmt::Display for ScriptContextError {
                 "Atleast one satisfaction in Miniscript would be larger than \
                 MAX_SCRIPTSIG_SIZE scriptsig"
             ),
-            ScriptContextError::NonStandardBareScript => write!(
-                f,
-                "Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) \
-                up to n=3 is invalid by standardness (bare).
-                "
-            ),
         }
     }
 }
@@ -137,8 +129,6 @@ pub trait ScriptContext:
     /// it is safe to discard them. (unless explicity disabled by non-standard flag)
     /// For example, in Segwit Context with MiniscriptKey as bitcoin::PublicKey
     /// scripts over 3600 bytes are invalid.
-    /// In Bare context, only c:pk(key) (P2PK),
-    /// c:pk_h(key) (P2PKH), and thresh_m(k,...) up to n=3 are allowed
     /// Post Tapscript upgrade, this would have to consider other nodes.
     /// This does *NOT* recursively check the miniscript fragments.
     fn check_non_satisfaction_policy_rules<Pk: MiniscriptKey, Ctx: ScriptContext>(
@@ -189,6 +179,39 @@ pub trait ScriptContext:
         Self::check_satisfaction_policy_rules(ms)?;
         Ok(())
     }
+
+    /// Check whether the top-level is type B
+    fn top_level_type_check<Pk: MiniscriptKey, Ctx: ScriptContext>(
+        ms: &Miniscript<Pk, Ctx>,
+    ) -> Result<(), Error> {
+        if ms.ty.corr.base != types::Base::B {
+            return Err(Error::NonTopLevel(format!("{:?}", ms)));
+        }
+        Ok(())
+    }
+
+    /// Other top level checks that are context specific
+    fn other_top_level_checks<Pk: MiniscriptKey, Ctx: ScriptContext>(
+        _ms: &Miniscript<Pk, Ctx>,
+    ) -> Result<(), Error> {
+        Ok(())
+    }
+
+    /// Check top level consensus rules.
+    // All the preivous check_ were applied at each fragment while parsing script
+    // Because if any of sub-miniscripts failed the reource level check, the entire
+    // miniscript would also be invalid. However, there are certain checks like
+    // in Bare context, only c:pk(key) (P2PK),
+    // c:pk_h(key) (P2PKH), and thresh_m(k,...) up to n=3 are allowed
+    // that are only applicable at the top-level
+    // We can also combine the top-level check for Base::B here
+    // even though it does not depend on context, but helps in cleaner code
+    fn top_level_checks<Pk: MiniscriptKey, Ctx: ScriptContext>(
+        ms: &Miniscript<Pk, Ctx>,
+    ) -> Result<(), Error> {
+        Self::top_level_type_check(ms)?;
+        Self::other_top_level_checks(ms)
+    }
 }
 
 /// Legacy ScriptContext
@@ -345,14 +368,17 @@ impl ScriptContext for Bare {
         Ok(())
     }
 
-    fn check_non_satisfaction_policy_rules<Pk: MiniscriptKey, Ctx: ScriptContext>(
+    fn other_top_level_checks<Pk: MiniscriptKey, Ctx: ScriptContext>(
         ms: &Miniscript<Pk, Ctx>,
-    ) -> Result<(), ScriptContextError> {
+    ) -> Result<(), Error> {
         match &ms.node {
-            Terminal::PkH(_pkh) => Ok(()),
-            Terminal::PkK(_pk) => Ok(()),
+            Terminal::Check(ref ms) => match &ms.node {
+                Terminal::PkH(_pkh) => Ok(()),
+                Terminal::PkK(_pk) => Ok(()),
+                _ => Err(Error::NonStandardBareScript),
+            },
             Terminal::Multi(_k, subs) if subs.len() <= 3 => Ok(()),
-            _ => Err(ScriptContextError::NonStandardBareScript),
+            _ => Err(Error::NonStandardBareScript),
         }
     }
 }

src/miniscript/mod.rs

@@ -30,8 +30,7 @@ use std::{fmt, str};
 use bitcoin;
 use bitcoin::blockdata::script;
 
-pub use self::context::Legacy;
-pub use self::context::Segwitv0;
+pub use self::context::{Bare, Legacy, Segwitv0};
 
 pub mod astelem;
 pub(crate) mod context;

src/policy/mod.rs

@@ -119,7 +119,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Liftable<Pk> for Terminal<Pk, Ctx> {
 impl<Pk: MiniscriptKey> Liftable<Pk> for Descriptor<Pk> {
     fn lift(&self) -> Result<Semantic<Pk>, Error> {
         Ok(match *self {
-            Descriptor::Bare(ref d) | Descriptor::Sh(ref d) => d.node.lift()?,
+            Descriptor::Bare(ref d) => d.node.lift()?,
+            Descriptor::Sh(ref d) => d.node.lift()?,
             Descriptor::Wsh(ref d) | Descriptor::ShWsh(ref d) => d.node.lift()?,
             Descriptor::Pk(ref p)
             | Descriptor::Pkh(ref p)

src/psbt/finalizer.rs

@@ -27,9 +27,8 @@ use bitcoin::util::bip143::SigHashCache;
 use bitcoin::{self, PublicKey, Script, SigHashType};
 use descriptor::{from_txin_with_witness_stack, SatisfiedConstraints};
 use Descriptor;
-use Legacy;
 use Miniscript;
-use Segwitv0;
+use {Bare, Legacy, Segwitv0};
 // Get the scriptpubkey for the psbt input
 fn get_scriptpubkey(psbt: &Psbt, index: usize) -> Result<&Script, InputError> {
     let script_pubkey;
@@ -188,7 +187,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
         if inp.redeem_script.is_some() {
             return Err(InputError::NonEmptyRedeemScript);
         }
-        let ms = Miniscript::<bitcoin::PublicKey, Legacy>::parse(script_pubkey)?;
+        let ms = Miniscript::<bitcoin::PublicKey, Bare>::parse(script_pubkey)?;
         Ok(Descriptor::Bare(ms))
     }
 }