expression: replace poorly-typed binary function with new one

Improves the typing of the errors (and the error messages themselves) by
eliminating another instance of `errstr`.

Saves another instance of `errstr` by removing the unused `unary`
method. There are now 5 left in the entire library :).

Strictly speaking, we ought to do a deprecation cycle on these. But
because we are changing the `expression` module so thoroughly, and we
don't expect any users are directly using it, it doesn't seem worth the
difficult (impossible?) task of preserving the old API for the sake of
deprecation messages.

Author: apoelstra

src/expression/mod.rs

@@ -137,6 +137,20 @@ impl<'a> Tree<'a> {
         }
     }
 
+    /// Check that a tree node has exactly two children.
+    ///
+    /// If so, return them.
+    ///
+    /// The `description` argument is only used to populate the error return,
+    /// and is not validated in any way.
+    pub fn verify_binary(
+        &self,
+        description: &'static str,
+    ) -> Result<(&Self, &Self), ParseTreeError> {
+        self.verify_n_children(description, 2..=2)?;
+        Ok((&self.args[0], &self.args[1]))
+    }
+
     /// Check that a tree has no curly-brace children in it.
     pub fn verify_no_curly_braces(&self) -> Result<(), ParseTreeError> {
         for tree in self.pre_order_iter() {
@@ -356,36 +370,6 @@ where
     }
 }
 
-/// Attempts to parse an expression with exactly one child
-pub fn unary<L, T, F>(term: &Tree, convert: F) -> Result<T, Error>
-where
-    L: FromTree,
-    F: FnOnce(L) -> T,
-{
-    if term.args.len() == 1 {
-        let left = FromTree::from_tree(&term.args[0])?;
-        Ok(convert(left))
-    } else {
-        Err(errstr(term.name))
-    }
-}
-
-/// Attempts to parse an expression with exactly two children
-pub fn binary<L, R, T, F>(term: &Tree, convert: F) -> Result<T, Error>
-where
-    L: FromTree,
-    R: FromTree,
-    F: FnOnce(L, R) -> T,
-{
-    if term.args.len() == 2 {
-        let left = FromTree::from_tree(&term.args[0])?;
-        let right = FromTree::from_tree(&term.args[1])?;
-        Ok(convert(left, right))
-    } else {
-        Err(errstr(term.name))
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/miniscript/astelem.rs

@@ -30,6 +30,18 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Arc<Ter
 
 impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Terminal<Pk, Ctx> {
     fn from_tree(top: &expression::Tree) -> Result<Terminal<Pk, Ctx>, Error> {
+        let binary =
+            |node: &expression::Tree, name, termfn: fn(_, _) -> Self| -> Result<Self, Error> {
+                node.verify_binary(name)
+                    .map_err(crate::ParseError::Tree)
+                    .map_err(Error::Parse)
+                    .and_then(|(x, y)| {
+                        let x = Arc::<Miniscript<Pk, Ctx>>::from_tree(x)?;
+                        let y = Arc::<Miniscript<Pk, Ctx>>::from_tree(y)?;
+                        Ok(termfn(x, y))
+                    })
+            };
+
         let (frag_name, frag_wrap) = super::split_expression_name(top.name)?;
         let unwrapped = match (frag_name, top.args.len()) {
             ("expr_raw_pkh", 1) => expression::terminal(&top.args[0], |x| {
@@ -65,8 +77,8 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina
             }),
             ("1", 0) => Ok(Terminal::True),
             ("0", 0) => Ok(Terminal::False),
-            ("and_v", 2) => expression::binary(top, Terminal::AndV),
-            ("and_b", 2) => expression::binary(top, Terminal::AndB),
+            ("and_v", _) => binary(top, "and_v", Terminal::AndV),
+            ("and_b", _) => binary(top, "and_b", Terminal::AndB),
             ("and_n", 2) => Ok(Terminal::AndOr(
                 expression::FromTree::from_tree(&top.args[0])?,
                 expression::FromTree::from_tree(&top.args[1])?,
@@ -77,10 +89,10 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina
                 expression::FromTree::from_tree(&top.args[1])?,
                 expression::FromTree::from_tree(&top.args[2])?,
             )),
-            ("or_b", 2) => expression::binary(top, Terminal::OrB),
-            ("or_d", 2) => expression::binary(top, Terminal::OrD),
-            ("or_c", 2) => expression::binary(top, Terminal::OrC),
-            ("or_i", 2) => expression::binary(top, Terminal::OrI),
+            ("or_b", _) => binary(top, "or_b", Terminal::OrB),
+            ("or_d", _) => binary(top, "or_d", Terminal::OrD),
+            ("or_c", _) => binary(top, "or_c", Terminal::OrC),
+            ("or_i", _) => binary(top, "or_i", Terminal::OrI),
             ("thresh", _) => top
                 .to_null_threshold()
                 .map_err(Error::ParseThreshold)?