New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
taproot: add TapNodeHash getter method on TapTree and NodeInfo #2467
Conversation
Pull Request Test Coverage Report for Build 7879485491Details
💛 - Coveralls |
I think the method on But I think you're correct that this is an API gap. I agree there's a mild footgun in the |
Concept ACK. Maybe |
Fixes a gap in the API of the taproot module. Callers can now use TapTree::root_hash or NodeInfo::node_hash to extract the taproot tree merkle root hash for fast validation without any ECC overhead.
2c9d911
to
1384330
Compare
I renamed Then squashed, rebased, and added a more detailed commit message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1384330
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1384330
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 rootTapNodeHash
.Instead, the API forces me to do
TaprootBuilder::finalize(secp_ctx, internal_key)
first to get aTaprootSpendInfo
, and then callTaprootSpendInfo::merkle_root()
to get the rootTapNodehash
. This requires ECC point addition/multiplication for the tweak operation (insideTaprootBuilder::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 rootTapNodeHash
while still using the safe API provided byTaprootBuilder
. I would simply useTaprootBuilder
to build aTapTree
orNodeInfo
, and then invoke thenode_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 aNodeInfo
orTapTree
in the first place would be to finalize the builder into it, which seems like an acceptable and intuitive-enough usage path to me.