Merge #612: Remove recursion in semantic module

f9307c8 semantic: Remove recursion in sorted (Tobin C. Harding)
09a47cb semantic: Remove recursion in minimum_n_keys (Tobin C. Harding)
eb75602 semantic: Remove recursion in n_keys (Tobin C. Harding)
d9f9d43 semantic: Remove recursion in at_age and at_lock_time (Tobin C. Harding)
c3c5e72 semantic: Remove recursion in real_*_timelocks (Tobin C. Harding)
b1ae1ff semantic: Remove recursion in n_terminals (Tobin C. Harding)
211abad semantic: Remove recursion in translate_pk (Tobin C. Harding)
97ba4e9 semantic: Remove recursion in for_each_key (Tobin C. Harding)
df3a85a Implement TreeLike for semantic::Policy (Tobin C. Harding)
e813ad0 Add Arc to the semantic::Policy::Thresh vector (Tobin C. Harding)
0ef5e54 Move TreeLike impl to concrete module (Tobin C. Harding)
161753c Fix imports in test module (Tobin C. Harding)
2fd42d6 Improve spacing in error string (Tobin C. Harding)
721e16b Use unwrap_or (Tobin C. Harding)
10e5982 Add line of whitespace (Tobin C. Harding)
61b45a9 Remove code comment (Tobin C. Harding)

Pull request description:

  Remove most of the recursion in the `semantic` module. Does not do `normalized` and associated functions (ones that either call it or take in a normalized policy).

  Includes 4 trivial preparatory clean up patches at the front.

ACKs for top commit:
  apoelstra:
    ACK f9307c8

Tree-SHA512: dd2eac15ecd2a672436ffe7b21277c72406c92d3e3434ac2046e7ebd4cba98e02db4ce5e39ed73f0bfcc2388610e871acd578655461bdb99e8beaa2aac4aefb5

src/descriptor/sortedmulti.rs

@@ -17,6 +17,7 @@ use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
 use crate::miniscript::satisfy::{Placeholder, Satisfaction};
 use crate::plan::AssetProvider;
 use crate::prelude::*;
+use crate::sync::Arc;
 use crate::{
     errstr, expression, policy, script_num_size, Error, ForEachKey, Miniscript, MiniscriptKey,
     Satisfier, ToPublicKey, TranslateErr, Translator,
@@ -201,7 +202,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> policy::Liftable<Pk> for SortedMulti
             self.k,
             self.pks
                 .iter()
-                .map(|k| policy::semantic::Policy::Key(k.clone()))
+                .map(|k| Arc::new(policy::semantic::Policy::Key(k.clone())))
                 .collect(),
         );
         Ok(ret)

src/descriptor/tr.rs

@@ -614,9 +614,10 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for TapTree<Pk> {
     fn lift(&self) -> Result<Policy<Pk>, Error> {
         fn lift_helper<Pk: MiniscriptKey>(s: &TapTree<Pk>) -> Result<Policy<Pk>, Error> {
             match *s {
-                TapTree::Tree { ref left, ref right, height: _ } => {
-                    Ok(Policy::Threshold(1, vec![lift_helper(left)?, lift_helper(right)?]))
-                }
+                TapTree::Tree { ref left, ref right, height: _ } => Ok(Policy::Threshold(
+                    1,
+                    vec![Arc::new(lift_helper(left)?), Arc::new(lift_helper(right)?)],
+                )),
                 TapTree::Leaf(ref leaf) => leaf.lift(),
             }
         }
@@ -629,9 +630,13 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for TapTree<Pk> {
 impl<Pk: MiniscriptKey> Liftable<Pk> for Tr<Pk> {
     fn lift(&self) -> Result<Policy<Pk>, Error> {
         match &self.tree {
-            Some(root) => {
-                Ok(Policy::Threshold(1, vec![Policy::Key(self.internal_key.clone()), root.lift()?]))
-            }
+            Some(root) => Ok(Policy::Threshold(
+                1,
+                vec![
+                    Arc::new(Policy::Key(self.internal_key.clone())),
+                    Arc::new(root.lift()?),
+                ],
+            )),
             None => Ok(Policy::Key(self.internal_key.clone())),
         }
     }

src/iter/mod.rs

@@ -15,7 +15,7 @@ pub use tree::{
 };
 
 use crate::sync::Arc;
-use crate::{policy, Miniscript, MiniscriptKey, ScriptContext, Terminal};
+use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal};
 
 impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript<Pk, Ctx> {
     fn as_node(&self) -> Tree<Self> {
@@ -68,29 +68,3 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for Arc<Miniscript<Pk, Ctx>
         }
     }
 }
-
-impl<'a, Pk: MiniscriptKey> TreeLike for &'a policy::Concrete<Pk> {
-    fn as_node(&self) -> Tree<Self> {
-        use policy::Concrete::*;
-        match *self {
-            Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
-            | Ripemd160(_) | Hash160(_) => Tree::Nullary,
-            And(ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
-            Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| p.as_ref()).collect()),
-            Threshold(_, ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
-        }
-    }
-}
-
-impl<Pk: MiniscriptKey> TreeLike for Arc<policy::Concrete<Pk>> {
-    fn as_node(&self) -> Tree<Self> {
-        use policy::Concrete::*;
-        match self.as_ref() {
-            Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
-            | Ripemd160(_) | Hash160(_) => Tree::Nullary,
-            And(ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
-            Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| Arc::clone(p)).collect()),
-            Threshold(_, ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
-        }
-    }
-}

src/policy/concrete.rs

@@ -23,7 +23,7 @@ use {
 
 use super::ENTAILMENT_MAX_TERMINALS;
 use crate::expression::{self, FromTree};
-use crate::iter::TreeLike;
+use crate::iter::{Tree, TreeLike};
 use crate::miniscript::types::extra_props::TimelockInfo;
 use crate::prelude::*;
 use crate::sync::Arc;
@@ -1114,6 +1114,34 @@ fn generate_combination<Pk: MiniscriptKey>(
     ret
 }
 
+impl<'a, Pk: MiniscriptKey> TreeLike for &'a Policy<Pk> {
+    fn as_node(&self) -> Tree<Self> {
+        use Policy::*;
+
+        match *self {
+            Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
+            | Ripemd160(_) | Hash160(_) => Tree::Nullary,
+            And(ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
+            Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| p.as_ref()).collect()),
+            Threshold(_, ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
+        }
+    }
+}
+
+impl<Pk: MiniscriptKey> TreeLike for Arc<Policy<Pk>> {
+    fn as_node(&self) -> Tree<Self> {
+        use Policy::*;
+
+        match self.as_ref() {
+            Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_)
+            | Ripemd160(_) | Hash160(_) => Tree::Nullary,
+            And(ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
+            Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| Arc::clone(p)).collect()),
+            Threshold(_, ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
+        }
+    }
+}
+
 #[cfg(all(test, feature = "compiler"))]
 mod compiler_tests {
     use core::str::FromStr;

src/policy/mod.rs

@@ -23,7 +23,7 @@ pub use self::semantic::Policy as Semantic;
 use crate::descriptor::Descriptor;
 use crate::miniscript::{Miniscript, ScriptContext};
 use crate::sync::Arc;
-use crate::{Error, MiniscriptKey, Terminal};
+use crate::{Error, MiniscriptKey, Terminal, Vec};
 
 /// Policy entailment algorithm maximum number of terminals allowed.
 const ENTAILMENT_MAX_TERMINALS: usize = 20;
@@ -136,28 +136,40 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Liftable<Pk> for Terminal<Pk, Ctx> {
             | Terminal::NonZero(ref sub)
             | Terminal::ZeroNotEqual(ref sub) => sub.node.lift()?,
             Terminal::AndV(ref left, ref right) | Terminal::AndB(ref left, ref right) => {
-                Semantic::Threshold(2, vec![left.node.lift()?, right.node.lift()?])
+                Semantic::Threshold(
+                    2,
+                    vec![Arc::new(left.node.lift()?), Arc::new(right.node.lift()?)],
+                )
             }
             Terminal::AndOr(ref a, ref b, ref c) => Semantic::Threshold(
                 1,
                 vec![
-                    Semantic::Threshold(2, vec![a.node.lift()?, b.node.lift()?]),
-                    c.node.lift()?,
+                    Arc::new(Semantic::Threshold(
+                        2,
+                        vec![Arc::new(a.node.lift()?), Arc::new(b.node.lift()?)],
+                    )),
+                    Arc::new(c.node.lift()?),
                 ],
             ),
             Terminal::OrB(ref left, ref right)
             | Terminal::OrD(ref left, ref right)
             | Terminal::OrC(ref left, ref right)
-            | Terminal::OrI(ref left, ref right) => {
-                Semantic::Threshold(1, vec![left.node.lift()?, right.node.lift()?])
-            }
+            | Terminal::OrI(ref left, ref right) => Semantic::Threshold(
+                1,
+                vec![Arc::new(left.node.lift()?), Arc::new(right.node.lift()?)],
+            ),
             Terminal::Thresh(k, ref subs) => {
-                let semantic_subs: Result<_, Error> = subs.iter().map(|s| s.node.lift()).collect();
-                Semantic::Threshold(k, semantic_subs?)
-            }
-            Terminal::Multi(k, ref keys) | Terminal::MultiA(k, ref keys) => {
-                Semantic::Threshold(k, keys.iter().map(|k| Semantic::Key(k.clone())).collect())
+                let semantic_subs: Result<Vec<Semantic<Pk>>, Error> =
+                    subs.iter().map(|s| s.node.lift()).collect();
+                let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect();
+                Semantic::Threshold(k, semantic_subs)
             }
+            Terminal::Multi(k, ref keys) | Terminal::MultiA(k, ref keys) => Semantic::Threshold(
+                k,
+                keys.iter()
+                    .map(|k| Arc::new(Semantic::Key(k.clone())))
+                    .collect(),
+            ),
         }
         .normalized();
         Ok(ret)
@@ -197,17 +209,22 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Concrete<Pk> {
             Concrete::Ripemd160(ref h) => Semantic::Ripemd160(h.clone()),
             Concrete::Hash160(ref h) => Semantic::Hash160(h.clone()),
             Concrete::And(ref subs) => {
-                let semantic_subs: Result<_, Error> = subs.iter().map(Liftable::lift).collect();
-                Semantic::Threshold(2, semantic_subs?)
+                let semantic_subs: Result<Vec<Semantic<Pk>>, Error> =
+                    subs.iter().map(Liftable::lift).collect();
+                let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect();
+                Semantic::Threshold(2, semantic_subs)
             }
             Concrete::Or(ref subs) => {
-                let semantic_subs: Result<_, Error> =
+                let semantic_subs: Result<Vec<Semantic<Pk>>, Error> =
                     subs.iter().map(|(_p, sub)| sub.lift()).collect();
-                Semantic::Threshold(1, semantic_subs?)
+                let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect();
+                Semantic::Threshold(1, semantic_subs)
             }
             Concrete::Threshold(k, ref subs) => {
-                let semantic_subs: Result<_, Error> = subs.iter().map(Liftable::lift).collect();
-                Semantic::Threshold(k, semantic_subs?)
+                let semantic_subs: Result<Vec<Semantic<Pk>>, Error> =
+                    subs.iter().map(Liftable::lift).collect();
+                let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect();
+                Semantic::Threshold(k, semantic_subs)
             }
         }
         .normalized();
@@ -223,14 +240,12 @@ mod tests {
     use core::str::FromStr;
 
     use bitcoin::Sequence;
-    #[cfg(feature = "compiler")]
-    use sync::Arc;
 
-    use super::super::miniscript::context::Segwitv0;
-    use super::super::miniscript::Miniscript;
-    use super::{Concrete, Liftable, Semantic};
+    use super::*;
     #[cfg(feature = "compiler")]
     use crate::descriptor::Tr;
+    use crate::miniscript::context::Segwitv0;
+    use crate::miniscript::Miniscript;
     use crate::prelude::*;
     #[cfg(feature = "compiler")]
     use crate::{descriptor::TapTree, Descriptor, Tap};
@@ -348,14 +363,14 @@ mod tests {
             Semantic::Threshold(
                 1,
                 vec![
-                    Semantic::Threshold(
+                    Arc::new(Semantic::Threshold(
                         2,
                         vec![
-                            Semantic::Key(key_a),
-                            Semantic::Older(Sequence::from_height(42))
+                            Arc::new(Semantic::Key(key_a)),
+                            Arc::new(Semantic::Older(Sequence::from_height(42)))
                         ]
-                    ),
-                    Semantic::Key(key_b)
+                    )),
+                    Arc::new(Semantic::Key(key_b))
                 ]
             ),
             ms_str.lift().unwrap()