From 00495a5442ff0f66c71728e317084ddcc1ed2ff7 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 2 May 2025 16:36:43 +0000 Subject: [PATCH 1/7] descriptor: add unit test from sanket --- src/descriptor/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 650b4ad74..5b7822564 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -1522,17 +1522,20 @@ mod tests { "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})", p1, p2, p3, p4, p5 )) - .unwrap() - .to_string(); + .unwrap(); // p5.to_pubkeyhash() = 516ca378e588a7ed71336147e2a72848b20aca1a assert_eq!( - descriptor, + descriptor.to_string(), format!( "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})#tvu28c0s", p1, p2, p3, p4, p5 ) - ) + ); + assert_eq!( + descriptor.spend_info().merkle_root().unwrap().to_string(), + "e1597abcb76f7cbc0792cf04a9c2d4f39caed1ede0afef772064126f28c69b09" + ); } #[test] From 27a30e61f7dd95f8719241b1add3438dd04aadce Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 30 Mar 2025 22:41:22 +0000 Subject: [PATCH 2/7] psbt: untangle some logic in `update_item_with_descriptor_helper` We have the function `update_item_with_descriptor_helper` which does a few things: it derives a descriptor (replacing all the xpubs with actual public keys) and updates the appropriate input or output map to map the derived keys to their keysources. It treats Tr outputs differently from other kinds of outputs, because the relevant maps are different. However, in doing so, it duplicates a bunch of work in ways that are hard to follow. Essentially, the algorithm does three things: (a) derives all the keys (and the descriptor), (b) optionally checks that the resulting scriptpubkey is what we expect, and (c) updates the maps. The existing code handles (a) separately for Tr and non-Tr descriptors. In the Tr case, we derive all the keys using Descriptor::::derived_descriptor which derives all the keys and throws away the conversion. Then separately it keeps around the un-derived descriptor, iterates through the keys, and populates the `tap_key_origins` map by re-computing the derivation. In the non-Tr case, we derive all the keys using the `KeySourceLookUp` object, which does exactly the same thing as `derived_descriptor` except that it stores its work in a BTreeMap, which is directly added to the PSBT's `item.bip32_derivation` field. This commit pulls out (a) into common code; it then copies all the data out of the key map into `item.tap_key_origins` along with an empty vector of tapleaves. It then goes through all the leaves, and for each key that appears in each leaf, appends that leaf's hash to the vector of tapleaves. This is still a little ineffecient but will be much cleaner after a later commit when we improve the Taproot SpendInfo structure. The original code dates to Lloyd's 2022 PR #339 which introduces logic to populate these maps. That algorithm underwent significant refactoring in response to review comments and I suspect that the duplicated logic went unnoticed after all the refactorings. --- src/psbt/mod.rs | 168 +++++++++++++++++++----------------------------- 1 file changed, 67 insertions(+), 101 deletions(-) diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 84924f42e..5bb96fda8 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -1073,119 +1073,87 @@ fn update_item_with_descriptor_helper( item: &mut F, descriptor: &Descriptor, check_script: Option<&Script>, - // the return value is a tuple here since the two internal calls to it require different info. - // One needs the derived descriptor and the other needs to know whether the script_pubkey check - // failed. + // We return an extra boolean here to indicate an error with `check_script`. We do this + // because the error is "morally" a UtxoUpdateError::MismatchedScriptPubkey, but some + // callers expect a `descriptor::ConversionError`, which cannot be produced from a + // `UtxoUpdateError`, and those callers can't get this error anyway because they pass + // `None` for `check_script`. ) -> Result<(Descriptor, bool), descriptor::ConversionError> { - let secp = secp256k1::Secp256k1::verification_only(); + let secp = Secp256k1::verification_only(); + + // 1. Derive the descriptor, recording each key derivation in a map from xpubs + // the keysource used to derive the key. + let mut bip32_derivation = KeySourceLookUp(BTreeMap::new(), secp); + let derived = descriptor + .translate_pk(&mut bip32_derivation) + .map_err(|e| e.expect_translator_err("No Outer Context errors in translations"))?; + + // 2. If we have a specific scriptpubkey we are targeting, bail out. + if let Some(check_script) = check_script { + if check_script != &derived.script_pubkey() { + return Ok((derived, false)); + } + } - let derived = if let Descriptor::Tr(_) = &descriptor { - let derived = descriptor.derived_descriptor(&secp)?; + // 3. Update the PSBT fields using the derived key map. + if let Descriptor::Tr(ref tr_derived) = &derived { + let spend_info = tr_derived.spend_info(); + let KeySourceLookUp(xpub_map, _) = bip32_derivation; - if let Some(check_script) = check_script { - if check_script != &derived.script_pubkey() { - return Ok((derived, false)); - } + *item.tap_internal_key() = Some(spend_info.internal_key()); + for (derived_key, key_source) in xpub_map { + item.tap_key_origins() + .insert(derived_key.to_x_only_pubkey(), (vec![], key_source)); + } + if let Some(merkle_root) = item.tap_merkle_root() { + *merkle_root = spend_info.merkle_root(); } - // NOTE: they will both always be Tr - if let (Descriptor::Tr(tr_derived), Descriptor::Tr(tr_xpk)) = (&derived, descriptor) { - let spend_info = tr_derived.spend_info(); - let ik_derived = spend_info.internal_key(); - let ik_xpk = tr_xpk.internal_key(); - if let Some(merkle_root) = item.tap_merkle_root() { - *merkle_root = spend_info.merkle_root(); + let mut builder = taproot::TaprootBuilder::new(); + for leaf_derived in tr_derived.leaves() { + let leaf_script = (leaf_derived.compute_script(), leaf_derived.leaf_version()); + let tapleaf_hash = TapLeafHash::from_script(&leaf_script.0, leaf_script.1); + builder = builder + .add_leaf(leaf_derived.depth(), leaf_script.0.clone()) + .expect("Computing spend data on a valid tree should always succeed"); + if let Some(tap_scripts) = item.tap_scripts() { + let control_block = spend_info + .control_block(&leaf_script) + .expect("Control block must exist in script map for every known leaf"); + tap_scripts.insert(control_block, leaf_script); } - *item.tap_internal_key() = Some(ik_derived); - item.tap_key_origins().insert( - ik_derived, - ( - vec![], - ( - ik_xpk.master_fingerprint(), - ik_xpk - .full_derivation_path() - .ok_or(descriptor::ConversionError::MultiKey)?, - ), - ), - ); - - let mut builder = taproot::TaprootBuilder::new(); - - for (leaf_derived, leaf) in tr_derived.leaves().zip(tr_xpk.leaves()) { - debug_assert_eq!(leaf_derived.depth(), leaf.depth()); - let leaf_script = (leaf_derived.compute_script(), leaf_derived.leaf_version()); - let tapleaf_hash = TapLeafHash::from_script(&leaf_script.0, leaf_script.1); - builder = builder - .add_leaf(leaf_derived.depth(), leaf_script.0.clone()) - .expect("Computing spend data on a valid tree should always succeed"); - if let Some(tap_scripts) = item.tap_scripts() { - let control_block = spend_info - .control_block(&leaf_script) - .expect("Control block must exist in script map for every known leaf"); - tap_scripts.insert(control_block, leaf_script); - } - for (pk_pkh_derived, pk_pkh_xpk) in leaf_derived - .miniscript() - .iter_pk() - .zip(leaf.miniscript().iter_pk()) - { - let (xonly, xpk) = (pk_pkh_derived.to_x_only_pubkey(), pk_pkh_xpk); - - let xpk_full_derivation_path = xpk - .full_derivation_path() - .ok_or(descriptor::ConversionError::MultiKey)?; - item.tap_key_origins() - .entry(xonly) - .and_modify(|(tapleaf_hashes, _)| { - if tapleaf_hashes.last() != Some(&tapleaf_hash) { - tapleaf_hashes.push(tapleaf_hash); - } - }) - .or_insert_with(|| { - ( - vec![tapleaf_hash], - (xpk.master_fingerprint(), xpk_full_derivation_path), - ) - }); + for leaf_pk in leaf_derived.miniscript().iter_pk() { + let tapleaf_hashes = &mut item + .tap_key_origins() + .get_mut(&leaf_pk.to_x_only_pubkey()) + .expect("inserted all keys above") + .0; + if tapleaf_hashes.last() != Some(&tapleaf_hash) { + tapleaf_hashes.push(tapleaf_hash); } } + } - // Ensure there are no duplicated leaf hashes. This can happen if some of them were - // already present in the map when this function is called, since this only appends new - // data to the psbt without checking what's already present. - for (tapleaf_hashes, _) in item.tap_key_origins().values_mut() { - tapleaf_hashes.sort(); - tapleaf_hashes.dedup(); - } - - match item.tap_tree() { - // Only set the tap_tree if the item supports it (it's an output) and the descriptor actually - // contains one, otherwise it'll just be empty - Some(tap_tree) if tr_derived.tap_tree().is_some() => { - *tap_tree = Some( - taproot::TapTree::try_from(builder) - .expect("The tree should always be valid"), - ); - } - _ => {} - } + // Ensure there are no duplicated leaf hashes. This can happen if some of them were + // already present in the map when this function is called, since this only appends new + // data to the psbt without checking what's already present. + for (tapleaf_hashes, _) in item.tap_key_origins().values_mut() { + tapleaf_hashes.sort(); + tapleaf_hashes.dedup(); } - derived - } else { - let mut bip32_derivation = KeySourceLookUp(BTreeMap::new(), Secp256k1::verification_only()); - let derived = descriptor - .translate_pk(&mut bip32_derivation) - .map_err(|e| e.expect_translator_err("No Outer Context errors in translations"))?; - - if let Some(check_script) = check_script { - if check_script != &derived.script_pubkey() { - return Ok((derived, false)); + match item.tap_tree() { + // Only set the tap_tree if the item supports it (it's an output) and the descriptor actually + // contains one, otherwise it'll just be empty + Some(tap_tree) if tr_derived.tap_tree().is_some() => { + *tap_tree = Some( + taproot::TapTree::try_from(builder).expect("The tree should always be valid"), + ); } + _ => {} } - + } else { item.bip32_derivation().append(&mut bip32_derivation.0); match &derived { @@ -1203,8 +1171,6 @@ fn update_item_with_descriptor_helper( Descriptor::Wsh(wsh) => *item.witness_script() = Some(wsh.inner_script()), Descriptor::Tr(_) => unreachable!("Tr is dealt with separately"), } - - derived }; Ok((derived, true)) From 795cd4fc8fd2228195d1d88a62cefc343eb5b04e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 30 Mar 2025 20:57:29 +0000 Subject: [PATCH 3/7] tr: introduce new `TrSpendInfo` structure which holds a full TapTree This commit introduces a new data structure but **does not** use it. The next commit will do this. I have separated them so that this one, which introduces a bunch of algorithmic code, can be reviewed separately from the API-breaking one. When computing a `Tr` output, we need to encode all its tapleaves into Script, put these into a Merkle tree and tweak the internal key with the root of this tree. When spending from one of the branches of this output, we need the Merkle path to that output. We currently do this by using the `TaprootSpendInfo` structure from rust-bitcoin. This is not a very good fit for rust-miniscript, because it constructs a map from Tapscripts to their control blocks. This is slow and memory-wasteful to construct, and while it makes random access fairly fast, it makes sequential access pretty slow. In Miniscript we almost always want sequential access, because all of our algorithms are some form of either "try every possibility and choose the optimum" or "aggregate every possibility". It also means that if there are multiple leaves with the same script, only one copy will ever be accessible. (If they are at different depths, the low-depth one will be yielded, but if they are at the same depth it's effectively random which one will get priority.) Having multiple copies of the same script is a pointless malleability vector, but this behavior is still surprising and annoying to have to think about. To replace `bitcoin::TaprootSpendInfo` we create a new structure `TrSpendInfo`. This structure doesn't maintain any maps: it stores a full Merkleized taptree in such a way that it can efficiently yield all of the leaves' control blocks in-order. It is likely that at some point we will want to upport this, or some variant of it, into rust-bitcoin, since for typical usecases it's much faster to use and construct. --- src/descriptor/mod.rs | 5 +- src/descriptor/tr/mod.rs | 2 + src/descriptor/tr/spend_info.rs | 325 ++++++++++++++++++++++++++++++++ src/descriptor/tr/taptree.rs | 8 +- 4 files changed, 337 insertions(+), 3 deletions(-) create mode 100644 src/descriptor/tr/spend_info.rs diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 5b7822564..37486dfb4 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -42,7 +42,10 @@ 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, TapTreeDepthError, TapTreeIter, TapTreeIterItem, Tr}; +pub use self::tr::{ + TapTree, TapTreeDepthError, TapTreeIter, TapTreeIterItem, Tr, TrSpendInfo, TrSpendInfoIter, + TrSpendInfoIterItem, +}; pub mod checksum; mod key; diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index c2cadc04a..ca19c817e 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -26,8 +26,10 @@ use crate::{ Threshold, ToPublicKey, TranslateErr, Translator, }; +mod spend_info; mod taptree; +pub use self::spend_info::{TrSpendInfo, TrSpendInfoIter, TrSpendInfoIterItem}; pub use self::taptree::{TapTree, TapTreeDepthError, TapTreeIter, TapTreeIterItem}; /// A taproot descriptor diff --git a/src/descriptor/tr/spend_info.rs b/src/descriptor/tr/spend_info.rs new file mode 100644 index 000000000..f6d5375ed --- /dev/null +++ b/src/descriptor/tr/spend_info.rs @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Taproot Spending Information +//! +//! Provides a structure which can be used to obtain control blocks and other information +//! needed for Taproot spends. +//! + +use bitcoin::key::{Parity, TapTweak as _, TweakedPublicKey, UntweakedPublicKey}; +use bitcoin::secp256k1::Secp256k1; +use bitcoin::taproot::{ControlBlock, LeafVersion, TapLeafHash, TapNodeHash, TaprootMerkleBranch}; +use bitcoin::{Script, ScriptBuf}; + +use crate::miniscript::context::Tap; +use crate::prelude::Vec; +use crate::sync::Arc; +use crate::{Miniscript, MiniscriptKey, ToPublicKey}; + +/// Utility structure which maintains a stack of bits (at most 128) using a u128. +/// +/// Will panic if the user attempts to push more than 128 bits; we assume in this +/// module that we are starting with a validated [`super::TapTree`] and therefore +/// that this can't happen. +#[derive(Default)] +struct BitStack128 { + inner: u128, + height: u8, +} + +impl BitStack128 { + fn push(&mut self, bit: bool) { + if bit { + self.inner |= 1u128 << self.height; + } else { + self.inner &= !(1u128 << self.height); + } + self.height += 1; + } + + fn pop(&mut self) -> Option { + if self.height > 0 { + self.height -= 1; + Some(self.inner & (1u128 << self.height) != 0) + } else { + None + } + } +} + +/// A structure which can be used to obtain control blocks and other information +/// needed for Taproot spends. +/// +/// Conceptually, this object is a copy of the Taproot tree with each leave annotated +/// with extra information that can be used to compute its control block. +pub struct TrSpendInfo { + internal_key: UntweakedPublicKey, + output_key: TweakedPublicKey, + output_key_parity: Parity, + /// The nodes of the tree, in pre-order, i.e. left-to-right depth-first order. + nodes: Vec>, +} + +impl TrSpendInfo { + fn nodes_from_tap_tree(tree: &super::TapTree) -> Vec> { + let mut nodes = vec![]; + let mut parent_stack = Vec::with_capacity(128); // FIXME use ArrayVec here + for leaf in tree.leaves() { + let depth = usize::from(leaf.depth()); + let script = leaf.miniscript().encode(); + + let leaf_hash = TapLeafHash::from_script(&script, leaf.leaf_version()); + let mut current_hash = TapNodeHash::from(leaf_hash); + + // 1. If this node increases our depth, add parents. + while parent_stack.len() < depth { + // When we encounter a leaf we put all of its parent nodes into the + // result. We set the "sibling hash" to a dummy value (specifically, + // `current_hash`, because it's convenient and the right type). + parent_stack.push((false, nodes.len())); + nodes.push(TrSpendInfoNode { sibling_hash: current_hash, leaf_data: None }); + } + // If parent_stack.len() < depth then we pushed things onto the stack in + // the previous step so that we now have equality. Meanwhile, it is + // impossible for parent_stack.len() > depth because we pop things off + // the stack in step 3 below. + assert_eq!(depth, parent_stack.len()); + + // 2. Add the node. + // + // Again, we don't know the sibling hash yet so we use the current hash. + // But this time the current hash isn't an arbitrary dummy value -- in the + // next step we will have an invariant that incomplete nodes' "sibling hashes" + // are set to the nodes' own hashes. + // + // We will use this hash to compute the parent's hash then replace it with + // the actual sibling hash. We do this for every node EXCEPT the root node, + // whose "sibling hash" will then wind up being equal to the Merkle root + // of the whole tree. + nodes.push(TrSpendInfoNode { + sibling_hash: current_hash, + leaf_data: Some(LeafData { + script, + miniscript: Arc::clone(leaf.miniscript()), + leaf_hash, + }), + }); + + // 3. Recursively complete nodes as long as we are on a right branch. + // + // As described above, for each parent node, we compute its hash and store it + // in `sibling_hash`. At that point we're done with the childrens' hashes so + // we finally replace those with their sibling hashes. + let mut cur_index = nodes.len() - 1; + while let Some((done_left_child, parent_idx)) = parent_stack.pop() { + if done_left_child { + let lchild_hash = nodes[parent_idx + 1].sibling_hash; + // Set current node's "sibling hash" to its own hash. + let new_merkle_root = TapNodeHash::from_node_hashes(lchild_hash, current_hash); + nodes[parent_idx].sibling_hash = new_merkle_root; + // Set the children's sibling hashes to each others' hashes. + nodes[parent_idx + 1].sibling_hash = current_hash; + nodes[cur_index].sibling_hash = lchild_hash; + // Recurse. + current_hash = new_merkle_root; + cur_index = parent_idx; + } else { + // Once we hit a left branch we can't do anything until we see the next leaf. + parent_stack.push((true, parent_idx)); + break; + } + } + } + debug_assert_eq!(parent_stack.len(), 0); + debug_assert_ne!(nodes.len(), 0); + + nodes + } + + /// Constructs a [`TrSpendInfo`] for a [`super::Tr`]. + pub fn from_tr(tr: &super::Tr) -> Self { + let internal_key = tr.internal_key().to_x_only_pubkey(); + + let nodes = match tr.tap_tree() { + Some(tree) => Self::nodes_from_tap_tree(tree), + None => vec![], + }; + + let secp = Secp256k1::verification_only(); + let (output_key, output_key_parity) = + internal_key.tap_tweak(&secp, nodes.first().map(|node| node.sibling_hash)); + + TrSpendInfo { internal_key, output_key, output_key_parity, nodes } + } + + /// If this [`TrSpendInfo`] has an associated Taproot tree, return its Merkle root. + pub fn merkle_root(&self) -> Option { + // As described in `nodes_from_tap_tree`, the "sibling hash" of the root node + // is actually the Merkle root of the whole tree. + self.nodes.first().map(|node| node.sibling_hash) + } + + /// The internal key of the Taproot output. + /// + /// This returns the x-only public key which appears on-chain. For the abstroct + /// public key, use the `internal_key` method on the original [`super::Tr`] used to + /// create this object. + pub fn internal_key(&self) -> UntweakedPublicKey { self.internal_key } + + // I don't really like these names, but they're used in rust-bitcoin so we'll stick + // with them and just doc-alias them to better names so they show up in search results. + /// The external key of the Taproot output. + #[doc(alias = "external_key")] + pub fn output_key(&self) -> TweakedPublicKey { self.output_key } + + /// The parity of the external key of the Taproot output. + #[doc(alias = "external_key_parity")] + pub fn output_key_parity(&self) -> Parity { self.output_key_parity } + + /// An iterator over the leaves of the Taptree. + /// + /// This yields the same leaves in the same order as [`super::Tr::leaves`] on the original + /// [`super::Tr`]. However, in addition to yielding the leaves and their depths, it also + /// yields their scripts, leafhashes, and control blocks. + pub fn leaves(&self) -> TrSpendInfoIter { + TrSpendInfoIter { + spend_info: self, + index: 0, + merkle_stack: Vec::with_capacity(128), + done_left_stack: BitStack128::default(), + } + } +} + +/// An internal node of the spend +#[derive(Debug)] +struct TrSpendInfoNode { + sibling_hash: TapNodeHash, + leaf_data: Option>, +} + +#[derive(Debug)] +struct LeafData { + script: ScriptBuf, + miniscript: Arc>, + leaf_hash: TapLeafHash, +} + +/// An iterator over the leaves of a Taproot tree. Produced by [`TrSpendInfo::leaves`]. +/// +/// This is conceptually similar to [`super::TapTreeIter`], which can be obtained by +/// calling [`super::TapTree::leaves`]. That iterator goes over the leaves of the tree, +/// yielding the Miniscripts of the leaves and their depth. +/// +/// This iterator goes over the leaves in the same order, yielding the data that actually +/// goes on chain: their scripts, control blocks, etc. +pub struct TrSpendInfoIter<'sp, Pk: MiniscriptKey> { + spend_info: &'sp TrSpendInfo, + index: usize, + merkle_stack: Vec, + done_left_stack: BitStack128, +} + +impl<'sp, Pk: MiniscriptKey> Iterator for TrSpendInfoIter<'sp, Pk> { + type Item = TrSpendInfoIterItem<'sp, Pk>; + + fn next(&mut self) -> Option { + while self.index < self.spend_info.nodes.len() { + let current_node = &self.spend_info.nodes[self.index]; + if self.index > 0 { + self.merkle_stack.push(current_node.sibling_hash); + } + self.index += 1; + + if let Some(ref leaf) = current_node.leaf_data { + // leaf + let mut merkle_stack = self.merkle_stack.clone(); + merkle_stack.reverse(); + self.merkle_stack.pop(); + + loop { + match self.done_left_stack.pop() { + None => break, // this leaf is the root node + Some(false) => { + self.done_left_stack.push(true); + break; + } + Some(true) => { + self.merkle_stack.pop(); + } + } + } + + return Some(TrSpendInfoIterItem { + script: &leaf.script, + miniscript: &leaf.miniscript, + leaf_hash: leaf.leaf_hash, + control_block: ControlBlock { + leaf_version: LeafVersion::TapScript, + output_key_parity: self.spend_info.output_key_parity, + internal_key: self.spend_info.internal_key, + merkle_branch: TaprootMerkleBranch::try_from(merkle_stack) + .expect("merkle stack guaranteed to be within allowable length"), + }, + }); + } else { + // internal node + self.done_left_stack.push(false); + } + } + None + } +} + +/// Item yielded from a [`TrSpendInfoIter`]. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct TrSpendInfoIterItem<'tr, Pk: MiniscriptKey> { + script: &'tr Script, + miniscript: &'tr Arc>, + leaf_hash: TapLeafHash, + control_block: ControlBlock, +} + +impl<'sp, Pk: MiniscriptKey> TrSpendInfoIterItem<'sp, Pk> { + /// The Tapscript of this leaf. + #[inline] + pub fn script(&self) -> &'sp Script { self.script } + + /// The Tapscript of this leaf, in Miniscript form. + #[inline] + pub fn miniscript(&self) -> &'sp Arc> { self.miniscript } + + /// The depth of the leaf in the tree. + /// + /// This value is returned as `u8` since it is guaranteed to be <= 128 by the Taproot + /// consensus rules. + #[inline] + pub fn depth(&self) -> u8 { + self.control_block.merkle_branch.len() as u8 // cast ok, length limited to 128 + } + + /// The Tapleaf version of this leaf. + /// + /// This function returns a constant value, since there is only one version in use + /// on the Bitcoin network; however, it may be useful to use this method in case + /// you wish to be forward-compatible with future versions supported by this + /// library. + #[inline] + pub fn leaf_version(&self) -> LeafVersion { self.control_block.leaf_version } + + /// The hash of this leaf. + /// + /// This hash, prefixed with the leaf's [`Self::leaf_version`], is what is directly + /// committed in the Taproot tree. + #[inline] + pub fn leaf_hash(&self) -> TapLeafHash { self.leaf_hash } + + /// The control block of this leaf. + /// + /// Unlike the other data obtainable from [`TrSpendInfoIterItem`], this one is computed + /// dynamically during iteration and therefore will not outlive the iterator item. If + /// you need access to multiple control blocks at once, will likely need to clone and + /// store them separately. + #[inline] + pub fn control_block(&self) -> &ControlBlock { &self.control_block } +} diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index 431c1ccab..35f23a95e 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -212,14 +212,18 @@ impl<'tr, Pk: MiniscriptKey> TapTreeIterItem<'tr, Pk> { impl TapTreeIterItem<'_, Pk> { /// Computes the Bitcoin Script of the leaf. /// - /// This function is potentially expensive. + /// This function is potentially expensive. If you are calling this method on + /// all (or many) of the leaves of the tree, you may instead want to call + /// [`super::Tr::spend_info`] and use the [`super::TrSpendInfo::leaves`] iterator instead. #[inline] pub fn compute_script(&self) -> bitcoin::ScriptBuf { self.node.encode() } /// Computes the [`TapLeafHash`] of the leaf. /// /// This function is potentially expensive, since it serializes the full Bitcoin - /// Script of the leaf and hashes this data. + /// Script of the leaf and hashes this data. If you are calling this method on + /// all (or many) of the leaves of the tree, you may instead want to call + /// [`super::Tr::spend_info`] and use the [`super::TrSpendInfo::leaves`] iterator instead. #[inline] pub fn compute_tap_leaf_hash(&self) -> TapLeafHash { TapLeafHash::from_script(&self.compute_script(), self.leaf_version()) From 5fe2d25decd8b31f72692c5750a9d59c760c4c1c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 31 Mar 2025 00:28:25 +0000 Subject: [PATCH 4/7] tr: replace `bitcoin::TaprootSpendInfo` with `TrSpendInfo`, update psbt See previous commit for details about this data structure. This commit stops using the rust-bitcoin `TaprootSpendInfo` in favor of our new `TrSpendInfo` structure. This one allows us to iterate over all the leaves of the Taptree, easily accessing their leaf hashes and control blocks in order, which simplifies satisfaction logic. --- bitcoind-tests/tests/test_desc.rs | 6 +-- src/descriptor/tr/mod.rs | 64 ++++++++----------------------- src/psbt/mod.rs | 10 ++--- 3 files changed, 23 insertions(+), 57 deletions(-) diff --git a/bitcoind-tests/tests/test_desc.rs b/bitcoind-tests/tests/test_desc.rs index 30c54f169..083ae44ec 100644 --- a/bitcoind-tests/tests/test_desc.rs +++ b/bitcoind-tests/tests/test_desc.rs @@ -10,6 +10,7 @@ use std::{error, fmt}; use actual_rand as rand; use bitcoin::blockdata::witness::Witness; use bitcoin::hashes::{sha256d, Hash}; +use bitcoin::key::TapTweak as _; use bitcoin::psbt::Psbt; use bitcoin::sighash::SighashCache; use bitcoin::taproot::{LeafVersion, TapLeafHash}; @@ -168,8 +169,7 @@ pub fn test_desc_satisfy( if let Some(internal_keypair) = internal_keypair { // ---------------------- Tr key spend -------------------- let internal_keypair = internal_keypair - .add_xonly_tweak(&secp, &tr.spend_info().tap_tweak().to_scalar()) - .expect("Tweaking failed"); + .tap_tweak(&secp, tr.spend_info().merkle_root()); let sighash_msg = sighash_cache .taproot_key_spend_signature_hash(0, &prevouts, sighash_type) .unwrap(); @@ -177,7 +177,7 @@ pub fn test_desc_satisfy( let mut aux_rand = [0u8; 32]; rand::thread_rng().fill_bytes(&mut aux_rand); let schnorr_sig = - secp.sign_schnorr_with_aux_rand(&msg, &internal_keypair, &aux_rand); + secp.sign_schnorr_with_aux_rand(&msg, &internal_keypair.to_inner(), &aux_rand); psbt.inputs[0].tap_key_sig = Some(taproot::Signature { signature: schnorr_sig, sighash_type }); } else { diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index ca19c817e..c7e3d7f7f 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -2,12 +2,7 @@ use core::{cmp, fmt, hash}; -#[cfg(not(test))] // https://github.com/rust-lang/rust/issues/121684 -use bitcoin::secp256k1; -use bitcoin::taproot::{ - LeafVersion, TaprootBuilder, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE, - TAPROOT_CONTROL_NODE_SIZE, -}; +use bitcoin::taproot::{TAPROOT_CONTROL_BASE_SIZE, TAPROOT_CONTROL_NODE_SIZE}; use bitcoin::{opcodes, Address, Network, ScriptBuf, Weight}; use sync::Arc; @@ -45,7 +40,7 @@ pub struct Tr { // The inner `Arc` here is because Rust does not allow us to return a reference // to the contents of the `Option` from inside a `MutexGuard`. There is no outer // `Arc` because when this structure is cloned, we create a whole new mutex. - spend_info: Mutex>>, + spend_info: Mutex>>>, } impl Clone for Tr { @@ -120,46 +115,21 @@ impl Tr { } } - /// Compute the [`TaprootSpendInfo`] associated with this descriptor if spend data is `None`. + /// Obtain the spending information for this [`Tr`]. /// - /// If spend data is already computed (i.e it is not `None`), this does not recompute it. + /// The first time this method is called, it computes the full Taproot Merkle tree of + /// all branches as well as the output key which appears on-chain. This is fairly + /// expensive since it requires hashing every branch and then doing an elliptic curve + /// operation. The result is cached and reused on subsequent calls. /// - /// [`TaprootSpendInfo`] is only required for spending via the script paths. - pub fn spend_info(&self) -> Arc + /// This data is needed to compute the Taproot output, so this method is implicitly + /// called through [`Self::script_pubkey`], [`Self::address`], etc. It is also needed + /// to compute the hash needed to sign the output. + pub fn spend_info(&self) -> TrSpendInfo where Pk: ToPublicKey, { - // If the value is already cache, read it - // read only panics if the lock is poisoned (meaning other thread having a lock panicked) - let read_lock = self.spend_info.lock().expect("Lock poisoned"); - if let Some(ref spend_info) = *read_lock { - return Arc::clone(spend_info); - } - drop(read_lock); - - // Get a new secp context - // This would be cheap operation after static context support from upstream - let secp = secp256k1::Secp256k1::verification_only(); - // Key spend path with no merkle root - let data = if self.tree.is_none() { - TaprootSpendInfo::new_key_spend(&secp, self.internal_key.to_x_only_pubkey(), None) - } else { - let mut builder = TaprootBuilder::new(); - for leaf in self.leaves() { - let script = leaf.miniscript().encode(); - builder = builder - .add_leaf(leaf.depth(), script) - .expect("Computing spend data on a valid Tree should always succeed"); - } - // Assert builder cannot error here because we have a well formed descriptor - match builder.finalize(&secp, self.internal_key.to_x_only_pubkey()) { - Ok(data) => data, - Err(_) => unreachable!("We know the builder can be finalized"), - } - }; - let spend_info = Arc::new(data); - *self.spend_info.lock().expect("Lock poisoned") = Some(Arc::clone(&spend_info)); - spend_info + TrSpendInfo::from_tr(self) } /// Checks whether the descriptor is safe. @@ -508,7 +478,7 @@ where absolute_timelock: None, }; let mut min_wit_len = None; - for leaf in desc.leaves() { + for leaf in spend_info.leaves() { let mut satisfaction = if allow_mall { match leaf.miniscript().build_template(provider) { s @ Satisfaction { stack: Witness::Stack(_), .. } => s, @@ -525,12 +495,10 @@ where _ => unreachable!(), }; - let leaf_script = (leaf.compute_script(), LeafVersion::TapScript); - let control_block = spend_info - .control_block(&leaf_script) - .expect("Control block must exist in script map for every known leaf"); + let script = ScriptBuf::from(leaf.script()); + let control_block = leaf.control_block().clone(); - wit.push(Placeholder::TapScript(leaf_script.0)); + wit.push(Placeholder::TapScript(script)); wit.push(Placeholder::TapControlBlock(control_block)); let wit_size = witness_size(wit); diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 5bb96fda8..ef5ae1314 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -1110,16 +1110,14 @@ fn update_item_with_descriptor_helper( } let mut builder = taproot::TaprootBuilder::new(); - for leaf_derived in tr_derived.leaves() { - let leaf_script = (leaf_derived.compute_script(), leaf_derived.leaf_version()); - let tapleaf_hash = TapLeafHash::from_script(&leaf_script.0, leaf_script.1); + for leaf_derived in spend_info.leaves() { + let leaf_script = (ScriptBuf::from(leaf_derived.script()), leaf_derived.leaf_version()); + let tapleaf_hash = leaf_derived.leaf_hash(); builder = builder .add_leaf(leaf_derived.depth(), leaf_script.0.clone()) .expect("Computing spend data on a valid tree should always succeed"); if let Some(tap_scripts) = item.tap_scripts() { - let control_block = spend_info - .control_block(&leaf_script) - .expect("Control block must exist in script map for every known leaf"); + let control_block = leaf_derived.control_block().clone(); tap_scripts.insert(control_block, leaf_script); } From 8bc940018b11ff3de18657cc8040f892a0ced652 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 31 Mar 2025 15:06:13 +0000 Subject: [PATCH 5/7] tr: add conversion from TrSpendInfo to bitcoin::TapTree Moves a bit of ugly logic out of the PSBT module into the spendinfo module so that it's available for other users. We can convert from a TrSpendInfo to a bitcoin::TapTree but we can't do the opposite conversion since TrSpendInfo expects to have a Miniscript for each leaf. --- src/descriptor/tr/spend_info.rs | 22 ++++++++++++++++++++++ src/psbt/mod.rs | 17 ++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/descriptor/tr/spend_info.rs b/src/descriptor/tr/spend_info.rs index f6d5375ed..574466692 100644 --- a/src/descriptor/tr/spend_info.rs +++ b/src/descriptor/tr/spend_info.rs @@ -189,6 +189,28 @@ impl TrSpendInfo { done_left_stack: BitStack128::default(), } } + + /// If the Taproot tree is not keyspend-only, converts it to a [`bitcoin::taproot::TapTree`] structure. + /// + /// This conversion is not particularly efficient but the resulting data structure is + /// useful for interacting with PSBTs. + pub fn to_tap_tree(&self) -> Option { + if self.nodes.is_empty() { + return None; + } + + let mut builder = bitcoin::taproot::TaprootBuilder::new(); + for leaf in self.leaves() { + builder = builder + .add_leaf_with_ver( + leaf.depth(), + ScriptBuf::from(leaf.script()), + leaf.leaf_version(), + ) + .expect("iterating through tree in correct DFS order") + } + Some(bitcoin::taproot::TapTree::try_from(builder).expect("tree is complete")) + } } /// An internal node of the spend diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index ef5ae1314..06c924c92 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -1109,13 +1109,9 @@ fn update_item_with_descriptor_helper( *merkle_root = spend_info.merkle_root(); } - let mut builder = taproot::TaprootBuilder::new(); for leaf_derived in spend_info.leaves() { let leaf_script = (ScriptBuf::from(leaf_derived.script()), leaf_derived.leaf_version()); let tapleaf_hash = leaf_derived.leaf_hash(); - builder = builder - .add_leaf(leaf_derived.depth(), leaf_script.0.clone()) - .expect("Computing spend data on a valid tree should always succeed"); if let Some(tap_scripts) = item.tap_scripts() { let control_block = leaf_derived.control_block().clone(); tap_scripts.insert(control_block, leaf_script); @@ -1141,15 +1137,10 @@ fn update_item_with_descriptor_helper( tapleaf_hashes.dedup(); } - match item.tap_tree() { - // Only set the tap_tree if the item supports it (it's an output) and the descriptor actually - // contains one, otherwise it'll just be empty - Some(tap_tree) if tr_derived.tap_tree().is_some() => { - *tap_tree = Some( - taproot::TapTree::try_from(builder).expect("The tree should always be valid"), - ); - } - _ => {} + // Only set the tap_tree if the item supports it (it's an output) and the descriptor actually + // contains one, otherwise it'll just be empty + if let Some(tap_tree) = item.tap_tree() { + *tap_tree = spend_info.to_tap_tree(); } } else { item.bip32_derivation().append(&mut bip32_derivation.0); From c1d4305db39cf38be823c1e44fcd9580fba15cd9 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 31 Mar 2025 15:33:42 +0000 Subject: [PATCH 6/7] tr: add a whole bunch of fixed vector unit tests for TrSpendInfo --- src/descriptor/tr/spend_info.rs | 321 ++++++++++++++++++++++++++++++++ 1 file changed, 321 insertions(+) diff --git a/src/descriptor/tr/spend_info.rs b/src/descriptor/tr/spend_info.rs index 574466692..b9f1d09c9 100644 --- a/src/descriptor/tr/spend_info.rs +++ b/src/descriptor/tr/spend_info.rs @@ -345,3 +345,324 @@ impl<'sp, Pk: MiniscriptKey> TrSpendInfoIterItem<'sp, Pk> { #[inline] pub fn control_block(&self) -> &ControlBlock { &self.control_block } } + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(PartialEq, Eq, Debug)] + struct ExpectedTree { + internal_key: UntweakedPublicKey, + output_key: TweakedPublicKey, + output_key_parity: Parity, + merkle_root: Option, + } + + #[derive(PartialEq, Eq, Debug)] + struct ExpectedLeaf { + leaf_hash: TapLeafHash, + branch: TaprootMerkleBranch, + } + + fn test_cases() -> Vec<(String, ExpectedTree, Vec)> { + let secp = Secp256k1::verification_only(); + let pk = "03cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115" + .parse::() + .unwrap(); + + // Hash of the FALSE script + let zero_hash = "e7e4d593fcb72926eedbe0d1e311f41acd6f6ef161dcba081a75168ec4dcd379" + .parse::() + .unwrap(); + // Hash of the TRUE script + let one_hash = "a85b2107f791b26a84e7586c28cec7cb61202ed3d01944d832500f363782d675" + .parse::() + .unwrap(); + + let mut ret = vec![]; + + // Empty tree + let merkle_root = None; + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + ret.push(( + format!("tr({pk})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![], + )); + + // Single-leaf tree + let merkle_root = Some(TapNodeHash::from(zero_hash)); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + ret.push(( + format!("tr({pk},0)"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![]).unwrap(), + }], + )); + + // Two-leaf tree, repeated leaf + let merkle_root = Some( + "e3208df58f4fae78044357451c8830698300cd7da47cf41957d82ac4ce1dd170" + .parse() + .unwrap(), + ); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + ret.push(( + format!("tr({pk},{{0,0}})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![TapNodeHash::from(zero_hash)]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![TapNodeHash::from(zero_hash)]) + .unwrap(), + }, + ], + )); + + // Two-leaf tree, non-repeated leaf + let merkle_root = Some( + "15526cd6108b4765640abe555e75f4bd11d9b1453b9db4cd36cf4189577a6f63" + .parse() + .unwrap(), + ); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + ret.push(( + format!("tr({pk},{{0,1}})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![TapNodeHash::from(one_hash)]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: one_hash, + branch: TaprootMerkleBranch::try_from(vec![TapNodeHash::from(zero_hash)]) + .unwrap(), + }, + ], + )); + + // Fuzz test vector 1 + let merkle_root = Some( + "d281962c67932b82e19b0da5ea437af316213e24509be0ef1bd7c5ee2b460d79" + .parse() + .unwrap(), + ); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + + ret.push(( + format!("tr({pk},{{0,{{0,tv:0}}}})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "573d619569d58a36b52187e56f168650ac17f66a9a3afaf054900a04001019b3" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "64ac241466a5e7032586718ff7465716f77a88d89946ce472daa4c3d0b81148f" + .parse::() + .unwrap(), + TapNodeHash::from(zero_hash), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: "64ac241466a5e7032586718ff7465716f77a88d89946ce472daa4c3d0b81148f" + .parse() + .unwrap(), + branch: TaprootMerkleBranch::try_from(vec![ + TapNodeHash::from(zero_hash), + TapNodeHash::from(zero_hash), + ]) + .unwrap(), + }, + ], + )); + + // Fuzz test vector 2 + let merkle_root = Some( + "2534e94c6ad06281b61fff86bad38a3911fb13436fb27fed6f5c057e4a71a911" + .parse() + .unwrap(), + ); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + + ret.push(( + format!("tr({pk},{{uuu:0,{{0,uu:0}}}})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ + ExpectedLeaf { + leaf_hash: "6498e1d56640a272493d1d87549f3347dc448ca674556a2110cdfe100e3c238b" + .parse() + .unwrap(), + branch: TaprootMerkleBranch::try_from(vec![ + "7e3e98bab404812c8eebd21c5d825527676b8e9f261f7ad479f3a08a83a43fb4" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "19417c32bc6ca7e0f6e65b006ac305107c6add73c8bef31181037e6faaa55e7f" + .parse::() + .unwrap(), + "6498e1d56640a272493d1d87549f3347dc448ca674556a2110cdfe100e3c238b" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: "19417c32bc6ca7e0f6e65b006ac305107c6add73c8bef31181037e6faaa55e7f" + .parse() + .unwrap(), + branch: TaprootMerkleBranch::try_from(vec![ + TapNodeHash::from(zero_hash), + "6498e1d56640a272493d1d87549f3347dc448ca674556a2110cdfe100e3c238b" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ], + )); + + // Fuzz test vector 3 + let merkle_root = Some( + "9f4bc03c65a88ffbbb3a8d4fe5e01be608109d9f875f35685d8865e181def26e" + .parse() + .unwrap(), + ); + let internal_key = pk.to_x_only_pubkey(); + let (output_key, output_key_parity) = internal_key.tap_tweak(&secp, merkle_root); + + ret.push(( + format!("tr({pk},{{{{0,{{uuu:0,0}}}},{{0,uu:0}}}})"), + ExpectedTree { internal_key, output_key, output_key_parity, merkle_root }, + vec![ + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "57e3b7d414075ff4864deec9efa99db4462c038706306e02c58e02e957c8a51e" + .parse::() + .unwrap(), + "7e3e98bab404812c8eebd21c5d825527676b8e9f261f7ad479f3a08a83a43fb4" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: "6498e1d56640a272493d1d87549f3347dc448ca674556a2110cdfe100e3c238b" + .parse() + .unwrap(), + branch: TaprootMerkleBranch::try_from(vec![ + TapNodeHash::from(zero_hash), + TapNodeHash::from(zero_hash), + "7e3e98bab404812c8eebd21c5d825527676b8e9f261f7ad479f3a08a83a43fb4" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "6498e1d56640a272493d1d87549f3347dc448ca674556a2110cdfe100e3c238b" + .parse::() + .unwrap(), + TapNodeHash::from(zero_hash), + "7e3e98bab404812c8eebd21c5d825527676b8e9f261f7ad479f3a08a83a43fb4" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: zero_hash, + branch: TaprootMerkleBranch::try_from(vec![ + "19417c32bc6ca7e0f6e65b006ac305107c6add73c8bef31181037e6faaa55e7f" + .parse::() + .unwrap(), + "e034d7d8b221034861bf3893c63cb0ff60d28a7a00090d0dc57c26fec91983cb" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ExpectedLeaf { + leaf_hash: "19417c32bc6ca7e0f6e65b006ac305107c6add73c8bef31181037e6faaa55e7f" + .parse() + .unwrap(), + branch: TaprootMerkleBranch::try_from(vec![ + TapNodeHash::from(zero_hash), + "e034d7d8b221034861bf3893c63cb0ff60d28a7a00090d0dc57c26fec91983cb" + .parse::() + .unwrap(), + ]) + .unwrap(), + }, + ], + )); + + ret + } + + #[test] + fn spend_info_fixed_vectors() { + for (s, tree, leaves) in test_cases() { + let tr = s + .parse::>() + .unwrap(); + let spend_info = tr.spend_info(); + + assert_eq!( + spend_info.internal_key(), + tree.internal_key, + "internal key mismatch (left: computed, right: expected)", + ); + assert_eq!( + spend_info.merkle_root(), + tree.merkle_root, + "merkle root mismatch (left: computed, right: expected)", + ); + assert_eq!( + spend_info.output_key(), + tree.output_key, + "output key mismatch (left: computed, right: expected)", + ); + + let got_leaves: Vec<_> = spend_info + .leaves() + .map(|leaf| ExpectedLeaf { + leaf_hash: leaf.leaf_hash(), + branch: leaf.control_block().merkle_branch.clone(), + }) + .collect(); + assert_eq!(got_leaves, leaves, "leaves mismatch (left: computed, right: expected)",); + } + } +} From 3d12b42a6527bce318ac618d25dd25e2d2946470 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 31 Mar 2025 12:33:27 +0000 Subject: [PATCH 7/7] fuzz: update taptree regression test to also check control blocks This is a bit of a pain because the old lookup-by-script behavior doesn't map cleanly onto the new iterate-through-everything behavior. But this test is definitely useful (the unit tests in the previous commit came from it.) --- fuzz/fuzz_targets/regression_taptree.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/fuzz/fuzz_targets/regression_taptree.rs b/fuzz/fuzz_targets/regression_taptree.rs index 296b68765..ab50d06bd 100644 --- a/fuzz/fuzz_targets/regression_taptree.rs +++ b/fuzz/fuzz_targets/regression_taptree.rs @@ -1,3 +1,5 @@ +use std::collections::{HashMap, HashSet}; + use descriptor_fuzz::FuzzPk; use honggfuzz::fuzz; use miniscript::descriptor::Tr; @@ -27,6 +29,22 @@ fn do_test(data: &[u8]) { new_si.output_key(), "merkle root mismatch (left is old, new is right)", ); + + // Map every leaf script to a set of all the control blocks + let mut new_cbs = HashMap::new(); + for leaf in new_si.leaves() { + new_cbs + .entry(leaf.script()) + .or_insert(HashSet::new()) + .insert(leaf.control_block().clone()); + } + // ...the old code will only ever yield one of them and it's not easy to predict which one + for leaf in new_si.leaves() { + let old_cb = old_si + .control_block(&(leaf.script().into(), leaf.leaf_version())) + .unwrap(); + assert!(new_cbs[leaf.script()].contains(&old_cb)); + } } } }