Rewrite expression parser to be non-recursive

src/descriptor/bare.rs

@@ -370,15 +370,11 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Pkh<Pk> {
 
 impl<Pk: 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))?)?)
-        } else {
-            Err(Error::Unexpected(format!(
-                "{}({} args) while parsing pkh descriptor",
-                top.name,
-                top.args.len(),
-            )))
-        }
+        let top = top
+            .verify_toplevel("pkh", 1..=1)
+            .map_err(From::from)
+            .map_err(Error::Parse)?;
+        Ok(Pkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?)
     }
 }
 

src/descriptor/checksum.rs

@@ -8,8 +8,8 @@
 //! [BIP-380]: <https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki>
 
 use core::convert::TryFrom;
-use core::fmt;
 use core::iter::FromIterator;
+use core::{array, fmt};
 
 use bech32::primitives::checksum::PackedFe32;
 use bech32::{Checksum, Fe32};
@@ -115,10 +115,10 @@ pub fn verify_checksum(s: &str) -> Result<&str, Error> {
         eng.input_unchecked(s[..last_hash_pos].as_bytes());
 
         let expected = eng.checksum_chars();
-        let mut actual = ['_'; CHECKSUM_LENGTH];
-        for (act, ch) in actual.iter_mut().zip(checksum_str.chars()) {
-            *act = ch;
-        }
+
+        let mut iter = checksum_str.chars();
+        let actual: [char; CHECKSUM_LENGTH] =
+            array::from_fn(|_| iter.next().expect("length checked above"));
 
         if expected != actual {
             return Err(Error::InvalidChecksum { actual, expected });

src/descriptor/mod.rs

@@ -21,6 +21,7 @@ use bitcoin::{
 };
 use sync::Arc;
 
+use crate::expression::FromTree as _;
 use crate::miniscript::decode::Terminal;
 use crate::miniscript::{satisfy, Legacy, Miniscript, Segwitv0};
 use crate::plan::{AssetProvider, Plan};
@@ -981,17 +982,17 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Descriptor<Pk> {
 impl<Pk: 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
-        // match "tr(" to handle more extensibly
-        let desc = if s.starts_with("tr(") {
-            Ok(Descriptor::Tr(Tr::from_str(s)?))
-        } else {
-            let top = expression::Tree::from_str(s)?;
-            expression::FromTree::from_tree(&top)
-        }?;
-
-        Ok(desc)
+        let top = expression::Tree::from_str(s)?;
+        let ret = Self::from_tree(&top)?;
+        if let Descriptor::Tr(ref inner) = ret {
+            // FIXME preserve weird/broken behavior from 12.x.
+            // See https://github.com/rust-bitcoin/rust-miniscript/issues/734
+            ret.sanity_check()?;
+            for (_, ms) in inner.iter_scripts() {
+                ms.ext_check(&crate::miniscript::analyzable::ExtParams::sane())?;
+            }
+        }
+        Ok(ret)
     }
 }
 
@@ -1102,7 +1103,7 @@ mod tests {
             StdDescriptor::from_str("sh(sortedmulti)")
                 .unwrap_err()
                 .to_string(),
-            "expected threshold, found terminal",
+            "sortedmulti must have at least 1 children, but found 0"
         ); //issue 202
         assert_eq!(
             StdDescriptor::from_str(&format!("sh(sortedmulti(2,{}))", &TEST_PK[3..69]))
@@ -1545,6 +1546,21 @@ mod tests {
         )
     }
 
+    #[test]
+    fn tr_named_branch() {
+        use crate::{ParseError, ParseTreeError};
+
+        assert!(matches!(
+            StdDescriptor::from_str(
+                "tr(0202d44008000010100000000084F0000000dd0dd00000000000201dceddd00d00,abc{0,0})"
+            ),
+            Err(Error::Parse(ParseError::Tree(ParseTreeError::IncorrectName {
+                expected: "",
+                ..
+            }))),
+        ));
+    }
+
     #[test]
     fn roundtrip_tests() {
         let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
@@ -1836,9 +1852,10 @@ mod tests {
         // https://github.com/bitcoin/bitcoin/blob/7ae86b3c6845873ca96650fc69beb4ae5285c801/src/test/descriptor_tests.cpp#L355-L360
         macro_rules! check_invalid_checksum {
             ($secp: ident,$($desc: expr),*) => {
+                use crate::{ParseError, ParseTreeError};
                 $(
                     match Descriptor::parse_descriptor($secp, $desc) {
-                        Err(Error::ParseTree(crate::ParseTreeError::Checksum(_))) => {},
+                        Err(Error::Parse(ParseError::Tree(ParseTreeError::Checksum(_)))) => {},
                         Err(e) => panic!("Expected bad checksum for {}, got '{}'", $desc, e),
                         _ => panic!("Invalid checksum treated as valid: {}", $desc),
                     };

src/descriptor/segwitv0.rs

@@ -248,21 +248,17 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wsh<Pk> {
 
 impl<Pk: 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];
-            if top.name == "sortedmulti" {
-                return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) });
-            }
-            let sub = Miniscript::from_tree(top)?;
-            Segwitv0::top_level_checks(&sub)?;
-            Ok(Wsh { inner: WshInner::Ms(sub) })
-        } else {
-            Err(Error::Unexpected(format!(
-                "{}({} args) while parsing wsh descriptor",
-                top.name,
-                top.args.len(),
-            )))
+        let top = top
+            .verify_toplevel("wsh", 1..=1)
+            .map_err(From::from)
+            .map_err(Error::Parse)?;
+
+        if top.name == "sortedmulti" {
+            return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) });
         }
+        let sub = Miniscript::from_tree(top)?;
+        Segwitv0::top_level_checks(&sub)?;
+        Ok(Wsh { inner: WshInner::Ms(sub) })
     }
 }
 
@@ -488,15 +484,11 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wpkh<Pk> {
 
 impl<Pk: 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))?)?)
-        } else {
-            Err(Error::Unexpected(format!(
-                "{}({} args) while parsing wpkh descriptor",
-                top.name,
-                top.args.len(),
-            )))
-        }
+        let top = top
+            .verify_toplevel("wpkh", 1..=1)
+            .map_err(From::from)
+            .map_err(Error::Parse)?;
+        Ok(Wpkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?)
     }
 }
 

src/descriptor/sh.rs

@@ -82,26 +82,22 @@ impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
 
 impl<Pk: 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];
-            let inner = match top.name {
-                "wsh" => ShInner::Wsh(Wsh::from_tree(top)?),
-                "wpkh" => ShInner::Wpkh(Wpkh::from_tree(top)?),
-                "sortedmulti" => ShInner::SortedMulti(SortedMultiVec::from_tree(top)?),
-                _ => {
-                    let sub = Miniscript::from_tree(top)?;
-                    Legacy::top_level_checks(&sub)?;
-                    ShInner::Ms(sub)
-                }
-            };
-            Ok(Sh { inner })
-        } else {
-            Err(Error::Unexpected(format!(
-                "{}({} args) while parsing sh descriptor",
-                top.name,
-                top.args.len(),
-            )))
-        }
+        let top = top
+            .verify_toplevel("sh", 1..=1)
+            .map_err(From::from)
+            .map_err(Error::Parse)?;
+
+        let inner = match top.name {
+            "wsh" => ShInner::Wsh(Wsh::from_tree(top)?),
+            "wpkh" => ShInner::Wpkh(Wpkh::from_tree(top)?),
+            "sortedmulti" => ShInner::SortedMulti(SortedMultiVec::from_tree(top)?),
+            _ => {
+                let sub = Miniscript::from_tree(top)?;
+                Legacy::top_level_checks(&sub)?;
+                ShInner::Ms(sub)
+            }
+        };
+        Ok(Sh { inner })
     }
 }
 

src/descriptor/sortedmulti.rs

@@ -64,6 +64,10 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
         Pk: FromStr,
         <Pk as FromStr>::Err: fmt::Display,
     {
+        tree.verify_toplevel("sortedmulti", 1..)
+            .map_err(From::from)
+            .map_err(Error::Parse)?;
+
         let ret = Self {
             inner: tree
                 .to_null_threshold()