Fix: ct() taproot descriptors with script tree don't parse

src/confidential/mod.rs

@@ -28,8 +28,8 @@ use elements::secp256k1_zkp;
 
 use crate::descriptor::checksum::{self, verify_checksum};
 use crate::descriptor::{
-    ConversionError, DefiniteDescriptorKey, DescriptorSecretKey, DescriptorPublicKey,
-    DescriptorXKey, Wildcard
+    ConversionError, DefiniteDescriptorKey, DescriptorPublicKey, DescriptorSecretKey,
+    DescriptorXKey, Wildcard,
 };
 use crate::expression::FromTree;
 use crate::extensions::{CovExtArgs, CovenantExt, Extension, ParseableExt};
@@ -644,4 +644,110 @@ mod tests {
         let desc_bare = Descriptor::<DefiniteDescriptorKey>::from_str(&desc_bare_str).unwrap();
         assert_eq!(desc_bare.address(&secp, params).unwrap_err(), Error::Unexpected("wildcard blinding key".into()));
     }
+
+    #[test]
+    fn confidential_descriptor_tr_script_tree() {
+        let secp = secp256k1_zkp::Secp256k1::new();
+
+        let nums = "0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0";
+        let key = "03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4";
+        let ct_key = DescriptorPublicKey::from_str(
+            "02dce16018bbbb8e36de7b394df5b5166e9adb7498be7d881a85a09aeecf76b623",
+        )
+        .unwrap();
+        let mbk = slip77::MasterBlindingKey::from_seed(b"abcd");
+
+        let leaf = crate::Descriptor::from_str(&format!("eltr({nums},pk({key}))")).unwrap();
+        let tree = crate::Descriptor::from_str(&format!("eltr({nums},{{pk({key}),pk({nums})}})"))
+            .unwrap();
+
+        let tests = vec![
+            // SLIP77, P2TR with one leaf
+            ConfidentialTest {
+                key: Key::Slip77(mbk),
+                descriptor: leaf.clone(),
+                descriptor_str: format!("ct(slip77({mbk}),eltr({nums},pk({key})))#0rh5eggz"),
+                conf_addr: "lq1pqv6pc3c9nlsr6my3tqxl8plf3t7hl73nlxgcuswhkxy8llz32z3p4940lgxlw8ksnmsl0x7kmazr648t5y2js6cl9nchna5s9jdpd0azfc2t8prf77qg",
+                unconf_addr: "ex1pj6hl5r0hrmgfac0hn0td73pa2n46z9fgdv0jeute76gzexskh73qzgwgq9",
+            },
+            // SLIP77, P2TR with a script tree
+            ConfidentialTest {
+                key: Key::Slip77(mbk),
+                descriptor: tree.clone(),
+                descriptor_str: format!("ct(slip77({mbk}),eltr({nums},{{pk({key}),pk({nums})}}))#jvtacgy4"),
+                conf_addr: "lq1pqf82jgz44dws8fh9jk759fuwkp72c2f7la0m88pfj4uz0uwgs5qhcyrdxthp89fwnguhrtkp0tlt8sjcen0lxtls357d9mv0rdy79xmufmvnf2d3e42s",
+                unconf_addr: "ex1pzpkn9msnj5hf5wt34mqh4l4ncfvvehln9lcg60xjak83kj0znd7qmfnsc7",
+            },
+            // Bare key, P2TR with one leaf
+            ConfidentialTest {
+                key: Key::Bare(ct_key.clone()),
+                descriptor: leaf,
+                descriptor_str: format!("ct({ct_key},eltr({nums},pk({key})))#ug7ku7uy"),
+                conf_addr: "lq1pq2nd3l90apnsrrssxkvzvpj8hrkjvqpm74aualy62d0akacxatm9l940lgxlw8ksnmsl0x7kmazr648t5y2js6cl9nchna5s9jdpd0azl2lt57872fd8",
+                unconf_addr: "ex1pj6hl5r0hrmgfac0hn0td73pa2n46z9fgdv0jeute76gzexskh73qzgwgq9",
+            },
+            // Bare key, P2TR with a script tree
+            ConfidentialTest {
+                key: Key::Bare(ct_key.clone()),
+                descriptor: tree,
+                descriptor_str: format!("ct({ct_key},eltr({nums},{{pk({key}),pk({nums})}}))#9h54dumz"),
+                conf_addr: "lq1pqtsl8ku9df0yrd7kh2ulz9wfrf7pdcfzh68sr9ge4dpxaphgjdxhuyrdxthp89fwnguhrtkp0tlt8sjcen0lxtls357d9mv0rdy79xmul76rz04xhhyn",
+                unconf_addr: "ex1pzpkn9msnj5hf5wt34mqh4l4ncfvvehln9lcg60xjak83kj0znd7qmfnsc7",
+            },
+        ];
+
+        for test in &tests {
+            test.check(&secp);
+        }
+    }
+
+    #[test]
+    fn confidential_descriptor_tr_extension_leaf() {
+        let secp = secp256k1_zkp::Secp256k1::new();
+        let params = &elements::AddressParams::LIQUID;
+
+        let nums = "0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0";
+        let key = "03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4";
+        let mbk = slip77::MasterBlindingKey::from_seed(b"abcd");
+        let tr = format!("eltr({nums},and_v(v:pk({key}),is_exp_asset(out_asset(0))))");
+
+        let desc = Descriptor::<DefiniteDescriptorKey>::from_str(&format!("ct(slip77({mbk}),{tr})")).unwrap();
+        assert!(matches!(desc.descriptor, crate::Descriptor::TrExt(_)));
+        assert_eq!(desc.descriptor, crate::Descriptor::from_str(&tr).unwrap());
+        assert_eq!(desc.to_string(), format!("ct(slip77({mbk}),{tr})#plnzm78w"));
+        assert_eq!(
+            desc.address(&secp, params).unwrap().to_string(),
+            "lq1pqfwyv29crkpngrt59ks2xhn8hjjrf48hsmyxh0nqz0y0k2d8hvxtfwfh2j8wk6jd9vg50jpg9hupjyvc9wef38x4rh9hslvagxa99x8wj3ssh4wjccnd",
+        );
+        assert_eq!(
+            desc.unconfidential_address(params).unwrap().to_string(),
+            "ex1phym4frhtdfxjky28eq5zm7qezxvzhv5cnn23mjmc0kw5rwjjnrhq2rfk85",
+        );
+    }
+
+    #[test]
+    fn elip151_tr_script_tree() {
+        let secp = secp256k1_zkp::Secp256k1::new();
+        let params = &elements::AddressParams::LIQUID;
+
+        let nums = "0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0";
+        let key = "03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4";
+        let xpub = "xpub661MyMwAqRbcFkPHucMnrGNzDwb6teAX1RbKQmqtEF8kK3Z7LZ59qafCjB9eCRLiTVG3uxBxgKvRgbubRhqSKXnGGb1aoaqLrpMBDrVxga8";
+        let view_key = "8a378c768713d53492f27f16c8b0b7cc45af4ec27a9bae002c645cdec5b8208b";
+
+        let tree = format!("{{pk({xpub}/0/*),pk({key})}}");
+        let desc = Descriptor::<DescriptorPublicKey>::from_str(&format!("ct(elip151,eltr({nums},{tree}))")).unwrap();
+        assert_eq!(desc.key.to_string(), view_key);
+        assert_eq!(desc.to_string(), format!("ct({view_key},eltr({nums},{tree}))#u3auuy5d"));
+
+        let desc = desc.at_derivation_index(0).unwrap();
+        assert_eq!(
+            desc.address(&secp, params).unwrap().to_string(),
+            "lq1pq2r4jza4g5p0x9xxunnx745ysh8a6ec5guvg5sh8eastahc55ud5n5y0s9psnfuvqqtl4fce5j92z6k6ttpufwth93djx7465tr5fsnjhwgt8yrzraqc",
+        );
+        assert_eq!(
+            desc.unconfidential_address(params).unwrap().to_string(),
+            "ex1p6z8czscf57xqq9l65uv6fz4pdtd94s7yh9mjcker02a2936ycfeq07d7qw",
+        );
+    }
 }

src/descriptor/mod.rs

@@ -1188,7 +1188,14 @@ impl_from_tree!(
             ("elsh", 1) => Descriptor::Sh(Sh::from_tree(top)?),
             ("elcovwsh", 2) => Descriptor::LegacyCSFSCov(LegacyCSFSCov::from_tree(top)?),
             ("elwsh", 1) => Descriptor::Wsh(Wsh::from_tree(top)?),
-            ("eltr", _) => Descriptor::Tr(Tr::from_tree(top)?),
+            ("eltr", _) => {
+                // First try parsing without extensions
+                match Tr::<Pk, NoExt>::from_tree(top) {
+                    Ok(tr) => Descriptor::Tr(tr),
+                    // Try parsing with extensions
+                    Err(_) => Descriptor::TrExt(Tr::<Pk, T>::from_tree(top)?),
+                }
+            }
             _ => Descriptor::Bare(Bare::from_tree(top)?),
         })
     }
@@ -1204,24 +1211,9 @@ impl_from_str!(
                 "Not an Elements Descriptor",
             )));
         }
-        // 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(&format!("{}tr", ELMTS_STR)) {
-            // First try parsing without extensions
-            match Tr::<Pk, NoExt>::from_str(s) {
-                Ok(tr) => Descriptor::Tr(tr),
-                Err(_) => {
-                    // Try parsing with extensions
-                    let tr = Tr::<Pk, T>::from_str(s)?;
-                    Descriptor::TrExt(tr)
-                }
-            }
-        } else {
-            let desc_str = verify_checksum(s)?;
-            let top = expression::Tree::from_str(desc_str)?;
-            expression::FromTree::from_tree(&top)?
-        };
+        let desc_str = verify_checksum(s)?;
+        let top = expression::Tree::from_str(desc_str)?;
+        let desc: Descriptor<Pk, T> = expression::FromTree::from_tree(&top)?;
 
         if desc.multipath_length_mismatch() {
             return Err(Error::MultipathDescLenMismatch);
@@ -1748,6 +1740,27 @@ mod tests {
         )
     }
 
+    #[test]
+    fn tr_malformed_script_tree() {
+        for s in [
+            "eltr(A,)",
+            "eltr({pk(A),pk(B)})",
+            "eltr(A,{})",
+            "eltr(A,{pk(B)})",
+            "eltr(A,{pk(B),pk(C),pk(D)})",
+            "eltr(A,{pk(B)),pk(C)}",
+            "eltr(A,{pk(B),pk(C)}",
+            "eltr(A,{pk(B),pk(C)}))",
+            "eltr(A,{pk(B),pk(C)},pk(D))",
+            "eltr(A,and_v(v:pk(B),{pk(C),pk(D)}))",
+            "elwsh({pk(A),pk(B)})",
+            "elwsh(or_d(pk(A),{pk(B),pk(C)}))",
+        ] {
+            assert!(Descriptor::<String>::from_str(s).is_err(), "{}", s);
+            assert!(Tr::<String>::from_str(s).is_err(), "{}", s);
+        }
+    }
+
     #[test]
     fn tr_script_pubkey() {
         let key = Descriptor::<bitcoin::PublicKey>::from_str(

src/descriptor/tr.rs

@@ -4,7 +4,6 @@ use std::str::FromStr;
 use std::sync::{Arc, Mutex};
 use std::{fmt, hash};
 
-use bitcoin_miniscript::expression::check_valid_chars;
 use elements::taproot::{
     LeafVersion, TaprootBuilder, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE,
     TAPROOT_CONTROL_MAX_NODE_COUNT, TAPROOT_CONTROL_NODE_SIZE,
@@ -21,7 +20,7 @@ use crate::policy::semantic::Policy;
 use crate::policy::Liftable;
 use crate::util::{varint_len, witness_size};
 use crate::{
-    errstr, Error, Extension, ForEachKey, MiniscriptKey, NoExt, Satisfier, Tap, ToPublicKey,
+    Error, Extension, ForEachKey, MiniscriptKey, NoExt, Satisfier, Tap, ToPublicKey,
     TranslateExt, TranslatePk, Translator,
 };
 
@@ -693,7 +692,7 @@ impl_from_str!(
     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)?;
+        let top = expression::Tree::from_str(desc_str)?;
         Self::from_tree(&top)
     }
 );
@@ -720,70 +719,6 @@ impl<Pk: MiniscriptKey, Ext: Extension> fmt::Display for Tr<Pk, Ext> {
     }
 }
 
-// Helper function to parse string into miniscript tree form
-fn parse_tr_tree(s: &str) -> Result<expression::Tree<'_>, Error> {
-    check_valid_chars(s)?;
-
-    if s.len() > 5 && &s[..5] == "eltr(" && s.as_bytes()[s.len() - 1] == b')' {
-        let rest = &s[5..s.len() - 1];
-        if !rest.contains(',') {
-            let internal_key = expression::Tree {
-                name: rest,
-                args: vec![],
-            };
-            return Ok(expression::Tree {
-                name: "eltr",
-                args: vec![internal_key],
-            });
-        }
-        // use str::split_once() method to refactor this when compiler version bumps up
-        let (key, script) = split_once(rest, ',')
-            .ok_or_else(|| Error::BadDescriptor("invalid taproot descriptor".to_string()))?;
-
-        let internal_key = expression::Tree {
-            name: key,
-            args: vec![],
-        };
-        if script.is_empty() {
-            return Ok(expression::Tree {
-                name: "eltr",
-                args: vec![internal_key],
-            });
-        }
-        let (tree, rest) = expression::Tree::from_slice_delim(script, 1, '{')?;
-        if rest.is_empty() {
-            Ok(expression::Tree {
-                name: "eltr",
-                args: vec![internal_key, tree],
-            })
-        } else {
-            Err(errstr(rest))
-        }
-    } else {
-        Err(Error::Unexpected("invalid taproot descriptor".to_string()))
-    }
-}
-
-fn split_once(inp: &str, delim: char) -> Option<(&str, &str)> {
-    if inp.is_empty() {
-        None
-    } else {
-        let mut found = inp.len();
-        for (idx, ch) in inp.chars().enumerate() {
-            if ch == delim {
-                found = idx;
-                break;
-            }
-        }
-        // No comma or trailing comma found
-        if found >= inp.len() - 1 {
-            Some((inp, ""))
-        } else {
-            Some((&inp[..found], &inp[found + 1..]))
-        }
-    }
-}
-
 impl<Pk: MiniscriptKey, Ext: Extension> Liftable<Pk> for TapTree<Pk, Ext> {
     fn lift(&self) -> Result<Policy<Pk>, Error> {
         fn lift_helper<Pk: MiniscriptKey, Ext: Extension>(

src/expression.rs

@@ -11,6 +11,9 @@ use bitcoin_miniscript::expression::check_valid_chars;
 
 use crate::{errstr, Error, MAX_RECURSION_DEPTH};
 
+/// Name of the taproot descriptor, whose second argument is a script tree.
+const SCRIPT_TREE_DESCRIPTOR: &str = "eltr";
+
 #[derive(Debug, Clone)]
 /// A token of the form `x(...)` or `x`
 pub struct Tree<'a> {
@@ -111,6 +114,11 @@ fn next_expr(sl: &str, delim: char) -> Found {
                     }
                 }
                 ')' => {
+                    if new_count == 0 {
+                        // Reached the parenthesis after the script tree, as in `eltr(KEY,TREE)`.
+                        found = Found::RBracket(n);
+                        break;
+                    }
                     new_count -= 1;
                 }
                 '}' => {
@@ -177,7 +185,17 @@ impl<'a> Tree<'a> {
 
                 sl = &sl[n + 1..];
                 loop {
-                    let (arg, new_sl) = Tree::from_slice_delim(sl, depth + 1, delim)?;
+                    // The script tree of a taproot descriptor is written with curly
+                    // braces, and its leaves are parsed later by the descriptor.
+                    let arg_delim = if delim == '('
+                        && ret.name == SCRIPT_TREE_DESCRIPTOR
+                        && ret.args.len() == 1
+                    {
+                        '{'
+                    } else {
+                        delim
+                    };
+                    let (arg, new_sl) = Tree::from_slice_delim(sl, depth + 1, arg_delim)?;
                     ret.args.push(arg);
 
                     if new_sl.is_empty() {