From 43f214b674832fa6ea8e5abc12fd5c599260eb21 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Tue, 28 Jun 2022 11:37:00 +0200 Subject: [PATCH 1/5] Switch to taproot leaf_scripts iterator --- scripts/src/taproot.rs | 120 ++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 68 deletions(-) diff --git a/scripts/src/taproot.rs b/scripts/src/taproot.rs index 2fbb8f8..946996b 100644 --- a/scripts/src/taproot.rs +++ b/scripts/src/taproot.rs @@ -27,7 +27,6 @@ use bitcoin::hashes::Hash; use bitcoin::psbt::TapTree; use bitcoin::util::taproot::{LeafVersion, TapBranchHash, TapLeafHash, TaprootBuilder}; use bitcoin::Script; -use secp256k1::{KeyPair, SECP256K1}; use strict_encoding::{StrictDecode, StrictEncode}; use crate::types::IntoNodeHash; @@ -1233,65 +1232,60 @@ impl TaprootScriptTree { impl From for TaprootScriptTree { fn from(tree: TapTree) -> Self { - // TODO: Do via iterator once #922 will be merged - let dumb_key = KeyPair::from_secret_key(SECP256K1, secp256k1::ONE_KEY).public_key(); - let spent_info = tree - .into_builder() - .finalize(SECP256K1, dumb_key) - .expect("non-final taptree"); - let mut root: Option = None; - for ((script, leaf_version), map) in spent_info.as_script_map() { - for merkle_branch in map { - let merkle_branch = merkle_branch.as_inner(); - let leaf_depth = merkle_branch.len() as u8; - - let mut curr_hash = - TapLeafHash::from_script(script, *leaf_version).into_node_hash(); - let merkle_branch = merkle_branch - .iter() - .map(|step| { - curr_hash = - TapBranchHash::from_node_hashes(*step, curr_hash).into_node_hash(); - curr_hash - }) - .collect::>(); - let mut hash_iter = merkle_branch.iter().rev(); - - match (root.is_some(), hash_iter.next()) { - (false, None) => { - root = Some(PartialTreeNode::with_leaf(*leaf_version, script.clone(), 0)) - } - (false, Some(hash)) => { - root = Some(PartialTreeNode::with_branch( - TapBranchHash::from_inner(hash.into_inner()), - 0, - )) - } - (true, None) => unreachable!("broken TapTree structure"), - (true, Some(_)) => {} + for leaf in tree.script_leaves() { + let merkle_branch = leaf.merkle_branch().as_inner(); + let leaf_depth = merkle_branch.len() as u8; + + let mut curr_hash = + TapLeafHash::from_script(leaf.script(), leaf.leaf_version()).into_node_hash(); + let merkle_branch = merkle_branch + .iter() + .map(|step| { + curr_hash = TapBranchHash::from_node_hashes(*step, curr_hash).into_node_hash(); + curr_hash + }) + .collect::>(); + let mut hash_iter = merkle_branch.iter().rev(); + + match (root.is_some(), hash_iter.next()) { + (false, None) => { + root = Some(PartialTreeNode::with_leaf( + leaf.leaf_version(), + leaf.script().clone(), + 0, + )) } - let mut node = root.as_mut().expect("unreachable"); - for (depth, hash) in hash_iter.enumerate() { - match node { - PartialTreeNode::Leaf(..) => unreachable!("broken TapTree structure"), - PartialTreeNode::Branch(branch, _) => { - let child = PartialTreeNode::with_branch( - TapBranchHash::from_inner(hash.into_inner()), - depth as u8 + 1, - ); - node = branch.push_child(child).expect("broken TapTree structure"); - } - } + (false, Some(hash)) => { + root = Some(PartialTreeNode::with_branch( + TapBranchHash::from_inner(hash.into_inner()), + 0, + )) } - let leaf = PartialTreeNode::with_leaf(*leaf_version, script.clone(), leaf_depth); + (true, None) => unreachable!("broken TapTree structure"), + (true, Some(_)) => {} + } + let mut node = root.as_mut().expect("unreachable"); + for (depth, hash) in hash_iter.enumerate() { match node { - PartialTreeNode::Leaf(..) => { /* nothing to do here */ } + PartialTreeNode::Leaf(..) => unreachable!("broken TapTree structure"), PartialTreeNode::Branch(branch, _) => { - branch.push_child(leaf); + let child = PartialTreeNode::with_branch( + TapBranchHash::from_inner(hash.into_inner()), + depth as u8 + 1, + ); + node = branch.push_child(child).expect("broken TapTree structure"); } } } + let leaf = + PartialTreeNode::with_leaf(leaf.leaf_version(), leaf.script().clone(), leaf_depth); + match node { + PartialTreeNode::Leaf(..) => { /* nothing to do here */ } + PartialTreeNode::Branch(branch, _) => { + branch.push_child(leaf); + } + } } let root = root @@ -1509,9 +1503,7 @@ mod test { use bitcoin::blockdata::opcodes::all; use bitcoin::hashes::hex::FromHex; - use bitcoin::schnorr::UntweakedPublicKey; use bitcoin::util::taproot::TaprootBuilder; - use secp256k1::ONE_KEY; use super::*; @@ -1534,21 +1526,13 @@ mod test { let taptree = compose_tree(opcode, depth_map); let script_tree = TaprootScriptTree::from(taptree.clone()); - let dumb = KeyPair::from_secret_key(SECP256K1, ONE_KEY); - - let map = taptree - .clone() - .into_builder() - .finalize(SECP256K1, UntweakedPublicKey::from_keypair(&dumb)) - .unwrap(); - let scripts = map - .as_script_map() - .iter() - .map(|((script, version), path)| { + let scripts = taptree + .script_leaves() + .map(|leaf| { ( - path.iter().next().unwrap().as_inner().len() as u8, - *version, - script, + leaf.merkle_branch().as_inner().len() as u8, + leaf.leaf_version(), + leaf.script(), ) }) .collect::>(); From 9fd13c283149a527b363a18f11351946b3169686 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Tue, 28 Jun 2022 11:37:09 +0200 Subject: [PATCH 2/5] More tests for taptree operations --- scripts/src/taproot.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/scripts/src/taproot.rs b/scripts/src/taproot.rs index 946996b..0aa26ce 100644 --- a/scripts/src/taproot.rs +++ b/scripts/src/taproot.rs @@ -1501,6 +1501,7 @@ impl From for TapTree { mod test { use std::collections::BTreeSet; + use amplify::Wrapper; use bitcoin::blockdata::opcodes::all; use bitcoin::hashes::hex::FromHex; use bitcoin::util::taproot::TaprootBuilder; @@ -1732,7 +1733,7 @@ mod test { } #[test] - fn instll_path_proof() { + fn instill_path_proof() { let path = DfsPath::from_str("00101").unwrap(); let taptree = compose_tree(0x51, [3, 5, 5, 4, 3, 3, 2, 3, 4, 5, 6, 8, 8, 7]); @@ -1788,4 +1789,42 @@ mod test { PartnerNode::Script(s!("Script(OP_PUSHNUM_3)")), ]); } + + #[test] + fn tapscripttree_roudtrip() { + let taptree = compose_tree(0x51, [3, 5, 5, 4, 3, 3, 2, 3, 4, 5, 6, 8, 8, 7]); + let script_tree = TaprootScriptTree::from(taptree.clone()); + let taptree_roundtrip = TapTree::from(script_tree); + assert_eq!(taptree, taptree_roundtrip); + } + + #[test] + fn tapscripttree_taptree_eq() { + let taptree = compose_tree(0x51, [3, 5, 5, 4, 3, 3, 2, 3, 4, 5, 6, 8, 8, 7]); + let script_tree = TaprootScriptTree::from(taptree.clone()); + assert!(script_tree.check().is_ok()); + for (leaf, (_, leaf_script)) in taptree.script_leaves().zip(script_tree.scripts()) { + assert_eq!(leaf.script(), leaf_script.script.as_inner()); + } + } + + #[test] + fn tapscripttree_dfs() { + let depth_map = [3, 5, 5, 4, 3, 3, 2, 3, 4, 5, 6, 8, 8, 7]; + let mut val = 0x51; + + let taptree = compose_tree(val, depth_map); + let script_tree = TaprootScriptTree::from(taptree.clone()); + assert!(script_tree.check().is_ok()); + + for (depth, leaf_script) in script_tree.scripts() { + let script = Script::from_hex(&format!("{:02x}", val)).unwrap(); + + assert_eq!(depth, depth_map[(val - 0x51) as usize]); + assert_eq!(script, leaf_script.script.to_inner()); + + let (new_val, _) = val.overflowing_add(1); + val = new_val; + } + } } From af348ed1dd44b1c9024df0ac60aba625c1631083 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Thu, 30 Jun 2022 18:45:34 +0200 Subject: [PATCH 3/5] Add docs on tap tree iterator ordering --- scripts/src/taproot.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/src/taproot.rs b/scripts/src/taproot.rs index 0aa26ce..c166f6b 100644 --- a/scripts/src/taproot.rs +++ b/scripts/src/taproot.rs @@ -980,15 +980,16 @@ impl TaprootScriptTree { /// Returns iterator over known scripts stored in the tree. /// - /// NB: the iterator ignores scripts behind hidden nodes. + /// NB: the iterator ignores scripts behind hidden nodes. It iterates the + /// scripts in DFS (and not consensus) order. #[inline] pub fn scripts(&self) -> TreeScriptIter { TreeScriptIter::from(self) } - /// Returns iterator over all known nodes of the tree. + /// Returns iterator over all known nodes of the tree in DFS order. #[inline] pub fn nodes(&self) -> TreeNodeIter { TreeNodeIter::from(self) } - /// Returns mutable iterator over all known nodes of the tree. + /// Returns mutable iterator over all known nodes of the tree in DFS order. #[inline] pub(self) fn nodes_mut(&mut self) -> TreeNodeIterMut { TreeNodeIterMut::from(self) } From 81a494c1d74ecb7eb538c0b056ad5521c8caf1e0 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Thu, 30 Jun 2022 18:45:59 +0200 Subject: [PATCH 4/5] Bugfix reversed order in rust-bitcoin taproot iterator See https://github.com/rust-bitcoin/rust-bitcoin/issues/1069 --- scripts/src/taproot.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/src/taproot.rs b/scripts/src/taproot.rs index c166f6b..ba27858 100644 --- a/scripts/src/taproot.rs +++ b/scripts/src/taproot.rs @@ -1234,7 +1234,10 @@ impl TaprootScriptTree { impl From for TaprootScriptTree { fn from(tree: TapTree) -> Self { let mut root: Option = None; - for leaf in tree.script_leaves() { + // TODO: This is a bugfix, which should be reversed once is fixed upstream + let mut script_leaves = tree.script_leaves().collect::>(); + script_leaves.reverse(); + for leaf in script_leaves { let merkle_branch = leaf.merkle_branch().as_inner(); let leaf_depth = merkle_branch.len() as u8; @@ -1421,6 +1424,8 @@ enum BranchDirection { /// Iterator over leaf scripts stored in the leaf nodes of the taproot script /// tree. +/// +/// NB: The scripts are iterated in the DFS order (not consensus). pub struct TreeScriptIter<'tree> { // Here we store vec of path elements, where each element is a tuple, consisting of: // 1. Tree node on the path @@ -1804,7 +1809,12 @@ mod test { let taptree = compose_tree(0x51, [3, 5, 5, 4, 3, 3, 2, 3, 4, 5, 6, 8, 8, 7]); let script_tree = TaprootScriptTree::from(taptree.clone()); assert!(script_tree.check().is_ok()); - for (leaf, (_, leaf_script)) in taptree.script_leaves().zip(script_tree.scripts()) { + + // TODO: This is a bugfix, which should be reversed once is fixed upstream + let mut script_leaves = taptree.script_leaves().collect::>(); + script_leaves.reverse(); + + for (leaf, (_, leaf_script)) in script_leaves.iter().zip(script_tree.scripts()) { assert_eq!(leaf.script(), leaf_script.script.as_inner()); } } From d53639f98e928152f70f4bf268da08bf938a3dbe Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Thu, 30 Jun 2022 18:49:47 +0200 Subject: [PATCH 5/5] Pin rust-bitcoin version --- scripts/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Cargo.toml b/scripts/Cargo.toml index 0a33ff3..07cfd11 100644 --- a/scripts/Cargo.toml +++ b/scripts/Cargo.toml @@ -15,7 +15,7 @@ exclude = [] [dependencies] amplify = "3.12.1" -bitcoin = "0.28.1" +bitcoin = "=0.28.1" # We pin this version due to an upstream bug. See setails: secp256k1 = { version = "0.22.1", features = ["global-context"] } # Remove after bitcoin #922 miniscript = { version = "7.0.0", optional = true } strict_encoding = "0.8.0"