Merge #296: Improve Taproot descriptor tree parsing

d3490d8 Improve parsing code in expression.rs (sanket1729)

Pull request description:

ACKs for top commit:
  apoelstra:
    ACK d3490d8

Tree-SHA512: 74c354cf0ebb8bd3693974be61000775364f4beb485cd8c13ea4f0e9ccf8594985d5a3a3415801774de3103bb65537b93d583f8262ed759ddf5f2a8e5ee8a394

Author: sanket1729

src/descriptor/tr.rs

@@ -483,7 +483,7 @@ fn parse_tr_tree(s: &str) -> Result<expression::Tree, Error> {
                 args: vec![internal_key],
             });
         }
-        let (tree, rest) = expression::Tree::from_slice_helper_curly(script, 1)?;
+        let (tree, rest) = expression::Tree::from_slice_delim(script, 1, '{')?;
         if rest.is_empty() {
             Ok(expression::Tree {
                 name: "tr",

src/expression.rs

@@ -37,110 +37,39 @@ pub trait FromTree: Sized {
     fn from_tree(top: &Tree) -> Result<Self, Error>;
 }
 
-impl<'a> Tree<'a> {
-    /// Parse an expression with round brackets
-    pub fn from_slice(sl: &'a str) -> Result<(Tree<'a>, &'a str), Error> {
-        // Parsing TapTree or just miniscript
-        Self::from_slice_helper_round(sl, 0u32)
-    }
-
-    fn from_slice_helper_round(mut sl: &'a str, depth: u32) -> Result<(Tree<'a>, &'a str), Error> {
-        if depth >= MAX_RECURSION_DEPTH {
-            return Err(Error::MaxRecursiveDepthExceeded);
-        }
-        enum Found {
-            Nothing,
-            Lparen(usize),
-            Comma(usize),
-            Rparen(usize),
-        }
+enum Found {
+    Nothing,
+    LBracket(usize), // Either a left ( or {
+    Comma(usize),
+    RBracket(usize), // Either a right ) or }
+}
 
-        let mut found = Found::Nothing;
+fn next_expr(sl: &str, delim: char) -> Found {
+    let mut found = Found::Nothing;
+    if delim == '(' {
         for (n, ch) in sl.char_indices() {
             match ch {
                 '(' => {
-                    found = Found::Lparen(n);
+                    found = Found::LBracket(n);
                     break;
                 }
                 ',' => {
                     found = Found::Comma(n);
                     break;
                 }
                 ')' => {
-                    found = Found::Rparen(n);
+                    found = Found::RBracket(n);
                     break;
                 }
                 _ => {}
             }
         }
-
-        match found {
-            // String-ending terminal
-            Found::Nothing => Ok((
-                Tree {
-                    name: &sl[..],
-                    args: vec![],
-                },
-                "",
-            )),
-            // Terminal
-            Found::Comma(n) | Found::Rparen(n) => Ok((
-                Tree {
-                    name: &sl[..n],
-                    args: vec![],
-                },
-                &sl[n..],
-            )),
-            // Function call
-            Found::Lparen(n) => {
-                let mut ret = Tree {
-                    name: &sl[..n],
-                    args: vec![],
-                };
-
-                sl = &sl[n + 1..];
-                loop {
-                    let (arg, new_sl) = Tree::from_slice_helper_round(sl, depth + 1)?;
-                    ret.args.push(arg);
-
-                    if new_sl.is_empty() {
-                        return Err(Error::ExpectedChar(')'));
-                    }
-
-                    sl = &new_sl[1..];
-                    match new_sl.as_bytes()[0] {
-                        b',' => {}
-                        b')' => break,
-                        _ => return Err(Error::ExpectedChar(',')),
-                    }
-                }
-                Ok((ret, sl))
-            }
-        }
-    }
-
-    // Helper function to parse expressions with curly braces
-    pub(crate) fn from_slice_helper_curly(
-        mut sl: &'a str,
-        depth: u32,
-    ) -> Result<(Tree<'a>, &'a str), Error> {
-        // contain the context of brackets
-        if depth >= MAX_RECURSION_DEPTH {
-            return Err(Error::MaxRecursiveDepthExceeded);
-        }
-        enum Found {
-            Nothing,
-            Lbrace(usize),
-            Comma(usize),
-            Rbrace(usize),
-        }
-
-        let mut found = Found::Nothing;
+    } else if delim == '{' {
         let mut new_count = 0;
         for (n, ch) in sl.char_indices() {
             match ch {
                 '{' => {
-                    found = Found::Lbrace(n);
+                    found = Found::LBracket(n);
                     break;
                 }
                 '(' => {
@@ -156,14 +85,44 @@ impl<'a> Tree<'a> {
                     new_count -= 1;
                 }
                 '}' => {
-                    found = Found::Rbrace(n);
+                    found = Found::RBracket(n);
                     break;
                 }
                 _ => {}
             }
         }
+    } else {
+        unreachable!("Internal: delimiters in parsing must be '(' or '{'");
+    }
+    found
+}
+
+// Get the corresponding delim
+fn closing_delim(delim: char) -> char {
+    match delim {
+        '(' => ')',
+        '{' => '}',
+        _ => unreachable!("Unknown delimiter"),
+    }
+}
 
-        match found {
+impl<'a> Tree<'a> {
+    /// Parse an expression with round brackets
+    pub fn from_slice(sl: &'a str) -> Result<(Tree<'a>, &'a str), Error> {
+        // Parsing TapTree or just miniscript
+        Self::from_slice_delim(sl, 0u32, '(')
+    }
+
+    pub(crate) fn from_slice_delim(
+        mut sl: &'a str,
+        depth: u32,
+        delim: char,
+    ) -> Result<(Tree<'a>, &'a str), Error> {
+        if depth >= MAX_RECURSION_DEPTH {
+            return Err(Error::MaxRecursiveDepthExceeded);
+        }
+
+        match next_expr(sl, delim) {
             // String-ending terminal
             Found::Nothing => Ok((
                 Tree {
@@ -173,34 +132,39 @@ impl<'a> Tree<'a> {
                 "",
             )),
             // Terminal
-            Found::Comma(n) | Found::Rbrace(n) => Ok((
+            Found::Comma(n) | Found::RBracket(n) => Ok((
                 Tree {
                     name: &sl[..n],
                     args: vec![],
                 },
                 &sl[n..],
             )),
             // Function call
-            Found::Lbrace(n) => {
+            Found::LBracket(n) => {
                 let mut ret = Tree {
-                    name: &sl[..n], // Would be empty for left and right assignments
+                    name: &sl[..n],
                     args: vec![],
                 };
 
                 sl = &sl[n + 1..];
                 loop {
-                    let (arg, new_sl) = Tree::from_slice_helper_curly(sl, depth + 1)?;
+                    let (arg, new_sl) = Tree::from_slice_delim(sl, depth + 1, delim)?;
                     ret.args.push(arg);
 
                     if new_sl.is_empty() {
-                        return Err(Error::ExpectedChar('}'));
+                        return Err(Error::ExpectedChar(closing_delim(delim)));
                     }
 
                     sl = &new_sl[1..];
                     match new_sl.as_bytes()[0] {
                         b',' => {}
-                        b'}' => break,
-                        _ => return Err(Error::ExpectedChar(',')),
+                        last_byte => {
+                            if last_byte == closing_delim(delim) as u8 {
+                                break;
+                            } else {
+                                return Err(Error::ExpectedChar(closing_delim(delim)));
+                            }
+                        }
                     }
                 }
                 Ok((ret, sl))