Merge rust-bitcoin/rust-bitcoin#924: Improvements to taproot script iterator

3c59897598 Removed IntoIterator for TapTree implementation (Dr Maxim Orlovsky)
7a5482d23a Rename LeafInfo into ScriptLeaf (Dr Maxim Orlovsky)
2b8d96581a Rename TapTree::iter into TapTree::script_leaves (Dr Maxim Orlovsky)
6f871ba47d Add convenience LeafInfo::depth method (Dr Maxim Orlovsky)
3c502ffc2d Making all LeafInfo fields private (Dr Maxim Orlovsky)
d655ff3e93 Make TapTreeIterator use LeafInfo (Dr Maxim Orlovsky)
79345fcd02 LeafInfo field accessor methods (Dr Maxim Orlovsky)
5958466678 Make LeafInfo::leaf_hash public and change its name and return type (Dr Maxim Orlovsky)
c83893d497 Make taproot LeafInfo public (Dr Maxim Orlovsky)

Pull request description:

  This PR makes existing taproot script iterator to iterate `LeafScript` values instead of constructed `(u8, &Script)`. First, this is more idiomatic (iterator should not construct value but iterate through real internal representation); second information about merkle path of the scripts is required for me downstream to implement OP_RETURN taproot commitments.

  The PR also removes unnecessary iterator type, replacing it with a slice iterator type from the core rust library.

  I am asking to include this PR into RC fix scope, since it is required downstream.

ACKs for top commit:
  sanket1729:
    ACK 3c59897598. Reviewed the range-diff with the post that I previously ACKed

Tree-SHA512: 99e341443987204a8aba20869c750bd80a725f3d49d1b5731d554dff7377181b02a4517f8b390101afb2957135dbb255c6e360f90cadd6ee07b17eb14fd30af5

src/util/psbt/map/output.rs

@@ -26,7 +26,7 @@ use util::psbt::map::Map;
 use util::psbt::raw;
 use util::psbt::Error;
 
-use util::taproot::{LeafInfo, TapLeafHash};
+use util::taproot::{ScriptLeaf, TapLeafHash};
 
 use util::taproot::{NodeInfo, TaprootBuilder};
 
@@ -155,34 +155,9 @@ impl TapTree {
         self.0
     }
 
-    /// Returns iterator for a taproot script tree, operating in DFS order over leaf depth and
+    /// Returns [`TapTreeIter`] iterator for a taproot script tree, operating in DFS order over
-    /// leaf script pairs.
+    /// tree [`ScriptLeaf`]s.
-    pub fn iter(&self) -> TapTreeIter {
+    pub fn script_leaves(&self) -> TapTreeIter {
-        self.into_iter()
-    }
-}
-
-/// Iterator for a taproot script tree, operating in DFS order over leaf depth and
-/// leaf script pairs.
-pub struct TapTreeIter<'tree> {
-    leaf_iter: core::slice::Iter<'tree, LeafInfo>,
-}
-
-impl<'tree> Iterator for TapTreeIter<'tree> {
-    type Item = (u8, &'tree Script);
-
-    fn next(&mut self) -> Option<Self::Item> {
-        self.leaf_iter.next().map(|leaf_info| {
-            (leaf_info.merkle_branch.as_inner().len() as u8, &leaf_info.script)
-        })
-    }
-}
-
-impl<'tree> IntoIterator for &'tree TapTree {
-    type Item = (u8, &'tree Script);
-    type IntoIter = TapTreeIter<'tree>;
-
-    fn into_iter(self) -> Self::IntoIter {
         match (self.0.branch().len(), self.0.branch().last()) {
             (1, Some(Some(root))) => {
                 TapTreeIter {
@@ -195,6 +170,21 @@ impl<'tree> IntoIterator for &'tree TapTree {
     }
 }
 
+/// Iterator for a taproot script tree, operating in DFS order over leaf depth and
+/// leaf script pairs.
+pub struct TapTreeIter<'tree> {
+    leaf_iter: core::slice::Iter<'tree, ScriptLeaf>,
+}
+
+impl<'tree> Iterator for TapTreeIter<'tree> {
+    type Item = &'tree ScriptLeaf;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        self.leaf_iter.next()
+    }
+}
+
 impl Output {
     pub(super) fn insert_pair(&mut self, pair: raw::Pair) -> Result<(), encode::Error> {
         let raw::Pair {

src/util/psbt/serialize.rs

@@ -327,9 +327,9 @@ impl Serialize for TapTree {
                     //
                     // TaprootMerkleBranch can only have len atmost 128(TAPROOT_CONTROL_MAX_NODE_COUNT).
                     // safe to cast from usize to u8
-                    buf.push(leaf_info.merkle_branch.as_inner().len() as u8);
+                    buf.push(leaf_info.merkle_branch().as_inner().len() as u8);
-                    buf.push(leaf_info.ver.to_consensus());
+                    buf.push(leaf_info.leaf_version().to_consensus());
-                    leaf_info.script.consensus_encode(&mut buf).expect("Vecs dont err");
+                    leaf_info.script().consensus_encode(&mut buf).expect("Vecs dont err");
                 }
                 buf
             }

src/util/taproot.rs

@@ -560,7 +560,7 @@ pub struct NodeInfo {
     /// Merkle hash for this node.
     pub(crate) hash: sha256::Hash,
     /// Information about leaves inside this node.
-    pub(crate) leaves: Vec<LeafInfo>,
+    pub(crate) leaves: Vec<ScriptLeaf>,
     /// Tracks information on hidden nodes below this node.
     pub(crate) has_hidden_nodes: bool,
 }
@@ -577,9 +577,9 @@ impl NodeInfo {
 
     /// Creates a new leaf [`NodeInfo`] with given [`Script`] and [`LeafVersion`].
     pub fn new_leaf_with_ver(script: Script, ver: LeafVersion) -> Self {
-        let leaf = LeafInfo::new(script, ver);
+        let leaf = ScriptLeaf::new(script, ver);
         Self {
-            hash: leaf.hash(),
+            hash: sha256::Hash::from_inner(leaf.leaf_hash().into_inner()),
             leaves: vec![leaf],
             has_hidden_nodes: false,
         }
@@ -608,17 +608,17 @@ impl NodeInfo {
 /// Store information about taproot leaf node.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub(crate) struct LeafInfo {
+pub struct ScriptLeaf {
     /// The underlying script.
-    pub(crate) script: Script,
+    script: Script,
     /// The leaf version.
-    pub(crate) ver: LeafVersion,
+    ver: LeafVersion,
     /// The merkle proof (hashing partners) to get this node.
-    pub(crate) merkle_branch: TaprootMerkleBranch,
+    merkle_branch: TaprootMerkleBranch,
 }
 
-impl LeafInfo {
+impl ScriptLeaf {
-    /// Creates an new [`LeafInfo`] from `script` and `ver` and no merkle branch.
+    /// Creates an new [`ScriptLeaf`] from `script` and `ver` and no merkle branch.
     fn new(script: Script, ver: LeafVersion) -> Self {
         Self {
             script: script,
@@ -627,10 +627,37 @@ impl LeafInfo {
         }
     }
 
-    /// Computes a leaf hash for this [`LeafInfo`].
+    /// Returns the depth of this script leaf in the tap tree.
-    fn hash(&self) -> sha256::Hash {
+    #[inline]
-        let leaf_hash = TapLeafHash::from_script(&self.script, self.ver);
+    pub fn depth(&self) -> u8 {
-        sha256::Hash::from_inner(leaf_hash.into_inner())
+        // The depth is guaranteed to be < 127 by the TaprootBuilder type.
+        // TODO: Following MSRV bump implement via `try_into().expect("")`.
+        self.merkle_branch.0.len() as u8
+    }
+
+    /// Computes a leaf hash for this [`ScriptLeaf`].
+    #[inline]
+    pub fn leaf_hash(&self) -> TapLeafHash {
+        TapLeafHash::from_script(&self.script, self.ver)
+    }
+
+    /// Returns reference to the leaf script.
+    #[inline]
+    pub fn script(&self) -> &Script {
+        &self.script
+    }
+
+    /// Returns leaf version of the script.
+    #[inline]
+    pub fn leaf_version(&self) -> LeafVersion {
+        self.ver
+    }
+
+    /// Returns reference to the merkle proof (hashing partners) to get this
+    /// node in form of [`TaprootMerkleBranch`].
+    #[inline]
+    pub fn merkle_branch(&self) -> &TaprootMerkleBranch {
+        &self.merkle_branch
     }
 }