milksad
/
rust-bitcoin-unsafe-fast
14
0
Fork 0
Code Issues Pull Requests Activity

Merge rust-bitcoin/rust-bitcoin#2467: taproot: add TapNodeHash getter method on TapTree and NodeInfo 

1384330029 taproot: add TapNodeHash getter method on TapTree and NodeInfo (conduition)

Pull request description:

  Submitting this to fix what I think is an API hole. Please correct me if I'm mistaken here and there is an easier way to do what I'm after.

  ## Problem

  From what I can tell of the existing 0.31.1 API, there doesn't seem to be any way for a consumer to build a taproot tree using `TaprootBuilder` and then simply output the resulting tap tree merkle root `TapNodeHash`.

  Instead, the API forces me to do `TaprootBuilder::finalize(secp_ctx, internal_key)` first to get a `TaprootSpendInfo`, and then call `TaprootSpendInfo::merkle_root()` to get the root `TapNodehash`. This requires ECC point addition/multiplication for the tweak operation (inside `TaprootBuilder::finalize`), so it is a lot less performant than the simple hashing operations needed to build a taproot tree.

  Obviously if I want to spend the taproot tree, I'll need to tweak an internal key. But if all I want is to examine the merkle root hashes of taproot trees (e.g. for quick validation), there should be a faster and more direct option for me.

  ## Suggested Solution

  My suggestion, demonstrated in this PR, would be to add a couple of simple getter methods:

  - `TapTree::node_hash() -> TapNodeHash`
  - `NodeInfo::node_hash() -> TapNodeHash`

  These rhyme with the existing `LeafNode::node_hash()` method. These provide a roundabout way for downstream consumers to extract a taptree merkle root `TapNodeHash` while still using the safe API provided by `TaprootBuilder`. I would simply use `TaprootBuilder` to build a `TapTree` or `NodeInfo`, and then invoke the `node_hash` method on that object. No point addition required.

  ## Footguns

  This does open up more opportunities for consumers to footgun themselves by, for example, committing a P2TR script pubkey to a leaf node by accident, instead of the root node. I'd argue this possibility already exists in the form of `LeafNode::node_hash()`. We're not making that problem much worse here.

  If the caller is using `TaprootBuilder` to construct their tree, the only way they'll be able to get a `NodeInfo` or `TapTree` in the first place would be to finalize the builder into it, which seems like an acceptable and intuitive-enough usage path to me.

ACKs for top commit:
  apoelstra:
    ACK 1384330029
  tcharding:
    ACK 1384330029

Tree-SHA512: 9a3c7313fcf281e6439241d0cdc9ea018329ecb5e8b959cba88b0e36d4f1f0df54e09e5318073e1067618e85aea991d5ac7c50fa336a91782896c9cb3c98a973
This commit is contained in:
Andrew Poelstra 2024-02-13 20:05:40 +00:00
parent 8f7cc4d6b3 1384330029
commit 668df1d22d
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
1 changed files with 6 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bitcoin/src/taproot/mod.rs
View File

@ -693,6 +693,9 @@ impl TapTree {
/// Returns [`TapTreeIter<'_>`] iterator for a taproot script tree, operating in DFS order over
/// tree [`ScriptLeaf`]s.
pub fn script_leaves(&self) -> ScriptLeaves { ScriptLeaves { leaf_iter: self.0.leaf_nodes() } }
/// Returns the root [`TapNodeHash`] of this tree.
pub fn root_hash(&self) -> TapNodeHash { self.0.hash }
}
impl TryFrom<TaprootBuilder> for TapTree {
@ -842,6 +845,9 @@ impl NodeInfo {
/// Creates an iterator over all leaves (including hidden leaves) in the tree.
pub fn leaf_nodes(&self) -> LeafNodes { LeafNodes { leaf_iter: self.leaves.iter() } }
/// Returns the root [`TapNodeHash`] of this node info.
pub fn node_hash(&self) -> TapNodeHash { self.hash }
}
impl TryFrom<TaprootBuilder> for NodeInfo {