Skip to content

Commit a7044cb

Browse files
apoelstraapoelstra
committed
expression: rewrite Tree module to no longer use a recursive data type
This significantly speeds up and simplifies tree parsing, at the cost of having a more complicated API (but we mostly addressed the API question in the previous commits). This completely eliminates recursion for the Tree data type, including in the Drop impl. Big diff but there are only two "real" changes -- expression/mod.rs is substantially rewritten of course since we replace the core datatype, and Tr::from_tree is substantially rewritten since doing so was the point of this change. The rest of the changes are mechanically changing the signature of expression::FromTree::from_tree everywhere.
1 parent 1db7df3 commit a7044cb

File tree

11 files changed

+536
-275
lines changed

11 files changed

+536
-275
lines changed

‎src/descriptor/bare.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Bare<Pk> {
175175
}
176176

177177
impl<Pk: FromStrKey> FromTree for Bare<Pk> {
178-
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
179-
let sub = Miniscript::<Pk, BareCtx>::from_tree(top)?;
178+
fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
179+
let sub = Miniscript::<Pk, BareCtx>::from_tree(root)?;
180180
BareCtx::top_level_checks(&sub)?;
181181
Bare::new(sub)
182182
}
@@ -186,7 +186,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Bare<Pk> {
186186
type Err = Error;
187187
fn from_str(s: &str) -> Result<Self, Self::Err> {
188188
let top = expression::Tree::from_str(s)?;
189-
Self::from_tree(&top)
189+
Self::from_tree(top.root())
190190
}
191191
}
192192

@@ -369,8 +369,8 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Pkh<Pk> {
369369
}
370370

371371
impl<Pk: FromStrKey> FromTree for Pkh<Pk> {
372-
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
373-
let pk = top
372+
fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
373+
let pk = root
374374
.verify_terminal_parent("pkh", "public key")
375375
.map_err(Error::Parse)?;
376376
Pkh::new(pk).map_err(Error::ContextError)
@@ -381,7 +381,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Pkh<Pk> {
381381
type Err = Error;
382382
fn from_str(s: &str) -> Result<Self, Self::Err> {
383383
let top = expression::Tree::from_str(s)?;
384-
Self::from_tree(&top)
384+
Self::from_tree(top.root())
385385
}
386386
}
387387

‎src/descriptor/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ impl Descriptor<DefiniteDescriptorKey> {
965965

966966
impl<Pk: FromStrKey> crate::expression::FromTree for Descriptor<Pk> {
967967
/// Parse an expression tree into a descriptor.
968-
fn from_tree(top: &expression::Tree) -> Result<Descriptor<Pk>, Error> {
968+
fn from_tree(top: expression::TreeIterItem) -> Result<Descriptor<Pk>, Error> {
969969
Ok(match (top.name(), top.n_children()) {
970970
("pkh", 1) => Descriptor::Pkh(Pkh::from_tree(top)?),
971971
("wpkh", 1) => Descriptor::Wpkh(Wpkh::from_tree(top)?),
@@ -981,7 +981,7 @@ impl<Pk: FromStrKey> FromStr for Descriptor<Pk> {
981981
type Err = Error;
982982
fn from_str(s: &str) -> Result<Descriptor<Pk>, Error> {
983983
let top = expression::Tree::from_str(s)?;
984-
let ret = Self::from_tree(&top)?;
984+
let ret = Self::from_tree(top.root())?;
985985
if let Descriptor::Tr(ref inner) = ret {
986986
// FIXME preserve weird/broken behavior from 12.x.
987987
// See https://github.com/rust-bitcoin/rust-miniscript/issues/734

‎src/descriptor/segwitv0.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wsh<Pk> {
247247
}
248248

249249
impl<Pk: FromStrKey> crate::expression::FromTree for Wsh<Pk> {
250-
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
250+
fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
251251
let top = top
252252
.verify_toplevel("wsh", 1..=1)
253253
.map_err(From::from)
@@ -284,7 +284,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Wsh<Pk> {
284284
type Err = Error;
285285
fn from_str(s: &str) -> Result<Self, Self::Err> {
286286
let top = expression::Tree::from_str(s)?;
287-
Wsh::<Pk>::from_tree(&top)
287+
Wsh::<Pk>::from_tree(top.root())
288288
}
289289
}
290290

@@ -483,7 +483,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Wpkh<Pk> {
483483
}
484484

485485
impl<Pk: FromStrKey> crate::expression::FromTree for Wpkh<Pk> {
486-
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
486+
fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
487487
let pk = top
488488
.verify_terminal_parent("wpkh", "public key")
489489
.map_err(Error::Parse)?;
@@ -495,7 +495,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Wpkh<Pk> {
495495
type Err = Error;
496496
fn from_str(s: &str) -> Result<Self, Self::Err> {
497497
let top = expression::Tree::from_str(s)?;
498-
Self::from_tree(&top)
498+
Self::from_tree(top.root())
499499
}
500500
}
501501

‎src/descriptor/sh.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
8181
}
8282

8383
impl<Pk: FromStrKey> crate::expression::FromTree for Sh<Pk> {
84-
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
84+
fn from_tree(top: expression::TreeIterItem) -> Result<Self, Error> {
8585
let top = top
8686
.verify_toplevel("sh", 1..=1)
8787
.map_err(From::from)
@@ -105,7 +105,7 @@ impl<Pk: FromStrKey> core::str::FromStr for Sh<Pk> {
105105
type Err = Error;
106106
fn from_str(s: &str) -> Result<Self, Self::Err> {
107107
let top = expression::Tree::from_str(s)?;
108-
Self::from_tree(&top)
108+
Self::from_tree(top.root())
109109
}
110110
}
111111

‎src/descriptor/sortedmulti.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
5959
}
6060

6161
/// Parse an expression tree into a SortedMultiVec
62-
pub fn from_tree(tree: &expression::Tree) -> Result<Self, Error>
62+
pub fn from_tree(tree: expression::TreeIterItem) -> Result<Self, Error>
6363
where
6464
Pk: FromStrKey,
6565
{

‎src/descriptor/tr.rs‎

Lines changed: 63 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use sync::Arc;
1414
use super::checksum;
1515
use crate::descriptor::DefiniteDescriptorKey;
1616
use crate::expression::{self, FromTree};
17-
use crate::iter::TreeLike as _;
1817
use crate::miniscript::satisfy::{Placeholder, Satisfaction, SchnorrSigType, Witness};
1918
use crate::miniscript::Miniscript;
2019
use crate::plan::AssetProvider;
@@ -495,99 +494,84 @@ impl<Pk: FromStrKey> core::str::FromStr for Tr<Pk> {
495494

496495
fn from_str(s: &str) -> Result<Self, Self::Err> {
497496
let expr_tree = expression::Tree::from_str(s)?;
498-
Self::from_tree(&expr_tree)
497+
Self::from_tree(expr_tree.root())
499498
}
500499
}
501500

502501
impl<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
503-
fn from_tree(expr_tree: &expression::Tree) -> Result<Self, Error> {
502+
fn from_tree(root: expression::TreeIterItem) -> Result<Self, Error> {
504503
use crate::expression::{Parens, ParseTreeError};
505504

506-
expr_tree
507-
.verify_toplevel("tr", 1..=2)
505+
struct TreeStack<'s, Pk: MiniscriptKey> {
506+
inner: Vec<(expression::TreeIterItem<'s>, TapTree<Pk>)>,
507+
}
508+
509+
impl<'s, Pk: MiniscriptKey> TreeStack<'s, Pk> {
510+
fn new() -> Self { Self { inner: Vec::with_capacity(128) } }
511+
512+
fn push(&mut self, parent: expression::TreeIterItem<'s>, tree: TapTree<Pk>) {
513+
let mut next_push = (parent, tree);
514+
while let Some(top) = self.inner.pop() {
515+
if next_push.0.index() == top.0.index() {
516+
next_push.0 = top.0.parent().unwrap();
517+
next_push.1 = TapTree::combine(top.1, next_push.1);
518+
} else {
519+
self.inner.push(top);
520+
break;
521+
}
522+
}
523+
self.inner.push(next_push);
524+
}
525+
526+
fn pop_final(&mut self) -> Option<TapTree<Pk>> {
527+
assert_eq!(self.inner.len(), 1);
528+
self.inner.pop().map(|x| x.1)
529+
}
530+
}
531+
532+
root.verify_toplevel("tr", 1..=2)
508533
.map_err(From::from)
509534
.map_err(Error::Parse)?;
510535

511-
let mut round_paren_depth = 0;
536+
let mut root_children = root.children();
537+
let internal_key: Pk = root_children
538+
.next()
539+
.unwrap() // `verify_toplevel` above checked that first child existed
540+
.verify_terminal("internal key")
541+
.map_err(Error::Parse)?;
512542

513-
let mut internal_key = None;
514-
let mut tree_stack = vec![];
543+
let tap_tree = match root_children.next() {
544+
None => return Tr::new(internal_key, None),
545+
Some(tree) => tree,
546+
};
515547

516-
for item in expr_tree.verbose_pre_order_iter() {
517-
// Top-level "tr" node.
518-
if item.index == 0 {
519-
if item.is_complete {
520-
debug_assert!(
521-
internal_key.is_some(),
522-
"checked above that top-level 'tr' has children"
523-
);
548+
let mut tree_stack = TreeStack::new();
549+
let mut tap_tree_iter = tap_tree.pre_order_iter();
550+
// while let construction needed because we modify the iterator inside the loop
551+
// (by calling skip_descendants to skip over the contents of the tapscripts).
552+
while let Some(node) = tap_tree_iter.next() {
553+
if node.parens() == Parens::Curly {
554+
if !node.name().is_empty() {
555+
return Err(Error::Parse(ParseError::Tree(ParseTreeError::IncorrectName {
556+
actual: node.name().to_owned(),
557+
expected: "",
558+
})));
524559
}
525-
} else if item.index == 1 {
526-
// First child of tr, which must be the internal key
527-
internal_key = item
528-
.node
529-
.verify_terminal("internal key")
530-
.map_err(Error::Parse)
531-
.map(Some)?;
560+
node.verify_n_children("taptree branch", 2..=2)
561+
.map_err(From::from)
562+
.map_err(Error::Parse)?;
532563
} else {
533-
// From here on we are into the taptree.
534-
if item.n_children_yielded == 0 {
535-
match item.node.parens() {
536-
Parens::Curly => {
537-
if !item.node.name().is_empty() {
538-
return Err(Error::Parse(ParseError::Tree(
539-
ParseTreeError::IncorrectName {
540-
actual: item.node.name().to_owned(),
541-
expected: "",
542-
},
543-
)));
544-
}
545-
if round_paren_depth > 0 {
546-
return Err(Error::Parse(ParseError::Tree(
547-
ParseTreeError::IllegalCurlyBrace {
548-
pos: item.node.children_pos(),
549-
},
550-
)));
551-
}
552-
}
553-
Parens::Round => round_paren_depth += 1,
554-
_ => {}
555-
}
556-
}
557-
if item.is_complete {
558-
if item.node.parens() == Parens::Curly {
559-
if item.n_children_yielded == 2 {
560-
let rchild = tree_stack.pop().unwrap();
561-
let lchild = tree_stack.pop().unwrap();
562-
tree_stack.push(TapTree::combine(lchild, rchild));
563-
} else {
564-
return Err(Error::Parse(ParseError::Tree(
565-
ParseTreeError::IncorrectNumberOfChildren {
566-
description: "Taptree node",
567-
n_children: item.n_children_yielded,
568-
minimum: Some(2),
569-
maximum: Some(2),
570-
},
571-
)));
572-
}
573-
} else {
574-
if item.node.parens() == Parens::Round {
575-
round_paren_depth -= 1;
576-
}
577-
if round_paren_depth == 0 {
578-
let script = Miniscript::from_tree(item.node)?;
579-
// FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734
580-
if script.ty.corr.base != crate::miniscript::types::Base::B {
581-
return Err(Error::NonTopLevel(format!("{:?}", script)));
582-
};
583-
tree_stack.push(TapTree::Leaf(Arc::new(script)));
584-
}
585-
}
586-
}
564+
let script = Miniscript::from_tree(node)?;
565+
// FIXME hack for https://github.com/rust-bitcoin/rust-miniscript/issues/734
566+
if script.ty.corr.base != crate::miniscript::types::Base::B {
567+
return Err(Error::NonTopLevel(format!("{:?}", script)));
568+
};
569+
570+
tree_stack.push(node.parent().unwrap(), TapTree::Leaf(Arc::new(script)));
571+
tap_tree_iter.skip_descendants();
587572
}
588573
}
589-
assert!(tree_stack.len() <= 1);
590-
Tr::new(internal_key.unwrap(), tree_stack.pop())
574+
Tr::new(internal_key, tree_stack.pop_final())
591575
}
592576
}
593577

0 commit comments

Comments
 (0)