bip360: use correct leafVersion in control block byte for non-default version leaves#2103
bip360: use correct leafVersion in control block byte for non-default version leaves#2103jasonandjay wants to merge 1 commit intobitcoin:masterfrom
Conversation
…rsion leaves The P2MR control block's first byte encodes both the leaf version and parity bit: controlBlock[0] = (leafVersion & 0xfe) | parityBit. For the leaf with leafVersion 250 (0xFA), the correct first byte is 0xFB (with parity bit 1), not 0xC1 (which corresponds to leafVersion 192). Fix p2mr_different_version_leaves scriptPathControlBlocks[1] first byte from c1 to fb.
|
@murchandamus Looking into it |
|
Given from the look of this, that a new PR by this author was authored by Openclaw, and from the general activity on this author’s profile, I assume that this PR is AI slop. It may be useful AI slop, which is why I asked for your opinion, but I tend to close this. Will close this in a couple weeks (on or after 2026-03-30), if I don’t hear more from @cryptoquick, or @EthanHeilman. |
This PR is obviously correct. |
|
@murchandamus @notmike-5 I'm confused about what this PR is trying to say. Let me try to work through it out loud tell me where I'm wrong. The only supported leaf version is The PR says:
Why should the leaf version be 250? 192 seems correct. |
| "scriptPathControlBlocks": [ | ||
| "c1f224a923cd0021ab202ab139cc56802ddb92dcfc172b9212261a539df79a112a", | ||
| "c18ad69ec7cf41c2a4001fd1f738bf1e505ce2277acdcaa63fe4765192497f47a7" | ||
| "fb8ad69ec7cf41c2a4001fd1f738bf1e505ce2277acdcaa63fe4765192497f47a7" |
There was a problem hiding this comment.
@EthanHeilman: The description of the test states that the two leaves have different leaf versions and the second leaf uses a hypothetical leaf version of 250 (see line 74).
One of the reasons I surface this is to ask whether you want to take the proposed action or whether you want to do something else, or additional actions if this is indicative of other issues in your reference implementation. E.g., what was this test with different leaf versions supposed to test? Is it an issue that it didn’t test that and still passed?
There was a problem hiding this comment.
Ahh, this test case is about different leaf versions, that makes sense. I probably should have noticed the name of the test case was p2mr_different_version_leaves I retract my comment.
I don't maintain the reference implementation. @cryptoquick is taking care of it.
There was a problem hiding this comment.
Sorry, I should have distinguished. The first line was in response to you, but the rest was generally directed at the authors.
|
I believe the correct behavior when encountering a non-standard leaf version in a P2MR merkle tree should be to throw an error. My pull request here implements this approach. |
|
Glad this was pointed out, thank you @jasonandjay. That said, we're going to discuss the proper approach together as a team and address concerns in the PR that @jbride linked: cryptoquick#44 We are also going to need more time to investigate #2102. Any other thoughts or improvements should be directed there to better tie up loose ends and polish things even further. |
|
Thanks for taking a look. I’m going to wait to hear what you want to do about this topic. |
|
According to the section "Script Validation" of BIP-360, which was adapted from BIP-341, it is implied that "for the future leaf versions (non-0xC0) the execution must succeed". As others have pointed to previously, there is a further issue of what to do for leaf version 0xC0 in the context of P2MR. Tapscript validation rules, as defined, explicitly do not apply in this context. So, might want to consider lumping these two issues together. |
Summary
Fix the first byte of
scriptPathControlBlocks[1]in thep2mr_different_version_leavestest vector. The control block incorrectly uses
0xC1(leafVersion 192) instead of0xFB(leafVersion 250) for a leaf explicitly declared with
"leafVersion": 250.Problem
The P2MR control block's first byte encodes both the leaf version and parity bit:
For P2MR, the parity bit is always 1. So:
0xC10xFBThe test vector defines two leaves:
{ "id": 0, "leafVersion": 192, "script": "...OP_CHECKSIG" } { "id": 1, "leafVersion": 250, "script": "06424950333431" }But the current control block for leaf1 starts with
0xC1:This is incorrect because a verifier would extract
leafVersion = 0xC1 & 0xFE = 0xC0 = 192from this byte, then compute
tapleafHash(script, version=192)which produces acompletely different hash than the correct
tapleafHash(script, version=250), makingit impossible to reconstruct the correct Merkle root.
Fix
Change the first byte from
0xC1to0xFB:Verification:
Changes
p2mr_different_version_leaves: changescriptPathControlBlocks[1]first byte fromc1tofb