Merge rust-bitcoin/rust-miniscript#431: Refactor PkH fragment to support Pk

8d0cf061e4fcc3f7784116e43b2d2afb712cd066 Remove `ForEach` Trait (Aman Rojjha)
5cb2bcffae4f5720868138eadae7b98de0d0a9bc Refactor PkH to include key. (Aman Rojjha)
09299b072e8bbb703406192239f67ada37d53798 Refactor ForEach to contain only Pk. (Aman Rojjha)
701c9ccd2c30d0c91e7b7130312caa5daaaba2da Remove un-necessary lifetimes (Aman Rojjha)
ca16bd86b9de807404bde882ef85801828d5b135 Fix formatter configuration errors with latest nightly (sanket1729)

Pull request description:

  As suggested in #427, this refactor helps to incorporate musig in the codebase.

ACKs for top commit:
  apoelstra:
    ACK 8d0cf061e4fcc3f7784116e43b2d2afb712cd066
  sanket1729:
    ACK 8d0cf061e4fcc3f7784116e43b2d2afb712cd066. The last commit message should be `Remove ForEach struct` instead of `Remove ForEach trait`. But won't holdoff this over that.

Tree-SHA512: 645974d9055e0458f64e214d95a73647420d46f05246be6809ae11dbfd8dacc05500c5e9f9f1b10b532b539b2d30883c014177a33e7666aea8ae383722910ff9

Author: heap-coder

rustfmt.toml

@@ -16,7 +16,6 @@ format_code_in_doc_comments = false
 comment_width = 80
 normalize_comments = false
 normalize_doc_attributes = false
-license_template_path = ""
 format_strings = false
 format_macro_matchers = false
 format_macro_bodies = true
@@ -53,7 +52,7 @@ trailing_comma = "Vertical"
 match_block_trailing_comma = false
 blank_lines_upper_bound = 1
 blank_lines_lower_bound = 0
-edition = "2015"
+edition = "2018"
 version = "One"
 inline_attribute_width = 0
 format_generated_files = true
@@ -63,15 +62,12 @@ use_field_init_shorthand = false
 force_explicit_abi = true
 condense_wildcard_suffixes = false
 color = "Auto"
-required_version = "1.4.38"
 unstable_features = false
 disable_all_formatting = false
 skip_children = false
 hide_parse_errors = false
 error_on_line_overflow = false
 error_on_unformatted = false
-report_todo = "Never"
-report_fixme = "Never"
 ignore = []
 emit_mode = "Files"
 make_backup = false

src/descriptor/bare.rs

@@ -30,8 +30,8 @@ use crate::policy::{semantic, Liftable};
 use crate::prelude::*;
 use crate::util::{varint_len, witness_to_scriptsig};
 use crate::{
-    BareCtx, Error, ForEach, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey,
-    TranslatePk, Translator,
+    BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, TranslatePk,
+    Translator,
 };
 
 /// Create a Bare Descriptor. That is descriptor that is
@@ -164,7 +164,7 @@ impl_from_str!(
 );
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Bare<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
@@ -326,12 +326,12 @@ impl_from_str!(
 );
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Pkh<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, mut pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
     {
-        pred(ForEach::Key(&self.pk))
+        pred(&self.pk)
     }
 }
 

src/descriptor/mod.rs

@@ -37,8 +37,8 @@ use self::checksum::verify_checksum;
 use crate::miniscript::{Legacy, Miniscript, Segwitv0};
 use crate::prelude::*;
 use crate::{
-    expression, miniscript, BareCtx, Error, ForEach, ForEachKey, MiniscriptKey, PkTranslator,
-    Satisfier, ToPublicKey, TranslatePk, Translator,
+    expression, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, PkTranslator, Satisfier,
+    ToPublicKey, TranslatePk, Translator,
 };
 
 mod bare;
@@ -495,7 +495,7 @@ where
 }
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Descriptor<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
@@ -514,7 +514,7 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Descriptor<Pk> {
 impl Descriptor<DescriptorPublicKey> {
     /// Whether or not the descriptor has any wildcards
     pub fn is_deriveable(&self) -> bool {
-        self.for_any_key(|key| key.as_key().is_deriveable())
+        self.for_any_key(|key| key.is_deriveable())
     }
 
     /// Derives all wildcard keys in the descriptor using the supplied index
@@ -1253,19 +1253,20 @@ mod tests {
         let p2 = "020000000000000000000000000000000000000000000000000000000000000002";
         let p3 = "020000000000000000000000000000000000000000000000000000000000000003";
         let p4 = "020000000000000000000000000000000000000000000000000000000000000004";
-        let p5 = "f54a5851e9372b87810a8e60cdd2e7cfd80b6e31";
+        let p5 = "03f8551772d66557da28c1de858124f365a8eb30ce6ad79c10e0f4c546d0ab0f82";
         let descriptor = Tr::<PublicKey>::from_str(&format!(
             "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})",
             p1, p2, p3, p4, p5
         ))
         .unwrap()
         .to_string();
 
+        // p5.to_pubkeyhash() = 516ca378e588a7ed71336147e2a72848b20aca1a
         assert_eq!(
             descriptor,
             format!(
-                "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})#fdhmu4fj",
-                p1, p2, p3, p4, p5
+                "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh(516ca378e588a7ed71336147e2a72848b20aca1a))}}}})#xz8ny8ae",
+                p1, p2, p3, p4,
             )
         )
     }

src/descriptor/segwitv0.rs

@@ -28,8 +28,8 @@ use crate::policy::{semantic, Liftable};
 use crate::prelude::*;
 use crate::util::varint_len;
 use crate::{
-    Error, ForEach, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey,
-    TranslatePk, Translator,
+    Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, TranslatePk,
+    Translator,
 };
 /// A Segwitv0 wsh descriptor
 #[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
@@ -246,7 +246,7 @@ impl_from_str!(
 );
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wsh<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
@@ -439,12 +439,12 @@ impl_from_str!(
 );
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wpkh<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, mut pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
     {
-        pred(ForEach::Key(&self.pk))
+        pred(&self.pk)
     }
 }
 

src/descriptor/sh.rs

@@ -31,8 +31,8 @@ use crate::policy::{semantic, Liftable};
 use crate::prelude::*;
 use crate::util::{varint_len, witness_to_scriptsig};
 use crate::{
-    push_opcode_size, Error, ForEach, ForEachKey, Legacy, Miniscript, MiniscriptKey, Satisfier,
-    Segwitv0, ToPublicKey, TranslatePk, Translator,
+    push_opcode_size, Error, ForEachKey, Legacy, Miniscript, MiniscriptKey, Satisfier, Segwitv0,
+    ToPublicKey, TranslatePk, Translator,
 };
 
 /// A Legacy p2sh Descriptor
@@ -377,7 +377,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Sh<Pk> {
 }
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Sh<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,

src/descriptor/sortedmulti.rs

@@ -27,8 +27,8 @@ use crate::miniscript::decode::Terminal;
 use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
 use crate::prelude::*;
 use crate::{
-    errstr, expression, miniscript, policy, script_num_size, Error, ForEach, ForEachKey,
-    Miniscript, MiniscriptKey, Satisfier, ToPublicKey, Translator,
+    errstr, expression, miniscript, policy, script_num_size, Error, ForEachKey, Miniscript,
+    MiniscriptKey, Satisfier, ToPublicKey, Translator,
 };
 
 /// Contents of a "sortedmulti" descriptor
@@ -112,12 +112,12 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
 }
 
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> ForEachKey<Pk> for SortedMultiVec<Pk, Ctx> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, mut pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
     {
-        self.pks.iter().all(|key| pred(ForEach::Key(key)))
+        self.pks.iter().all(|key| pred(key))
     }
 }
 

src/descriptor/tr.rs

@@ -19,8 +19,7 @@ use crate::policy::Liftable;
 use crate::prelude::*;
 use crate::util::{varint_len, witness_size};
 use crate::{
-    errstr, Error, ForEach, ForEachKey, MiniscriptKey, Satisfier, Tap, ToPublicKey, TranslatePk,
-    Translator,
+    errstr, Error, ForEachKey, MiniscriptKey, Satisfier, Tap, ToPublicKey, TranslatePk, Translator,
 };
 
 /// A Taproot Tree representation.
@@ -579,15 +578,15 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Tr<Pk> {
 }
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Tr<Pk> {
-    fn for_each_key<'a, F: FnMut(ForEach<'a, Pk>) -> bool>(&'a self, mut pred: F) -> bool
+    fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
         Pk::Hash: 'a,
     {
         let script_keys_res = self
             .iter_scripts()
             .all(|(_d, ms)| ms.for_each_key(&mut pred));
-        script_keys_res && pred(ForEach::Key(&self.internal_key))
+        script_keys_res && pred(&self.internal_key)
     }
 }
 
@@ -702,9 +701,6 @@ mod tests {
         let desc = desc.replace(&[' ', '\n'][..], "");
         let tr = Tr::<String>::from_str(&desc).unwrap();
         // Note the last ac12 only has ac and fails the predicate
-        assert!(!tr.for_each_key(|k| match k {
-            ForEach::Key(k) => k.starts_with("acc"),
-            ForEach::Hash(_h) => unreachable!(),
-        }));
+        assert!(!tr.for_each_key(|k| k.starts_with("acc")));
     }
 }

src/interpreter/mod.rs

@@ -590,15 +590,25 @@ where
                 Terminal::PkK(ref pk) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self.stack.evaluate_pk(&mut self.verify_sig, pk);
+                    let res = self.stack.evaluate_pk(&mut self.verify_sig, *pk);
                     if res.is_some() {
                         return res;
                     }
                 }
-                Terminal::PkH(ref pkh) => {
+                Terminal::PkH(ref pk) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self.stack.evaluate_pkh(&mut self.verify_sig, pkh);
+                    let res = self
+                        .stack
+                        .evaluate_pkh(&mut self.verify_sig, pk.to_pubkeyhash());
+                    if res.is_some() {
+                        return res;
+                    }
+                }
+                Terminal::RawPkH(ref pkh) => {
+                    debug_assert_eq!(node_state.n_evaluated, 0);
+                    debug_assert_eq!(node_state.n_satisfied, 0);
+                    let res = self.stack.evaluate_pkh(&mut self.verify_sig, *pkh);
                     if res.is_some() {
                         return res;
                     }
@@ -857,7 +867,7 @@ where
                         // push 1 on satisfied sigs and push 0 on empty sigs
                         match self
                             .stack
-                            .evaluate_pk(&mut self.verify_sig, &subs[node_state.n_evaluated])
+                            .evaluate_pk(&mut self.verify_sig, subs[node_state.n_evaluated])
                         {
                             Some(Ok(x)) => {
                                 self.push_evaluation_state(
@@ -1138,7 +1148,7 @@ mod tests {
         }
 
         let pk = no_checks_ms(&format!("c:pk_k({})", pks[0]));
-        let pkh = no_checks_ms(&format!("c:pk_h({})", pks[1].to_pubkeyhash()));
+        let pkh = no_checks_ms(&format!("c:pk_h({})", pks[1]));
         //Time
         let after = no_checks_ms(&format!("after({})", 1000));
         let older = no_checks_ms(&format!("older({})", 1000));
@@ -1270,11 +1280,7 @@ mod tests {
             stack::Element::Push(&pk_bytes),
             stack::Element::Push(&der_sigs[0]),
         ]);
-        let elem = no_checks_ms(&format!(
-            "and_v(vc:pk_k({}),c:pk_h({}))",
-            pks[0],
-            pks[1].to_pubkeyhash()
-        ));
+        let elem = no_checks_ms(&format!("and_v(vc:pk_k({}),c:pk_h({}))", pks[0], pks[1]));
         let vfyfn = vfyfn_.clone(); // sigh rust 1.29...
         let constraints = from_stack(Box::new(vfyfn), stack, &elem);
 
@@ -1325,9 +1331,7 @@ mod tests {
         ]);
         let elem = no_checks_ms(&format!(
             "andor(c:pk_k({}),jtv:sha256({}),c:pk_h({}))",
-            pks[0],
-            sha256_hash,
-            pks[1].to_pubkeyhash(),
+            pks[0], sha256_hash, pks[1],
         ));
         let vfyfn = vfyfn_.clone(); // sigh rust 1.29...
         let constraints = from_stack(Box::new(vfyfn), stack, &elem);

src/interpreter/stack.rs

@@ -134,7 +134,7 @@ impl<'txin> Stack<'txin> {
     pub(super) fn evaluate_pk<'intp>(
         &mut self,
         verify_sig: &mut Box<dyn FnMut(&KeySigPair) -> bool + 'intp>,
-        pk: &'intp BitcoinKey,
+        pk: BitcoinKey,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
         if let Some(sigser) = self.pop() {
             match sigser {
@@ -143,7 +143,7 @@ impl<'txin> Stack<'txin> {
                     None
                 }
                 Element::Push(sigser) => {
-                    let key_sig = verify_sersig(verify_sig, pk, sigser);
+                    let key_sig = verify_sersig(verify_sig, &pk, sigser);
                     match key_sig {
                         Ok(key_sig) => {
                             self.push(Element::Satisfied);
@@ -152,9 +152,7 @@ impl<'txin> Stack<'txin> {
                         Err(e) => Some(Err(e)),
                     }
                 }
-                Element::Satisfied => {
-                    Some(Err(Error::PkEvaluationError(PkEvalErrInner::from(*pk))))
-                }
+                Element::Satisfied => Some(Err(Error::PkEvaluationError(PkEvalErrInner::from(pk)))),
             }
         } else {
             Some(Err(Error::UnexpectedStackEnd))
@@ -170,7 +168,7 @@ impl<'txin> Stack<'txin> {
     pub(super) fn evaluate_pkh<'intp>(
         &mut self,
         verify_sig: &mut Box<dyn FnMut(&KeySigPair) -> bool + 'intp>,
-        pkh: &'intp TypedHash160,
+        pkh: TypedHash160,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
         // Parse a bitcoin key from witness data slice depending on hash context
         // when we encounter a pkh(hash)
@@ -189,7 +187,7 @@ impl<'txin> Stack<'txin> {
             if pk_hash != pkh.hash160() {
                 return Some(Err(Error::PkHashVerifyFail(pkh.hash160())));
             }
-            match bitcoin_key_from_slice(pk, *pkh) {
+            match bitcoin_key_from_slice(pk, pkh) {
                 Some(pk) => {
                     if let Some(sigser) = self.pop() {
                         match sigser {