From 1db7df32b719f15a27479228408045fcc9e23555 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 2 Sep 2024 19:18:06 +0000 Subject: [PATCH] expression: hide all Tree fields and encapsulate API This removes the ability to randomly access children of tree nodes. You can get them in order using the `children` iterator, and get them recursively using the `TreeLike` iterator methods. (In the next commits we will specialize these a bit, providing a `pre_order_iter` and `rtl_post_order_iter` which let you efficiently skip over subtrees. We will need these to parse Taproot expression trees and they don't fit into the `TreeLike` trait, at least not efficiently.) --- src/descriptor/mod.rs | 2 +- src/descriptor/segwitv0.rs | 2 +- src/descriptor/sh.rs | 2 +- src/descriptor/tr.rs | 12 ++++----- src/expression/mod.rs | 51 +++++++++++++++++++++++++++++--------- src/miniscript/astelem.rs | 14 +++++++---- src/policy/concrete.rs | 8 +++--- src/policy/semantic.rs | 8 +++--- 8 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index ce7654958..f9e673c2b 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -966,7 +966,7 @@ impl Descriptor { impl crate::expression::FromTree for 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) { + Ok(match (top.name(), top.n_children()) { ("pkh", 1) => Descriptor::Pkh(Pkh::from_tree(top)?), ("wpkh", 1) => Descriptor::Wpkh(Wpkh::from_tree(top)?), ("sh", 1) => Descriptor::Sh(Sh::from_tree(top)?), diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 6679f062a..b09a9f887 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -253,7 +253,7 @@ impl crate::expression::FromTree for Wsh { .map_err(From::from) .map_err(Error::Parse)?; - if top.name == "sortedmulti" { + if top.name() == "sortedmulti" { return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) }); } let sub = Miniscript::from_tree(top)?; diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index f04ddae1c..f00e3f093 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -87,7 +87,7 @@ impl crate::expression::FromTree for Sh { .map_err(From::from) .map_err(Error::Parse)?; - let inner = match top.name { + 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)?), diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 372c7911e..ba68a73fa 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -532,12 +532,12 @@ impl crate::expression::FromTree for Tr { } else { // From here on we are into the taptree. if item.n_children_yielded == 0 { - match item.node.parens { + match item.node.parens() { Parens::Curly => { - if !item.node.name.is_empty() { + if !item.node.name().is_empty() { return Err(Error::Parse(ParseError::Tree( ParseTreeError::IncorrectName { - actual: item.node.name.to_owned(), + actual: item.node.name().to_owned(), expected: "", }, ))); @@ -545,7 +545,7 @@ impl crate::expression::FromTree for Tr { if round_paren_depth > 0 { return Err(Error::Parse(ParseError::Tree( ParseTreeError::IllegalCurlyBrace { - pos: item.node.children_pos, + pos: item.node.children_pos(), }, ))); } @@ -555,7 +555,7 @@ impl crate::expression::FromTree for Tr { } } if item.is_complete { - if item.node.parens == Parens::Curly { + if item.node.parens() == Parens::Curly { if item.n_children_yielded == 2 { let rchild = tree_stack.pop().unwrap(); let lchild = tree_stack.pop().unwrap(); @@ -571,7 +571,7 @@ impl crate::expression::FromTree for Tr { ))); } } else { - if item.node.parens == Parens::Round { + if item.node.parens() == Parens::Round { round_paren_depth -= 1; } if round_paren_depth == 0 { diff --git a/src/expression/mod.rs b/src/expression/mod.rs index cdd688b31..390082cd1 100644 --- a/src/expression/mod.rs +++ b/src/expression/mod.rs @@ -22,14 +22,14 @@ pub const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVW /// A token of the form `x(...)` or `x` pub struct Tree<'a> { /// The name `x` - pub name: &'a str, + 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, + children_pos: usize, /// The type of parentheses surrounding the node's children. - pub parens: Parens, + parens: Parens, /// The comma-separated contents of the `(...)`, if any - pub args: Vec>, + args: Vec>, } impl PartialEq for Tree<'_> { @@ -79,6 +79,32 @@ pub trait FromTree: Sized { } impl<'a> Tree<'a> { + /// The name of this tree node. + pub fn name(&self) -> &str { self.name } + + /// The 0-indexed byte-position of the name in the original expression tree. + pub fn name_pos(&self) -> usize { self.children_pos - self.name.len() - 1 } + + /// The 0-indexed byte-position of the '(' or '{' character which starts the + /// expression's children. + /// + /// If the expression has no children, returns one past the end of the name. + pub fn children_pos(&self) -> usize { self.children_pos - self.name.len() - 1 } + + /// The number of children this node has. + pub fn n_children(&self) -> usize { self.args.len() } + + /// The type of parenthesis surrounding this node's children. + /// + /// If the node has no children, this will be `Parens::None`. + pub fn parens(&self) -> Parens { self.parens } + + /// An iterator over the direct children of this node. + /// + /// If you want to iterate recursively, use the [`TreeLike`] API which + /// provides methods `pre_order_iter` and `post_order_iter`. + pub fn children(&self) -> impl ExactSizeIterator { self.args.iter() } + /// Split the name by a separating character. /// /// If the separator is present, returns the prefix before the separator and @@ -260,20 +286,21 @@ impl<'a> Tree<'a> { &self, mut map_child: F, ) -> Result, E> { + let mut child_iter = self.children(); + let kchild = match child_iter.next() { + Some(k) => k, + None => return Err(ParseThresholdError::NoChildren.into()), + }; // First, special case "no arguments" so we can index the first argument without panics. - if self.args.is_empty() { - return Err(ParseThresholdError::NoChildren.into()); - } - - if !self.args[0].args.is_empty() { + if kchild.n_children() > 0 { return Err(ParseThresholdError::KNotTerminal.into()); } - let k = parse_num(self.args[0].name).map_err(ParseThresholdError::ParseK)? as usize; - Threshold::new(k, vec![(); self.args.len() - 1]) + let k = parse_num(kchild.name()).map_err(ParseThresholdError::ParseK)? as usize; + Threshold::new(k, vec![(); self.n_children() - 1]) .map_err(ParseThresholdError::Threshold) .map_err(From::from) - .and_then(|thresh| thresh.translate_by_index(|i| map_child(&self.args[1 + i]))) + .and_then(|thresh| thresh.translate_by_index(|_| map_child(child_iter.next().unwrap()))) } /// Check that a tree has no curly-brace children in it. diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index d28617dd8..779ca7122 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -112,10 +112,14 @@ impl crate::expression::FromTree for Termina top.verify_n_children("andor", 3..=3) .map_err(From::from) .map_err(Error::Parse)?; - let x = Arc::>::from_tree(&top.args[0])?; - let y = Arc::>::from_tree(&top.args[1])?; - let z = Arc::>::from_tree(&top.args[2])?; - Ok(Terminal::AndOr(x, y, z)) + let mut child_iter = top + .children() + .map(|x| Arc::>::from_tree(x)); + Ok(Terminal::AndOr( + child_iter.next().unwrap()?, + child_iter.next().unwrap()?, + child_iter.next().unwrap()?, + )) } "or_b" => binary(top, "or_b", Terminal::OrB), "or_d" => binary(top, "or_d", Terminal::OrD), @@ -137,7 +141,7 @@ impl crate::expression::FromTree for Termina if frag_wrap == Some("") { return Err(Error::Parse(crate::ParseError::Tree( - crate::ParseTreeError::UnknownName { name: top.name.to_owned() }, + crate::ParseTreeError::UnknownName { name: top.name().to_owned() }, ))); } let ms = super::wrap_into_miniscript(unwrapped, frag_wrap.unwrap_or(""))?; diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index b5eec2215..f1401c583 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -856,7 +856,7 @@ impl Policy { .map_err(From::from) .map_err(Error::Parse)? } else { - (None, top.name) + (None, top.name()) }; let frag_prob = match frag_prob { @@ -906,8 +906,7 @@ impl Policy { .map_err(From::from) .map_err(Error::Parse)?; let subs = top - .args - .iter() + .children() .map(|arg| Self::from_tree(arg).map(Arc::new)) .collect::>()?; Ok(Policy::And(subs)) @@ -917,8 +916,7 @@ impl Policy { .map_err(From::from) .map_err(Error::Parse)?; let subs = top - .args - .iter() + .children() .map(|arg| { Self::from_tree_prob(arg, true).map(|(prob, sub)| (prob, Arc::new(sub))) }) diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index e23851292..6f16b3d67 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -285,7 +285,7 @@ serde_string_impl_pk!(Policy, "a miniscript semantic policy"); impl expression::FromTree for Policy { fn from_tree(top: &expression::Tree) -> Result, Error> { - match top.name { + match top.name() { "UNSATISFIABLE" => { top.verify_n_children("UNSATISFIABLE", 0..=0) .map_err(From::from) @@ -325,8 +325,7 @@ impl expression::FromTree for Policy { .map_err(From::from) .map_err(Error::Parse)?; let subs = top - .args - .iter() + .children() .map(|arg| Self::from_tree(arg).map(Arc::new)) .collect::, Error>>()?; Ok(Policy::Thresh(Threshold::new(subs.len(), subs).map_err(Error::Threshold)?)) @@ -336,8 +335,7 @@ impl expression::FromTree for Policy { .map_err(From::from) .map_err(Error::Parse)?; let subs = top - .args - .iter() + .children() .map(|arg| Self::from_tree(arg).map(Arc::new)) .collect::, Error>>()?; Ok(Policy::Thresh(Threshold::new(1, subs).map_err(Error::Threshold)?))