Merge #650: Replace macros with traits; other cleanups

164bde9 miniscript: add parsing benchmarks (Andrew Poelstra)
bbafc39 expression: add parsing benchmark (Andrew Poelstra)
30d2d11 miniscript: factor out a couple parts of Terminal::from_tree (Andrew Poelstra)
f22dc3a miniscript: use new TRUE/FALSE constants throughout the codebase (Andrew Poelstra)
be41d8b miniscript: add constants for 1 and 0 (Andrew Poelstra)
270e65d Replace macros with blanket impls (Andrew Poelstra)
5ec0f44 add `blanket_traits` module to replace very noisy trait definitions (Andrew Poelstra)

Pull request description:

  There are several macros we use to implement functions when our `Pk` types satisfy a large bundle of trait conditions. There is a trick rust-lang/rust#20671 (comment) that we can use instead.

  While I'm at it, do several other small cleanups.

  As a weekend project I rewrote the expression module and was able to get a 10-15% speedup on parsing miniscripts, while eliminating recursion and simplifying the algorithms. I am still working on cleaning up the code and improving the error handling. This is the first set of commits from that branch, which should be simple and uncontroversial.

ACKs for top commit:
  apoelstra:
    > ACK [164bde9](164bde9). Are you taking a performance refactor stab at rust-miniscript?
  sanket1729:
    ACK 164bde9. Are you taking a performance refactor stab at rust-miniscript?

Tree-SHA512: 42e11f5f45fa705e14334e79126a66c97577fc6e807f804beaa1532bf3693a6c41a8b714bc4d1436209a1e0d808dc6a3ccc7198f20ff467f40f12126d1ee02f3

Author: apoelstra

src/blanket_traits.rs

@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Blanket Traits
+//!
+//! Because of this library's heavy use of generics, we often require complicated
+//! trait bounds (especially when it comes to [`FromStr`] and its
+//! associated error types). These blanket traits act as aliases, allowing easier
+//! descriptions of them.
+//!
+//! While these traits are not sealed, they have blanket-impls which prevent you
+//! from directly implementing them on your own types. The traits will be
+//! automatically implemented if you satisfy all the bounds.
+//!
+
+use core::fmt;
+use core::str::FromStr;
+
+use crate::MiniscriptKey;
+
+/// Blanket trait describing a key where all associated types implement `FromStr`,
+/// and all `FromStr` errors can be displayed.
+pub trait FromStrKey:
+    MiniscriptKey<
+        Sha256 = Self::_Sha256,
+        Hash256 = Self::_Hash256,
+        Ripemd160 = Self::_Ripemd160,
+        Hash160 = Self::_Hash160,
+    > + FromStr<Err = Self::_FromStrErr>
+{
+    /// Dummy type. Do not use.
+    type _Sha256: FromStr<Err = Self::_Sha256FromStrErr>;
+    /// Dummy type. Do not use.
+    type _Sha256FromStrErr: fmt::Debug + fmt::Display;
+    /// Dummy type. Do not use.
+    type _Hash256: FromStr<Err = Self::_Hash256FromStrErr>;
+    /// Dummy type. Do not use.
+    type _Hash256FromStrErr: fmt::Debug + fmt::Display;
+    /// Dummy type. Do not use.
+    type _Ripemd160: FromStr<Err = Self::_Ripemd160FromStrErr>;
+    /// Dummy type. Do not use.
+    type _Ripemd160FromStrErr: fmt::Debug + fmt::Display;
+    /// Dummy type. Do not use.
+    type _Hash160: FromStr<Err = Self::_Hash160FromStrErr>;
+    /// Dummy type. Do not use.
+    type _Hash160FromStrErr: fmt::Debug + fmt::Display;
+    /// Dummy type. Do not use.
+    type _FromStrErr: fmt::Debug + fmt::Display;
+}
+
+impl<T> FromStrKey for T
+where
+    Self: MiniscriptKey + FromStr,
+    <Self as MiniscriptKey>::Sha256: FromStr,
+    Self::Hash256: FromStr,
+    Self::Ripemd160: FromStr,
+    Self::Hash160: FromStr,
+    <Self as FromStr>::Err: fmt::Debug + fmt::Display,
+    <<Self as MiniscriptKey>::Sha256 as FromStr>::Err: fmt::Debug + fmt::Display,
+    <Self::Hash256 as FromStr>::Err: fmt::Debug + fmt::Display,
+    <Self::Ripemd160 as FromStr>::Err: fmt::Debug + fmt::Display,
+    <Self::Hash160 as FromStr>::Err: fmt::Debug + fmt::Display,
+{
+    type _Sha256 = <T as MiniscriptKey>::Sha256;
+    type _Sha256FromStrErr = <<T as MiniscriptKey>::Sha256 as FromStr>::Err;
+    type _Hash256 = <T as MiniscriptKey>::Hash256;
+    type _Hash256FromStrErr = <<T as MiniscriptKey>::Hash256 as FromStr>::Err;
+    type _Ripemd160 = <T as MiniscriptKey>::Ripemd160;
+    type _Ripemd160FromStrErr = <<T as MiniscriptKey>::Ripemd160 as FromStr>::Err;
+    type _Hash160 = <T as MiniscriptKey>::Hash160;
+    type _Hash160FromStrErr = <<T as MiniscriptKey>::Hash160 as FromStr>::Err;
+    type _FromStrErr = <T as FromStr>::Err;
+}

src/descriptor/bare.rs

@@ -166,24 +166,22 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Bare<Pk> {
     fn lift(&self) -> Result<semantic::Policy<Pk>, Error> { self.ms.lift() }
 }
 
-impl_from_tree!(
-    Bare<Pk>,
+impl<Pk: crate::FromStrKey> FromTree for Bare<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         let sub = Miniscript::<Pk, BareCtx>::from_tree(top)?;
         BareCtx::top_level_checks(&sub)?;
         Bare::new(sub)
     }
-);
+}
 
-impl_from_str!(
-    Bare<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Bare<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = expression::Tree::from_str(desc_str)?;
         Self::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Bare<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool {
@@ -366,8 +364,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Pkh<Pk> {
     }
 }
 
-impl_from_tree!(
-    Pkh<Pk>,
+impl<Pk: crate::FromStrKey> FromTree for Pkh<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         if top.name == "pkh" && top.args.len() == 1 {
             Ok(Pkh::new(expression::terminal(&top.args[0], |pk| Pk::from_str(pk))?)?)
@@ -379,17 +376,16 @@ impl_from_tree!(
             )))
         }
     }
-);
+}
 
-impl_from_str!(
-    Pkh<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Pkh<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = expression::Tree::from_str(desc_str)?;
         Self::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Pkh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool { pred(&self.pk) }

src/descriptor/mod.rs

@@ -918,8 +918,7 @@ impl Descriptor<DefiniteDescriptorKey> {
     }
 }
 
-impl_from_tree!(
-    Descriptor<Pk>,
+impl<Pk: crate::FromStrKey> crate::expression::FromTree for Descriptor<Pk> {
     /// Parse an expression tree into a descriptor.
     fn from_tree(top: &expression::Tree) -> Result<Descriptor<Pk>, Error> {
         Ok(match (top.name, top.args.len() as u32) {
@@ -931,11 +930,10 @@ impl_from_tree!(
             _ => Descriptor::Bare(Bare::from_tree(top)?),
         })
     }
-);
+}
 
-impl_from_str!(
-    Descriptor<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> FromStr for Descriptor<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Descriptor<Pk>, Error> {
         // tr tree parsing has special code
         // Tr::from_str will check the checksum
@@ -950,7 +948,7 @@ impl_from_str!(
 
         Ok(desc)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> fmt::Debug for Descriptor<Pk> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

src/descriptor/segwitv0.rs

@@ -231,8 +231,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wsh<Pk> {
     }
 }
 
-impl_from_tree!(
-    Wsh<Pk>,
+impl<Pk: crate::FromStrKey> crate::expression::FromTree for Wsh<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         if top.name == "wsh" && top.args.len() == 1 {
             let top = &top.args[0];
@@ -250,7 +249,7 @@ impl_from_tree!(
             )))
         }
     }
-);
+}
 
 impl<Pk: MiniscriptKey> fmt::Debug for Wsh<Pk> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -270,15 +269,14 @@ impl<Pk: MiniscriptKey> fmt::Display for Wsh<Pk> {
     }
 }
 
-impl_from_str!(
-    Wsh<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Wsh<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = expression::Tree::from_str(desc_str)?;
         Wsh::<Pk>::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wsh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool {
@@ -475,8 +473,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wpkh<Pk> {
     }
 }
 
-impl_from_tree!(
-    Wpkh<Pk>,
+impl<Pk: crate::FromStrKey> crate::expression::FromTree for Wpkh<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         if top.name == "wpkh" && top.args.len() == 1 {
             Ok(Wpkh::new(expression::terminal(&top.args[0], |pk| Pk::from_str(pk))?)?)
@@ -488,17 +485,16 @@ impl_from_tree!(
             )))
         }
     }
-);
+}
 
-impl_from_str!(
-    Wpkh<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Wpkh<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = expression::Tree::from_str(desc_str)?;
         Self::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wpkh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool { pred(&self.pk) }

src/descriptor/sh.rs

@@ -81,8 +81,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
     }
 }
 
-impl_from_tree!(
-    Sh<Pk>,
+impl<Pk: crate::FromStrKey> crate::expression::FromTree for Sh<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         if top.name == "sh" && top.args.len() == 1 {
             let top = &top.args[0];
@@ -105,17 +104,16 @@ impl_from_tree!(
             )))
         }
     }
-);
+}
 
-impl_from_str!(
-    Sh<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Sh<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = expression::Tree::from_str(desc_str)?;
         Self::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> Sh<Pk> {
     /// Get the Inner

src/descriptor/sortedmulti.rs

@@ -60,7 +60,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
     pub fn from_tree(tree: &expression::Tree) -> Result<Self, Error>
     where
         Pk: FromStr,
-        <Pk as FromStr>::Err: ToString,
+        <Pk as FromStr>::Err: fmt::Display,
     {
         if tree.args.is_empty() {
             return Err(errstr("no arguments given for sortedmulti"));

src/descriptor/tr.rs

@@ -467,8 +467,7 @@ where
 }
 
 #[rustfmt::skip]
-impl_block_str!(
-    Tr<Pk>,
+impl<Pk: crate::FromStrKey> Tr<Pk> {
     // Helper function to parse taproot script path
     fn parse_tr_script_spend(tree: &expression::Tree,) -> Result<TapTree<Pk>, Error> {
         match tree {
@@ -487,10 +486,9 @@ impl_block_str!(
             )),
         }
     }
-);
+}
 
-impl_from_tree!(
-    Tr<Pk>,
+impl<Pk: crate::FromStrKey> crate::expression::FromTree for Tr<Pk> {
     fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
         if top.name == "tr" {
             match top.args.len() {
@@ -530,17 +528,16 @@ impl_from_tree!(
             )))
         }
     }
-);
+}
 
-impl_from_str!(
-    Tr<Pk>,
-    type Err = Error;,
+impl<Pk: crate::FromStrKey> core::str::FromStr for Tr<Pk> {
+    type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let desc_str = verify_checksum(s)?;
         let top = parse_tr_tree(desc_str)?;
         Self::from_tree(&top)
     }
-);
+}
 
 impl<Pk: MiniscriptKey> fmt::Debug for Tr<Pk> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

src/expression.rs

@@ -2,6 +2,7 @@
 
 //! # Function-like Expression Language
 //!
+use core::fmt;
 use core::str::FromStr;
 
 use crate::prelude::*;
@@ -218,7 +219,7 @@ pub fn parse_num(s: &str) -> Result<u32, Error> {
 pub fn terminal<T, F, Err>(term: &Tree, convert: F) -> Result<T, Error>
 where
     F: FnOnce(&str) -> Result<T, Err>,
-    Err: ToString,
+    Err: fmt::Display,
 {
     if term.args.is_empty() {
         convert(term.name).map_err(|e| Error::Unexpected(e.to_string()))
@@ -259,7 +260,6 @@ where
 
 #[cfg(test)]
 mod tests {
-
     use super::parse_num;
 
     #[test]
@@ -281,3 +281,30 @@ mod tests {
         assert_eq!(valid_chars, super::VALID_CHARS);
     }
 }
+
+#[cfg(bench)]
+mod benches {
+    use test::{black_box, Bencher};
+
+    use super::*;
+
+    #[bench]
+    pub fn parse_tree(bh: &mut Bencher) {
+        bh.iter(|| {
+            let tree = Tree::from_str(
+                "and(thresh(2,and(sha256(H),or(sha256(H),pk(A))),pk(B),pk(C),pk(D),sha256(H)),pk(E))",
+            ).unwrap();
+            black_box(tree);
+        });
+    }
+
+    #[bench]
+    pub fn parse_tree_deep(bh: &mut Bencher) {
+        bh.iter(|| {
+            let tree = Tree::from_str(
+                "and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(and(1,2),3),4),5),6),7),8),9),10),11),12),13),14),15),16),17),18),19),20),21)"
+            ).unwrap();
+            black_box(tree);
+        });
+    }
+}

src/interpreter/inner.rs

@@ -46,12 +46,8 @@ fn script_from_stack_elem<Ctx: ScriptContext>(
             Miniscript::parse_with_ext(bitcoin::Script::from_bytes(sl), &ExtParams::allow_all())
                 .map_err(Error::from)
         }
-        stack::Element::Satisfied => {
-            Miniscript::from_ast(crate::Terminal::True).map_err(Error::from)
-        }
-        stack::Element::Dissatisfied => {
-            Miniscript::from_ast(crate::Terminal::False).map_err(Error::from)
-        }
+        stack::Element::Satisfied => Ok(Miniscript::TRUE),
+        stack::Element::Dissatisfied => Ok(Miniscript::FALSE),
     }
 }