Add ToNoChecks trait

Rename TaggedHash160 -> TypedHash160

Author: sanket1729

src/interpreter/inner.rs

@@ -20,7 +20,7 @@ use bitcoin::util::taproot::{ControlBlock, TAPROOT_ANNEX_PREFIX};
 
 use {BareCtx, Legacy, Segwitv0, Tap};
 
-use super::{stack, BitcoinKey, Error, Stack, TaggedHash160};
+use super::{stack, BitcoinKey, Error, Stack, TypedHash160};
 use miniscript::context::{NoChecks, ScriptContext};
 use {Miniscript, MiniscriptKey};
 
@@ -186,7 +186,7 @@ pub(super) fn from_txdata<'txin>(
                 Some(elem) => {
                     let miniscript = script_from_stackelem::<Segwitv0>(&elem)?;
                     let script = miniscript.encode();
-                    let miniscript = pre_taproot_to_no_checks(&miniscript);
+                    let miniscript = miniscript.to_no_checks_ms();
                     let scripthash = sha256::Hash::hash(&script[..]);
                     if *spk == bitcoin::Script::new_v0_p2wsh(&scripthash.into()) {
                         Ok((
@@ -230,7 +230,7 @@ pub(super) fn from_txdata<'txin>(
                     let ctrl_blk = ControlBlock::from_slice(ctrl_blk)
                         .map_err(|e| Error::ControlBlockParse(e))?;
                     let tap_script = script_from_stackelem::<Tap>(&tap_script)?;
-                    let ms = taproot_to_no_checks(&tap_script);
+                    let ms = tap_script.to_no_checks_ms();
                     // Creating new contexts is cheap
                     let secp = bitcoin::secp256k1::Secp256k1::verification_only();
                     let tap_script = tap_script.encode();
@@ -306,7 +306,7 @@ pub(super) fn from_txdata<'txin>(
                                     // parse wsh with Segwitv0 context
                                     let miniscript = script_from_stackelem::<Segwitv0>(&elem)?;
                                     let script = miniscript.encode();
-                                    let miniscript = pre_taproot_to_no_checks(&miniscript);
+                                    let miniscript = miniscript.to_no_checks_ms();
                                     let scripthash = sha256::Hash::hash(&script[..]);
                                     if slice
                                         == &bitcoin::Script::new_v0_p2wsh(&scripthash.into())[..]
@@ -328,7 +328,7 @@ pub(super) fn from_txdata<'txin>(
                 // normal p2sh parsed in Legacy context
                 let miniscript = script_from_stackelem::<Legacy>(&elem)?;
                 let script = miniscript.encode();
-                let miniscript = pre_taproot_to_no_checks(&miniscript);
+                let miniscript = miniscript.to_no_checks_ms();
                 if wit_stack.is_empty() {
                     let scripthash = hash160::Hash::hash(&script[..]);
                     if *spk == bitcoin::Script::new_p2sh(&scripthash.into()) {
@@ -351,7 +351,7 @@ pub(super) fn from_txdata<'txin>(
         if wit_stack.is_empty() {
             // Bare script parsed in BareCtx
             let miniscript = Miniscript::<bitcoin::PublicKey, BareCtx>::parse_insane(spk)?;
-            let miniscript = pre_taproot_to_no_checks(&miniscript);
+            let miniscript = miniscript.to_no_checks_ms();
             Ok((
                 Inner::Script(miniscript, ScriptType::Bare),
                 ssig_stack,
@@ -363,29 +363,39 @@ pub(super) fn from_txdata<'txin>(
     }
 }
 
-// Convert a miniscript from a specified context into an insane miniscript
-// We need to remember how the hash160 was translated because while doing a checksig
-// we need to know whether to parse the public key provided in witness as x-only or full
-pub(super) fn pre_taproot_to_no_checks<Ctx: ScriptContext>(
-    ms: &Miniscript<bitcoin::PublicKey, Ctx>,
-) -> Miniscript<BitcoinKey, NoChecks> {
-    // specify the () error type as this cannot error
-    ms.real_translate_pk::<_, _, _, (), _>(&mut |pk| Ok(BitcoinKey::Fullkey(*pk)), &mut |pkh| {
-        Ok(TaggedHash160::FullKey(*pkh))
-    })
-    .expect("Translation should succeed")
+// Convert a miniscript from a well-defined context to a no checks context.
+// We need to parse insane scripts because these scripts are obtained from already
+// created transaction possibly already confirmed in a block.
+// In order to avoid code duplication for various contexts related interpreter checks,
+// we convert all the scripts to from a well-defined context to NoContexts.
+//
+// While executing Pkh(<hash>) in NoChecks, we need to pop a public key from stack
+// However, NoChecks context does not know whether to parse the key as 33 bytes or 32 bytes
+// While converting into NoChecks we store explicitly in TypedHash160 enum.
+pub(super) trait ToNoChecks {
+    fn to_no_checks_ms(&self) -> Miniscript<BitcoinKey, NoChecks>;
 }
 
-// Convert a miniscript from a specified context into an insane miniscript
-pub(super) fn taproot_to_no_checks<Ctx: ScriptContext>(
-    ms: &Miniscript<bitcoin::XOnlyPublicKey, Ctx>,
-) -> Miniscript<BitcoinKey, NoChecks> {
-    // specify the () error type as this cannot error
-    ms.real_translate_pk::<_, _, _, (), _>(
-        &mut |xpk| Ok(BitcoinKey::XOnlyPublicKey(*xpk)),
-        &mut |pkh| Ok(TaggedHash160::XonlyKey(*pkh)),
-    )
-    .expect("Translation should succeed")
+impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::PublicKey, Ctx> {
+    fn to_no_checks_ms(&self) -> Miniscript<BitcoinKey, NoChecks> {
+        // specify the () error type as this cannot error
+        self.real_translate_pk::<_, _, _, (), _>(
+            &mut |pk| Ok(BitcoinKey::Fullkey(*pk)),
+            &mut |pkh| Ok(TypedHash160::FullKey(*pkh)),
+        )
+        .expect("Translation should succeed")
+    }
+}
+
+impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::XOnlyPublicKey, Ctx> {
+    fn to_no_checks_ms(&self) -> Miniscript<BitcoinKey, NoChecks> {
+        // specify the () error type as this cannot error
+        self.real_translate_pk::<_, _, _, (), _>(
+            &mut |xpk| Ok(BitcoinKey::XOnlyPublicKey(*xpk)),
+            &mut |pkh| Ok(TypedHash160::XonlyKey(*pkh)),
+        )
+        .expect("Translation should succeed")
+    }
 }
 
 #[cfg(test)]
@@ -737,7 +747,7 @@ mod tests {
     fn ms_inner_script(ms: &str) -> (Miniscript<BitcoinKey, NoChecks>, bitcoin::Script) {
         let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::from_str_insane(ms).unwrap();
         let spk = ms.encode();
-        let miniscript = pre_taproot_to_no_checks(&ms);
+        let miniscript = ms.to_no_checks_ms();
         (miniscript, spk)
     }
     #[test]

src/interpreter/mod.rs

@@ -91,7 +91,7 @@ impl KeySigPair {
 // - This does not implement ToPublicKey to avoid context dependant encoding/decoding of 33/32
 //   byte keys. This allows us to keep a single NoChecks context instead of a context for
 //   for NoChecksSchnorr/NoChecksEcdsa.
-// Long term TODO: There really should be any need for Miniscript<Pk: MiniscriptKey> struct
+// Long term TODO: There really should be not be any need for Miniscript<Pk: MiniscriptKey> struct
 // to have the Pk: MiniscriptKey bound. The bound should be on all of it's methods. That would
 // require changing Miniscript struct to three generics Miniscript<Pk, Pkh, Ctx> and bound on
 // all of the methods of Miniscript to ensure that Pkh = Pk::Hash
@@ -127,34 +127,34 @@ impl From<bitcoin::XOnlyPublicKey> for BitcoinKey {
 
 // While parsing we need to remember how to the hash was parsed so that we can
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-enum TaggedHash160 {
+enum TypedHash160 {
     XonlyKey(hash160::Hash),
     FullKey(hash160::Hash),
 }
 
-impl fmt::Display for TaggedHash160 {
+impl fmt::Display for TypedHash160 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            TaggedHash160::FullKey(pkh) | TaggedHash160::XonlyKey(pkh) => pkh.fmt(f),
+            TypedHash160::FullKey(pkh) | TypedHash160::XonlyKey(pkh) => pkh.fmt(f),
         }
     }
 }
 
-impl TaggedHash160 {
+impl TypedHash160 {
     fn hash160(&self) -> hash160::Hash {
         match self {
-            TaggedHash160::XonlyKey(hash) | TaggedHash160::FullKey(hash) => *hash,
+            TypedHash160::XonlyKey(hash) | TypedHash160::FullKey(hash) => *hash,
         }
     }
 }
 
 impl MiniscriptKey for BitcoinKey {
-    type Hash = TaggedHash160;
+    type Hash = TypedHash160;
 
     fn to_pubkeyhash(&self) -> Self::Hash {
         match self {
-            BitcoinKey::Fullkey(pk) => TaggedHash160::FullKey(pk.to_pubkeyhash()),
-            BitcoinKey::XOnlyPublicKey(pk) => TaggedHash160::XonlyKey(pk.to_pubkeyhash()),
+            BitcoinKey::Fullkey(pk) => TypedHash160::FullKey(pk.to_pubkeyhash()),
+            BitcoinKey::XOnlyPublicKey(pk) => TypedHash160::XonlyKey(pk.to_pubkeyhash()),
         }
     }
 }
@@ -988,6 +988,7 @@ fn verify_sersig<'txin>(
 #[cfg(test)]
 mod tests {
 
+    use super::inner::ToNoChecks;
     use super::*;
     use bitcoin;
     use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d, Hash};
@@ -1460,6 +1461,6 @@ mod tests {
     fn no_checks_ms(ms: &str) -> Miniscript<BitcoinKey, NoChecks> {
         let elem: Miniscript<bitcoin::PublicKey, NoChecks> =
             Miniscript::from_str_insane(ms).unwrap();
-        inner::pre_taproot_to_no_checks(&elem)
+        elem.to_no_checks_ms()
     }
 }

src/interpreter/stack.rs

@@ -20,7 +20,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d, Hash};
 
 use super::error::PkEvalErrInner;
 use super::{
-    verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint, TaggedHash160,
+    verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint, TypedHash160,
 };
 
 /// Definition of Stack Element of the Stack used for interpretation of Miniscript.
@@ -175,17 +175,17 @@ impl<'txin> Stack<'txin> {
     pub(super) fn evaluate_pkh<'intp>(
         &mut self,
         verify_sig: &mut Box<dyn FnMut(&KeySigPair) -> bool + 'intp>,
-        pkh: &'intp TaggedHash160,
+        pkh: &'intp TypedHash160,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
         // Parse a bitcoin key from witness data slice depending on hash context
         // when we encounter a pkh(hash)
         // Depending on the tag of hash, we parse the as full key or x-only-key
         // TODO: All keys parse errors are currently captured in a single BadPubErr
         // We don't really store information about which key error.
-        fn bitcoin_key_from_slice(sl: &[u8], tag: TaggedHash160) -> Option<BitcoinKey> {
+        fn bitcoin_key_from_slice(sl: &[u8], tag: TypedHash160) -> Option<BitcoinKey> {
             let key: BitcoinKey = match tag {
-                TaggedHash160::XonlyKey(_) => bitcoin::XOnlyPublicKey::from_slice(sl).ok()?.into(),
-                TaggedHash160::FullKey(_) => bitcoin::PublicKey::from_slice(sl).ok()?.into(),
+                TypedHash160::XonlyKey(_) => bitcoin::XOnlyPublicKey::from_slice(sl).ok()?.into(),
+                TypedHash160::FullKey(_) => bitcoin::PublicKey::from_slice(sl).ok()?.into(),
             };
             Some(key)
         }