diff --git a/fuzz/README.md b/fuzz/README.md index 2b4a2730b..40827c54f 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -118,28 +118,12 @@ as follows: ``` #[cfg(test)] mod tests { - fn extend_vec_from_hex(hex: &str, out: &mut Vec) { - let mut b = 0; - for (idx, c) in hex.as_bytes().iter().enumerate() { - b <<= 4; - match *c { - b'A'...b'F' => b |= c - b'A' + 10, - b'a'...b'f' => b |= c - b'a' + 10, - b'0'...b'9' => b |= c - b'0', - _ => panic!("Bad hex"), - } - if (idx & 1) == 1 { - out.push(b); - b = 0; - } - } - } + use miniscript::bitcoin::hex::FromHex; #[test] fn duplicate_crash() { - let mut a = Vec::new(); - extend_vec_from_hex("048531e80700ae6400670000af5168", &mut a); - super::do_test(&a); + let v = Vec::from_hex("abcd").unwrap(); + super::do_test(&v); } } ``` diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 63cb8168a..3ff49cc39 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -370,15 +370,11 @@ impl Liftable for Pkh { impl FromTree for Pkh { fn from_tree(top: &expression::Tree) -> Result { - if top.name == "pkh" && top.args.len() == 1 { - Ok(Pkh::new(expression::terminal(&top.args[0], |pk| Pk::from_str(pk))?)?) - } else { - Err(Error::Unexpected(format!( - "{}({} args) while parsing pkh descriptor", - top.name, - top.args.len(), - ))) - } + let top = top + .verify_toplevel("pkh", 1..=1) + .map_err(From::from) + .map_err(Error::Parse)?; + Ok(Pkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?) } } diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 6ae296433..04c76dc09 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -8,8 +8,8 @@ //! [BIP-380]: use core::convert::TryFrom; -use core::fmt; use core::iter::FromIterator; +use core::{array, fmt}; use bech32::primitives::checksum::PackedFe32; use bech32::{Checksum, Fe32}; @@ -115,10 +115,10 @@ pub fn verify_checksum(s: &str) -> Result<&str, Error> { eng.input_unchecked(s[..last_hash_pos].as_bytes()); let expected = eng.checksum_chars(); - let mut actual = ['_'; CHECKSUM_LENGTH]; - for (act, ch) in actual.iter_mut().zip(checksum_str.chars()) { - *act = ch; - } + + let mut iter = checksum_str.chars(); + let actual: [char; CHECKSUM_LENGTH] = + array::from_fn(|_| iter.next().expect("length checked above")); if expected != actual { return Err(Error::InvalidChecksum { actual, expected }); diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 6c973cc5d..72d2f57a2 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -21,6 +21,7 @@ use bitcoin::{ }; use sync::Arc; +use crate::expression::FromTree as _; use crate::miniscript::decode::Terminal; use crate::miniscript::{satisfy, Legacy, Miniscript, Segwitv0}; use crate::plan::{AssetProvider, Plan}; @@ -981,17 +982,17 @@ impl crate::expression::FromTree for Descriptor { impl FromStr for Descriptor { type Err = Error; fn from_str(s: &str) -> Result, Error> { - // tr tree parsing has special code - // Tr::from_str will check the checksum - // match "tr(" to handle more extensibly - let desc = if s.starts_with("tr(") { - Ok(Descriptor::Tr(Tr::from_str(s)?)) - } else { - let top = expression::Tree::from_str(s)?; - expression::FromTree::from_tree(&top) - }?; - - Ok(desc) + let top = expression::Tree::from_str(s)?; + let ret = Self::from_tree(&top)?; + if let Descriptor::Tr(ref inner) = ret { + // FIXME preserve weird/broken behavior from 12.x. + // See https://github.com/rust-bitcoin/rust-miniscript/issues/734 + ret.sanity_check()?; + for (_, ms) in inner.iter_scripts() { + ms.ext_check(&crate::miniscript::analyzable::ExtParams::sane())?; + } + } + Ok(ret) } } @@ -1102,7 +1103,7 @@ mod tests { StdDescriptor::from_str("sh(sortedmulti)") .unwrap_err() .to_string(), - "expected threshold, found terminal", + "sortedmulti must have at least 1 children, but found 0" ); //issue 202 assert_eq!( StdDescriptor::from_str(&format!("sh(sortedmulti(2,{}))", &TEST_PK[3..69])) @@ -1545,6 +1546,21 @@ mod tests { ) } + #[test] + fn tr_named_branch() { + use crate::{ParseError, ParseTreeError}; + + assert!(matches!( + StdDescriptor::from_str( + "tr(0202d44008000010100000000084F0000000dd0dd00000000000201dceddd00d00,abc{0,0})" + ), + Err(Error::Parse(ParseError::Tree(ParseTreeError::IncorrectName { + expected: "", + .. + }))), + )); + } + #[test] fn roundtrip_tests() { let descriptor = Descriptor::::from_str("multi"); @@ -1836,9 +1852,10 @@ mod tests { // https://github.com/bitcoin/bitcoin/blob/7ae86b3c6845873ca96650fc69beb4ae5285c801/src/test/descriptor_tests.cpp#L355-L360 macro_rules! check_invalid_checksum { ($secp: ident,$($desc: expr),*) => { + use crate::{ParseError, ParseTreeError}; $( match Descriptor::parse_descriptor($secp, $desc) { - Err(Error::ParseTree(crate::ParseTreeError::Checksum(_))) => {}, + Err(Error::Parse(ParseError::Tree(ParseTreeError::Checksum(_)))) => {}, Err(e) => panic!("Expected bad checksum for {}, got '{}'", $desc, e), _ => panic!("Invalid checksum treated as valid: {}", $desc), }; diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index c8552eb4b..bfb33b935 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -248,21 +248,17 @@ impl Liftable for Wsh { impl crate::expression::FromTree for Wsh { fn from_tree(top: &expression::Tree) -> Result { - if top.name == "wsh" && top.args.len() == 1 { - let top = &top.args[0]; - if top.name == "sortedmulti" { - return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) }); - } - let sub = Miniscript::from_tree(top)?; - Segwitv0::top_level_checks(&sub)?; - Ok(Wsh { inner: WshInner::Ms(sub) }) - } else { - Err(Error::Unexpected(format!( - "{}({} args) while parsing wsh descriptor", - top.name, - top.args.len(), - ))) + let top = top + .verify_toplevel("wsh", 1..=1) + .map_err(From::from) + .map_err(Error::Parse)?; + + if top.name == "sortedmulti" { + return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) }); } + let sub = Miniscript::from_tree(top)?; + Segwitv0::top_level_checks(&sub)?; + Ok(Wsh { inner: WshInner::Ms(sub) }) } } @@ -488,15 +484,11 @@ impl Liftable for Wpkh { impl crate::expression::FromTree for Wpkh { fn from_tree(top: &expression::Tree) -> Result { - if top.name == "wpkh" && top.args.len() == 1 { - Ok(Wpkh::new(expression::terminal(&top.args[0], |pk| Pk::from_str(pk))?)?) - } else { - Err(Error::Unexpected(format!( - "{}({} args) while parsing wpkh descriptor", - top.name, - top.args.len(), - ))) - } + let top = top + .verify_toplevel("wpkh", 1..=1) + .map_err(From::from) + .map_err(Error::Parse)?; + Ok(Wpkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?) } } diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index f9f21544d..f04ddae1c 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -82,26 +82,22 @@ impl fmt::Display for Sh { impl crate::expression::FromTree for Sh { fn from_tree(top: &expression::Tree) -> Result { - if top.name == "sh" && top.args.len() == 1 { - let top = &top.args[0]; - let inner = match top.name { - "wsh" => ShInner::Wsh(Wsh::from_tree(top)?), - "wpkh" => ShInner::Wpkh(Wpkh::from_tree(top)?), - "sortedmulti" => ShInner::SortedMulti(SortedMultiVec::from_tree(top)?), - _ => { - let sub = Miniscript::from_tree(top)?; - Legacy::top_level_checks(&sub)?; - ShInner::Ms(sub) - } - }; - Ok(Sh { inner }) - } else { - Err(Error::Unexpected(format!( - "{}({} args) while parsing sh descriptor", - top.name, - top.args.len(), - ))) - } + let top = top + .verify_toplevel("sh", 1..=1) + .map_err(From::from) + .map_err(Error::Parse)?; + + let inner = match top.name { + "wsh" => ShInner::Wsh(Wsh::from_tree(top)?), + "wpkh" => ShInner::Wpkh(Wpkh::from_tree(top)?), + "sortedmulti" => ShInner::SortedMulti(SortedMultiVec::from_tree(top)?), + _ => { + let sub = Miniscript::from_tree(top)?; + Legacy::top_level_checks(&sub)?; + ShInner::Ms(sub) + } + }; + Ok(Sh { inner }) } } diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index b8a378a26..1aa96c094 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -64,6 +64,10 @@ impl SortedMultiVec { Pk: FromStr, ::Err: fmt::Display, { + tree.verify_toplevel("sortedmulti", 1..) + .map_err(From::from) + .map_err(Error::Parse)?; + let ret = Self { inner: tree .to_null_threshold() diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 77cceb7e0..cc0dd1945 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -1,6 +1,5 @@ // SPDX-License-Identifier: CC0-1.0 -use core::str::FromStr; use core::{cmp, fmt, hash}; #[cfg(not(test))] // https://github.com/rust-lang/rust/issues/121684 @@ -12,9 +11,10 @@ use bitcoin::taproot::{ use bitcoin::{opcodes, Address, Network, ScriptBuf, Weight}; use sync::Arc; -use super::checksum::{self, verify_checksum}; +use super::checksum; use crate::descriptor::DefiniteDescriptorKey; use crate::expression::{self, FromTree}; +use crate::iter::TreeLike as _; use crate::miniscript::satisfy::{Placeholder, Satisfaction, SchnorrSigType, Witness}; use crate::miniscript::Miniscript; use crate::plan::AssetProvider; @@ -23,8 +23,8 @@ use crate::policy::Liftable; use crate::prelude::*; use crate::util::{varint_len, witness_size}; use crate::{ - errstr, Error, ForEachKey, FromStrKey, MiniscriptKey, Satisfier, ScriptContext, Tap, Threshold, - ToPublicKey, TranslateErr, Translator, + Error, ForEachKey, FromStrKey, MiniscriptKey, ParseError, Satisfier, ScriptContext, Tap, + Threshold, ToPublicKey, TranslateErr, Translator, }; /// A Taproot Tree representation. @@ -490,78 +490,114 @@ where } } -#[rustfmt::skip] -impl Tr { - // Helper function to parse taproot script path - fn parse_tr_script_spend(tree: &expression::Tree,) -> Result, Error> { - match tree { - 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])?; - Ok(TapTree::combine(left, right)) - } - _ => Err(Error::Unexpected( - "unknown format for script spending paths while parsing taproot descriptor" - .to_string(), - )), - } +impl core::str::FromStr for Tr { + type Err = Error; + + fn from_str(s: &str) -> Result { + let expr_tree = expression::Tree::from_str(s)?; + Self::from_tree(&expr_tree) } } impl crate::expression::FromTree for Tr { - fn from_tree(top: &expression::Tree) -> Result { - if top.name == "tr" { - 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(expr_tree: &expression::Tree) -> Result { + use crate::expression::{Parens, ParseTreeError}; + + expr_tree + .verify_toplevel("tr", 1..=2) + .map_err(From::from) + .map_err(Error::Parse)?; + + let mut round_paren_depth = 0; + + let mut internal_key = None; + let mut tree_stack = vec![]; + + for item in expr_tree.verbose_pre_order_iter() { + // Top-level "tr" node. + if item.index == 0 { + if item.is_complete { + debug_assert!( + internal_key.is_some(), + "checked above that top-level 'tr' has children" + ); + } + } else if item.index == 1 { + // First child of tr, which must be the internal key + if !item.node.args.is_empty() { + return Err(Error::Parse(ParseError::Tree( + ParseTreeError::IncorrectNumberOfChildren { + description: "internal key", + n_children: item.node.args.len(), + minimum: Some(0), + maximum: Some(0), + }, + ))); + } + internal_key = Some( + Pk::from_str(item.node.name) + .map_err(ParseError::box_from_str) + .map_err(Error::Parse)?, + ); + } else { + // From here on we are into the taptree. + if item.n_children_yielded == 0 { + match item.node.parens { + Parens::Curly => { + if !item.node.name.is_empty() { + return Err(Error::Parse(ParseError::Tree( + ParseTreeError::IncorrectName { + actual: item.node.name.to_owned(), + expected: "", + }, + ))); + } + if round_paren_depth > 0 { + return Err(Error::Parse(ParseError::Tree( + ParseTreeError::IllegalCurlyBrace { + pos: item.node.children_pos, + }, + ))); + } + } + Parens::Round => round_paren_depth += 1, + _ => {} } - 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() - ))); + if item.is_complete { + if item.node.parens == Parens::Curly { + if item.n_children_yielded == 2 { + let rchild = tree_stack.pop().unwrap(); + let lchild = tree_stack.pop().unwrap(); + tree_stack.push(TapTree::combine(lchild, rchild)); + } else { + return Err(Error::Parse(ParseError::Tree( + ParseTreeError::IncorrectNumberOfChildren { + description: "Taptree node", + n_children: item.n_children_yielded, + minimum: Some(2), + maximum: Some(2), + }, + ))); + } + } else { + if item.node.parens == Parens::Round { + round_paren_depth -= 1; + } + if round_paren_depth == 0 { + let script = Miniscript::from_tree(item.node)?; + // FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734 + if script.ty.corr.base != crate::miniscript::types::Base::B { + return Err(Error::NonTopLevel(format!("{:?}", script))); + }; + tree_stack.push(TapTree::Leaf(Arc::new(script))); + } } - let tree = &top.args[1]; - 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() - ))), } - } else { - Err(Error::Unexpected(format!( - "{}[#{} args] while parsing taproot descriptor", - top.name, - top.args.len() - ))) } - } -} - -impl core::str::FromStr for Tr { - type Err = Error; - fn from_str(s: &str) -> Result { - let desc_str = verify_checksum(s) - .map_err(From::from) - .map_err(Error::ParseTree)?; - let top = parse_tr_tree(desc_str)?; - Self::from_tree(&top) + assert!(tree_stack.len() <= 1); + Tr::new(internal_key.unwrap(), tree_stack.pop()) } } @@ -587,52 +623,6 @@ impl fmt::Display for Tr { } } -// Helper function to parse string into miniscript tree form -fn parse_tr_tree(s: &str) -> Result { - if s.len() > 3 && &s[..3] == "tr(" && s.as_bytes()[s.len() - 1] == b')' { - let rest = &s[3..s.len() - 1]; - if !rest.contains(',') { - let key = expression::Tree::from_str(rest)?; - if !key.args.is_empty() { - return Err(Error::Unexpected("invalid taproot internal key".to_string())); - } - let internal_key = expression::Tree { name: key.name, args: vec![] }; - return Ok(expression::Tree { name: "tr", 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 key = expression::Tree::from_str(key)?; - if !key.args.is_empty() { - return Err(Error::Unexpected("invalid taproot internal key".to_string())); - } - let internal_key = expression::Tree { name: key.name, args: vec![] }; - let (tree, rest) = expression::Tree::from_slice_delim(script, 1, '{')?; - if rest.is_empty() { - Ok(expression::Tree { name: "tr", 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 { - // find the first character that matches delim - let res = inp - .chars() - .position(|ch| ch == delim) - .map(|idx| (&inp[..idx], &inp[idx + 1..])) - .unwrap_or((inp, "")); - Some(res) - } -} - impl Liftable for TapTree { fn lift(&self) -> Result, Error> { fn lift_helper(s: &TapTree) -> Result, Error> { @@ -749,6 +739,8 @@ where #[cfg(test)] mod tests { + use core::str::FromStr; + use super::*; fn descriptor() -> String { diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 000000000..f66cd42f1 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,53 @@ +// Written in 2019 by Andrew Poelstra +// SPDX-License-Identifier: CC0-1.0 + +//! Errors + +use core::fmt; +#[cfg(feature = "std")] +use std::error; + +use crate::blanket_traits::StaticDebugAndDisplay; +use crate::Box; +/// An error parsing a Miniscript object (policy, descriptor or miniscript) +/// from a string. +#[derive(Debug)] +pub enum ParseError { + /// Failed to parse a public key or hash. + /// + /// Note that the error information is lost for nostd compatibility reasons. See + /// . + FromStr(Box), + /// Error parsing a string into an expression tree. + Tree(crate::ParseTreeError), +} + +impl ParseError { + /// Boxes a `FromStr` error for a `Pk` (or associated types) into a `ParseError` + pub(crate) fn box_from_str(e: E) -> Self { + ParseError::FromStr(Box::new(e)) + } +} + +impl From for ParseError { + fn from(e: crate::ParseTreeError) -> Self { Self::Tree(e) } +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ParseError::FromStr(ref e) => e.fmt(f), + ParseError::Tree(ref e) => e.fmt(f), + } + } +} + +#[cfg(feature = "std")] +impl error::Error for ParseError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match self { + ParseError::FromStr(..) => None, + ParseError::Tree(ref e) => Some(e), + } + } +} diff --git a/src/expression/error.rs b/src/expression/error.rs index d705e2df5..b9292a445 100644 --- a/src/expression/error.rs +++ b/src/expression/error.rs @@ -53,6 +53,29 @@ pub enum ParseTreeError { /// The position of the closing parethesis. close_pos: usize, }, + /// A node had the wrong name. + IncorrectName { + /// The name that was found. + actual: String, + /// The name that was expected. + expected: &'static str, + }, + /// A node had the wrong number of children. + IncorrectNumberOfChildren { + /// A description of the node in question. + description: &'static str, + /// The number of children the node had. + n_children: usize, + /// The minimum of children the node should have had. + minimum: Option, + /// The minimum of children the node should have had. + maximum: Option, + }, + /// A Taproot child occurred somewhere it was not allowed. + IllegalCurlyBrace { + /// The position of the opening curly brace. + pos: usize, + }, /// Data occurred after the final ). TrailingCharacter { /// The first trailing character. @@ -93,6 +116,34 @@ impl fmt::Display for ParseTreeError { open_ch, open_pos, close_ch, close_pos ) } + ParseTreeError::IllegalCurlyBrace { pos } => { + write!(f, "illegal `{{` at position {} (Taproot branches not allowed here)", pos) + } + ParseTreeError::IncorrectName { actual, expected } => { + if expected.is_empty() { + write!(f, "found node '{}', expected nameless node", actual) + } else { + write!(f, "expected node '{}', found '{}'", expected, actual) + } + } + ParseTreeError::IncorrectNumberOfChildren { + description, + n_children, + minimum, + maximum, + } => { + write!(f, "{} must have ", description)?; + match (minimum, maximum) { + (_, Some(0)) => f.write_str("no children"), + (Some(min), Some(max)) if min == max => write!(f, "{} children", min), + (Some(min), None) if n_children < min => write!(f, "at least {} children", min), + (Some(min), Some(max)) if n_children < min => write!(f, "at least {} children (maximum {})", min, max), + (None, Some(max)) if n_children > max => write!(f, "at most {} children", max), + (Some(min), Some(max)) if n_children > max => write!(f, "at most {} children (minimum {})", max, min), + (x, y) => panic!("IncorrectNumberOfChildren error was constructed inconsistently (min {:?} max {:?})", x, y), + }?; + write!(f, ", but found {}", n_children) + } ParseTreeError::TrailingCharacter { ch, pos } => { write!(f, "trailing data `{}...` (position {})", ch, pos) } @@ -109,6 +160,9 @@ impl std::error::Error for ParseTreeError { | ParseTreeError::UnmatchedOpenParen { .. } | ParseTreeError::UnmatchedCloseParen { .. } | ParseTreeError::MismatchedParens { .. } + | ParseTreeError::IllegalCurlyBrace { .. } + | ParseTreeError::IncorrectName { .. } + | ParseTreeError::IncorrectNumberOfChildren { .. } | ParseTreeError::TrailingCharacter { .. } => None, } } diff --git a/src/expression/mod.rs b/src/expression/mod.rs index 844b3c869..14978def2 100644 --- a/src/expression/mod.rs +++ b/src/expression/mod.rs @@ -5,11 +5,12 @@ mod error; -use core::fmt; use core::str::FromStr; +use core::{fmt, ops}; pub use self::error::{ParseThresholdError, ParseTreeError}; use crate::descriptor::checksum::verify_checksum; +use crate::iter::{self, TreeLike}; use crate::prelude::*; use crate::{errstr, Error, Threshold, MAX_RECURSION_DEPTH}; @@ -21,6 +22,11 @@ pub const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVW pub struct Tree<'a> { /// The name `x` pub name: &'a str, + /// Position one past the last character of the node's name. If it has + /// children, the position of the '(' or '{'. + pub children_pos: usize, + /// The type of parentheses surrounding the node's children. + pub parens: Parens, /// The comma-separated contents of the `(...)`, if any pub args: Vec>, } @@ -38,11 +44,32 @@ impl PartialEq for Tree<'_> { } } impl Eq for Tree<'_> {} -// or_b(pk(A),pk(B)) -// -// A = musig(musig(B,C),D,E) -// or_b() -// pk(A), pk(B) + +impl<'a, 't> TreeLike for &'t Tree<'a> { + type NaryChildren = &'t [Tree<'a>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] } + + fn as_node(&self) -> iter::Tree { + if self.args.is_empty() { + iter::Tree::Nullary + } else { + iter::Tree::Nary(&self.args) + } + } +} + +/// The type of parentheses surrounding a node's children. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum Parens { + /// Node has no children. + None, + /// Round parentheses: `(` and `)`. + Round, + /// Curly braces: `{` and `}`. + Curly, +} /// A trait for extracting a structure from a Tree representation in token form pub trait FromTree: Sized { @@ -50,100 +77,95 @@ pub trait FromTree: Sized { fn from_tree(top: &Tree) -> Result; } -enum Found { - Nothing, - LBracket(usize), // Either a left ( or { - Comma(usize), - RBracket(usize), // Either a right ) or } -} - -fn next_expr(sl: &str, delim: char) -> Found { - let mut found = Found::Nothing; - if delim == '(' { - for (n, ch) in sl.char_indices() { - match ch { - '(' => { - found = Found::LBracket(n); - break; - } - ',' => { - found = Found::Comma(n); - break; - } - ')' => { - found = Found::RBracket(n); - break; - } - _ => {} - } - } - } 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; - } - _ => {} - } +impl<'a> Tree<'a> { + /// Check that a tree node has the given number of children. + /// + /// The `description` argument is only used to populate the error return, + /// and is not validated in any way. + pub fn verify_n_children( + &self, + description: &'static str, + n_children: impl ops::RangeBounds, + ) -> Result<(), ParseTreeError> { + if n_children.contains(&self.n_children()) { + Ok(()) + } else { + let minimum = match n_children.start_bound() { + ops::Bound::Included(n) => Some(*n), + ops::Bound::Excluded(n) => Some(*n + 1), + ops::Bound::Unbounded => None, + }; + let maximum = match n_children.end_bound() { + ops::Bound::Included(n) => Some(*n), + ops::Bound::Excluded(n) => Some(*n - 1), + ops::Bound::Unbounded => None, + }; + Err(ParseTreeError::IncorrectNumberOfChildren { + description, + n_children: self.n_children(), + minimum, + maximum, + }) } - } 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"), + /// Check that a tree node has the given name, one child, and round braces. + /// + /// Returns the first child. + /// + /// # Panics + /// + /// Panics if zero is in bounds for `n_children` (since then there may be + /// no sensible value to return). + pub fn verify_toplevel( + &self, + name: &'static str, + n_children: impl ops::RangeBounds, + ) -> Result<&Self, ParseTreeError> { + assert!( + !n_children.contains(&0), + "verify_toplevel is intended for nodes with >= 1 child" + ); + + if self.name != name { + Err(ParseTreeError::IncorrectName { actual: self.name.to_owned(), expected: name }) + } else if self.parens == Parens::Curly { + Err(ParseTreeError::IllegalCurlyBrace { pos: self.children_pos }) + } else { + self.verify_n_children(name, n_children)?; + Ok(&self.args[0]) + } } -} -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, '(') + /// Check that a tree has no curly-brace children in it. + pub fn verify_no_curly_braces(&self) -> Result<(), ParseTreeError> { + for tree in self.pre_order_iter() { + if tree.parens == Parens::Curly { + return Err(ParseTreeError::IllegalCurlyBrace { pos: tree.children_pos }); + } + } + Ok(()) } /// Check that a string is a well-formed expression string, with optional /// checksum. /// - /// Returns the string with the checksum removed. - fn parse_pre_check(s: &str, open: u8, close: u8) -> Result<&str, ParseTreeError> { - // Do ASCII check first; after this we can use .bytes().enumerate() rather + /// Returns the string with the checksum removed and its tree depth. + fn parse_pre_check(s: &str) -> Result<(&str, usize), ParseTreeError> { + // First, scan through string to make sure it is well-formed. + // Do ASCII/checksum check first; after this we can use .bytes().enumerate() rather // than .char_indices(), which is *significantly* faster. let s = verify_checksum(s)?; let mut max_depth = 0; let mut open_paren_stack = Vec::with_capacity(128); for (pos, ch) in s.bytes().enumerate() { - if ch == open { + if ch == b'(' || ch == b'{' { open_paren_stack.push((ch, pos)); if max_depth < open_paren_stack.len() { max_depth = open_paren_stack.len(); } - } else if ch == close { + } else if ch == b')' || ch == b'}' { if let Some((open_ch, open_pos)) = open_paren_stack.pop() { if (open_ch == b'(' && ch == b'}') || (open_ch == b'{' && ch == b')') { return Err(ParseTreeError::MismatchedParens { @@ -211,68 +233,73 @@ impl<'a> Tree<'a> { }); } - Ok(s) + Ok((s, max_depth)) } - pub(crate) fn from_slice_delim( - mut sl: &'a str, - depth: u32, - delim: char, - ) -> Result<(Tree<'a>, &'a str), Error> { - if depth == 0 { - if delim == '{' { - sl = Self::parse_pre_check(sl, b'{', b'}').map_err(Error::ParseTree)?; - } else { - sl = Self::parse_pre_check(sl, b'(', b')').map_err(Error::ParseTree)?; - } - } - - 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() { - unreachable!() - } + /// Parses a tree from a string + #[allow(clippy::should_implement_trait)] // Cannot use std::str::FromStr because of lifetimes. + pub fn from_str(s: &'a str) -> Result { + Self::from_str_inner(s) + .map_err(From::from) + .map_err(Error::Parse) + } - sl = &new_sl[1..]; - match new_sl.as_bytes()[0] { - b',' => {} - last_byte => { - if last_byte == closing_delim(delim) as u8 { - break; - } else { - unreachable!() - } - } - } - } - Ok((ret, sl)) + fn from_str_inner(s: &'a str) -> Result { + // First, scan through string to make sure it is well-formed. + let (s, max_depth) = Self::parse_pre_check(s)?; + + // Now, knowing it is sane and well-formed, we can easily parse it backward, + // which will yield a post-order right-to-left iterator of its nodes. + let mut stack = Vec::with_capacity(max_depth); + let mut children_parens: Option<(Vec<_>, usize, Parens)> = None; + let mut node_name_end = s.len(); + for (pos, ch) in s.bytes().enumerate().rev() { + if ch == b')' || ch == b'}' { + stack.push(vec![]); + node_name_end = pos; + } else if ch == b',' { + let (mut args, children_pos, parens) = + children_parens + .take() + .unwrap_or((vec![], node_name_end, Parens::None)); + args.reverse(); + + let top = stack.last_mut().unwrap(); + let new_tree = + Tree { name: &s[pos + 1..node_name_end], children_pos, parens, args }; + top.push(new_tree); + node_name_end = pos; + } else if ch == b'(' || ch == b'{' { + let (mut args, children_pos, parens) = + children_parens + .take() + .unwrap_or((vec![], node_name_end, Parens::None)); + args.reverse(); + + let mut top = stack.pop().unwrap(); + let new_tree = + Tree { name: &s[pos + 1..node_name_end], children_pos, parens, args }; + top.push(new_tree); + children_parens = Some(( + top, + pos, + match ch { + b'(' => Parens::Round, + b'{' => Parens::Curly, + _ => unreachable!(), + }, + )); + node_name_end = pos; } } - } - /// Parses a tree from a string - #[allow(clippy::should_implement_trait)] // Cannot use std::str::FromStr because of lifetimes. - pub fn from_str(s: &'a str) -> Result, Error> { - let (top, rem) = Tree::from_slice(s)?; - if rem.is_empty() { - Ok(top) - } else { - unreachable!() - } + assert_eq!(stack.len(), 0); + let (mut args, children_pos, parens) = + children_parens + .take() + .unwrap_or((vec![], node_name_end, Parens::None)); + args.reverse(); + Ok(Tree { name: &s[..node_name_end], children_pos, parens, args }) } /// Parses an expression tree as a threshold (a term with at least one child, @@ -362,11 +389,32 @@ where #[cfg(test)] mod tests { use super::*; + use crate::ParseError; /// Test functions to manually build trees - fn leaf(name: &str) -> Tree { Tree { name, args: vec![] } } + fn leaf(name: &str) -> Tree { + Tree { name, parens: Parens::None, children_pos: name.len(), args: vec![] } + } + + fn paren_node<'a>(name: &'a str, mut args: Vec>) -> Tree<'a> { + let mut offset = name.len() + 1; // +1 for open paren + for arg in &mut args { + arg.children_pos += offset; + offset += arg.name.len() + 1; // +1 for comma + } - fn paren_node<'a>(name: &'a str, args: Vec>) -> Tree<'a> { Tree { name, args } } + Tree { name, parens: Parens::Round, children_pos: name.len(), args } + } + + fn brace_node<'a>(name: &'a str, mut args: Vec>) -> Tree<'a> { + let mut offset = name.len() + 1; // +1 for open paren + for arg in &mut args { + arg.children_pos += offset; + offset += arg.name.len() + 1; // +1 for comma + } + + Tree { name, parens: Parens::Curly, children_pos: name.len(), args } + } #[test] fn test_parse_num() { @@ -384,29 +432,35 @@ mod tests { assert!(matches!( Tree::from_str("thresh,").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 }), + Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 })), )); assert!(matches!( Tree::from_str("thresh,thresh").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 }), + Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 })), )); assert!(matches!( Tree::from_str("thresh()thresh()").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: 't', pos: 8 }), + Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: 't', pos: 8 })), )); assert_eq!(Tree::from_str("thresh()").unwrap(), paren_node("thresh", vec![leaf("")])); assert!(matches!( Tree::from_str("thresh(a()b)"), - Err(Error::ParseTree(ParseTreeError::ExpectedParenOrComma { ch: 'b', pos: 10 })), + Err(Error::Parse(ParseError::Tree(ParseTreeError::ExpectedParenOrComma { + ch: 'b', + pos: 10 + }))), )); assert!(matches!( Tree::from_str("thresh()xyz"), - Err(Error::ParseTree(ParseTreeError::TrailingCharacter { ch: 'x', pos: 8 })), + Err(Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { + ch: 'x', + pos: 8 + }))), )); } @@ -414,56 +468,55 @@ mod tests { fn parse_tree_parens() { assert!(matches!( Tree::from_str("a(").unwrap_err(), - Error::ParseTree(ParseTreeError::UnmatchedOpenParen { ch: '(', pos: 1 }), + Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedOpenParen { ch: '(', pos: 1 })), )); assert!(matches!( Tree::from_str(")").unwrap_err(), - Error::ParseTree(ParseTreeError::UnmatchedCloseParen { ch: ')', pos: 0 }), + Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedCloseParen { ch: ')', pos: 0 })), )); assert!(matches!( Tree::from_str("x(y))").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ')', pos: 4 }), + Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ')', pos: 4 })), )); /* Will be enabled in a later PR which unifies TR and non-TR parsing. assert!(matches!( Tree::from_str("a{").unwrap_err(), - Error::ParseTree(ParseTreeError::UnmatchedOpenParen { ch: '{', pos: 1 }), + Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedOpenParen { ch: '{', pos: 1 })), )); assert!(matches!( Tree::from_str("}").unwrap_err(), - Error::ParseTree(ParseTreeError::UnmatchedCloseParen { ch: '}', pos: 0 }), + Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedCloseParen { ch: '}', pos: 0 })), )); */ assert!(matches!( Tree::from_str("x(y)}").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: '}', pos: 4 }), + Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: '}', pos: 4 })), )); /* Will be enabled in a later PR which unifies TR and non-TR parsing. assert!(matches!( Tree::from_str("x{y)").unwrap_err(), - Error::ParseTree(ParseTreeError::MismatchedParens { + Error::Parse(ParseError::Tree(ParseTreeError::MismatchedParens { open_ch: '{', open_pos: 1, close_ch: ')', close_pos: 3, - }), + }),) )); */ } #[test] fn parse_tree_taproot() { - // This test will change in a later PR which unifies TR and non-TR parsing. - assert!(matches!( - Tree::from_str("a{b(c),d}").unwrap_err(), - Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 }), - )); + assert_eq!( + Tree::from_str("a{b(c),d}").unwrap(), + brace_node("a", vec![paren_node("b", vec![leaf("c")]), leaf("d")]), + ); } #[test] diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs index 49134c6de..03e6f2840 100644 --- a/src/interpreter/inner.rs +++ b/src/interpreter/inner.rs @@ -429,8 +429,7 @@ mod tests { ", ) .unwrap(); - let mut dummy_sig = [0u8; 48]; - dummy_sig.copy_from_slice(&dummy_sig_vec[..]); + let dummy_sig = <[u8; 48]>::try_from(&dummy_sig_vec[..]).unwrap(); let pkhash = key.to_pubkeyhash(SigType::Ecdsa).into(); let wpkhash = key.to_pubkeyhash(SigType::Ecdsa).into(); diff --git a/src/interpreter/stack.rs b/src/interpreter/stack.rs index 4cf5450af..647a84713 100644 --- a/src/interpreter/stack.rs +++ b/src/interpreter/stack.rs @@ -261,7 +261,7 @@ impl<'txin> Stack<'txin> { self.push(Element::Satisfied); Some(Ok(SatisfiedConstraint::HashLock { hash: HashLockType::Sha256(*hash), - preimage: preimage_from_sl(preimage), + preimage: <[u8; 32]>::try_from(preimage).expect("length checked above"), })) } else { self.push(Element::Dissatisfied); @@ -286,7 +286,7 @@ impl<'txin> Stack<'txin> { self.push(Element::Satisfied); Some(Ok(SatisfiedConstraint::HashLock { hash: HashLockType::Hash256(*hash), - preimage: preimage_from_sl(preimage), + preimage: <[u8; 32]>::try_from(preimage).expect("length checked above"), })) } else { self.push(Element::Dissatisfied); @@ -311,7 +311,7 @@ impl<'txin> Stack<'txin> { self.push(Element::Satisfied); Some(Ok(SatisfiedConstraint::HashLock { hash: HashLockType::Hash160(*hash), - preimage: preimage_from_sl(preimage), + preimage: <[u8; 32]>::try_from(preimage).expect("length checked above"), })) } else { self.push(Element::Dissatisfied); @@ -336,7 +336,7 @@ impl<'txin> Stack<'txin> { self.push(Element::Satisfied); Some(Ok(SatisfiedConstraint::HashLock { hash: HashLockType::Ripemd160(*hash), - preimage: preimage_from_sl(preimage), + preimage: <[u8; 32]>::try_from(preimage).expect("length checked above"), })) } else { self.push(Element::Dissatisfied); @@ -376,14 +376,3 @@ impl<'txin> Stack<'txin> { } } } - -// Helper function to compute preimage from slice -fn preimage_from_sl(sl: &[u8]) -> [u8; 32] { - if sl.len() != 32 { - unreachable!("Internal: Preimage length checked to be 32") - } else { - let mut preimage = [0u8; 32]; - preimage.copy_from_slice(sl); - preimage - } -} diff --git a/src/lib.rs b/src/lib.rs index f15d99e46..fde7ac66b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,6 +114,7 @@ mod pub_macros; mod benchmarks; mod blanket_traits; pub mod descriptor; +mod error; pub mod expression; pub mod interpreter; pub mod iter; @@ -128,8 +129,6 @@ mod test_utils; mod util; use core::{fmt, hash, str}; -#[cfg(feature = "std")] -use std::error; use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; use bitcoin::hex::DisplayHex; @@ -137,6 +136,7 @@ use bitcoin::{script, Opcode}; pub use crate::blanket_traits::FromStrKey; pub use crate::descriptor::{DefiniteDescriptorKey, Descriptor, DescriptorPublicKey}; +pub use crate::error::ParseError; pub use crate::expression::{ParseThresholdError, ParseTreeError}; pub use crate::interpreter::Interpreter; pub use crate::miniscript::analyzable::{AnalysisError, ExtParams}; @@ -423,14 +423,6 @@ pub enum Error { AddrError(bitcoin::address::ParseError), /// rust-bitcoin p2sh address error AddrP2shError(bitcoin::address::P2shError), - /// A `CHECKMULTISIG` opcode was preceded by a number > 20 - CmsTooManyKeys(u32), - /// A tapscript multi_a cannot support more than Weight::MAX_BLOCK/32 keys - MultiATooManyKeys(u64), - /// Encountered unprintable character in descriptor - Unprintable(u8), - /// 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 @@ -451,8 +443,6 @@ pub enum Error { CouldNotSatisfy, /// Typechecking failed TypeCheck(String), - /// General error in creating descriptor - BadDescriptor(String), /// Forward-secp related errors Secp(bitcoin::secp256k1::Error), #[cfg(feature = "compiler")] @@ -493,7 +483,7 @@ pub enum Error { /// Invalid threshold. ParseThreshold(ParseThresholdError), /// Invalid expression tree. - ParseTree(ParseTreeError), + Parse(ParseError), } // https://github.com/sipa/miniscript/pull/5 for discussion on this number @@ -510,9 +500,6 @@ impl fmt::Display for Error { Error::Script(ref e) => fmt::Display::fmt(e, f), Error::AddrError(ref e) => fmt::Display::fmt(e, f), Error::AddrP2shError(ref e) => fmt::Display::fmt(e, f), - Error::CmsTooManyKeys(n) => write!(f, "checkmultisig with {} keys", n), - Error::Unprintable(x) => write!(f, "unprintable character 0x{:02x}", x), - 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), @@ -523,7 +510,6 @@ impl fmt::Display for Error { Error::MissingSig(ref pk) => write!(f, "missing signature for key {:?}", pk), Error::CouldNotSatisfy => f.write_str("could not satisfy"), Error::TypeCheck(ref e) => write!(f, "typecheck: {}", e), - Error::BadDescriptor(ref e) => write!(f, "Invalid descriptor: {}", e), Error::Secp(ref e) => fmt::Display::fmt(e, f), Error::ContextError(ref e) => fmt::Display::fmt(e, f), #[cfg(feature = "compiler")] @@ -548,31 +534,26 @@ impl fmt::Display for Error { Error::PubKeyCtxError(ref pk, ref ctx) => { write!(f, "Pubkey error: {} under {} scriptcontext", pk, ctx) } - Error::MultiATooManyKeys(k) => write!(f, "MultiA too many keys {}", k), Error::TrNoScriptCode => 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::AbsoluteLockTime(ref e) => e.fmt(f), Error::RelativeLockTime(ref e) => e.fmt(f), Error::Threshold(ref e) => e.fmt(f), Error::ParseThreshold(ref e) => e.fmt(f), - Error::ParseTree(ref e) => e.fmt(f), + Error::Parse(ref e) => e.fmt(f), } } } #[cfg(feature = "std")] -impl error::Error for Error { - fn cause(&self) -> Option<&dyn error::Error> { +impl std::error::Error for Error { + fn cause(&self) -> Option<&dyn std::error::Error> { use self::Error::*; match self { InvalidOpcode(_) | NonMinimalVerify(_) | InvalidPush(_) - | CmsTooManyKeys(_) - | MultiATooManyKeys(_) - | Unprintable(_) - | ExpectedChar(_) | UnexpectedStart | Unexpected(_) | MultiColon(_) @@ -583,7 +564,6 @@ impl error::Error for Error { | MissingSig(_) | CouldNotSatisfy | TypeCheck(_) - | BadDescriptor(_) | MaxRecursiveDepthExceeded | NonStandardBareScript | ImpossibleSatisfaction @@ -606,7 +586,7 @@ impl error::Error for Error { RelativeLockTime(e) => Some(e), Threshold(e) => Some(e), ParseThreshold(e) => Some(e), - ParseTree(e) => Some(e), + Parse(e) => Some(e), } } } diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index 0090b7516..a8be0bc63 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -17,6 +17,7 @@ use crate::miniscript::lex::{Token as Tk, TokenIter}; use crate::miniscript::limits::{MAX_PUBKEYS_IN_CHECKSIGADD, MAX_PUBKEYS_PER_MULTISIG}; use crate::miniscript::ScriptContext; use crate::prelude::*; +use crate::primitives::threshold; #[cfg(doc)] use crate::Descriptor; use crate::{ @@ -538,10 +539,8 @@ pub fn parse( }, // CHECKMULTISIG based multisig Tk::CheckMultiSig, Tk::Num(n) => { - // Check size before allocating keys - if n as usize > MAX_PUBKEYS_PER_MULTISIG { - return Err(Error::CmsTooManyKeys(n)); - } + threshold::validate_k_n::(1, n as usize).map_err(Error::Threshold)?; + let mut keys = Vec::with_capacity(n as usize); for _ in 0..n { match_token!( @@ -562,11 +561,9 @@ pub fn parse( }, // MultiA Tk::NumEqual, Tk::Num(k) => { - // Check size before allocating keys - if k as usize > MAX_PUBKEYS_IN_CHECKSIGADD { - return Err(Error::MultiATooManyKeys(MAX_PUBKEYS_IN_CHECKSIGADD as u64)) - } - let mut keys = Vec::with_capacity(k as usize); // atleast k capacity + threshold::validate_k_n::(k as usize, k as usize).map_err(Error::Threshold)?; + + let mut keys = Vec::with_capacity(k as usize); // at least k capacity while tokens.peek() == Some(&Tk::CheckSigAdd) { match_token!( tokens, diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 03c242ec8..763df9a31 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -802,6 +802,9 @@ impl crate::expression::FromTree for Miniscr /// 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> { + top.verify_no_curly_braces() + .map_err(From::from) + .map_err(Error::Parse)?; let inner: Terminal = expression::FromTree::from_tree(top)?; Miniscript::from_ast(inner) } diff --git a/src/primitives/threshold.rs b/src/primitives/threshold.rs index 0045f9183..7c40f7ccd 100644 --- a/src/primitives/threshold.rs +++ b/src/primitives/threshold.rs @@ -40,6 +40,15 @@ impl std::error::Error for ThresholdError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } } +/// Check whether `k` and `n` are valid for an instance of [`Self`]. +pub fn validate_k_n(k: usize, n: usize) -> Result<(), ThresholdError> { + if k == 0 || k > n || (MAX > 0 && n > MAX) { + Err(ThresholdError { k, n, max: (MAX > 0).then_some(MAX) }) + } else { + Ok(()) + } +} + /// Structure representing a k-of-n threshold collection of some arbitrary /// object `T`. /// @@ -54,11 +63,8 @@ pub struct Threshold { impl Threshold { /// Constructs a threshold directly from a threshold value and collection. pub fn new(k: usize, inner: Vec) -> Result { - if k == 0 || k > inner.len() || (MAX > 0 && inner.len() > MAX) { - Err(ThresholdError { k, n: inner.len(), max: (MAX > 0).then_some(MAX) }) - } else { - Ok(Threshold { k, inner }) - } + validate_k_n::(k, inner.len())?; + Ok(Threshold { k, inner }) } /// Constructs a threshold from a threshold value and an iterator that yields collection diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 655878062..65e8c854d 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -340,38 +340,28 @@ impl Satisfier for PsbtInputSatisfier<'_> { self.psbt.inputs[self.index] .hash160_preimages .get(&Pk::to_hash160(h)) - .and_then(|x: &Vec| try_vec_as_preimage32(x)) + .and_then(|x: &Vec| <[u8; 32]>::try_from(&x[..]).ok()) } fn lookup_sha256(&self, h: &Pk::Sha256) -> Option { self.psbt.inputs[self.index] .sha256_preimages .get(&Pk::to_sha256(h)) - .and_then(|x: &Vec| try_vec_as_preimage32(x)) + .and_then(|x: &Vec| <[u8; 32]>::try_from(&x[..]).ok()) } fn lookup_hash256(&self, h: &Pk::Hash256) -> Option { self.psbt.inputs[self.index] .hash256_preimages .get(&sha256d::Hash::from_byte_array(Pk::to_hash256(h).to_byte_array())) // upstream psbt operates on hash256 - .and_then(|x: &Vec| try_vec_as_preimage32(x)) + .and_then(|x: &Vec| <[u8; 32]>::try_from(&x[..]).ok()) } fn lookup_ripemd160(&self, h: &Pk::Ripemd160) -> Option { self.psbt.inputs[self.index] .ripemd160_preimages .get(&Pk::to_ripemd160(h)) - .and_then(|x: &Vec| try_vec_as_preimage32(x)) - } -} - -fn try_vec_as_preimage32(vec: &[u8]) -> Option { - if vec.len() == 32 { - let mut arr = [0u8; 32]; - arr.copy_from_slice(vec); - Some(arr) - } else { - None + .and_then(|x: &Vec| <[u8; 32]>::try_from(&x[..]).ok()) } }