Skip to content

Commit 2c422af

Browse files
apoelstraapoelstra
committed
policy: make Concrete::from_tree non-recursive
1 parent 61a89a2 commit 2c422af

File tree

1 file changed

+99
-87
lines changed

1 file changed

+99
-87
lines changed

‎src/policy/concrete.rs‎

Lines changed: 99 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -843,100 +843,112 @@ impl<Pk: FromStrKey> str::FromStr for Policy<Pk> {
843843

844844
serde_string_impl_pk!(Policy, "a miniscript concrete policy");
845845

846-
impl<Pk: FromStrKey> Policy<Pk> {
847-
/// Helper function for `from_tree` to parse subexpressions with
848-
/// names of the form x@y
849-
fn from_tree_prob(
850-
top: expression::TreeIterItem,
851-
allow_prob: bool,
852-
) -> Result<(usize, Policy<Pk>), Error> {
853-
// When 'allow_prob' is true we parse '@' signs out of node names.
854-
let (frag_prob, frag_name) = if allow_prob {
855-
top.name_separated('@')
856-
.map_err(From::from)
857-
.map_err(Error::Parse)?
858-
} else {
859-
(None, top.name())
860-
};
861-
862-
let frag_prob = match frag_prob {
863-
None => 1,
864-
Some(s) => expression::parse_num(s)
865-
.map_err(From::from)
866-
.map_err(Error::Parse)? as usize,
867-
};
868-
869-
match frag_name {
870-
"UNSATISFIABLE" => {
871-
top.verify_n_children("UNSATISFIABLE", 0..=0)
872-
.map_err(From::from)
873-
.map_err(Error::Parse)?;
874-
Ok(Policy::Unsatisfiable)
875-
}
876-
"TRIVIAL" => {
877-
top.verify_n_children("TRIVIAL", 0..=0)
846+
impl<Pk: FromStrKey> expression::FromTree for Policy<Pk> {
847+
fn from_tree(root: expression::TreeIterItem) -> Result<Policy<Pk>, Error> {
848+
root.verify_no_curly_braces()
849+
.map_err(From::from)
850+
.map_err(Error::Parse)?;
851+
852+
let mut stack = Vec::<(usize, _)>::with_capacity(128);
853+
for node in root.pre_order_iter().rev() {
854+
let allow_prob;
855+
// Before doing anything else, check if this is the inner value of a terminal.
856+
// In that case, just skip the node. Conveniently, there are no combinators
857+
// in policy that have a single child that these might be confused with (we
858+
// require and, or and thresholds to all have >1 child).
859+
if let Some(parent) = node.parent() {
860+
if parent.n_children() == 1 {
861+
continue;
862+
}
863+
let (_, parent_name) = parent
864+
.name_separated('@')
878865
.map_err(From::from)
879866
.map_err(Error::Parse)?;
880-
Ok(Policy::Trivial)
867+
if node.is_first_child() && parent_name == "thresh" {
868+
continue;
869+
}
870+
allow_prob = parent_name == "or";
871+
} else {
872+
allow_prob = false;
881873
}
882-
"pk" => top
883-
.verify_terminal_parent("pk", "public key")
884-
.map(Policy::Key)
885-
.map_err(Error::Parse),
886-
"after" => top.verify_after().map_err(Error::Parse).map(Policy::After),
887-
"older" => top.verify_older().map_err(Error::Parse).map(Policy::Older),
888-
"sha256" => top
889-
.verify_terminal_parent("sha256", "hash")
890-
.map(Policy::Sha256)
891-
.map_err(Error::Parse),
892-
"hash256" => top
893-
.verify_terminal_parent("hash256", "hash")
894-
.map(Policy::Hash256)
895-
.map_err(Error::Parse),
896-
"ripemd160" => top
897-
.verify_terminal_parent("ripemd160", "hash")
898-
.map(Policy::Ripemd160)
899-
.map_err(Error::Parse),
900-
"hash160" => top
901-
.verify_terminal_parent("hash160", "hash")
902-
.map(Policy::Hash160)
903-
.map_err(Error::Parse),
904-
"and" => {
905-
top.verify_n_children("and", 2..=2)
874+
875+
// When 'allow_prob' is true we parse '@' signs out of node names.
876+
let (frag_prob, frag_name) = if allow_prob {
877+
node.name_separated('@')
906878
.map_err(From::from)
907-
.map_err(Error::Parse)?;
908-
let subs = top
909-
.children()
910-
.map(|arg| Self::from_tree(arg).map(Arc::new))
911-
.collect::<Result<_, Error>>()?;
912-
Ok(Policy::And(subs))
913-
}
914-
"or" => {
915-
top.verify_n_children("or", 2..=2)
879+
.map_err(Error::Parse)?
880+
} else {
881+
(None, node.name())
882+
};
883+
884+
let frag_prob = match frag_prob {
885+
None => 1,
886+
Some(s) => expression::parse_num(s)
916887
.map_err(From::from)
917-
.map_err(Error::Parse)?;
918-
let subs = top
919-
.children()
920-
.map(|arg| {
921-
Self::from_tree_prob(arg, true).map(|(prob, sub)| (prob, Arc::new(sub)))
922-
})
923-
.collect::<Result<_, Error>>()?;
924-
Ok(Policy::Or(subs))
925-
}
926-
"thresh" => top
927-
.verify_threshold(|sub| Self::from_tree(sub).map(Arc::new))
928-
.map(Self::Thresh),
929-
x => Err(Error::Parse(crate::ParseError::Tree(crate::ParseTreeError::UnknownName {
930-
name: x.to_owned(),
931-
}))),
888+
.map_err(Error::Parse)? as usize,
889+
};
890+
891+
let new =
892+
match frag_name {
893+
"UNSATISFIABLE" => {
894+
node.verify_n_children("UNSATISFIABLE", 0..=0)
895+
.map_err(From::from)
896+
.map_err(Error::Parse)?;
897+
Ok(Policy::Unsatisfiable)
898+
}
899+
"TRIVIAL" => {
900+
node.verify_n_children("TRIVIAL", 0..=0)
901+
.map_err(From::from)
902+
.map_err(Error::Parse)?;
903+
Ok(Policy::Trivial)
904+
}
905+
"pk" => node
906+
.verify_terminal_parent("pk", "public key")
907+
.map(Policy::Key)
908+
.map_err(Error::Parse),
909+
"after" => node.verify_after().map_err(Error::Parse).map(Policy::After),
910+
"older" => node.verify_older().map_err(Error::Parse).map(Policy::Older),
911+
"sha256" => node
912+
.verify_terminal_parent("sha256", "hash")
913+
.map(Policy::Sha256)
914+
.map_err(Error::Parse),
915+
"hash256" => node
916+
.verify_terminal_parent("hash256", "hash")
917+
.map(Policy::Hash256)
918+
.map_err(Error::Parse),
919+
"ripemd160" => node
920+
.verify_terminal_parent("ripemd160", "hash")
921+
.map(Policy::Ripemd160)
922+
.map_err(Error::Parse),
923+
"hash160" => node
924+
.verify_terminal_parent("hash160", "hash")
925+
.map(Policy::Hash160)
926+
.map_err(Error::Parse),
927+
"and" => {
928+
node.verify_n_children("and", 2..=2)
929+
.map_err(From::from)
930+
.map_err(Error::Parse)?;
931+
Ok(Policy::And(vec![stack.pop().unwrap().1, stack.pop().unwrap().1]))
932+
}
933+
"or" => {
934+
node.verify_n_children("or", 2..=2)
935+
.map_err(From::from)
936+
.map_err(Error::Parse)?;
937+
Ok(Policy::Or(vec![stack.pop().unwrap(), stack.pop().unwrap()]))
938+
}
939+
"thresh" => node
940+
.verify_threshold(|_| Ok(stack.pop().unwrap().1))
941+
.map(Self::Thresh),
942+
x => Err(Error::Parse(crate::ParseError::Tree(
943+
crate::ParseTreeError::UnknownName { name: x.to_owned() },
944+
))),
945+
}?;
946+
947+
stack.push((frag_prob, Arc::new(new)));
932948
}
933-
.map(|res| (frag_prob, res))
934-
}
935-
}
936949

937-
impl<Pk: FromStrKey> expression::FromTree for Policy<Pk> {
938-
fn from_tree(root: expression::TreeIterItem) -> Result<Policy<Pk>, Error> {
939-
Policy::from_tree_prob(root, false).map(|(_, result)| result)
950+
assert_eq!(stack.len(), 1);
951+
Ok(Arc::try_unwrap(stack.pop().unwrap().1).unwrap())
940952
}
941953
}
942954

0 commit comments

Comments
 (0)