Fix BIP-360 P2MR consensus, descriptor, policy, and test gaps#1
Open
starius wants to merge 17 commits into
Open
Fix BIP-360 P2MR consensus, descriptor, policy, and test gaps#1starius wants to merge 17 commits into
starius wants to merge 17 commits into
Conversation
P2MR validation was wired into standard and mandatory policy flags, but block validation still built its script flag set from P2SH, witness, and taproot only. A native v2 32-byte witness program could therefore be spent arbitrarily inside a block because it continued down the unknown-witness-program success path. Include SCRIPT_VERIFY_P2MR in the block script flags so block connection evaluates the P2MR control block, Merkle root, and tapscript rules instead of treating these outputs as anyone-can-spend witness upgrades.
A native v2 32-byte output must not remain spendable through the generic unknown-witness-program path once P2MR validation is active. This regression test mines such an output, matures it, and submits a block containing a one-element witness spend that has no P2MR script path. With block script flags enforcing P2MR, submitblock fails with block-script-verify-flag-failed. If the consensus flag wiring is reverted, the same block connects because the spend is treated as an unknown witness program.
BIP-360 assigns Pay-to-Merkle-Root to native witness version 2 with a 32-byte Merkle root program. The tmr() descriptor builder was emitting OP_3, so descriptors produced v3/bech32m outputs that decoded as witness_unknown and never exercised the P2MR validation path. Emit OP_2 for tmr() outputs so descriptor-derived scripts match the interpreter, address encoder, and solver's WITNESS_V2_P2MR type.
tmr() descriptors must materialize native witness v2 P2MR outputs. Without coverage, the descriptor code could emit OP_3 and still parse successfully while producing addresses that decode as witness_unknown. Add a descriptor regression that expands tmr(pk(...)), asserts the script starts with OP_2 OP_PUSHBYTES_32, checks Solver returns WITNESS_V2_P2MR, and confirms the encoded address uses the bc1z witness-v2 prefix.
A P2MR control block with no Merkle path commits directly to the witness program. After the root check succeeds, BIP-360 specifies that this case terminates successfully instead of executing the leaf script. The interpreter was executing c1/tapscript leaves even when the control block length was one byte, so a committed depth-zero OP_FALSE-style script could fail. Return success after the commitment and parity checks when the P2MR path depth is zero.
Depth-zero P2MR spends are anyone-can-spend after the committed root is verified; the leaf script is data used for the commitment, not executed script. Update the existing unequal OP_EQUAL vector to expect success under P2MR activation. Reverting the interpreter fix makes this vector fail with EVAL_FALSE, which is the old incorrect execution path.
BIP-360 uses a one-byte P2MR control header and permits at most 128 Merkle branch nodes. The implementation derived the maximum from the Taproot control size and then added another 128 nodes, allowing 257 P2MR path elements. Set P2MR_CONTROL_MAX_SIZE to 1 + 32 * 128 so over-deep P2MR script-path spends fail at the control-size check before commitment validation.
P2MR has a maximum Merkle path depth of 128 branch nodes. A control block with 129 nodes should fail because of its size, not after trying to verify a commitment. Add a script test that builds a 129-node P2MR control block and asserts P2MR_WRONG_CONTROL_SIZE. Reverting the size cap changes the symptom to WITNESS_PROGRAM_MISMATCH, proving the over-deep path was accepted past the size check.
BIP-360 reserves the low bit of the P2MR control byte and requires it to be set for every leaf version. The interpreter only enforced that rule for tapscript leaves, so a future leaf version with low bit zero could satisfy the commitment and be accepted as an upgradable leaf. Reject any P2MR control byte with parity bit zero before leaf-version dispatch, and make the error text describe the generic control-byte rule rather than only the 0xc1 tapscript case.
The P2MR parity-bit rule is not limited to tapscript leaf version 0xc0. Future leaf versions must also use control bytes with the low bit set. Add a depth-zero future-leaf spend whose root matches when interpreted as leaf version 0xc2 but whose control byte has parity bit zero. Reverting the parity fix makes the spend return OK instead of P2MR_WRONG_PARITY_BIT.
OP_SUCCESSx opcodes in tapscript must short-circuit to success unless policy discouragement flags are being applied. This branch had redefined OP_SUCCESS127/OP_SUBSTR as an SLH-DSA verifier for all tapscript spends, including ordinary P2TR, so scripts that BIP-342 treats as unconditional success could fail. Remove the OP_SUCCESS127 verifier path, its oversized stack exception, and the PQC/debug plumbing from the script interpreter. OP_SUBSTR now follows the same OP_SUCCESS pre-scan as the other reserved success opcodes.
OP_SUCCESS127 is reserved by BIP-342 like the other OP_SUCCESSx opcodes. The functional taproot test had skipped opcode 127 because the branch redefined it for SLH-DSA verification, so the regression was hidden. Re-enable the existing OP_SUCCESSx cases for OP_SUBSTR. If the interpreter again special-cases opcode 127, these spends fail instead of following the normal OP_SUCCESS success path.
The P2QRH functional test did not validate P2MR behavior. It funded a witness-v3 output that decoded as witness_unknown, then caught and logged the testmempoolaccept and sendrawtransaction failures before reporting success. Remove the obsolete runner entry and test file so the suite no longer carries coverage that can pass without exercising native witness-v2 P2MR validation.
Functional P2MR coverage should prove that native witness-v2 outputs reach the P2MR interpreter path, not the unknown-witness path. A depth-zero P2MR leaf with a matching TapLeaf root can be spent without a signature after the commitment check succeeds. Extend feature_p2mr.py to fund a native v2 P2MR output, assert testmempoolaccept allows the matching depth-zero spend, broadcast it, and mine it. Reverting the descriptor/version or depth-zero validation fixes makes this spend fail with a symptomatic policy or script error.
Several tests had temporary skips or version checks that avoided native witness-v2 P2MR data. key_io_valid_parse skipped all 0x52 scripts, the script fuzzer skipped WitnessV2P2MR destination round-trips, and script_assets_tests looked for OP_3 while describing witness version 2. Restore the normal key/address assertions for v2 scripts, let the fuzzer round-trip WitnessV2P2MR destinations, and detect P2MR scriptPubKeys with OP_2 OP_PUSHBYTES_32.
The generic key_io_valid fixtures include short witness-v2 unknown-program examples, but they did not pin the native v2/32 P2MR destination path directly. Add a unit test that constructs a WitnessV2P2MR root, encodes it as a mainnet bech32m address, decodes it back to WitnessV2P2MR, and verifies the generated scriptPubKey starts with OP_2 OP_PUSHBYTES_32.
P2MR tapscript leaves share the BIP 342 execution rules, so their standard witness stack item limit should match the existing tapscript policy limit. The branch still carried an 8000-byte P2MR-specific limit and comments about a removed SLH-DSA OP_SUCCESS127 experiment. That made standard P2MR policy looser than taproot for no BIP-360 reason, and the witness-standardness branch also described the output as witness version 3 even though P2MR is native witness v2. Set the P2MR stack item limit back to 80 bytes, remove the stale SLH-DSA wording, and apply this policy branch only to native v2/32 P2MR spends. Add a transaction unit test that accepts an 80-byte initial stack item and rejects an 81-byte item, so reverting the limit produces a direct policy regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes several BIP-360 P2MR implementation issues found while comparing the branch against the BIP text and the existing Taproot/Tapscript behavior.
P2MR was not enforced during block validation
SCRIPT_VERIFY_P2MRwas wired into policy flags, but block script flags omitted it. A native witness-v2 32-byte output could still be spent inside a block through the unknown-witness-program success path.Relevant broken code: block script flags omit P2MR and the interpreter returns success when P2MR is not enabled.
Fix: add P2MR to block script verification flags and add a functional block-rejection test proving an invalid native P2MR spend is rejected by consensus validation.
tmr(...)descriptors emitted witness version 3tmr(...)generatedOP_3 <32-byte-root>, while BIP-360 and the interpreter use native witness v2 (bc1z...).Relevant broken code: descriptor output script construction.
Fix: emit
OP_2 <32-byte-root>and add descriptor coverage that checks the script, solver type, and encoded address prefix.Depth-zero P2MR executed the committed leaf script
BIP-360 says
m = 0succeeds immediately after the committed root matches. The implementation instead executed the committed tapscript leaf, so a valid depth-zero P2MR spend could fail if the committed script evaluated false.Relevant broken code and test expectation: depth-zero execution path and test vector expecting failure.
Fix: return success after commitment and parity checks for zero-depth control blocks, and update script/functional tests to cover the intended anyone-can-spend behavior.
P2MR allowed over-deep control blocks
BIP-360 caps the Merkle path at 128 nodes. The implementation derived the maximum from Taproot's 33-byte control block and then extended it again, allowing up to 257 P2MR nodes instead of
1 + 32 * 128.Relevant broken code: control block constants and P2MR control-size check.
Fix: cap P2MR control blocks at
1 + 32 * 128and add a regression test for a 129-node control path.P2MR parity was only enforced for tapscript leaves
BIP-360 requires the low bit of
c[0]to be 1 for every leaf version, including future leaf versions. The implementation only enforced that rule for tapscript leaves.Relevant broken code: parity check limited to tapscript.
Fix: reject low-bit-zero P2MR control bytes before leaf-version dispatch and add future-leaf parity coverage.
OP_SUCCESS127 was redefined globally for tapscript
The branch treated OP_SUCCESS127 as an SLH-DSA verifier for all tapscript spends, including ordinary P2TR spends, and skipped upstream OP_SUCCESS127 functional coverage. That changed BIP-342 behavior outside the P2MR proposal.
Relevant broken code and skipped coverage: global OP_SUCCESS127 verifier path and functional test skip.
Fix: remove the global OP_SUCCESS127 verifier path and restore the normal OP_SUCCESSx functional coverage.
The added P2QRH functional test did not validate P2MR
The test funded a witness-v3 output that decoded as
witness_unknown, swallowed mempool/broadcast failures, and still reported success.Relevant broken test: broad exception handling around failed spend paths.
Fix: remove that obsolete test and replace it with native witness-v2 P2MR functional coverage that must pass
testmempoolaccept, broadcast, and mining.P2MR address and script tests were weakened
Key I/O skipped normal checks for
0x52scripts, the script fuzzer skippedWitnessV2P2MR, and script asset tests looked forOP_3while describing witness version 2.Relevant broken tests: key I/O skip, script fuzzer skip, and asset test OP_3 detection.
Fix: restore native v2/32 P2MR address round trips, fuzzer coverage, and OP_2 script detection, with an explicit P2MR key I/O regression test.
P2MR standardness policy used an oversized stack item limit
P2MR tapscript leaves follow BIP-342 execution rules, but policy still carried an 8000-byte P2MR-specific stack item limit and stale SLH-DSA comments.
Relevant broken policy: oversized P2MR stack item limit and stale SLH-DSA policy comments.
Fix: align P2MR standard stack item policy with the existing 80-byte tapscript limit, remove the stale comments, restrict the policy branch to native P2MR spends, and add a direct unit test for the 80/81-byte boundary.
Verification
build/bin/test_bitcoin --run_test=script_tests,script_standard_tests,key_io_tests,descriptor_tests,script_assets_tests,transaction_tests --catch_system_errors=no --log_level=test_suitebuild/test/functional/test_runner.py --jobs=1 --combinedlogslen=0 feature_p2mr.py feature_taproot.py🤖 Assisted by Codex GPT-5