diff --git a/CHANGELOG.md b/CHANGELOG.md index 51ca41d6..629200b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +# Unreleased + +- Reject a legacy-pegin miniscript outside `or_d(multi,and_v(v:older,multi))` with `Error::BadDescriptor`. +- Remove the unused `Error::MultiAt`, `Error::MissingHash`, `Error::TaprootSpendInfoUnavialable` and `Error::TrNoExplicitScript` variants. +- Parse expressions with the rust-miniscript `expression` module instead of a local parser. `expression::Tree`, `expression::TreeIterItem` and `expression::Parens` are the rust-miniscript types. `Tree` no longer has public `name` and `args` fields, a `Display` implementation or a `from_slice` method, and `Tree::from_str` returns a rust-miniscript `Error`. +- `FromTree::from_tree`, `expression::{terminal, unary, binary}`, `SortedMultiVec::from_tree` and `Extension::from_name_tree` take `TreeIterItem` nodes instead of `&Tree` references. +- Malformed expressions are reported as rust-miniscript parse errors in `Error::BtcError`. `Error::ExpectedChar` is removed. The expression parser accepts up to 402 nested brackets across the whole expression, including the `eltr` wrapper, script tree branches and the Miniscript in each leaf, and rejects deeper expressions with a rust-miniscript `MaxRecursionDepthExceeded` parse error. The previous limit was 401, and each Taproot leaf had its own separate limit. `Error::MaxRecursiveDepthExceeded` now only reports a Taproot script tree deeper than the 128-level Taproot limit. +- In Elements expressions, curly braces are only accepted for the branches of an `eltr` script tree, which must be nameless and have exactly two children. Keys can no longer contain curly braces. The Bitcoin federation descriptor in `pegin()` follows rust-miniscript rules, which accept curly braces in `tr` script trees. +- Only `,`, `)` or `}` may follow a closing bracket, so key text such as `musig(A,B)/0`, which key types like `String` previously accepted, is rejected. +- A `sortedmulti` now contains a check that threshold must be a number without arguments. +- An empty script tree argument, as in `eltr(A,)`, is rejected. It was previously read as part of the internal key. +- `eltr` accepts a `musig(...)` internal key for key types that parse `musig(...)` text, such as `String`. +- `ct()` descriptors accept `eltr` with script leaves that contain no extension fragments. +- Miniscript and policy strings accept a trailing descriptor checksum, which is verified. The text after the last `#` is read as the checksum, so a key containing `#` is only accepted when a valid checksum follows. +- `pegin()` and `legacy_pegin()` descriptors can be parsed from strings. `LegacyPegin` displays its inner descriptor without the inner checksum. A successful parse does not check the restrictions that pegin spending places on the descriptor. Unsupported forms, such as an `eltr` Elements descriptor, still parse and can panic in the unchanged Bitcoin-side methods, including `bitcoin_address`, `bitcoin_witness_script` and `get_bitcoin_satisfaction`. + # 0.5.0 - Sep 20, 2026 - Update rust-elements to 0.27.0 and simplicity-lang to 0.9.0. diff --git a/Cargo-latest.lock b/Cargo-latest.lock index c78d9bc8..854484e6 100644 --- a/Cargo-latest.lock +++ b/Cargo-latest.lock @@ -240,12 +240,6 @@ version = "3.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46c5e41b57b8bba42a04676d81cb89e9ee8e859a1a66f80a5a72e1cb76b34d43" -[[package]] -name = "byteorder" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" - [[package]] name = "cc" version = "1.2.34" @@ -307,7 +301,6 @@ dependencies = [ "serde", "serde_json", "serde_test", - "simplicity-lang", ] [[package]] @@ -342,12 +335,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "ghost-cell" -version = "0.2.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8449d342b1c67f49169e92e71deb7b9b27f30062301a16dbc27a4cc8d2351b7" - [[package]] name = "hex-conservative" version = "0.1.2" @@ -467,12 +454,13 @@ dependencies = [ [[package]] name = "miniscript" -version = "12.3.5" +version = "13.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "487906208f38448e186e3deb02f2b8ef046a9078b0de00bdb28bf4fb9b76951c" +checksum = "cd35e2c377504e50159561884b03610711db6c2fec6c89f2b98d94016684726b" dependencies = [ "bech32 0.11.0", "bitcoin 0.32.102", + "hex-conservative 1.2.0", ] [[package]] @@ -719,33 +707,6 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" -[[package]] -name = "simplicity-lang" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cff7b10251528d46d9cf60238267911984fedbb0948cccea98dd74fc6bf9f729" -dependencies = [ - "bitcoin 0.32.102", - "bitcoin_hashes 1.2.0", - "byteorder", - "elements", - "getrandom", - "ghost-cell", - "hex-conservative 1.2.0", - "miniscript", - "simplicity-sys", -] - -[[package]] -name = "simplicity-sys" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "955935733b2735665b4735d9b1361eade6be4c7aea6bca2640345e4c81756f58" -dependencies = [ - "bitcoin_hashes 1.2.0", - "cc", -] - [[package]] name = "syn" version = "2.0.106" diff --git a/Cargo.toml b/Cargo.toml index f7918393..7149567a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,8 +22,7 @@ base64 = ["bitcoin/base64", "elements/base64"] [dependencies] bitcoin = "0.32.0" elements = "0.27.0" -bitcoin-miniscript = { package = "miniscript", version = "12.0" } -simplicity = { package = "simplicity-lang", version = "0.9.0", optional = true } +bitcoin-miniscript = { package = "miniscript", version = "13.1" } # Do NOT use this as a feature! Use the `serde` feature instead. actual-serde = { package = "serde", version = "1.0", optional = true } diff --git a/contrib/test_vars.sh b/contrib/test_vars.sh index 227b292c..307179e3 100644 --- a/contrib/test_vars.sh +++ b/contrib/test_vars.sh @@ -8,7 +8,7 @@ FEATURES_WITH_STD="" # Test all these features without "std" enabled. -FEATURES_WITHOUT_STD="compiler trace serde rand base64 simplicity" +FEATURES_WITHOUT_STD="compiler trace serde rand base64" # Run these examples. EXAMPLES="htlc:compiler parse: sign_multisig: verify_tx: xpub_descriptors: taproot:compiler psbt_sign_finalize:base64" diff --git a/src/confidential/elip151.rs b/src/confidential/elip151.rs index 14622781..0a4fa7c7 100644 --- a/src/confidential/elip151.rs +++ b/src/confidential/elip151.rs @@ -207,7 +207,7 @@ mod test { (&format!("elwsh(multi(2,{xpub}/<0;1>/*,{xpub}/0/<0;1;2>/*))"), Error::MultipathDescLenMismatch), ] { let err = confidential_descriptor(invalid_desc).unwrap_err(); - assert_eq!(err, expected_err); + assert_eq!(format!("{:?}", err), format!("{:?}", expected_err)); /* _i = _i + 1; println!("* Invalid Test vector {}", _i); diff --git a/src/confidential/mod.rs b/src/confidential/mod.rs index 828600a3..4e8703e0 100644 --- a/src/confidential/mod.rs +++ b/src/confidential/mod.rs @@ -222,34 +222,38 @@ impl_from_str!( type Err = Error;, fn from_str(s: &str) -> Result, Error> { let desc_str = verify_checksum(s)?; - let top = expression::Tree::from_str(desc_str)?; + let tree = expression::Tree::from_str(desc_str)?; + let top = tree.root(); - if top.name != "ct" { + if top.name() != "ct" { return Err(Error::BadDescriptor(String::from( "Not a CT Descriptor", ))); } - if top.args.len() != 2 { + expression::verify_round_parens(top)?; + let mut args = top.children(); + let (Some(keyexpr), Some(desc), None) = (args.next(), args.next(), args.next()) else { return Err(Error::BadDescriptor( - format!("CT descriptor had {} arguments rather than 2", top.args.len()) + format!("CT descriptor had {} arguments rather than 2", top.n_children()) )); - } + }; - let keyexpr = &top.args[0]; + expression::verify_round_parens(keyexpr)?; + let mut keyargs = keyexpr.children(); Ok(Descriptor { - key: match (keyexpr.name, keyexpr.args.len()) { - ("elip151", 0) => { - let d = crate::Descriptor::::from_tree(&top.args[1])?; + key: match (keyexpr.name(), keyargs.next(), keyargs.next()) { + ("elip151", None, None) => { + let d = crate::Descriptor::::from_tree(desc)?; Key::from_elip151(&d)? } - ("slip77", 1) => Key::Slip77(expression::terminal(&keyexpr.args[0], slip77::MasterBlindingKey::from_str)?), - ("slip77", _) => return Err(Error::BadDescriptor( + ("slip77", Some(key), None) => Key::Slip77(expression::terminal(key, slip77::MasterBlindingKey::from_str)?), + ("slip77", _, _) => return Err(Error::BadDescriptor( "slip77() must have exactly one argument".to_owned() )), _ => expression::terminal(keyexpr, |s: &str| DescriptorSecretKey::from_str_inner(s, true)).map(Key::View) .or_else(|_| expression::terminal(keyexpr, DescriptorPublicKey::from_str).map(Key::Bare))?, }, - descriptor: crate::Descriptor::from_tree(&top.args[1])?, + descriptor: crate::Descriptor::from_tree(desc)?, }) } ); @@ -586,10 +590,10 @@ mod tests { .unwrap(), }; - assert_eq!( + assert!(matches!( desc.address(&secp, params).unwrap_err(), Error::Conversion(ConversionError::HardenedChild) - ); + )); } // Same case, reached through `at_derivation_index` on a hardened wildcard blinding @@ -605,10 +609,10 @@ mod tests { let desc = Descriptor::::from_str(&desc_str).unwrap(); let definite_desc = desc.at_derivation_index(1).unwrap(); - assert_eq!( + assert!(matches!( definite_desc.address(&secp, params).unwrap_err(), Error::Conversion(ConversionError::HardenedChild) - ); + )); } #[test] @@ -638,10 +642,10 @@ mod tests { // but we can't enforce this with the Descriptor generic. let desc_view_str = format!("ct({}/*,elwpkh({}))#ls6mx2ac", xprv, xpub); let desc_view = Descriptor::::from_str(&desc_view_str).unwrap(); - assert_eq!(desc_view.address(&secp, params).unwrap_err(), Error::Unexpected("wildcard blinding key".into())); + assert!(matches!(desc_view.address(&secp, params).unwrap_err(), Error::Unexpected(e) if e == "wildcard blinding key")); let desc_bare_str = format!("ct({}/*,elwpkh({}))#czkz0hwn", xpub, xpub); let desc_bare = Descriptor::::from_str(&desc_bare_str).unwrap(); - assert_eq!(desc_bare.address(&secp, params).unwrap_err(), Error::Unexpected("wildcard blinding key".into())); + assert!(matches!(desc_bare.address(&secp, params).unwrap_err(), Error::Unexpected(e) if e == "wildcard blinding key")); } } diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 96f84322..a5c05700 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -20,7 +20,7 @@ use crate::policy::{semantic, Liftable}; use crate::util::{varint_len, witness_to_scriptsig}; use crate::{ elementssig_to_rawsig, BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, - ToPublicKey, TranslatePk, Translator, + Terminal, ToPublicKey, TranslatePk, Translator, }; /// Create a Bare Descriptor. That is descriptor that is @@ -157,14 +157,10 @@ impl Liftable for Bare { impl_from_tree!( Bare, - fn from_tree(top: &expression::Tree<'_>) -> Result { - // extra allocations to use the existing code as is. - if top.name.starts_with("el") { - let new_tree = expression::Tree { - name: top.name.split_at(2).1, - args: top.args.clone(), - }; - let sub = Miniscript::::from_tree(&new_tree)?; + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + if let Some(name) = top.name().strip_prefix("el") { + let sub = + Miniscript::::from_ast(Terminal::from_tree_with_name(name, top)?)?; BareCtx::top_level_checks(&sub)?; Bare::new(sub) } else { @@ -179,7 +175,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(&desc_str[2..])?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); @@ -345,16 +341,19 @@ impl Liftable for Pkh { impl_from_tree!( Pkh, - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "elpkh" && top.args.len() == 1 { - Ok(Pkh::new(expression::terminal(&top.args[0], |pk| { - Pk::from_str(pk) - })?)) + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("elpkh", Some(child), None) = (top.name(), args.next(), args.next()) { + Ok(Pkh::new(expression::terminal( + child, + |pk| Pk::from_str(pk), + )?)) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing pkh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -366,7 +365,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 85ee4ec4..76f6f8ce 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -8,7 +8,8 @@ use core::fmt; use core::iter::FromIterator; -use bitcoin_miniscript::expression::check_valid_chars; +use bitcoin_miniscript::descriptor::checksum::Error as ChecksumError; +use bitcoin_miniscript::ParseTreeError; use crate::Error; @@ -48,25 +49,24 @@ pub fn desc_checksum(desc: &str) -> Result { Ok(eng.checksum()) } -/// Helper function for FromStr for various -/// descriptor types. Checks and verifies the checksum -/// if it is present and returns the descriptor string -/// without the checksum +/// Validate descriptor characters and strip a valid optional checksum. +/// +/// Preserve descriptor parsing's first-`#` split and `BadDescriptor` checksum errors. +/// The upstream validator splits at the last `#`. pub(crate) fn verify_checksum(s: &str) -> Result<&str, Error> { - check_valid_chars(s)?; - - let mut parts = s.splitn(2, '#'); - let desc_str = parts.next().unwrap(); - if let Some(checksum_str) = parts.next() { - let expected_sum = desc_checksum(desc_str)?; - if checksum_str != expected_sum { - return Err(Error::BadDescriptor(format!( - "Invalid checksum '{}', expected '{}'", - checksum_str, expected_sum - ))); - } + let verified = bitcoin_miniscript::descriptor::checksum::verify_checksum(s); + if let Err(e @ ChecksumError::InvalidCharacter { .. }) = verified { + return Err(Error::from(ParseTreeError::Checksum(e))); + } + match s.split_once('#') { + None => Ok(s), + Some((desc, checksum)) if verified.is_ok() && !checksum.contains('#') => Ok(desc), + Some((desc, checksum)) => Err(Error::BadDescriptor(format!( + "Invalid checksum '{}', expected '{}'", + checksum, + desc_checksum(desc)? + ))), } - Ok(desc_str) } /// An engine to compute a checksum from a string @@ -187,6 +187,55 @@ mod test { }; } + #[test] + fn descriptor_checksum_compatibility() { + let desc = "elpkh(A)"; + let expected = desc_checksum(desc).unwrap(); + assert_eq!(verify_checksum(desc).unwrap(), desc); + assert_eq!(verify_checksum(&format!("{desc}#{expected}")).unwrap(), desc); + let embedded = format!("{desc}#extra"); + let last_checksum = desc_checksum(&embedded).unwrap(); + let valid_last = format!("{embedded}#{last_checksum}"); + assert_eq!( + bitcoin_miniscript::descriptor::checksum::verify_checksum(&valid_last).unwrap(), + embedded + ); + for suffix in [ + "", + "short", + "qqqqqqqq", + "extra#bad", + &format!("extra#{last_checksum}"), + ] { + let input = format!("{desc}#{suffix}"); + match verify_checksum(&input) { + Err(Error::BadDescriptor(message)) => assert_eq!( + message, + format!("Invalid checksum '{suffix}', expected '{expected}'") + ), + result => panic!("unexpected checksum result for {}: {:?}", input, result), + } + } + assert_eq!(verify_checksum(" ~").unwrap(), " ~"); + for ch in ['\0', '\x1f', '\x7f', 'é', '💖'] { + for prefix in ["", "el", "elpkh(A)#bad", "elpkh(A)#bad#"] { + let input = format!("{prefix}{ch}"); + assert!(matches!( + verify_checksum(&input), + Err(Error::BtcError(bitcoin_miniscript::Error::Parse( + bitcoin_miniscript::ParseError::Tree(ParseTreeError::Checksum( + ChecksumError::InvalidCharacter { ch: actual, pos } + )) + ))) if actual == ch && pos == prefix.len() + )); + } + } + assert!(matches!( + "€".parse::>(), + Err(Error::BtcError(_)) + )); + } + #[test] fn test_valid_descriptor_checksum() { check_expected!( diff --git a/src/descriptor/csfs_cov/cov.rs b/src/descriptor/csfs_cov/cov.rs index 1e5b82a2..8dab3184 100644 --- a/src/descriptor/csfs_cov/cov.rs +++ b/src/descriptor/csfs_cov/cov.rs @@ -290,18 +290,21 @@ impl LegacyCSFSCov { impl_from_tree!( LegacyCSFSCov, => Ext; Extension, - fn from_tree(top: &expression::Tree<'_>) -> Result { - if top.name == "elcovwsh" && top.args.len() == 2 { - let pk = expression::terminal(&top.args[0], |pk| Pk::from_str(pk))?; - let top = &top.args[1]; - let sub = Miniscript::from_tree(top)?; + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("elcovwsh", Some(key), Some(script), None) = + (top.name(), args.next(), args.next(), args.next()) + { + let pk = expression::terminal(key, |pk| Pk::from_str(pk))?; + let sub = Miniscript::from_tree(script)?; Segwitv0::top_level_checks(&sub)?; Ok(LegacyCSFSCov { pk, ms: sub }) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing elcovwsh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -337,7 +340,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - LegacyCSFSCov::::from_tree(&top) + LegacyCSFSCov::::from_tree(top.root()) } ); diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 6b60e922..68f67928 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -1181,8 +1181,8 @@ impl_from_tree!( ;T; Extension, Descriptor, /// Parse an expression tree into a descriptor. - fn from_tree(top: &expression::Tree) -> Result, Error> { - Ok(match (top.name, top.args.len() as u32) { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result, Error> { + Ok(match (top.name(), top.n_children()) { ("elpkh", 1) => Descriptor::Pkh(Pkh::from_tree(top)?), ("elwpkh", 1) => Descriptor::Wpkh(Wpkh::from_tree(top)?), ("elsh", 1) => Descriptor::Sh(Sh::from_tree(top)?), @@ -1204,8 +1204,8 @@ impl_from_str!( "Not an Elements Descriptor", ))); } - // tr tree parsing has special code - // Tr::from_str will check the checksum + // eltr is parsed without extensions first, and then with the extension + // type T. 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 @@ -1220,7 +1220,7 @@ impl_from_str!( } else { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - expression::FromTree::from_tree(&top)? + expression::FromTree::from_tree(top.root())? }; if desc.multipath_length_mismatch() { @@ -1748,6 +1748,18 @@ mod tests { ) } + #[test] + fn sortedmulti_threshold_is_terminal() { + for (prefix, suffix) in [("elwsh(", ")"), ("elsh(", ")"), ("elsh(wsh(", "))")] { + let valid = format!("{}sortedmulti(1,A,B){}", prefix, suffix); + assert!(Descriptor::::from_str(&valid).is_ok(), "{}", valid); + for k in ["1(X)", "1{X}", "1()", "1{}"] { + let s = format!("{}sortedmulti({},A,B){}", prefix, k, suffix); + assert!(Descriptor::::from_str(&s).is_err(), "{}", s); + } + } + } + #[test] fn tr_script_pubkey() { let key = Descriptor::::from_str( diff --git a/src/descriptor/pegin/dynafed_pegin.rs b/src/descriptor/pegin/dynafed_pegin.rs index 233d5311..68911caf 100644 --- a/src/descriptor/pegin/dynafed_pegin.rs +++ b/src/descriptor/pegin/dynafed_pegin.rs @@ -33,7 +33,7 @@ use crate::expression::{self, FromTree}; use crate::extensions::{CovExtArgs, CovenantExt}; use crate::policy::{semantic, Liftable}; use crate::{ - tweak_key, BtcDescriptor, BtcError, BtcFromTree, BtcLiftable, BtcPolicy, BtcSatisfier, BtcTree, + tweak_key, BtcDescriptor, BtcError, BtcFromTree, BtcLiftable, BtcPolicy, BtcSatisfier, Descriptor, DescriptorPublicKey, Error, MiniscriptKey, ToPublicKey, }; @@ -104,24 +104,24 @@ impl BtcLiftable for Pegin { impl_from_tree!( Pegin, - fn from_tree(top: &expression::Tree<'_>) -> Result { - if top.name == "pegin" && top.args.len() == 2 { - // a roundtrip hack to use FromTree from bitcoin::Miniscript from - // expression::Tree in elements. - let ms_str = top.args[0].to_string(); - let ms_expr = BtcTree::from_str(&ms_str)?; - // + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("pegin", Some(fed), Some(elem), None) = + (top.name(), args.next(), args.next(), args.next()) + { // TODO: Confirm with Andrew about the descriptor type for dynafed // Assuming sh(wsh) for now. - let fed_desc = BtcDescriptor::::from_tree(&ms_expr)?; - let elem_desc = Descriptor::>::from_tree(&top.args[1])?; + let fed_desc = BtcDescriptor::::from_tree(fed)?; + let elem_desc = + Descriptor::>::from_tree(elem)?; Ok(Pegin::new(fed_desc, elem_desc)) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing legacy_pegin descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -133,7 +133,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); @@ -252,7 +252,9 @@ impl Pegin { .into_bytes(); let mut t = TranslateTweak(&claim_script[..], secp); - let tweaked_desc = bitcoin_miniscript::TranslatePk::translate_pk(&self.fed_desc, &mut t) + let tweaked_desc = self + .fed_desc + .translate_pk(&mut t) .expect("Tweaking must succeed"); let res = tweaked_desc.get_satisfaction(satisfier)?; @@ -303,7 +305,8 @@ fn bitcoin_witness_script( ) -> Result { let mut t = TranslateTweak(claim_script, secp); - let tweaked_desc = bitcoin_miniscript::TranslatePk::translate_pk(fed_desc, &mut t) + let tweaked_desc = fed_desc + .translate_pk(&mut t) .expect("Tweaking must succeed"); Ok(tweaked_desc.explicit_script()?) } @@ -313,12 +316,14 @@ struct TranslateTweak<'a, 'b, C: secp256k1_zkp::Verification>( &'b secp256k1_zkp::Secp256k1, ); -impl<'a, 'b, Pk, C> bitcoin_miniscript::Translator - for TranslateTweak<'a, 'b, C> +impl<'a, 'b, Pk, C> bitcoin_miniscript::Translator for TranslateTweak<'a, 'b, C> where Pk: MiniscriptKey + ToPublicKey, C: secp256k1_zkp::Verification, { + type TargetPk = bitcoin::PublicKey; + type Error = (); + fn pk(&mut self, pk: &Pk) -> Result { Ok(tweak_key(&pk.to_public_key(), self.1, self.0)) } @@ -342,7 +347,7 @@ mod tests { type Segwitv0Script = bitcoin_miniscript::Miniscript; - let m = Segwitv0Script::parse(&s).unwrap(); + let m = Segwitv0Script::decode(&s).unwrap(); assert_eq!(m.encode(), s); BtcDescriptor::<_>::new_wsh(m).unwrap() } @@ -385,6 +390,10 @@ mod tests { let pegin = Pegin::new(d.clone(), elem_desc); assert_eq!(pegin.to_string(), "pegin(wsh(or_d(multi(11,020e0338c96a8870479f2396c373cc7696ba124e8635d41b0ea581112b67817261,02675333a4e4b8fb51d9d4e22fa5a8eaced3fdac8a8cbf9be8c030f75712e6af99,02896807d54bc55c24981f24a453c60ad3e8993d693732288068a23df3d9f50d48,029e51a5ef5db3137051de8323b001749932f2ff0d34c82e96a2c2461de96ae56c,02a4e1a9638d46923272c266631d94d36bdb03a64ee0e14c7518e49d2f29bc4010,031c41fdbcebe17bec8d49816e00ca1b5ac34766b91c9f2ac37d39c63e5e008afb,03079e252e85abffd3c401a69b087e590a9b86f33f574f08129ccbd3521ecf516b,03111cf405b627e22135b3b3733a4a34aa5723fb0f58379a16d32861bf576b0ec2,0318f331b3e5d38156da6633b31929c5b220349859cc9ca3d33fb4e68aa0840174,03230dae6b4ac93480aeab26d000841298e3b8f6157028e47b0897c1e025165de1,035abff4281ff00660f99ab27bb53e6b33689c2cd8dcd364bc3c90ca5aea0d71a6,03bd45cddfacf2083b14310ae4a84e25de61e451637346325222747b157446614c,03cc297026b06c71cbfa52089149157b5ff23de027ac5ab781800a578192d17546,03d3bde5d63bdb3a6379b461be64dad45eabff42f758543a9645afd42f6d424828,03ed1e8d5109c9ed66f7941bc53cc71137baa76d50d274bda8d5e8ffbd6e61fe9a),and_v(v:older(4032),multi(2,03aab896d53a8e7d6433137bbba940f9c521e085dd07e60994579b64a6d992cf79,0291b7d0b1b692f8f524516ed950872e5da10fb1b808b5a526dedc6fed1cf29807,0386aa9372fbab374593466bc5451dc59954e90787f08060964d95c87ef34ca5bb)))),elwpkh(0321da398ca2ddc09be89caa26e6730ae84751b6ea3a1ca46aa365bb5e1c3d9620))#qp4fan9q"); + assert_eq!( + pegin.to_string().parse::>().unwrap(), + pegin + ); } #[test] diff --git a/src/descriptor/pegin/legacy_pegin.rs b/src/descriptor/pegin/legacy_pegin.rs index 64fb9d5d..d0c49403 100644 --- a/src/descriptor/pegin/legacy_pegin.rs +++ b/src/descriptor/pegin/legacy_pegin.rs @@ -32,7 +32,6 @@ use bitcoin::blockdata::{opcodes, script}; use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; use bitcoin::{self, hashes, ScriptBuf as BtcScript}; use bitcoin_miniscript::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; -use bitcoin_miniscript::TranslatePk as BtcTranslatePk; use elements::secp256k1_zkp; use crate::descriptor::checksum::{self, verify_checksum}; @@ -42,7 +41,7 @@ use crate::policy::{semantic, Liftable}; use crate::util::varint_len; use crate::{ hash256, tweak_key, BtcError, BtcFromTree, BtcLiftable, BtcMiniscript, BtcPolicy, BtcSatisfier, - BtcSegwitv0, BtcTerminal, BtcTree, Descriptor, Error, MiniscriptKey, ToPublicKey, + BtcSegwitv0, BtcTerminal, Descriptor, Error, MiniscriptKey, ToPublicKey, }; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -167,7 +166,12 @@ impl LegacyPegin { fn from_ms_and_desc( desc: Descriptor>, ms: BtcMiniscript, - ) -> Self { + ) -> Result { + let invalid = || { + Error::BadDescriptor( + "legacy_pegin miniscript must be or_d(multi,and_v(v:older,multi))".to_string(), + ) + }; // Miniscript is a bunch of Arc's. So, cloning is not as bad. // Can we avoid this without NLL? let ms_clone = ms.clone(); @@ -175,31 +179,31 @@ impl LegacyPegin { if let (BtcTerminal::Multi(t), right) = (&a.node, &b.node) { (t.clone(), right) } else { - unreachable!("Only valid pegin miniscripts"); + return Err(invalid()); } } else { - unreachable!("Only valid pegin miniscripts"); + return Err(invalid()); }; let (timelock, emer) = if let BtcTerminal::AndV(l, r) = right { if let (BtcTerminal::Verify(csv), BtcTerminal::Multi(t)) = (&l.node, &r.node) { if let BtcTerminal::Older(timelock) = csv.node { (timelock, t.clone()) } else { - unreachable!("Only valid pegin miniscripts"); + return Err(invalid()); } } else { - unreachable!("Only valid pegin miniscripts"); + return Err(invalid()); } } else { - unreachable!("Only valid pegin miniscripts"); + return Err(invalid()); }; - Self { + Ok(Self { fed, emer, timelock, desc, ms, - } + }) } fn explicit_script( @@ -237,7 +241,10 @@ impl LegacyPegin { }; struct TranslateUnTweak; - impl bitcoin_miniscript::Translator for TranslateUnTweak { + impl bitcoin_miniscript::Translator for TranslateUnTweak { + type TargetPk = bitcoin::PublicKey; + type Error = (); + fn pk(&mut self, pk: &LegacyPeginKey) -> Result { Ok(*pk.as_untweaked()) } @@ -322,7 +329,7 @@ impl fmt::Display for LegacyPegin { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use fmt::Write; let mut wrapped_f = checksum::Formatter::new(f); - write!(wrapped_f, "legacy_pegin({},{})", self.ms, self.desc)?; + write!(wrapped_f, "legacy_pegin({},{:#})", self.ms, self.desc)?; wrapped_f.write_checksum_if_not_alt() } } @@ -342,21 +349,20 @@ impl BtcLiftable for LegacyPegin { impl_from_tree!( LegacyPegin, - fn from_tree(top: &expression::Tree<'_>) -> Result { - if top.name == "legacy_pegin" && top.args.len() == 2 { - // a roundtrip hack to use FromTree from bitcoin::Miniscript from - // expression::Tree in elements. - let ms_str = top.args[0].to_string(); - let ms_expr = BtcTree::from_str(&ms_str)?; - // - let ms = BtcMiniscript::::from_tree(&ms_expr); - let desc = Descriptor::>::from_tree(&top.args[1]); - Ok(LegacyPegin::from_ms_and_desc(desc?, ms?)) + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("legacy_pegin", Some(btc), Some(elem), None) = + (top.name(), args.next(), args.next(), args.next()) + { + let ms = BtcMiniscript::::from_tree(btc); + let desc = Descriptor::>::from_tree(elem); + LegacyPegin::from_ms_and_desc(desc?, ms?) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing legacy_pegin descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -368,7 +374,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); @@ -532,3 +538,71 @@ impl LegacyPegin { self.desc } } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use super::*; + + #[test] + fn legacy_pegin_from_str() { + let fed_key = "03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4"; + let emer_key = "0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0"; + let user = "0321da398ca2ddc09be89caa26e6730ae84751b6ea3a1ca46aa365bb5e1c3d9620"; + + let pegin = LegacyPegin::::from_str(&format!( + "legacy_pegin(or_d(multi(1,f{fed_key}),and_v(v:older(4032),multi(1,u{emer_key}))),elwpkh({user}))" + )) + .unwrap(); + let fed = LegacyPeginKey::Functionary(bitcoin::PublicKey::from_str(fed_key).unwrap()); + let emer = LegacyPeginKey::NonFunctionary(bitcoin::PublicKey::from_str(emer_key).unwrap()); + let expected = LegacyPegin::new( + bitcoin_miniscript::Threshold::new(1, vec![fed]).unwrap(), + bitcoin_miniscript::Threshold::new(1, vec![emer]).unwrap(), + bitcoin_miniscript::RelLockTime::from_consensus(4032).unwrap(), + Descriptor::from_str(&format!("elwpkh({user})")).unwrap(), + ); + assert_eq!(pegin, expected); + + let s = pegin.to_string(); + assert_eq!(s.matches('#').count(), 1, "{}", s); + assert_eq!( + LegacyPegin::::from_str(&s).unwrap(), + pegin + ); + + let invalid = format!("legacy_pegin(multi(1,f{fed_key}),elwpkh({user}))"); + assert!(matches!( + LegacyPegin::::from_str(&invalid), + Err(Error::BadDescriptor(_)) + )); + } + + #[test] + fn legacy_pegin_miniscript_shape() { + let fed = "f03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4"; + let emer = "u0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0"; + let desc = Descriptor::>::from_str( + "elwpkh(0321da398ca2ddc09be89caa26e6730ae84751b6ea3a1ca46aa365bb5e1c3d9620)", + ) + .unwrap(); + let valid = format!("or_d(multi(1,{fed}),and_v(v:older(4032),multi(1,{emer})))"); + let ms = BtcMiniscript::::from_str(&valid).unwrap(); + assert!(LegacyPegin::from_ms_and_desc(desc.clone(), ms).is_ok()); + + for text in [ + format!("multi(1,{fed})"), + format!("or_d(pk({fed}),and_v(v:older(4032),multi(1,{emer})))"), + format!("or_d(multi(1,{fed}),and_v(v:after(4032),multi(1,{emer})))"), + format!("or_d(multi(1,{fed}),and_v(v:older(4032),pk({emer})))"), + format!("or_d(multi(1,{fed}),pk({emer}))"), + ] { + let ms = BtcMiniscript::::from_str(&text).unwrap(); + assert!(matches!( + LegacyPegin::from_ms_and_desc(desc.clone(), ms), + Err(Error::BadDescriptor(_)) + )); + } + } +} diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 1bf6feca..9ba42702 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -139,11 +139,12 @@ impl Wsh { impl_block_str!( Wsh, // Constructor for creating inner wsh for the sh fragment - pub(super) fn from_inner_tree(top: &expression::Tree<'_>, ) -> Result + pub(super) fn from_inner_tree(top: expression::TreeIterItem<'_>, ) -> Result { - if top.name == "wsh" && top.args.len() == 1 { - let top = &top.args[0]; - if top.name == "sortedmulti" { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("wsh", Some(top), None) = (top.name(), args.next(), args.next()) { + if top.name() == "sortedmulti" { return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?), }); @@ -156,8 +157,8 @@ impl_block_str!( } else { Err(Error::Unexpected(format!( "{}({} args) while parsing wsh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -252,10 +253,11 @@ impl Liftable for Wsh { impl_from_tree!( Wsh, - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "elwsh" && top.args.len() == 1 { - let top = &top.args[0]; - if top.name == "sortedmulti" { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("elwsh", Some(top), None) = (top.name(), args.next(), args.next()) { + if top.name() == "sortedmulti" { return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?), }); @@ -268,8 +270,8 @@ impl_from_tree!( } else { Err(Error::Unexpected(format!( "{}({} args) while parsing wsh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -301,7 +303,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Wsh::::from_tree(&top) + Wsh::::from_tree(top.root()) } ); @@ -402,20 +404,23 @@ impl Wpkh { } // Parse a bitcoin style wpkh tree. Useful when parsing nested trees - pub(super) fn from_inner_tree(top: &expression::Tree<'_>) -> Result + pub(super) fn from_inner_tree(top: expression::TreeIterItem<'_>) -> Result where Pk: FromStr, ::Err: ToString, { - if top.name == "wpkh" && top.args.len() == 1 { - Ok(Wpkh::new(expression::terminal(&top.args[0], |pk| { - Pk::from_str(pk) - })?)?) + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("wpkh", Some(child), None) = (top.name(), args.next(), args.next()) { + Ok(Wpkh::new(expression::terminal( + child, + |pk| Pk::from_str(pk), + )?)?) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing wpkh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -508,16 +513,19 @@ impl Liftable for Wpkh { impl_from_tree!( Wpkh, - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "elwpkh" && top.args.len() == 1 { - Ok(Wpkh::new(expression::terminal(&top.args[0], |pk| { - Pk::from_str(pk) - })?)?) + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("elwpkh", Some(child), None) = (top.name(), args.next(), args.next()) { + Ok(Wpkh::new(expression::terminal( + child, + |pk| Pk::from_str(pk), + )?)?) } else { Err(Error::Unexpected(format!( "{}({} args) while parsing wpkh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -529,7 +537,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 55784ad2..16dfd51e 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -91,10 +91,11 @@ impl fmt::Display for Sh { impl_from_tree!( Sh, - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "elsh" && top.args.len() == 1 { - let top = &top.args[0]; - let inner = match top.name { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + let mut args = top.children(); + if let ("elsh", Some(top), None) = (top.name(), args.next(), args.next()) { + let inner = match top.name() { "wsh" => ShInner::Wsh(Wsh::from_inner_tree(top)?), "wpkh" => ShInner::Wpkh(Wpkh::from_inner_tree(top)?), "sortedmulti" => ShInner::SortedMulti(SortedMultiVec::from_tree(top)?), @@ -108,8 +109,8 @@ impl_from_tree!( } else { Err(Error::Unexpected(format!( "{}({} args) while parsing sh descriptor", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))) } } @@ -121,7 +122,7 @@ impl_from_str!( fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; let top = expression::Tree::from_str(desc_str)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } ); diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index 37bc09b6..196bf0f9 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -60,22 +60,26 @@ impl SortedMultiVec { }) } /// Parse an expression tree into a SortedMultiVec - pub fn from_tree(tree: &expression::Tree<'_>) -> Result + pub fn from_tree(tree: expression::TreeIterItem<'_>) -> Result where Pk: FromStr, ::Err: ToString, { - if tree.args.is_empty() { - return Err(errstr("no arguments given for sortedmulti")); + expression::verify_round_parens(tree)?; + let mut args = tree.children(); + let k = args + .next() + .ok_or_else(|| errstr("no arguments given for sortedmulti"))?; + if k.n_children() > 0 { + return Err(errstr(k.name())); } - let k = expression::parse_num::(tree.args[0].name)?; - if k > (tree.args.len() - 1) as u32 { + let k = expression::parse_num::(k.name())?; + if k > (tree.n_children() - 1) as u32 { return Err(errstr( "higher threshold than there were keys in sortedmulti", )); } - let pks: Result, _> = tree.args[1..] - .iter() + let pks: Result, _> = args .map(|sub| expression::terminal(sub, Pk::from_str)) .collect(); diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 02c16295..1e9bf761 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -1,10 +1,8 @@ // SPDX-License-Identifier: CC0-1.0 use std::cmp::{self, max}; -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, @@ -14,15 +12,16 @@ use elements::{self, opcodes, secp256k1_zkp, Script}; use super::checksum::verify_checksum; use super::ELMTS_STR; use crate::descriptor::checksum; -use crate::expression::{self, FromTree}; +use crate::expression::{self, FromTree, Parens}; use crate::extensions::ParseableExt; +use crate::miniscript::analyzable::ExtParams; use crate::miniscript::Miniscript; 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, - TranslateExt, TranslatePk, Translator, + Error, Extension, ForEachKey, MiniscriptKey, NoExt, Satisfier, Tap, ToPublicKey, TranslateExt, + TranslatePk, Translator, }; /// A Taproot Tree representation. @@ -38,9 +37,6 @@ pub enum TapTree { // in adding a LeafVersion with Leaf type here. All Miniscripts right now // are of Leafversion::default Leaf(Arc>), - /// A taproot leaf denoting a spending condition in terms of Simplicity - #[cfg(feature = "simplicity")] - SimplicityLeaf(Arc>), } /// A taproot descriptor @@ -119,8 +115,6 @@ impl TapTree { 1 + max(left_tree.taptree_height(), right_tree.taptree_height()) } TapTree::Leaf(..) => 0, - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(..) => 0, } } @@ -139,33 +133,12 @@ impl TapTree { Q: MiniscriptKey, Ext: Extension, { - #[cfg(feature = "simplicity")] - struct SimTranslator<'a, T>(&'a mut T); - - #[cfg(feature = "simplicity")] - impl<'a, Pk, T, Q, Error> simplicity::Translator for SimTranslator<'a, T> - where - Pk: MiniscriptKey, - T: Translator, - Q: MiniscriptKey, - { - fn pk(&mut self, pk: &Pk) -> Result { - self.0.pk(pk) - } - - fn sha256(&mut self, sha256: &Pk::Sha256) -> Result { - self.0.sha256(sha256) - } - } - let frag = match self { TapTree::Tree(l, r) => TapTree::Tree( Arc::new(l.translate_helper(t)?), Arc::new(r.translate_helper(t)?), ), TapTree::Leaf(ms) => TapTree::Leaf(Arc::new(ms.translate_pk(t)?)), - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(sim) => TapTree::SimplicityLeaf(Arc::new(sim.translate(&mut SimTranslator(t))?)) }; Ok(frag) } @@ -183,8 +156,6 @@ impl TapTree { Arc::new(r.translate_ext_helper(t)?), ), TapTree::Leaf(ms) => TapTree::Leaf(Arc::new(ms.translate_ext(t)?)), - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(sim) => TapTree::SimplicityLeaf(Arc::clone(sim)), }; Ok(frag) } @@ -195,8 +166,6 @@ impl fmt::Display for TapTree { match self { TapTree::Tree(ref left, ref right) => write!(f, "{{{},{}}}", *left, *right), TapTree::Leaf(ref script) => write!(f, "{}", *script), - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(ref policy) => write!(f, "sim{{{}}}", policy), } } } @@ -206,8 +175,6 @@ impl fmt::Debug for TapTree { match self { TapTree::Tree(ref left, ref right) => write!(f, "{{{:?},{:?}}}", *left, *right), TapTree::Leaf(ref script) => write!(f, "{:?}", *script), - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(ref policy) => write!(f, "{:?}", policy), } } } @@ -294,9 +261,6 @@ impl Tr { for (_depth, script) in self.iter_scripts() { match script { TapLeafScript::Miniscript(ms) => ms.sanity_check()?, - // TODO: Add sanity check for Simplicity policies - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => {}, } } Ok(()) @@ -444,9 +408,6 @@ impl Tr { pub enum TapLeafScript<'a, Pk: MiniscriptKey, Ext: Extension> { /// Miniscript leaf Miniscript(&'a Miniscript), - /// Simplicity leaf - #[cfg(feature = "simplicity")] - Simplicity(&'a simplicity::Policy) } impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { @@ -454,17 +415,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn as_miniscript(&self) -> Option<&'a Miniscript> { match self { TapLeafScript::Miniscript(ms) => Some(ms), - #[cfg(feature = "simplicity")] - _ => None, - } - } - - /// Get the Simplicity policy at the leaf, if it exists. - #[cfg(feature = "simplicity")] - pub fn as_simplicity(&self) -> Option<&'a simplicity::Policy> { - match self { - TapLeafScript::Simplicity(sim) => Some(sim), - _ => None, } } @@ -472,8 +422,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn version(&self) -> LeafVersion { match self { TapLeafScript::Miniscript(..) => LeafVersion::default(), - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => simplicity::leaf_version(), } } @@ -481,9 +429,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn script_size(&self) -> usize { match self { TapLeafScript::Miniscript(ms) => ms.script_size(), - // Simplicity's witness script is always a 32-byte CMR - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => 32, } } @@ -492,12 +437,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn max_satisfaction_witness_elements(&self) -> Result { match self { TapLeafScript::Miniscript(ms) => ms.max_satisfaction_witness_elements(), - // Simplicity always has one witness element plus leaf script: - // (1) Encoded program+witness - // (2) CMR program - // The third element is the control block, which is not counted by this method. - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => Ok(2), } } @@ -505,11 +444,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn max_satisfaction_size(&self) -> Result { match self { TapLeafScript::Miniscript(ms) => ms.max_satisfaction_size(), - // There is currently no way to bound the Simplicity witness size without producing one - // We mark the witness size as malleable since it depends on the chosen spending path - // TODO: Add method to simplicity::Policy and use it here - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => Err(Error::AnalysisError(crate::AnalysisError::Malleable)) } } @@ -517,8 +451,6 @@ impl<'a, Pk: MiniscriptKey, Ext: Extension> TapLeafScript<'a, Pk, Ext> { pub fn iter_pk(&self) -> Box + 'a> { match self { TapLeafScript::Miniscript(ms) => Box::new(ms.iter_pk()), - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(sim) => Box::new(sim.iter_pk()), } } } @@ -528,39 +460,20 @@ impl<'a, Pk: ToPublicKey, Ext: ParseableExt> TapLeafScript<'a, Pk, Ext> { pub fn encode(&self) -> Script { match self { TapLeafScript::Miniscript(ms) => ms.encode(), - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(sim) => { - Script::from(sim.cmr().as_ref().to_vec()) - } } } /// Attempt to produce a malleable satisfying witness for the leaf script. - /// - /// Returns [`Error::CouldNotSatisfy`] for Simplicity leaves until their descriptor - /// integration is rewritten. pub fn satisfy_malleable>(&self, satisfier: S) -> Result>, Error> { match self { TapLeafScript::Miniscript(ms) => ms.satisfy_malleable(satisfier), - // There doesn't (yet?) exist a malleable satisfaction of Simplicity policy - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => self.satisfy(satisfier), } } /// Attempt to produce a non-malleable satisfying witness for the leaf script. - /// - /// Returns [`Error::CouldNotSatisfy`] for Simplicity leaves until their descriptor - /// integration is rewritten. pub fn satisfy>(&self, satisfier: S) -> Result>, Error> { match self { TapLeafScript::Miniscript(ms) => ms.satisfy(satisfier), - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(..) => { - // TODO: The descriptor rewrite must supply a real ElementsEnv and - // branded inference context for Simplicity 0.9 satisfaction. - Err(Error::CouldNotSatisfy) - } } } } @@ -602,10 +515,6 @@ where TapTree::Leaf(ref ms) => { return Some((depth, TapLeafScript::Miniscript(ms))) }, - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(ref sim) => { - return Some((depth, TapLeafScript::Simplicity(sim))) - } } } None @@ -617,20 +526,16 @@ impl_block_str!( Tr, => Ext; Extension, // Helper function to parse taproot script path - fn parse_tr_script_spend(tree: &expression::Tree,) -> Result, Error> { - match tree { - #[cfg(feature = "simplicity")] - expression::Tree { name, args } if *name == "sim" && args.len() == 1 => { - let policy = crate::simplicity::PolicyWrapper::::from_str(args[0].name)?; - Ok(TapTree::SimplicityLeaf(Arc::new(policy.0))) - } - expression::Tree { name, args } if !name.is_empty() && args.is_empty() => { - let script = Miniscript::::from_str(name)?; - Ok(TapTree::Leaf(Arc::new(script))) - } - expression::Tree { name, args } if name.is_empty() && args.len() == 2 => { - let left = Self::parse_tr_script_spend(&args[0])?; - let right = Self::parse_tr_script_spend(&args[1])?; + fn parse_tr_script_spend(tree: expression::TreeIterItem<'_>,) -> Result, Error> { + if tree.parens() != Parens::Curly { + let script = Miniscript::::from_tree_ext(tree, &ExtParams::sane())?; + return Ok(TapTree::Leaf(Arc::new(script))); + } + let mut children = tree.children(); + match (tree.name(), children.next(), children.next(), children.next()) { + ("", Some(left), Some(right), None) => { + let left = Self::parse_tr_script_spend(left)?; + let right = Self::parse_tr_script_spend(right)?; Ok(TapTree::Tree(Arc::new(left), Arc::new(right))) } _ => Err(Error::Unexpected( @@ -644,44 +549,31 @@ impl_block_str!( impl_from_tree!( Tr, => Ext; Extension, - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "eltr" { - match top.args.len() { - 1 => { - let key = &top.args[0]; - if !key.args.is_empty() { - return Err(Error::Unexpected(format!( - "#{} script associated with `key-path` while parsing taproot descriptor", - key.args.len() - ))); - } + fn from_tree(top: expression::TreeIterItem<'_>) -> Result { + if top.name() == "eltr" { + expression::verify_round_parens(top)?; + let mut args = top.children(); + match (args.next(), args.next(), args.next()) { + (Some(key), None, None) => { Tr::new(expression::terminal(key, Pk::from_str)?, None) } - 2 => { - let key = &top.args[0]; - if !key.args.is_empty() { - return Err(Error::Unexpected(format!( - "#{} script associated with `key-path` while parsing taproot descriptor", - key.args.len() - ))); - } - let tree = &top.args[1]; + (Some(key), Some(tree), None) => { let ret = Self::parse_tr_script_spend(tree)?; Tr::new(expression::terminal(key, Pk::from_str)?, Some(ret)) } _ => { Err(Error::Unexpected(format!( "{}[#{} args] while parsing taproot descriptor", - top.name, - top.args.len() + top.name(), + top.n_children() ))) } } } else { Err(Error::Unexpected(format!( "{}[#{} args] while parsing taproot descriptor", - top.name, - top.args.len() + top.name(), + top.n_children() ))) } } @@ -693,8 +585,8 @@ impl_from_str!( type Err = Error;, fn from_str(s: &str) -> Result { let desc_str = verify_checksum(s)?; - let top = parse_tr_tree(desc_str)?; - Self::from_tree(&top) + let top = expression::Tree::from_str(desc_str)?; + Self::from_tree(top.root()) } ); @@ -720,70 +612,6 @@ impl fmt::Display for Tr { } } -// Helper function to parse string into miniscript tree form -fn parse_tr_tree(s: &str) -> Result, 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 Liftable for TapTree { fn lift(&self) -> Result, Error> { fn lift_helper( @@ -794,8 +622,6 @@ impl Liftable for TapTree { Ok(Policy::Threshold(1, vec![lift_helper(l)?, lift_helper(r)?])) } TapTree::Leaf(ref leaf) => leaf.lift(), - #[cfg(feature = "simplicity")] - TapTree::SimplicityLeaf(..) => panic!("FIXME: Cannot lift Simplicity policy to Miniscript semantic policy"), } } @@ -826,8 +652,6 @@ impl ForEachKey for Tr { .all(|(_d, script)| { match script { TapLeafScript::Miniscript(ms) => ms.for_each_key(&mut pred), - #[cfg(feature = "simplicity")] - TapLeafScript::Simplicity(sim) => crate::simplicity::for_each_key(sim, &mut pred), } }); script_keys_res && pred(&self.internal_key) @@ -952,55 +776,11 @@ where #[cfg(test)] mod tests { + use std::str::FromStr; + use super::*; use crate::{ForEachKey, NoExt}; - #[cfg(feature = "simplicity")] - #[test] - fn mixed_tree_satisfaction_skips_simplicity() { - let key = bitcoin::XOnlyPublicKey::from_str( - "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", - ) - .unwrap(); - let ms = - Miniscript::::from_str(&format!("pk({})", key)).unwrap(); - let script = ms.encode(); - let leaf_hash = - elements::taproot::TapLeafHash::from_script(&script, LeafVersion::default()); - let sig = elements::SchnorrSig { - sig: secp256k1_zkp::schnorr::Signature::from_slice(&[1; 64]).unwrap(), - hash_ty: elements::SchnorrSighashType::Default, - }; - let mut satisfier = std::collections::HashMap::new(); - satisfier.insert((key, leaf_hash), sig); - - for tree in [ - format!("{{pk({0}),sim{{pk({0})}}}}", key), - format!("{{sim{{pk({0})}},pk({0})}}", key), - ] { - let desc = Tr::::from_str(&format!("eltr({},{})", key, tree)) - .unwrap(); - let control_block = desc - .spend_info() - .control_block(&(script.clone(), LeafVersion::default())) - .unwrap(); - let expected = ( - vec![sig.to_vec(), script.to_bytes(), control_block.serialize()], - Script::new(), - ); - assert_eq!(desc.get_satisfaction(&satisfier).unwrap(), expected); - assert_eq!(desc.get_satisfaction_mall(&satisfier).unwrap(), expected); - assert!(matches!( - desc.get_satisfaction(()), - Err(Error::CouldNotSatisfy) - )); - assert!(matches!( - desc.get_satisfaction_mall(()), - Err(Error::CouldNotSatisfy) - )); - } - } - #[test] fn test_for_each() { let desc = "eltr(acc0, { @@ -1046,21 +826,35 @@ mod tests { "eltr(internal,pk(a))#vadmk9gd", "internal", &[TapLeafScript::Miniscript(&ms)] ); + } - #[cfg(feature = "simplicity")] - { - // Simplicity key spend - let sim = simplicity::Policy::Key("a".to_string()); - verify_from_str( - "eltr(internal,sim{pk(a)})#duhmnzmm", "internal", - &[TapLeafScript::Simplicity(&sim)] - ); - - // Mixed Miniscript and Simplicity - verify_from_str( - "eltr(internal,{pk(a),sim{pk(a)}})#7vmfhpaj", "internal", - &[TapLeafScript::Miniscript(&ms), TapLeafScript::Simplicity(&sim)] - ); + #[test] + fn tr_script_tree_branches() { + for s in [ + "eltr(A,pk(B))", + "eltr(A,{pk(B),pk(C)})", + "eltr(A,{pk(B),{pk(C),pk(D)}})", + "eltr(musig(A,B),{pk(C),pk(D)})", + ] { + assert!(Tr::::from_str(s).is_ok(), "{}", s); + } + // The script tree cannot be empty. Branches are nameless, use curly braces + // and have exactly two children. Curly braces are not allowed anywhere else. + for s in [ + "eltr", + "eltr(A,pk(B),pk(C))", + "eltr(A,)", + "eltr(A,{})", + "eltr(A,{pk(B)})", + "eltr(A,{pk(B),pk(C),pk(D)})", + "eltr(A,x{pk(B),pk(C)})", + "eltr(A,(pk(B),pk(C)))", + "eltr(A,and_v(v:pk(B),{pk(C),pk(D)}))", + "eltr(A,{pk(B),pk{C}})", + "eltr({A,B},pk(C))", + "eltr{A,pk(B)}", + ] { + assert!(Tr::::from_str(s).is_err(), "{}", s); } } } diff --git a/src/expression.rs b/src/expression.rs index a884c78f..46221a01 100644 --- a/src/expression.rs +++ b/src/expression.rs @@ -3,219 +3,58 @@ //! # Function-like Expression Language //! +//! Expression strings are parsed into trees by the rust-miniscript +//! [expression module](bitcoin_miniscript::expression), whose tree types are +//! re-exported here. This module provides the helpers used to convert the +//! nodes of such a tree into Elements Miniscript types. -use std::fmt; use std::str::FromStr; -use bitcoin_miniscript::expression::check_valid_chars; +pub use bitcoin_miniscript::expression::{Parens, Tree, TreeIterItem}; +use bitcoin_miniscript::ParseTreeError; -use crate::{errstr, Error, MAX_RECURSION_DEPTH}; - -#[derive(Debug, Clone)] -/// A token of the form `x(...)` or `x` -pub struct Tree<'a> { - /// The name `x` - pub name: &'a str, - /// The comma-separated contents of the `(...)`, if any - pub args: Vec>, -} -// or_b(pk(A),pk(B)) -// -// A = musig(musig(B,C),D,E) -// or_b() -// pk(A), pk(B) +use crate::{errstr, Error}; /// A trait for extracting a structure from a Tree representation in token form pub trait FromTree: Sized { /// Extract a structure from Tree representation - fn from_tree(top: &Tree<'_>) -> Result; -} - -impl<'a> fmt::Display for Tree<'a> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "({}", self.name)?; - for arg in &self.args { - write!(f, ",{}", arg)?; - } - write!(f, ")") - } -} - -enum Found { - Nothing, - LBracket(usize), // Either a left ( or { - Comma(usize), - RBracket(usize), // Either a right ) or } + fn from_tree(root: TreeIterItem<'_>) -> Result; } -fn next_expr(sl: &str, delim: char) -> Found { - // Decide whether we are parsing a key or not. - // When parsing a key ignore all the '(' and ')'. - // We keep count of lparan whenever we are inside a key context - // We exit the context whenever we find the corresponding ')' - // in which we entered the context. This allows to special case - // parse the '(' ')' inside key expressions.(key or musig(keys)). - let mut key_ctx = false; - let mut key_lparan_count = 0; - let mut found = Found::Nothing; - if delim == '(' { - for (n, ch) in sl.char_indices() { - match ch { - '(' => { - // already inside a key context - if key_ctx { - key_lparan_count += 1; - } else if &sl[..n] == "musig" { - key_lparan_count = 1; - key_ctx = true; - } else { - found = Found::LBracket(n); - break; - } - } - ',' => { - if !key_ctx { - found = Found::Comma(n); - break; - } - } - ')' => { - if key_ctx { - key_lparan_count -= 1; - if key_lparan_count == 0 { - key_ctx = false; - } - } else { - found = Found::RBracket(n); - break; - } - } - _ => {} +/// Writes a `musig(...)` key expression node back into its string form. +fn write_key_expression(node: TreeIterItem<'_>, s: &mut String) -> Result<(), Error> { + verify_round_parens(node)?; + s.push_str(node.name()); + if node.n_children() > 0 { + s.push('('); + for (n, child) in node.children().enumerate() { + if n > 0 { + s.push(','); } + write_key_expression(child, s)?; } - } else if delim == '{' { - let mut new_count = 0; - for (n, ch) in sl.char_indices() { - match ch { - '{' => { - found = Found::LBracket(n); - break; - } - '(' => { - new_count += 1; - } - ',' => { - if new_count == 0 { - found = Found::Comma(n); - break; - } - } - ')' => { - new_count -= 1; - } - '}' => { - 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"), + s.push(')'); } + Ok(()) } -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 { - name: sl, - args: vec![], - }, - "", - )), - // Terminal - Found::Comma(n) | Found::RBracket(n) => Ok(( - Tree { - name: &sl[..n], - args: vec![], - }, - &sl[n..], - )), - // Function call - Found::LBracket(n) => { - let mut ret = Tree { - name: &sl[..n], - args: vec![], - }; - - sl = &sl[n + 1..]; - loop { - 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(closing_delim(delim))); - } - - sl = &new_sl[1..]; - match new_sl.as_bytes()[0] { - b',' => {} - last_byte => { - if last_byte == closing_delim(delim) as u8 { - break; - } else { - return Err(Error::ExpectedChar(closing_delim(delim))); - } - } - } - } - Ok((ret, sl)) - } - } - } - - /// Parses a tree from a string - #[allow(clippy::should_implement_trait)] // seems to be a false positive - pub fn from_str(s: &'a str) -> Result, Error> { - check_valid_chars(s)?; - - let (top, rem) = Tree::from_slice(s)?; - if rem.is_empty() { - Ok(top) - } else { - Err(errstr(rem)) - } +/// Checks that the children of a node, if it has any, are enclosed in round brackets. +/// +/// Curly braces are only allowed in the script tree of a Taproot descriptor. +pub(crate) fn verify_round_parens(node: TreeIterItem<'_>) -> Result<(), Error> { + if node.parens() == Parens::Curly { + Err(Error::from(ParseTreeError::IllegalCurlyBrace { + pos: node.children_pos(), + })) + } else { + Ok(()) } } -/// Parse a string as a u32, for timelocks or thresholds +/// Parse a string as a number, for timelocks, thresholds, indices or values +/// +/// Unlike the rust-miniscript function of the same name, which only returns +/// `u32`, this parses any integer type, including signed ones. pub fn parse_num(s: &str) -> Result { if s.len() > 1 { let ch = s.chars().next().unwrap(); @@ -236,52 +75,68 @@ pub fn parse_num(s: &str) -> Result { } /// Attempts to parse a terminal expression -pub fn terminal(term: &Tree<'_>, convert: F) -> Result +/// +/// A `musig(...)` key expression is converted as a whole, including its children. +pub fn terminal(term: TreeIterItem<'_>, convert: F) -> Result where F: FnOnce(&str) -> Result, Err: ToString, { - if term.args.is_empty() { - convert(term.name).map_err(|e| Error::Unexpected(e.to_string())) + if term.n_children() == 0 { + convert(term.name()).map_err(|e| Error::Unexpected(e.to_string())) + } else if term.name() == "musig" { + let mut key = String::new(); + write_key_expression(term, &mut key)?; + convert(&key).map_err(|e| Error::Unexpected(e.to_string())) } else { - Err(errstr(term.name)) + Err(errstr(term.name())) } } /// Attempts to parse an expression with exactly one child -pub fn unary(term: &Tree<'_>, convert: F) -> Result +/// +/// This checks the number of children of `term`, not the type of its brackets. +pub fn unary(term: TreeIterItem<'_>, convert: F) -> Result 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)) + match (term.n_children(), term.first_child()) { + (1, Some(child)) => { + let left = FromTree::from_tree(child)?; + Ok(convert(left)) + } + _ => Err(errstr(term.name())), } } /// Attempts to parse an expression with exactly two children -pub fn binary(term: &Tree<'_>, convert: F) -> Result +/// +/// This checks the number of children of `term`, not the type of its brackets. +pub fn binary(term: TreeIterItem<'_>, convert: F) -> Result 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)) - } + let (left, right) = term + .verify_binary("binary expression") + .map_err(|_| errstr(term.name()))?; + let left = FromTree::from_tree(left)?; + let right = FromTree::from_tree(right)?; + Ok(convert(left, right)) } #[cfg(test)] mod tests { + use std::fmt; + use std::str::FromStr; use super::parse_num; + use crate::descriptor::pegin::{LegacyPegin, Pegin}; + use crate::extensions::{Arith, CovExtArgs, CovOps, CovenantExt, Expr}; + use crate::policy::{concrete, semantic}; + use crate::{ConfidentialDescriptor, Descriptor, DescriptorPublicKey, Miniscript, Segwitv0}; #[test] fn test_parse_num() { @@ -292,4 +147,135 @@ mod tests { assert!(parse_num::("+6").is_err()); assert!(parse_num::("-6").is_err()); } + + #[test] + fn extension_child_arity_and_order() { + use crate::extensions::Extension; + + for name in ["num64_eq", "num64_geq", "num64_gt", "num64_lt", "num64_leq"] { + let input = format!("{name}(1,2)"); + let parsed = Arith::::from_str(&input).unwrap(); + assert_eq!(parsed.to_string(), input); + + for args in ["", "1", "1,2,3", "1,2,3,4"] { + let input = format!("{name}({args})"); + let tree = super::Tree::from_str(&input).unwrap(); + let children: Vec<_> = tree.root().children().collect(); + assert!(Arith::::from_name_tree(name, &children).is_err()); + assert!(Arith::::from_str(&input).is_err()); + } + } + + for (name, args) in [ + ("is_exp_asset", "out_asset(0)"), + ("is_exp_value", "out_value(0)"), + ("asset_eq", "inp_asset(0),out_asset(1)"), + ("value_eq", "inp_value(0),out_value(1)"), + ("spk_eq", "inp_spk(0),out_spk(1)"), + ("curr_idx_eq", "3"), + ("idx_eq", "1,2"), + ] { + let input = format!("{name}({args})"); + let tree = super::Tree::from_str(&input).unwrap(); + let children: Vec<_> = tree.root().children().collect(); + let parsed = CovOps::::from_str(&input).unwrap(); + assert_eq!(parsed.to_string(), input); + assert_eq!( + CovOps::::from_name_tree(name, &children).ok(), + Some(parsed) + ); + + assert!( + CovOps::::from_name_tree(name, &children[..children.len() - 1]) + .is_err() + ); + let extra = format!("{name}({args},0,1,2)"); + let tree = super::Tree::from_str(&extra).unwrap(); + let children: Vec<_> = tree.root().children().collect(); + assert!(CovOps::::from_name_tree(name, &children).is_err()); + let err = CovOps::::from_str(&extra).unwrap_err(); + assert_eq!( + err.to_string(), + format!( + "unexpected «{}({} args) while parsing Extension»", + name, + children.len() + ) + ); + } + } + + /// Checks that `s` parses, and that it is rejected whenever any one pair of its + /// round brackets is replaced with curly braces. + fn check_curly_braces(s: &str) + where + T: FromStr, + T::Err: fmt::Debug, + { + T::from_str(s).unwrap(); + + let mut open = vec![]; + let mut n_variants = 0; + for (pos, ch) in s.char_indices() { + match ch { + '(' => open.push(pos), + ')' => { + let mut variant = s.as_bytes().to_vec(); + variant[open.pop().unwrap()] = b'{'; + variant[pos] = b'}'; + let variant = String::from_utf8(variant).unwrap(); + assert!(T::from_str(&variant).is_err(), "accepted {}", variant); + n_variants += 1; + } + _ => {} + } + } + assert!(n_variants > 0); + } + + #[test] + fn curly_braces_only_in_script_trees() { + type Desc = Descriptor>; + + check_curly_braces::>( + "and_v(v:pk(A),or_d(multi(1,B,C),thresh(1,pk(musig(D,E)),s:pk(F))))", + ); + check_curly_braces::>( + "or(1@pk(A),3@thresh(2,pk(B),sha256(H),and(pk(C),older(10))))", + ); + check_curly_braces::>("or(pk(A),and(pk(B),older(10)))"); + check_curly_braces::>("add(inp_v(0),mul(out_v(idx_add(1,curr_idx)),2))"); + check_curly_braces::>( + "num64_eq(price_oracle1(K,123213),add(inp_v(0),28004))", + ); + check_curly_braces::>("asset_eq(inp_asset(0),out_asset(curr_idx))"); + + for desc in [ + "elpkh(A)", + "elwpkh(A)", + "elsh(wpkh(A))", + "elsh(wsh(sortedmulti(1,A,B)))", + "elwsh(and_v(v:pk(A),or_d(pk(B),older(10))))", + "elc:pk_k(A)", + "elcovwsh(A,pk(B))", + "eltr(A,pk(B))", + "eltr(musig(A,B),{pk(C),and_v(v:pk(D),older(10))})", + "eltr(A,{pk(B),{and_v(v:pk(C),is_exp_asset(out_asset(0))),pk(D)}})", + ] { + check_curly_braces::(desc); + } + + let key = "03774eec7a3d550d18e9f89414152025b3b0ad6a342b19481f702d843cff06dfc4"; + let nums = "0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0"; + let mbk = "ab5824f4477b4ebb00a132adfd8eb0b7935cf24f6ac151add5d1913db374ce92"; + check_curly_braces::>(&format!( + "ct(slip77({mbk}),elwsh(or_d(pk({key}),and_v(v:pk({nums}),older(10)))))" + )); + check_curly_braces::>(&format!( + "pegin(wsh(multi(1,{key},{nums})),elwpkh({key}))" + )); + check_curly_braces::>(&format!( + "legacy_pegin(or_d(multi(1,f{key}),and_v(v:older(4032),multi(1,u{nums}))),elwpkh({key}))" + )); + } } diff --git a/src/extensions/arith.rs b/src/extensions/arith.rs index 583a57db..dd63151e 100644 --- a/src/extensions/arith.rs +++ b/src/extensions/arith.rs @@ -12,7 +12,7 @@ use elements::{opcodes, script, secp256k1_zkp as secp256k1, SchnorrSig, Transact use super::param::{ExtParamTranslator, TranslateExtParam}; use super::{CovExtArgs, CsfsKey, ExtParam, FromTokenIterError, IdxExpr, ParseableExt, TxEnv}; -use crate::expression::{FromTree, Tree}; +use crate::expression::{FromTree, TreeIterItem}; use crate::extensions::check_sig_price_oracle_1; use crate::miniscript::context::ScriptContextError; use crate::miniscript::lex::{Token as Tk, TokenIter}; @@ -1151,35 +1151,20 @@ impl FromStr for Expr { fn from_str(s: &str) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } } impl FromTree for Box> { - fn from_tree(top: &expression::Tree<'_>) -> Result { + fn from_tree(top: TreeIterItem<'_>) -> Result { expression::FromTree::from_tree(top).map(Box::new) } } impl FromTree for Expr { - fn from_tree(top: &expression::Tree<'_>) -> Result { - fn unary(top: &expression::Tree<'_>, frag: F) -> Result, Error> - where - F: FnOnce(Box>) -> ExprInner, - { - let l: Expr = FromTree::from_tree(&top.args[0])?; - Ok(Expr::from_inner(frag(Box::new(l)))) - } - - fn binary(top: &expression::Tree<'_>, frag: F) -> Result, Error> - where - F: FnOnce(Box>, Box>) -> ExprInner, - { - let l: Expr = FromTree::from_tree(&top.args[0])?; - let r: Expr = FromTree::from_tree(&top.args[1])?; - Ok(Expr::from_inner(frag(Box::new(l), Box::new(r)))) - } - match (top.name, top.args.len()) { + fn from_tree(top: TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + match (top.name(), top.n_children()) { ("inp_v", 1) => Ok(Expr::from_inner(expression::unary(top, ExprInner::Input)?)), ("curr_inp_v", 0) => Ok(Expr::from_inner(ExprInner::CurrInputIdx)), ("out_v", 1) => Ok(Expr::from_inner(expression::unary(top, ExprInner::Output)?)), @@ -1192,29 +1177,33 @@ impl FromTree for Expr { ExprInner::InputReIssue, )?)), ("price_oracle1", 2) | ("price_oracle1_w", 2) => { - if !top.args[0].args.is_empty() || !top.args[1].args.is_empty() { + let (pk, t) = top.verify_binary("price_oracle1")?; + if pk.n_children() != 0 || t.n_children() != 0 { return Err(Error::Unexpected(String::from( "price_oracle1 expects 2 terminal arguments", ))); } - let pk = T::arg_from_str(top.args[0].name, top.name, 0)?; - let t: u64 = expression::parse_num::(top.args[1].name)?; - if top.name == "price_oracle1" { + let pk = T::arg_from_str(pk.name(), top.name(), 0)?; + let t: u64 = expression::parse_num::(t.name())?; + if top.name() == "price_oracle1" { Ok(Expr::from_inner(ExprInner::PriceOracle1(pk, t))) } else { Ok(Expr::from_inner(ExprInner::PriceOracle1W(pk, t))) } } - ("add", 2) => binary(top, ExprInner::Add), - ("sub", 2) => binary(top, ExprInner::Sub), - ("mul", 2) => binary(top, ExprInner::Mul), - ("div", 2) => binary(top, ExprInner::Div), - ("mod", 2) => binary(top, ExprInner::Mod), - ("bitand", 2) => binary(top, ExprInner::BitAnd), - ("bitor", 2) => binary(top, ExprInner::BitOr), - ("bitxor", 2) => binary(top, ExprInner::Xor), - ("bitinv", 1) => unary(top, ExprInner::Invert), - ("neg", 1) => unary(top, ExprInner::Negate), + ("add", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Add)?)), + ("sub", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Sub)?)), + ("mul", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Mul)?)), + ("div", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Div)?)), + ("mod", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Mod)?)), + ("bitand", 2) => Ok(Expr::from_inner(expression::binary( + top, + ExprInner::BitAnd, + )?)), + ("bitor", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::BitOr)?)), + ("bitxor", 2) => Ok(Expr::from_inner(expression::binary(top, ExprInner::Xor)?)), + ("bitinv", 1) => Ok(Expr::from_inner(expression::unary(top, ExprInner::Invert)?)), + ("neg", 1) => Ok(Expr::from_inner(expression::unary(top, ExprInner::Negate)?)), (_num, 0) => { Ok(Expr { inner: expression::terminal(top, expression::parse_num::) @@ -1225,8 +1214,8 @@ impl FromTree for Expr { } _ => Err(Error::Unexpected(format!( "{}({} args) while parsing Extension", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))), } } @@ -1237,7 +1226,7 @@ impl FromStr for ArithInner { fn from_str(s: &str) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } } @@ -1251,26 +1240,41 @@ impl FromStr for Arith { } impl FromTree for Box> { - fn from_tree(top: &expression::Tree<'_>) -> Result { + fn from_tree(top: TreeIterItem<'_>) -> Result { ArithInner::from_tree(top).map(Box::new) } } impl FromTree for ArithInner { - fn from_tree(top: &expression::Tree<'_>) -> Result { - match (top.name, top.args.len()) { - // Disambiguiate with num64_eq to avoid confusion with asset_eq - ("num64_eq", 2) => expression::binary(top, ArithInner::Eq), - ("num64_geq", 2) => expression::binary(top, ArithInner::Geq), - ("num64_gt", 2) => expression::binary(top, ArithInner::Gt), - ("num64_lt", 2) => expression::binary(top, ArithInner::Lt), - ("num64_leq", 2) => expression::binary(top, ArithInner::Leq), - _ => Err(Error::Unexpected(format!( - "{}({} args) while parsing Extension", - top.name, - top.args.len(), - ))), - } + fn from_tree(top: TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + Self::from_name_children(top.name(), top.n_children(), top.children()) + } +} + +impl ArithInner { + fn from_name_children<'a>( + name: &str, + n_children: usize, + mut args: impl Iterator>, + ) -> Result { + let children = [args.next(), args.next(), args.next()]; + let (frag, left, right): (fn(Expr, Expr) -> Self, _, _) = match (name, children) { + ("num64_eq", [Some(left), Some(right), None]) => (ArithInner::Eq, left, right), + ("num64_geq", [Some(left), Some(right), None]) => (ArithInner::Geq, left, right), + ("num64_gt", [Some(left), Some(right), None]) => (ArithInner::Gt, left, right), + ("num64_lt", [Some(left), Some(right), None]) => (ArithInner::Lt, left, right), + ("num64_leq", [Some(left), Some(right), None]) => (ArithInner::Leq, left, right), + _ => { + return Err(Error::Unexpected(format!( + "{}({} args) while parsing Extension", + name, n_children, + ))) + } + }; + let l = Expr::from_tree(left)?; + let r = Expr::from_tree(right)?; + Ok(frag(l, r)) } } @@ -1351,14 +1355,10 @@ impl Extension for Arith { fn from_name_tree( name: &str, - children: &[expression::Tree<'_>], + children: &[TreeIterItem<'_>], ) -> Result { - let tree = Tree { - name, - args: children.to_vec(), // Cloning two references here, it is possible to avoid the to_vec() here, - // but it requires lot of refactor. - }; - let inner = ArithInner::from_tree(&tree).map_err(|_| FromTokenIterError)?; + let inner = ArithInner::from_name_children(name, children.len(), children.iter().copied()) + .map_err(|_| FromTokenIterError)?; Arith::new(inner).map_err(|_e| FromTokenIterError) } } diff --git a/src/extensions/csfs.rs b/src/extensions/csfs.rs index f11e3afe..f03ebc85 100644 --- a/src/extensions/csfs.rs +++ b/src/extensions/csfs.rs @@ -100,14 +100,16 @@ impl Extension for CheckSigFromStack { fn from_name_tree( name: &str, - children: &[expression::Tree<'_>], + children: &[expression::TreeIterItem<'_>], ) -> Result { if children.len() == 2 && name == "csfs" { - if !children[0].args.is_empty() || !children[1].args.is_empty() { + if children[0].n_children() != 0 || children[1].n_children() != 0 { return Err(FromTokenIterError); } - let pk = T::arg_from_str(children[0].name, name, 0).map_err(|_| FromTokenIterError)?; - let msg = T::arg_from_str(children[1].name, name, 1).map_err(|_| FromTokenIterError)?; + let pk = + T::arg_from_str(children[0].name(), name, 0).map_err(|_| FromTokenIterError)?; + let msg = + T::arg_from_str(children[1].name(), name, 1).map_err(|_| FromTokenIterError)?; Ok(Self { pk, msg }) } else { // Correct error handling while parsing fromtree diff --git a/src/extensions/index_ops.rs b/src/extensions/index_ops.rs index 00cd0d50..ebd82ace 100644 --- a/src/extensions/index_ops.rs +++ b/src/extensions/index_ops.rs @@ -7,7 +7,7 @@ use elements::opcodes::{self}; use elements::script; use super::{EvalError, TxEnv}; -use crate::expression::{FromTree, Tree}; +use crate::expression::{FromTree, TreeIterItem}; use crate::miniscript::lex::Token as Tk; use crate::{expression, script_num_size, Error}; @@ -76,29 +76,30 @@ impl fmt::Debug for IdxExpr { } impl FromTree for IdxExpr { - fn from_tree(top: &Tree<'_>) -> Result { - match (top.name, top.args.len()) { + fn from_tree(top: TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + match (top.name(), top.n_children()) { ("curr_idx", 0) => Ok(IdxExpr::CurrIdx), - ("idx_add", 2) => Ok(IdxExpr::Add( - Box::new(Self::from_tree(&top.args[0])?), - Box::new(Self::from_tree(&top.args[1])?), - )), - ("idx_sub", 2) => Ok(IdxExpr::Sub( - Box::new(Self::from_tree(&top.args[0])?), - Box::new(Self::from_tree(&top.args[1])?), - )), - ("idx_mul", 2) => Ok(IdxExpr::Mul( - Box::new(Self::from_tree(&top.args[0])?), - Box::new(Self::from_tree(&top.args[1])?), - )), - ("idx_div", 2) => Ok(IdxExpr::Div( - Box::new(Self::from_tree(&top.args[0])?), - Box::new(Self::from_tree(&top.args[1])?), - )), + ("idx_add", 2) => expression::binary(top, |left, right| { + IdxExpr::Add(Box::new(left), Box::new(right)) + }), + ("idx_sub", 2) => expression::binary(top, |left, right| { + IdxExpr::Sub(Box::new(left), Box::new(right)) + }), + ("idx_mul", 2) => expression::binary(top, |left, right| { + IdxExpr::Mul(Box::new(left), Box::new(right)) + }), + ("idx_div", 2) => expression::binary(top, |left, right| { + IdxExpr::Div(Box::new(left), Box::new(right)) + }), (_num, 0) => { expression::terminal(top, expression::parse_num::).map(IdxExpr::Const) } - _ => Err(Error::Unexpected(format!("Unexpected token: {:?}", top))), + _ => Err(Error::Unexpected(format!( + "Unexpected token: {}({} args)", + top.name(), + top.n_children() + ))), } } } diff --git a/src/extensions/introspect_ops.rs b/src/extensions/introspect_ops.rs index d4dc4f61..9c5cd8ca 100644 --- a/src/extensions/introspect_ops.rs +++ b/src/extensions/introspect_ops.rs @@ -14,7 +14,7 @@ use elements::{confidential, encode, script, Address, AddressParams}; use super::index_ops::IdxExpr; use super::param::{ExtParamTranslator, TranslateExtParam}; use super::{ArgFromStr, CovExtArgs, EvalError, ExtParam, FromTokenIterError, ParseableExt, TxEnv}; -use crate::expression::{FromTree, Tree}; +use crate::expression::{FromTree, TreeIterItem}; use crate::miniscript::context::ScriptContextError; use crate::miniscript::lex::{Token as Tk, TokenIter}; use crate::miniscript::satisfy::{Satisfaction, Witness}; @@ -176,21 +176,22 @@ impl fmt::Debug for AssetExpr { impl ArgFromStr for AssetExpr { fn arg_from_str(s: &str, parent: &str, pos: usize) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree_parent(&top, parent, pos) + Self::from_tree_parent(top.root(), parent, pos) } } impl AssetExpr { - fn from_tree_parent(top: &Tree<'_>, parent: &str, pos: usize) -> Result { - match (top.name, top.args.len()) { + fn from_tree_parent(top: TreeIterItem<'_>, parent: &str, pos: usize) -> Result { + expression::verify_round_parens(top)?; + match (top.name(), top.n_children()) { ("curr_inp_asset", 0) => Ok(AssetExpr::CurrInputAsset), ("inp_asset", 1) => expression::unary(top, AssetExpr::Input), ("out_asset", 1) => expression::unary(top, AssetExpr::Output), (asset, 0) => Ok(AssetExpr::Const(T::arg_from_str(asset, parent, pos)?)), _ => Err(Error::Unexpected(format!( "{}({} args) while parsing Extension", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))), } } @@ -248,21 +249,22 @@ impl fmt::Debug for ValueExpr { impl ArgFromStr for ValueExpr { fn arg_from_str(s: &str, parent: &str, pos: usize) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree_parent(&top, parent, pos) + Self::from_tree_parent(top.root(), parent, pos) } } impl ValueExpr { - fn from_tree_parent(top: &Tree<'_>, parent: &str, pos: usize) -> Result { - match (top.name, top.args.len()) { + fn from_tree_parent(top: TreeIterItem<'_>, parent: &str, pos: usize) -> Result { + expression::verify_round_parens(top)?; + match (top.name(), top.n_children()) { ("curr_inp_value", 0) => Ok(ValueExpr::CurrInputValue), ("inp_value", 1) => expression::unary(top, ValueExpr::Input), ("out_value", 1) => expression::unary(top, ValueExpr::Output), (value, 0) => Ok(ValueExpr::Const(T::arg_from_str(value, parent, pos)?)), _ => Err(Error::Unexpected(format!( "{}({} args) while parsing Extension", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))), } } @@ -320,21 +322,22 @@ impl fmt::Debug for SpkExpr { impl ArgFromStr for SpkExpr { fn arg_from_str(s: &str, parent: &str, pos: usize) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree_parent(&top, parent, pos) + Self::from_tree_parent(top.root(), parent, pos) } } impl SpkExpr { - fn from_tree_parent(top: &Tree<'_>, parent: &str, pos: usize) -> Result { - match (top.name, top.args.len()) { + fn from_tree_parent(top: TreeIterItem<'_>, parent: &str, pos: usize) -> Result { + expression::verify_round_parens(top)?; + match (top.name(), top.n_children()) { ("curr_inp_spk", 0) => Ok(SpkExpr::CurrInputSpk), ("inp_spk", 1) => expression::unary(top, SpkExpr::Input), ("out_spk", 1) => expression::unary(top, SpkExpr::Output), (asset, 0) => Ok(SpkExpr::Const(T::arg_from_str(asset, parent, pos)?)), _ => Err(Error::Unexpected(format!( "{}({} args) while parsing Extension", - top.name, - top.args.len(), + top.name(), + top.n_children(), ))), } } @@ -373,47 +376,57 @@ impl FromStr for CovOps { fn from_str(s: &str) -> Result { let top = expression::Tree::from_str(s)?; - Self::from_tree(&top) + Self::from_tree(top.root()) } } impl FromTree for CovOps { - fn from_tree(top: &Tree<'_>) -> Result { - match (top.name, top.args.len()) { - ("is_exp_asset", 1) => { - AssetExpr::from_tree_parent(&top.args[0], top.name, 0).map(CovOps::IsExpAsset) + fn from_tree(top: TreeIterItem<'_>) -> Result { + expression::verify_round_parens(top)?; + Self::from_name_children(top.name(), top.n_children(), top.children()) + } +} + +impl CovOps { + fn from_name_children<'a>( + name: &str, + n_children: usize, + mut args: impl Iterator>, + ) -> Result { + let children = [args.next(), args.next(), args.next()]; + match (name, children) { + ("is_exp_asset", [Some(child), None, None]) => { + AssetExpr::from_tree_parent(child, name, 0).map(CovOps::IsExpAsset) } - ("is_exp_value", 1) => { - ValueExpr::from_tree_parent(&top.args[0], top.name, 0).map(CovOps::IsExpValue) + ("is_exp_value", [Some(child), None, None]) => { + ValueExpr::from_tree_parent(child, name, 0).map(CovOps::IsExpValue) } - ("asset_eq", 2) => { - let l = AssetExpr::from_tree_parent(&top.args[0], top.name, 0)?; - let r = AssetExpr::from_tree_parent(&top.args[1], top.name, 1)?; + ("asset_eq", [Some(left), Some(right), None]) => { + let l = AssetExpr::from_tree_parent(left, name, 0)?; + let r = AssetExpr::from_tree_parent(right, name, 1)?; Ok(CovOps::AssetEq(l, r)) } - ("value_eq", 2) => { - let l = ValueExpr::from_tree_parent(&top.args[0], top.name, 0)?; - let r = ValueExpr::from_tree_parent(&top.args[1], top.name, 1)?; + ("value_eq", [Some(left), Some(right), None]) => { + let l = ValueExpr::from_tree_parent(left, name, 0)?; + let r = ValueExpr::from_tree_parent(right, name, 1)?; Ok(CovOps::ValueEq(l, r)) } - ("spk_eq", 2) => { - let l = SpkExpr::from_tree_parent(&top.args[0], top.name, 0)?; - let r = SpkExpr::from_tree_parent(&top.args[1], top.name, 1)?; + ("spk_eq", [Some(left), Some(right), None]) => { + let l = SpkExpr::from_tree_parent(left, name, 0)?; + let r = SpkExpr::from_tree_parent(right, name, 1)?; Ok(CovOps::SpkEq(l, r)) } - ("curr_idx_eq", 1) => { - expression::terminal(&top.args[0], expression::parse_num::) - .map(CovOps::CurrIndEq) + ("curr_idx_eq", [Some(child), None, None]) => { + expression::terminal(child, expression::parse_num::).map(CovOps::CurrIndEq) } - ("idx_eq", 2) => { - let l = IdxExpr::from_tree(&top.args[0])?; - let r = IdxExpr::from_tree(&top.args[1])?; + ("idx_eq", [Some(left), Some(right), None]) => { + let l = IdxExpr::from_tree(left)?; + let r = IdxExpr::from_tree(right)?; Ok(CovOps::IdxEq(l, r)) } _ => Err(Error::Unexpected(format!( "{}({} args) while parsing Extension", - top.name, - top.args.len(), + name, n_children, ))), } } @@ -470,13 +483,12 @@ impl Extension for CovOps { } } - fn from_name_tree(name: &str, children: &[Tree<'_>]) -> Result { - let tree = Tree { - name, - args: children.to_vec(), // Cloning references here, it is possible to avoid the to_vec() here, - // but it requires lot of refactor. - }; - Self::from_tree(&tree).map_err(|_| FromTokenIterError) + fn from_name_tree( + name: &str, + children: &[TreeIterItem<'_>], + ) -> Result { + Self::from_name_children(name, children.len(), children.iter().copied()) + .map_err(|_| FromTokenIterError) } fn segwit_ctx_checks(&self) -> Result<(), ScriptContextError> { diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index 0234a0bf..ea15d2bc 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -8,7 +8,7 @@ use bitcoin::hashes::{sha256, Hash}; use elements::script::Builder; use elements::{secp256k1_zkp, Transaction, TxOut}; -use crate::expression::Tree; +use crate::expression::TreeIterItem; use crate::interpreter::{self, Stack}; use crate::miniscript::context::ScriptContextError; use crate::miniscript::lex::TokenIter; @@ -64,11 +64,14 @@ pub trait Extension: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash { Ok(()) } - /// Create an instance of this object from a Tree with root name and children as - /// `Vec`. + /// Create an instance of this object from the name of an expression tree node, + /// with any wrappers removed, and the node's children. // Ideally, we would want a FromTree implementation here, but that is not possible - // as we would need to create a new Tree by removing wrappers from root. - fn from_name_tree(_name: &str, children: &[Tree<'_>]) -> Result; + // as the node name still includes the wrappers. + fn from_name_tree( + _name: &str, + children: &[TreeIterItem<'_>], + ) -> Result; } /// Support for parsing/serializing/satisfaction of extensions. @@ -143,7 +146,10 @@ impl Extension for NoExt { match *self {} } - fn from_name_tree(_name: &str, _children: &[Tree<'_>]) -> Result { + fn from_name_tree( + _name: &str, + _children: &[TreeIterItem<'_>], + ) -> Result { // No extensions should not parse any extensions from String Err(FromTokenIterError) } @@ -280,7 +286,10 @@ impl Extension for CovenantExt { all_arms_fn!(self, Extension, script_size,) } - fn from_name_tree(name: &str, children: &[Tree<'_>]) -> Result { + fn from_name_tree( + name: &str, + children: &[TreeIterItem<'_>], + ) -> Result { try_from_arms!(Extension, T, from_name_tree, name, children,) } diff --git a/src/extensions/outputs_pref.rs b/src/extensions/outputs_pref.rs index 8bc19d4d..e99c3397 100644 --- a/src/extensions/outputs_pref.rs +++ b/src/extensions/outputs_pref.rs @@ -107,10 +107,10 @@ impl Extension for LegacyOutputsPref { fn from_name_tree( name: &str, - children: &[expression::Tree<'_>], + children: &[expression::TreeIterItem<'_>], ) -> Result { if children.len() == 1 && name == "outputs_pref" { - let pref = expression::terminal(&children[0], Vec::::from_hex) + let pref = expression::terminal(children[0], Vec::::from_hex) .map_err(|_| FromTokenIterError)?; Ok(Self { pref }) } else { diff --git a/src/extensions/tx_ver.rs b/src/extensions/tx_ver.rs index 7b69ffca..b01bf4fd 100644 --- a/src/extensions/tx_ver.rs +++ b/src/extensions/tx_ver.rs @@ -86,10 +86,10 @@ impl Extension for LegacyVerEq { fn from_name_tree( name: &str, - children: &[expression::Tree<'_>], + children: &[expression::TreeIterItem<'_>], ) -> Result { if children.len() == 1 && name == "ver_eq" { - let n = expression::terminal(&children[0], expression::parse_num) + let n = expression::terminal(children[0], expression::parse_num) .map_err(|_| FromTokenIterError)?; Ok(Self { n }) } else { diff --git a/src/lib.rs b/src/lib.rs index 489f6495..d76928e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -108,7 +108,7 @@ extern crate test; // It can be confusing to code when we have two miniscript libraries // As a rule, only import the library here and pub use all the required // items. Should help in faster code development in the long run -use bitcoin_miniscript::expression::{FromTree as BtcFromTree, Tree as BtcTree}; +use bitcoin_miniscript::expression::FromTree as BtcFromTree; use bitcoin_miniscript::policy::semantic::Policy as BtcPolicy; use bitcoin_miniscript::policy::Liftable as BtcLiftable; // re-export imports @@ -134,15 +134,12 @@ pub mod miniscript; pub mod policy; pub mod psbt; -#[cfg(feature = "simplicity")] -mod simplicity; #[cfg(test)] mod test_utils; mod util; use std::{cmp, error, fmt, str}; -use bitcoin::hashes::sha256; use elements::secp256k1_zkp::Secp256k1; use elements::{locktime, opcodes, script, secp256k1_zkp}; @@ -279,7 +276,7 @@ where } /// Miniscript Error -#[derive(Debug, PartialEq)] +#[derive(Debug)] pub enum Error { /// Opcode appeared which is not part of the script subset InvalidOpcode(opcodes::All), @@ -296,16 +293,12 @@ pub enum Error { CmsTooManyKeys(u32), /// A tapscript multi_a cannot support more than MAX_BLOCK_WEIGHT/32 keys MultiATooManyKeys(u32), - /// expected character while parsing descriptor; didn't find one - ExpectedChar(char), /// While parsing backward, hit beginning of script UnexpectedStart, /// Got something we were not expecting Unexpected(String), /// Name of a fragment contained `:` multiple times MultiColon(String), - /// Name of a fragment contained `@` multiple times - MultiAt(String), /// Name of a fragment contained `@` but we were not parsing an OR AtOutsideOr(String), /// Encountered a `l:0` which is syntactically equal to `u:0` except stupid @@ -320,8 +313,6 @@ pub enum Error { BadPubkey(bitcoin::key::ParsePublicKeyError), /// Failed to parse a slice as public key BadPubkeySlice(bitcoin::key::FromSliceError), - /// Could not satisfy a script (fragment) because of a missing hash preimage - MissingHash(sha256::Hash), /// Could not satisfy a script (fragment) because of a missing signature MissingSig(bitcoin::PublicKey), /// Could not satisfy, relative locktime not met @@ -345,7 +336,10 @@ pub enum Error { LiftError(policy::LiftError), /// Forward script context related errors ContextError(miniscript::context::ScriptContextError), - /// Recursion depth exceeded when parsing policy/miniscript from string + /// A Taproot script tree is deeper than the 128 levels a Taproot control + /// block allows. [`descriptor::Tr::new`] returns it, including when parsing + /// `eltr` descriptors. Expressions nested too deeply for the expression + /// parser are reported as rust-miniscript parse errors in [`Error::BtcError`]. MaxRecursiveDepthExceeded, /// Script size too large ScriptSizeTooLarge, @@ -364,12 +358,8 @@ pub enum Error { CovError(descriptor::CovError), /// PubKey invalid under current context PubKeyCtxError(miniscript::decode::KeyParseError, &'static str), - /// Attempted to call function that requires PreComputed taproot info - TaprootSpendInfoUnavialable, /// No script code for Tr descriptors TrNoScriptCode, - /// No explicit script for Tr descriptors - TrNoExplicitScript, /// At least two BIP389 key expressions in the descriptor contain tuples of /// derivation indexes of different lengths. MultipathDescLenMismatch, @@ -400,6 +390,14 @@ impl From for Error { } } +#[doc(hidden)] +impl From for Error { + fn from(e: bitcoin_miniscript::ParseTreeError) -> Error { + let e = bitcoin_miniscript::ParseError::Tree(e); + Error::BtcError(bitcoin_miniscript::Error::Parse(e)) + } +} + #[doc(hidden)] impl From for Error { fn from(e: policy::LiftError) -> Error { @@ -465,8 +463,6 @@ fn errstr(s: &str) -> Error { Error::Unexpected(s.to_owned()) } -// https://github.com/sipa/miniscript/pull/5 for discussion on this number -const MAX_RECURSION_DEPTH: u32 = 402; // https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki const MAX_SCRIPT_SIZE: u32 = 10000; @@ -482,17 +478,14 @@ impl fmt::Display for Error { Error::Script(ref e) => fmt::Display::fmt(e, f), Error::AddrError(ref e) => fmt::Display::fmt(e, f), Error::CmsTooManyKeys(n) => write!(f, "checkmultisig with {} keys", n), - Error::ExpectedChar(c) => write!(f, "expected {}", c), Error::UnexpectedStart => f.write_str("unexpected start of script"), Error::Unexpected(ref s) => write!(f, "unexpected «{}»", s), Error::MultiColon(ref s) => write!(f, "«{}» has multiple instances of «:»", s), - Error::MultiAt(ref s) => write!(f, "«{}» has multiple instances of «@»", s), Error::AtOutsideOr(ref s) => write!(f, "«{}» contains «@» in non-or() context", s), Error::LikelyFalse => write!(f, "0 is not very likely (use «u:0»)"), Error::UnknownWrapper(ch) => write!(f, "unknown wrapper «{}:»", ch), Error::NonTopLevel(ref s) => write!(f, "non-T miniscript: {}", s), Error::Trailing(ref s) => write!(f, "trailing tokens: {}", s), - Error::MissingHash(ref h) => write!(f, "missing preimage of hash {}", h), Error::MissingSig(ref pk) => write!(f, "missing signature for key {:?}", pk), Error::RelativeLocktimeNotMet(n) => { write!(f, "required relative locktime CSV of {} blocks, not met", n) @@ -515,8 +508,8 @@ impl fmt::Display for Error { Error::LiftError(ref e) => fmt::Display::fmt(e, f), Error::MaxRecursiveDepthExceeded => write!( f, - "Recursive depth over {} not permitted", - MAX_RECURSION_DEPTH + "Taproot script tree depth over {} not permitted", + elements::taproot::TAPROOT_CONTROL_MAX_NODE_COUNT ), Error::ScriptSizeTooLarge => write!( f, @@ -538,9 +531,7 @@ impl fmt::Display for Error { write!(f, "Pubkey error: {} under {} scriptcontext", pk, ctx) } Error::MultiATooManyKeys(k) => write!(f, "MultiA too many keys {}", k), - Error::TaprootSpendInfoUnavialable => write!(f, "Taproot Spend Info not computed."), Error::TrNoScriptCode => write!(f, "No script code for Tr descriptors"), - Error::TrNoExplicitScript => write!(f, "No script code for Tr descriptors"), Error::MultipathDescLenMismatch => write!(f, "At least two BIP389 key expressions in the descriptor contain tuples of derivation indexes of different lengths"), Error::Conversion(ref e) => e.fmt(f), Error::UnsupportedAddressForPegin => write!(f, "Cannot create the address from the pegin descriptor, the federation descriptor is of an unsuppported type"), @@ -559,17 +550,14 @@ impl error::Error for Error { | InvalidPush(_) | CmsTooManyKeys(_) | MultiATooManyKeys(_) - | ExpectedChar(_) | UnexpectedStart | Unexpected(_) | MultiColon(_) - | MultiAt(_) | AtOutsideOr(_) | LikelyFalse | UnknownWrapper(_) | NonTopLevel(_) | Trailing(_) - | MissingHash(_) | MissingSig(_) | RelativeLocktimeNotMet(_) | AbsoluteLocktimeNotMet(_) @@ -581,10 +569,8 @@ impl error::Error for Error { | NonStandardBareScript | ImpossibleSatisfaction | BareDescriptorAddr - | TaprootSpendInfoUnavialable | TrNoScriptCode - | UnsupportedAddressForPegin - | TrNoExplicitScript => None, + | UnsupportedAddressForPegin => None, MultipathDescLenMismatch => None, BtcError(e) => Some(e), CovError(e) => Some(e), diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index 18b06341..527c8b6e 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -508,7 +508,7 @@ impl_from_tree!( ;Ctx; ScriptContext, Arc>, => Ext ; Extension, - fn from_tree(top: &expression::Tree<'_>) -> Result>, Error> { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result>, Error> { Ok(Arc::new(expression::FromTree::from_tree(top)?)) } ); @@ -517,11 +517,24 @@ impl_from_tree!( ;Ctx; ScriptContext, Terminal, => Ext ; Extension, - fn from_tree(top: &expression::Tree<'_>) -> Result, Error> { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result, Error> { + Terminal::from_tree_with_name(top.name(), top) + } +); + +impl_block_str!( + ;Ctx; ScriptContext, + Terminal, + => Ext ; Extension, + /// Parse an expression tree node into a fragment, using `node_name` in place + /// of the node's own name (e.g. with a descriptor prefix removed). + pub(crate) fn from_tree_with_name(node_name: &str, top: expression::TreeIterItem<'_>,) -> Result, Error> { + expression::verify_round_parens(top)?; + let mut args = top.children(); let mut aliased_wrap; let frag_name; let frag_wrap; - let mut name_split = top.name.split(':'); + let mut name_split = node_name.split(':'); match (name_split.next(), name_split.next(), name_split.next()) { (None, _, _) => { frag_name = ""; @@ -541,7 +554,7 @@ impl_from_tree!( } (Some(wrap), Some(name), None) => { if wrap.is_empty() { - return Err(Error::Unexpected(top.name.to_owned())); + return Err(Error::Unexpected(node_name.to_owned())); } if name == "pk" { frag_name = "pk_k"; @@ -559,58 +572,64 @@ impl_from_tree!( } } (Some(_), Some(_), Some(_)) => { - return Err(Error::MultiColon(top.name.to_owned())); + return Err(Error::MultiColon(node_name.to_owned())); } } - let mut unwrapped = match (frag_name, top.args.len()) { - ("expr_raw_pkh", 1) => expression::terminal(&top.args[0], |x| { + let mut unwrapped = match (frag_name, top.n_children(), top.first_child()) { + ("expr_raw_pkh", 1, Some(child)) => expression::terminal(child, |x| { hash160::Hash::from_str(x).map(Terminal::RawPkH) }), - ("pk_k", 1) => { - expression::terminal(&top.args[0], |x| Pk::from_str(x).map(Terminal::PkK)) + ("pk_k", 1, Some(child)) => { + expression::terminal(child, |x| Pk::from_str(x).map(Terminal::PkK)) } - ("pk_h", 1) => expression::terminal(&top.args[0], |x| Pk::from_str(x).map(Terminal::PkH)), - ("after", 1) => expression::terminal(&top.args[0], |x| { + ("pk_h", 1, Some(child)) => expression::terminal(child, |x| Pk::from_str(x).map(Terminal::PkH)), + ("after", 1, Some(child)) => expression::terminal(child, |x| { expression::parse_num::(x).map(|x| Terminal::After(AbsLockTime::from_consensus(x))) }), - ("older", 1) => expression::terminal(&top.args[0], |x| { + ("older", 1, Some(child)) => expression::terminal(child, |x| { expression::parse_num::(x).map(|x| Terminal::Older(Sequence::from_consensus(x))) }), - ("sha256", 1) => expression::terminal(&top.args[0], |x| { + ("sha256", 1, Some(child)) => expression::terminal(child, |x| { Pk::Sha256::from_str(x).map(Terminal::Sha256) }), - ("hash256", 1) => expression::terminal(&top.args[0], |x| { + ("hash256", 1, Some(child)) => expression::terminal(child, |x| { Pk::Hash256::from_str(x).map(Terminal::Hash256) }), - ("ripemd160", 1) => expression::terminal(&top.args[0], |x| { + ("ripemd160", 1, Some(child)) => expression::terminal(child, |x| { Pk::Ripemd160::from_str(x).map(Terminal::Ripemd160) }), - ("hash160", 1) => expression::terminal(&top.args[0], |x| { + ("hash160", 1, Some(child)) => expression::terminal(child, |x| { Pk::Hash160::from_str(x).map(Terminal::Hash160) }), - ("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_n", 2) => Ok(Terminal::AndOr( - expression::FromTree::from_tree(&top.args[0])?, - expression::FromTree::from_tree(&top.args[1])?, - Arc::new(Miniscript::from_ast(Terminal::False)?), - )), - ("andor", 3) => Ok(Terminal::AndOr( - expression::FromTree::from_tree(&top.args[0])?, - 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), - ("thresh", n) => { - if n == 0 { - return Err(errstr("no arguments given")); - } - let k = expression::terminal(&top.args[0], expression::parse_num::)? as usize; + ("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_n", 2, _) => { + let (left, right) = top.verify_binary("and_n")?; + Ok(Terminal::AndOr( + expression::FromTree::from_tree(left)?, + expression::FromTree::from_tree(right)?, + Arc::new(Miniscript::from_ast(Terminal::False)?), + )) + } + ("andor", 3, _) => { + let (Some(a), Some(b), Some(c)) = (args.next(), args.next(), args.next()) else { + return Err(errstr(node_name)); + }; + Ok(Terminal::AndOr( + expression::FromTree::from_tree(a)?, + expression::FromTree::from_tree(b)?, + expression::FromTree::from_tree(c)?, + )) + } + ("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), + ("thresh", n, _) => { + let threshold = args.next().ok_or_else(|| errstr("no arguments given"))?; + let k = expression::terminal(threshold, expression::parse_num::)? as usize; if k > n - 1 { return Err(errstr("higher threshold than there are subexpressions")); } @@ -618,26 +637,20 @@ impl_from_tree!( return Err(errstr("empty thresholds not allowed in descriptors")); } - let subs: Result>>, _> = top.args[1..] - .iter() - .map(expression::FromTree::from_tree) - .collect(); + let subs: Result>>, _> = + args.map(expression::FromTree::from_tree).collect(); Ok(Terminal::Thresh(k, subs?)) } - ("multi", n) | ("multi_a", n) => { - if n == 0 { - return Err(errstr("no arguments given")); - } - let k = expression::terminal(&top.args[0], expression::parse_num::)? as usize; + ("multi", n, _) | ("multi_a", n, _) => { + let threshold = args.next().ok_or_else(|| errstr("no arguments given"))?; + let k = expression::terminal(threshold, expression::parse_num::)? as usize; if k > n - 1 { return Err(errstr("higher threshold than there were keys in multi")); } - let pks: Result, _> = top.args[1..] - .iter() - .map(|sub| expression::terminal(sub, Pk::from_str)) - .collect(); + let pks: Result, _> = + args.map(|sub| expression::terminal(sub, Pk::from_str)).collect(); if frag_name == "multi" { pks.map(|pks| Terminal::Multi(k, pks)) @@ -646,14 +659,16 @@ impl_from_tree!( pks.map(|pks| Terminal::MultiA(k, pks)) } } - (name, _num_child) => { - // If nothing matches try to parse as extension - match Ext::from_name_tree(name, &top.args) { + (name, _num_child, _) => { + // If nothing matches try to parse as extension. Extensions are + // leaves, so the brackets of their whole subtree are checked here. + top.verify_no_curly_braces()?; + match Ext::from_name_tree(name, &args.collect::>()) { Ok(e) => Ok(Terminal::Ext(e)), Err(..) => Err(Error::Unexpected(format!( "{}({} args) while parsing Miniscript", - top.name, - top.args.len(), + node_name, + top.n_children(), ))), } } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index f5fd378d..a7f54972 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -416,7 +416,19 @@ impl_block_str!( { // This checks for invalid ASCII chars let top = expression::Tree::from_str(s)?; - let ms: Miniscript = expression::FromTree::from_tree(&top)?; + Miniscript::from_tree_ext(top.root(), ext) + } +); + +impl_block_str!( + ;Ctx; ScriptContext, + Miniscript, + => Ext ; Extension, + /// Parse an expression tree node into a top-level Miniscript, applying the + /// same checks as [`Miniscript::from_str_ext`]. + pub(crate) fn from_tree_ext(top: expression::TreeIterItem<'_>, ext: &ExtParams,) -> Result, Error> + { + let ms: Miniscript = expression::FromTree::from_tree(top)?; ms.ext_check(ext)?; if ms.ty.corr.base != types::Base::B { @@ -476,7 +488,7 @@ impl_from_tree!( ;Ctx; ScriptContext, Arc>, => Ext ; Extension, - fn from_tree(top: &expression::Tree<'_>) -> Result>, Error> { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result>, Error> { Ok(Arc::new(expression::FromTree::from_tree(top)?)) } ); @@ -487,14 +499,8 @@ impl_from_tree!( => Ext ; Extension, /// Parse an expression tree into a Miniscript. As a general rule, this /// should not be called directly; rather go through the descriptor API. - fn from_tree(top: &expression::Tree<'_>) -> Result, Error> { - let inner: Terminal = expression::FromTree::from_tree(top)?; - Ok(Miniscript { - ty: Type::type_check(&inner)?, - ext: ExtData::type_check(&inner)?, - node: inner, - phantom: PhantomData, - }) + fn from_tree(top: expression::TreeIterItem<'_>) -> Result, Error> { + Miniscript::from_ast(Terminal::from_tree_with_name(top.name(), top)?) } ); @@ -1077,7 +1083,7 @@ mod tests { assert!(Segwitv0Script::from_str_insane("🌏") .unwrap_err() .to_string() - .contains("unprintable character")); + .contains("invalid character")); } #[test] diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 949e8e87..dd191955 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -9,7 +9,6 @@ use std::collections::HashSet; use std::{error, fmt, str}; -use bitcoin_miniscript::expression::check_valid_chars; use elements::{LockTime, Sequence}; #[cfg(feature = "compiler")] use { @@ -1101,10 +1100,8 @@ impl_from_str!( Policy, type Err = Error;, fn from_str(s: &str) -> Result, Error> { - check_valid_chars(s)?; - let tree = expression::Tree::from_str(s)?; - let policy: Policy = FromTree::from_tree(&tree)?; + let policy: Policy = FromTree::from_tree(tree.root())?; policy.check_timelocks()?; Ok(policy) } @@ -1117,12 +1114,14 @@ impl_block_str!( Policy, /// Helper function for `from_tree` to parse subexpressions with /// names of the form x@y - fn from_tree_prob(top: &expression::Tree, allow_prob: bool,) + fn from_tree_prob(top: expression::TreeIterItem<'_>, allow_prob: bool,) -> Result<(usize, Policy), Error> { + expression::verify_round_parens(top)?; + let mut args = top.children(); let frag_prob; let frag_name; - let mut name_split = top.name.split('@'); + let mut name_split = top.name().split('@'); match (name_split.next(), name_split.next(), name_split.next()) { (None, _, _) => { frag_prob = 1; @@ -1134,21 +1133,21 @@ impl_block_str!( } (Some(prob), Some(name), None) => { if !allow_prob { - return Err(Error::AtOutsideOr(top.name.to_owned())); + return Err(Error::AtOutsideOr(top.name().to_owned())); } frag_prob = expression::parse_num::(prob)? as usize; frag_name = name; } (Some(_), Some(_), Some(_)) => { - return Err(Error::MultiColon(top.name.to_owned())); + return Err(Error::MultiColon(top.name().to_owned())); } } - match (frag_name, top.args.len() as u32) { - ("UNSATISFIABLE", 0) => Ok(Policy::Unsatisfiable), - ("TRIVIAL", 0) => Ok(Policy::Trivial), - ("pk", 1) => expression::terminal(&top.args[0], |pk| Pk::from_str(pk).map(Policy::Key)), - ("after", 1) => { - let num = expression::terminal(&top.args[0], expression::parse_num)?; + match (frag_name, top.n_children() as u32, top.first_child()) { + ("UNSATISFIABLE", 0, _) => Ok(Policy::Unsatisfiable), + ("TRIVIAL", 0, _) => Ok(Policy::Trivial), + ("pk", 1, Some(child)) => expression::terminal(child, |pk| Pk::from_str(pk).map(Policy::Key)), + ("after", 1, Some(child)) => { + let num = expression::terminal(child, expression::parse_num)?; if num > 2u32.pow(31) { return Err(Error::PolicyError(PolicyError::TimeTooFar)); } else if num == 0 { @@ -1156,8 +1155,8 @@ impl_block_str!( } Ok(Policy::after(num)) } - ("older", 1) => { - let num = expression::terminal(&top.args[0], expression::parse_num)?; + ("older", 1, Some(child)) => { + let num = expression::terminal(child, expression::parse_num)?; if num > 2u32.pow(31) { return Err(Error::PolicyError(PolicyError::TimeTooFar)); } else if num == 0 { @@ -1165,55 +1164,56 @@ impl_block_str!( } Ok(Policy::older(num)) } - ("sha256", 1) => expression::terminal(&top.args[0], |x| { + ("sha256", 1, Some(child)) => expression::terminal(child, |x| { ::from_str(x).map(Policy::Sha256) }), - ("hash256", 1) => expression::terminal(&top.args[0], |x| { + ("hash256", 1, Some(child)) => expression::terminal(child, |x| { ::from_str(x).map(Policy::Hash256) }), - ("ripemd160", 1) => expression::terminal(&top.args[0], |x| { + ("ripemd160", 1, Some(child)) => expression::terminal(child, |x| { ::from_str(x).map(Policy::Ripemd160) }), - ("hash160", 1) => expression::terminal(&top.args[0], |x| { + ("hash160", 1, Some(child)) => expression::terminal(child, |x| { ::from_str(x).map(Policy::Hash160) }), - ("and", _) => { - if top.args.len() != 2 { + ("and", _, _) => { + if top.n_children() != 2 { return Err(Error::PolicyError(PolicyError::NonBinaryArgAnd)); } - let mut subs = Vec::with_capacity(top.args.len()); - for arg in &top.args { + let mut subs = Vec::with_capacity(top.n_children()); + for arg in args { subs.push(Policy::from_tree(arg)?); } Ok(Policy::And(subs)) } - ("or", _) => { - if top.args.len() != 2 { + ("or", _, _) => { + if top.n_children() != 2 { return Err(Error::PolicyError(PolicyError::NonBinaryArgOr)); } - let mut subs = Vec::with_capacity(top.args.len()); - for arg in &top.args { + let mut subs = Vec::with_capacity(top.n_children()); + for arg in args { subs.push(Policy::from_tree_prob(arg, true)?); } Ok(Policy::Or(subs)) } - ("thresh", nsubs) => { - if top.args.is_empty() || !top.args[0].args.is_empty() { - return Err(Error::PolicyError(PolicyError::IncorrectThresh)); - } + ("thresh", nsubs, _) => { + let k = match args.next() { + Some(k) if k.n_children() == 0 => k, + _ => return Err(Error::PolicyError(PolicyError::IncorrectThresh)), + }; - let thresh = expression::parse_num::(top.args[0].name)?; + let thresh = expression::parse_num::(k.name())?; if thresh >= nsubs || thresh == 0 { return Err(Error::PolicyError(PolicyError::IncorrectThresh)); } - let mut subs = Vec::with_capacity(top.args.len() - 1); - for arg in &top.args[1..] { + let mut subs = Vec::with_capacity(top.n_children() - 1); + for arg in args { subs.push(Policy::from_tree(arg)?); } Ok(Policy::Threshold(thresh as usize, subs)) } - _ => Err(errstr(top.name)), + _ => Err(errstr(top.name())), } .map(|res| (frag_prob, res)) } @@ -1221,7 +1221,7 @@ impl_block_str!( impl_from_tree!( Policy, - fn from_tree(top: &expression::Tree) -> Result, Error> { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result, Error> { Policy::from_tree_prob(top, false).map(|(_, result)| result) } ); diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 77193a5f..0df42f90 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -6,7 +6,6 @@ use std::str::FromStr; use std::{fmt, str}; -use bitcoin_miniscript::expression::check_valid_chars; use elements::{LockTime, Sequence}; use super::concrete::PolicyError; @@ -287,10 +286,8 @@ impl_from_str!( Policy, type Err = Error;, fn from_str(s: &str) -> Result, Error> { - check_valid_chars(s)?; - let tree = expression::Tree::from_str(s)?; - expression::FromTree::from_tree(&tree) + expression::FromTree::from_tree(tree.root()) } ); @@ -298,59 +295,63 @@ serde_string_impl_pk!(Policy, "a miniscript semantic policy"); impl_from_tree!( Policy, - fn from_tree(top: &expression::Tree) -> Result, Error> { - match (top.name, top.args.len()) { - ("UNSATISFIABLE", 0) => Ok(Policy::Unsatisfiable), - ("TRIVIAL", 0) => Ok(Policy::Trivial), - ("pk", 1) => expression::terminal(&top.args[0], |pk| Pk::from_str(pk).map(Policy::Key)), - ("after", 1) => expression::terminal(&top.args[0], |x| { + fn from_tree(top: expression::TreeIterItem<'_>) -> Result, Error> { + expression::verify_round_parens(top)?; + let mut args = top.children(); + match (top.name(), top.n_children(), top.first_child()) { + ("UNSATISFIABLE", 0, _) => Ok(Policy::Unsatisfiable), + ("TRIVIAL", 0, _) => Ok(Policy::Trivial), + ("pk", 1, Some(child)) => { + expression::terminal(child, |pk| Pk::from_str(pk).map(Policy::Key)) + } + ("after", 1, Some(child)) => expression::terminal(child, |x| { expression::parse_num::(x).map(|x| Policy::after(x)) }), - ("older", 1) => expression::terminal(&top.args[0], |x| { + ("older", 1, Some(child)) => expression::terminal(child, |x| { expression::parse_num::(x).map(|x| Policy::older(x)) }), - ("sha256", 1) => expression::terminal(&top.args[0], |x| { - Pk::Sha256::from_str(x).map(Policy::Sha256) - }), - ("hash256", 1) => expression::terminal(&top.args[0], |x| { - Pk::Hash256::from_str(x).map(Policy::Hash256) - }), - ("ripemd160", 1) => expression::terminal(&top.args[0], |x| { - Pk::Ripemd160::from_str(x).map(Policy::Ripemd160) - }), - ("hash160", 1) => expression::terminal(&top.args[0], |x| { - Pk::Hash160::from_str(x).map(Policy::Hash160) - }), - ("and", nsubs) => { + ("sha256", 1, Some(child)) => { + expression::terminal(child, |x| Pk::Sha256::from_str(x).map(Policy::Sha256)) + } + ("hash256", 1, Some(child)) => { + expression::terminal(child, |x| Pk::Hash256::from_str(x).map(Policy::Hash256)) + } + ("ripemd160", 1, Some(child)) => { + expression::terminal(child, |x| Pk::Ripemd160::from_str(x).map(Policy::Ripemd160)) + } + ("hash160", 1, Some(child)) => { + expression::terminal(child, |x| Pk::Hash160::from_str(x).map(Policy::Hash160)) + } + ("and", nsubs, _) => { if nsubs < 2 { return Err(Error::PolicyError(PolicyError::InsufficientArgsforAnd)); } let mut subs = Vec::with_capacity(nsubs); - for arg in &top.args { + for arg in args { subs.push(Policy::from_tree(arg)?); } Ok(Policy::Threshold(nsubs, subs)) } - ("or", nsubs) => { + ("or", nsubs, _) => { if nsubs < 2 { return Err(Error::PolicyError(PolicyError::InsufficientArgsforOr)); } let mut subs = Vec::with_capacity(nsubs); - for arg in &top.args { + for arg in args { subs.push(Policy::from_tree(arg)?); } Ok(Policy::Threshold(1, subs)) } - ("thresh", nsubs) => { - if nsubs == 0 || nsubs == 1 { - // thresh() and thresh(k) are err - return Err(errstr("thresh without args")); - } - if !top.args[0].args.is_empty() { - return Err(errstr(top.args[0].args[0].name)); + ("thresh", nsubs, _) => { + let k = match (nsubs, args.next()) { + (2.., Some(k)) => k, + _ => return Err(errstr("thresh without args")), + }; + if let Some(arg) = k.first_child() { + return Err(errstr(arg.name())); } - let thresh = expression::parse_num::(top.args[0].name)?; + let thresh = expression::parse_num::(k.name())?; // thresh(1) and thresh(n) are disallowed in semantic policies if thresh <= 1 || thresh >= (nsubs as u32 - 1) { @@ -359,16 +360,16 @@ impl_from_tree!( )); } if thresh >= (nsubs as u32) { - return Err(errstr(top.args[0].name)); + return Err(errstr(k.name())); } - let mut subs = Vec::with_capacity(top.args.len() - 1); - for arg in &top.args[1..] { + let mut subs = Vec::with_capacity(nsubs - 1); + for arg in args { subs.push(Policy::from_tree(arg)?); } Ok(Policy::Threshold(thresh as usize, subs)) } - _ => Err(errstr(top.name)), + _ => Err(errstr(top.name())), } } ); diff --git a/src/simplicity.rs b/src/simplicity.rs deleted file mode 100644 index 2871a1a1..00000000 --- a/src/simplicity.rs +++ /dev/null @@ -1,178 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 -use std::fmt; -use std::str::FromStr; -use std::sync::Arc; - -use simplicity::{FailEntropy, Policy}; - -use crate::policy::concrete::PolicyError; -use crate::{expression, Error, MiniscriptKey}; - -impl_from_tree!( - Policy, - fn from_tree(top: &expression::Tree) -> Result { - match (top.name, top.args.len() as u32) { - ("UNSATISFIABLE", 0) => Ok(Policy::Unsatisfiable(FailEntropy::ZERO)), - ("TRIVIAL", 0) => Ok(Policy::Trivial), - ("pk", 1) => expression::terminal(&top.args[0], |pk| Pk::from_str(pk).map(Policy::Key)), - ("after", 1) => expression::terminal(&top.args[0], |x| { - expression::parse_num(x).map(Policy::After) - }), - ("older", 1) => expression::terminal(&top.args[0], |x| { - expression::parse_num(x).map(Policy::Older) - }), - ("sha256", 1) => expression::terminal(&top.args[0], |x| { - Pk::Sha256::from_str(x).map(Policy::Sha256) - }), - ("and", _) => { - if top.args.len() != 2 { - return Err(Error::PolicyError(PolicyError::NonBinaryArgAnd)); - } - let left = Arc::new(Policy::from_tree(&top.args[0])?); - let right = Arc::new(Policy::from_tree(&top.args[0])?); - Ok(Policy::And { left, right }) - } - ("or", _) => { - if top.args.len() != 2 { - return Err(Error::PolicyError(PolicyError::NonBinaryArgOr)); - } - let left = Arc::new(Policy::from_tree(&top.args[0])?); - let right = Arc::new(Policy::from_tree(&top.args[0])?); - Ok(Policy::Or { left, right }) - } - ("thresh", nsubs) => { - if nsubs == 0 { - return Err(Error::Unexpected("thresh without args".to_owned())); - } - if nsubs < 3 { - return Err(Error::Unexpected( - "thresh must have a threshold value and at least 2 children".to_owned(), - )); - } - if !top.args[0].args.is_empty() { - return Err(Error::Unexpected(top.args[0].args[0].name.to_owned())); - } - - let thresh: u32 = expression::parse_num(top.args[0].name)?; - if thresh >= nsubs { - return Err(Error::Unexpected(top.args[0].name.to_owned())); - } - - let mut subs = Vec::with_capacity(top.args.len() - 1); - for arg in &top.args[1..] { - subs.push(Policy::from_tree(arg)?); - } - Ok(Policy::Threshold(thresh as usize, subs)) - } - _ => Err(Error::Unexpected(top.name.to_owned())), - } - } -); - -// We cannot implement FromStr for Policy because neither is defined in this crate -// Use a crate-local wrapper type to avoid code repetition -// Users use `Tr` / `Descriptor` and never encounter this wrapper -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, std::hash::Hash)] -pub(crate) struct PolicyWrapper(pub Policy); - -impl fmt::Debug for PolicyWrapper { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(&self.0, f) - } -} - -impl_from_str!( - PolicyWrapper, - type Err = Error;, - fn from_str(s: &str) -> Result { - let tree = expression::Tree::from_str(s)?; - as expression::FromTree>::from_tree(&tree).map(PolicyWrapper) - } -); - -// We cannot implement ForEachKey for Policy because it is not defined in this crate -// We cannot use our wrapper because we don't own the Policy (we have a reference) -// Implementing a wrapper of Cow<'a, Policy> leads to lifetime issues -// when implementing ForEachKey, because for_each_key() has its own lifetime 'a -pub fn for_each_key<'a, Pk: MiniscriptKey + 'a, F: FnMut(&'a Pk) -> bool>(policy: &'a Policy, mut pred: F) -> bool -{ - let mut stack = vec![policy]; - - while let Some(top) = stack.pop() { - match top { - Policy::Key(key) => { - if !pred(key) { - return false; - } - } - Policy::And { left, right } | Policy::Or { left, right } => { - stack.push(right); - stack.push(left); - } - Policy::Threshold(_, sub_policies) => { - stack.extend(sub_policies.iter()); - } - _ => {} - } - } - - true -} - -#[cfg(test)] -mod tests { - use secp256k1::XOnlyPublicKey; - use crate::DescriptorPublicKey; - use super::*; - - #[test] - fn parse_bad_thresh() { - assert_eq!( - PolicyWrapper::::from_str("thresh()"), - Err(Error::Unexpected( - "thresh must have a threshold value and at least 2 children".to_string() - )), - ); - - assert_eq!( - PolicyWrapper::::from_str("thresh"), - Err(Error::Unexpected("thresh without args".to_string())), - ); - - assert_eq!( - PolicyWrapper::::from_str("thresh(0)"), - Err(Error::Unexpected( - "thresh must have a threshold value and at least 2 children".to_string() - )), - ); - - assert_eq!( - PolicyWrapper::::from_str("thresh(0,TRIVIAL)"), - Err(Error::Unexpected( - "thresh must have a threshold value and at least 2 children".to_string() - )), - ); - - assert!(PolicyWrapper::::from_str("thresh(0,TRIVIAL,TRIVIAL)").is_ok()); - assert!(PolicyWrapper::::from_str("thresh(2,TRIVIAL,TRIVIAL)").is_ok()); - - assert_eq!( - PolicyWrapper::::from_str("thresh(3,TRIVIAL,TRIVIAL)"), - Err(Error::Unexpected("3".to_string())), - ); - } - - #[test] - fn decode_xpub() { - let s = "[78412e3a/44'/0'/0']xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1/*"; - let decoded_key = DescriptorPublicKey::from_str(s).expect("constant key"); - let s = format!("pk({})", s); - let decoded_policy = PolicyWrapper::::from_str(&s).expect("decode policy").0; - - if let Policy::Key(key) = decoded_policy { - assert_eq!(decoded_key, key); - } else { - panic!("Decoded policy should be public key") - } - } -}