tr: move TapTree depth-checking to construction rather than embedding in Tr

This makes it impossible to construct a `TapTree` whose depth exceeds
the limit, which means that we can assume that our maximum depth is 128
in all of our code.

Also introduces a new error type for this case distinct from "maximum
recursive depth exceeded" which is supposed to be about limitations of
this library, or at least of Script, rather than about the Taproot
limit.

Author: apoelstra

src/descriptor/mod.rs

@@ -42,7 +42,7 @@ pub use self::bare::{Bare, Pkh};
 pub use self::segwitv0::{Wpkh, Wsh, WshInner};
 pub use self::sh::{Sh, ShInner};
 pub use self::sortedmulti::SortedMultiVec;
-pub use self::tr::{TapTree, TapTreeIter, TapTreeIterItem, Tr};
+pub use self::tr::{TapTree, TapTreeDepthError, TapTreeIter, TapTreeIterItem, Tr};
 
 pub mod checksum;
 mod key;

src/descriptor/tr/mod.rs

@@ -6,7 +6,7 @@ use core::{cmp, fmt, hash};
 use bitcoin::secp256k1;
 use bitcoin::taproot::{
     LeafVersion, TaprootBuilder, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE,
-    TAPROOT_CONTROL_MAX_NODE_COUNT, TAPROOT_CONTROL_NODE_SIZE,
+    TAPROOT_CONTROL_NODE_SIZE,
 };
 use bitcoin::{opcodes, Address, Network, ScriptBuf, Weight};
 use sync::Arc;
@@ -28,7 +28,7 @@ use crate::{
 
 mod taptree;
 
-pub use self::taptree::{TapTree, TapTreeIter, TapTreeIterItem};
+pub use self::taptree::{TapTree, TapTreeDepthError, TapTreeIter, TapTreeIterItem};
 
 /// A taproot descriptor
 pub struct Tr<Pk: MiniscriptKey> {
@@ -98,13 +98,7 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
     /// Create a new [`Tr`] descriptor from internal key and [`TapTree`]
     pub fn new(internal_key: Pk, tree: Option<TapTree<Pk>>) -> Result<Self, Error> {
         Tap::check_pk(&internal_key)?;
-        let nodes = tree.as_ref().map(|t| t.height()).unwrap_or(0);
-
-        if nodes <= TAPROOT_CONTROL_MAX_NODE_COUNT {
-            Ok(Self { internal_key, tree, spend_info: Mutex::new(None) })
-        } else {
-            Err(Error::MaxRecursiveDepthExceeded)
-        }
+        Ok(Self { internal_key, tree, spend_info: Mutex::new(None) })
     }
 
     /// Obtain the internal key of [`Tr`] descriptor
@@ -399,18 +393,23 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
         impl<'s, Pk: MiniscriptKey> TreeStack<'s, Pk> {
             fn new() -> Self { Self { inner: Vec::with_capacity(128) } }
 
-            fn push(&mut self, parent: expression::TreeIterItem<'s>, tree: TapTree<Pk>) {
+            fn push(
+                &mut self,
+                parent: expression::TreeIterItem<'s>,
+                tree: TapTree<Pk>,
+            ) -> Result<(), Error> {
                 let mut next_push = (parent, tree);
                 while let Some(top) = self.inner.pop() {
                     if next_push.0.index() == top.0.index() {
                         next_push.0 = top.0.parent().unwrap();
-                        next_push.1 = TapTree::combine(top.1, next_push.1);
+                        next_push.1 = TapTree::combine(top.1, next_push.1)?;
                     } else {
                         self.inner.push(top);
                         break;
                     }
                 }
                 self.inner.push(next_push);
+                Ok(())
             }
 
             fn pop_final(&mut self) -> Option<TapTree<Pk>> {
@@ -457,7 +456,7 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Tr<Pk> {
                     return Err(Error::NonTopLevel(format!("{:?}", script)));
                 };
 
-                tree_stack.push(node.parent().unwrap(), TapTree::Leaf(Arc::new(script)));
+                tree_stack.push(node.parent().unwrap(), TapTree::Leaf(Arc::new(script)))?;
                 tap_tree_iter.skip_descendants();
             }
         }

src/descriptor/tr/taptree.rs

@@ -2,14 +2,28 @@
 
 use core::{cmp, fmt};
 
-use bitcoin::taproot::{LeafVersion, TapLeafHash};
+use bitcoin::taproot::{LeafVersion, TapLeafHash, TAPROOT_CONTROL_MAX_NODE_COUNT};
 
 use crate::miniscript::context::Tap;
 use crate::policy::{Liftable, Semantic};
 use crate::prelude::Vec;
 use crate::sync::Arc;
 use crate::{Miniscript, MiniscriptKey, Threshold, ToPublicKey, TranslateErr, Translator};
 
+/// Tried to construct Taproot tree which was too deep.
+#[derive(PartialEq, Eq, Debug)]
+#[non_exhaustive]
+pub struct TapTreeDepthError;
+
+impl fmt::Display for TapTreeDepthError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str("maximum Taproot tree depth (128) exceeded")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for TapTreeDepthError {}
+
 /// A Taproot Tree representation.
 // Hidden leaves are not yet supported in descriptor spec. Conceptually, it should
 // be simple to integrate those here, but it is best to wait on core for the exact syntax.
@@ -33,9 +47,13 @@ pub enum TapTree<Pk: MiniscriptKey> {
 
 impl<Pk: MiniscriptKey> TapTree<Pk> {
     /// Creates a `TapTree` by combining `left` and `right` tree nodes.
-    pub fn combine(left: TapTree<Pk>, right: TapTree<Pk>) -> Self {
+    pub fn combine(left: TapTree<Pk>, right: TapTree<Pk>) -> Result<Self, TapTreeDepthError> {
         let height = 1 + cmp::max(left.height(), right.height());
-        TapTree::Tree { left: Arc::new(left), right: Arc::new(right), height }
+        if height <= TAPROOT_CONTROL_MAX_NODE_COUNT {
+            Ok(TapTree::Tree { left: Arc::new(left), right: Arc::new(right), height })
+        } else {
+            Err(TapTreeDepthError)
+        }
     }
 
     /// Returns the height of this tree.

src/lib.rs

@@ -450,6 +450,8 @@ pub enum Error {
     LiftError(policy::LiftError),
     /// Forward script context related errors
     ContextError(miniscript::context::ScriptContextError),
+    /// Tried to construct a Taproot tree which was too deep.
+    TapTreeDepthError(crate::descriptor::TapTreeDepthError),
     /// Recursion depth exceeded when parsing policy/miniscript from string
     MaxRecursiveDepthExceeded,
     /// Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...)
@@ -509,6 +511,7 @@ impl fmt::Display for Error {
             Error::TypeCheck(ref e) => write!(f, "typecheck: {}", e),
             Error::Secp(ref e) => fmt::Display::fmt(e, f),
             Error::ContextError(ref e) => fmt::Display::fmt(e, f),
+            Error::TapTreeDepthError(ref e) => fmt::Display::fmt(e, f),
             #[cfg(feature = "compiler")]
             Error::CompilerError(ref e) => fmt::Display::fmt(e, f),
             Error::ConcretePolicy(ref e) => fmt::Display::fmt(e, f),
@@ -573,6 +576,7 @@ impl std::error::Error for Error {
             ConcretePolicy(e) => Some(e),
             LiftError(e) => Some(e),
             ContextError(e) => Some(e),
+            TapTreeDepthError(e) => Some(e),
             AnalysisError(e) => Some(e),
             PubKeyCtxError(e, _) => Some(e),
             AbsoluteLockTime(e) => Some(e),
@@ -594,6 +598,11 @@ impl From<policy::LiftError> for Error {
     fn from(e: policy::LiftError) -> Error { Error::LiftError(e) }
 }
 
+#[doc(hidden)]
+impl From<crate::descriptor::TapTreeDepthError> for Error {
+    fn from(e: crate::descriptor::TapTreeDepthError) -> Error { Error::TapTreeDepthError(e) }
+}
+
 #[doc(hidden)]
 impl From<miniscript::context::ScriptContextError> for Error {
     fn from(e: miniscript::context::ScriptContextError) -> Error { Error::ContextError(e) }

src/policy/concrete.rs

@@ -965,7 +965,11 @@ fn with_huffman_tree<Pk: MiniscriptKey>(ms: Vec<(OrdF64, Miniscript<Pk, Tap>)>)
         let (p2, s2) = node_weights.pop().expect("len must at least be two");
 
         let p = (p1.0).0 + (p2.0).0;
-        node_weights.push((Reverse(OrdF64(p)), TapTree::combine(s1, s2)));
+        node_weights.push((
+            Reverse(OrdF64(p)),
+            TapTree::combine(s1, s2)
+                .expect("huffman tree cannot produce depth > 128 given sane weights"),
+        ));
     }
 
     debug_assert!(node_weights.len() == 1);

src/policy/mod.rs

@@ -393,7 +393,7 @@ mod tests {
 
             let left = TapTree::Leaf(left_ms_compilation);
             let right = TapTree::Leaf(right_ms_compilation);
-            let tree = TapTree::combine(left, right);
+            let tree = TapTree::combine(left, right).unwrap();
 
             let expected_descriptor =
                 Descriptor::new_tr(unspendable_key.clone(), Some(tree)).unwrap();
@@ -460,18 +460,23 @@ mod tests {
 
             // Arrange leaf compilations (acc. to probabilities) using huffman encoding into a TapTree
             let tree = TapTree::combine(
-                TapTree::combine(node_compilations[4].clone(), node_compilations[5].clone()),
+                TapTree::combine(node_compilations[4].clone(), node_compilations[5].clone())
+                    .unwrap(),
                 TapTree::combine(
                     TapTree::combine(
                         TapTree::combine(
                             node_compilations[0].clone(),
                             node_compilations[1].clone(),
-                        ),
+                        )
+                        .unwrap(),
                         node_compilations[3].clone(),
-                    ),
+                    )
+                    .unwrap(),
                     node_compilations[6].clone(),
-                ),
-            );
+                )
+                .unwrap(),
+            )
+            .unwrap();
 
             let expected_descriptor = Descriptor::new_tr("E".to_string(), Some(tree)).unwrap();
             assert_eq!(descriptor, expected_descriptor);