#36699 test: strengthen the tapscript CHECKMULTISIG asset tests and dump the expected error
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36699 test: strengthen the tapscript CHECKMULTISIG asset tests and dump the expected error | Ready | 0 | Crickets | 0 + 19 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Adds 0-of-0 OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY test cases for tapscript in feature_taproot.py to verify that failure is triggered specifically by the opcode ban rather than DER signature validation errors. Also dumps the expected error string for failure cases into the test assets and verifies in script_assets_tests that evaluation fails with the exact expected script error code under the declared flags.
Problem: Existing tapscript test cases for disabled CHECKMULTISIG opcodes supplied Schnorr signatures, which fail DER signature validation when the opcode is executed. As a result, an implementation that incorrectly executes CHECKMULTISIG in tapscript still passed all tests. Additionally, the script asset test runner only verified that a test failed, without ensuring it failed for the correct reason.
Evidence of importance:
- The author found this flaw against external software: "This came up while running script_assets_test.json against a third-party implementation which executed OP_CHECKMULTISIG in tapscript and passed every case... it accepts the new failure case while its fixed version rejects it with SCRIPT_ERR_TAPSCRIPT_CHECKMULTISIG" (PR description).
Risk of a mistake: low (tests). Changes are strictly confined to functional test script generation and test harness assertions.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: review from instagibbs or others 2026-10-30
- reviewer: review of the PR 2026-10-30
Agreement
Crickets
no substantive comment either way
Thread notes: Initial record. fanquake pinged instagibbs; awaiting review. / Initial record. fanquake only pinged instagibbs.
Objections, suggestions, and questions: none recorded.
Participants:
- fanquake (member, neutral, 1 statements 2026-10-30 to 2026-10-30): pinged instagibbs
Files
20 lines under test/bench/ci.
- test/functional/feature_taproot.py +11/-0
- src/test/script_assets_tests.cpp +8/-1
Card
The short text the ranking pass compares across PRs.
Adds 0-of-0 OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY test vectors for tapscript to feature_taproot.py, and records expected error strings in the generated test asset dump for script_assets_tests to verify. Existing test cases used Schnorr signatures that failed DER validation, allowing an implementation that failed to disable CHECKMULTISIG in tapscript to pass all existing asset tests. The asset test runner also previously only verified that negative test cases failed, rather than verifying they failed with the expected error code. The author identified the issue after discovering a third-party tapscript implementation executing CHECKMULTISIG passed the test suite.
Data
thread read through 2026-10-30 (1 statements), head 420decb170
labels: Tests
Categories
Test infrastructure
P3 · test coverage (ranking pass: Moved up: this is a misleading-test fix (asset tests passed for an implementation that runs CHECKMULTISIG in tapscript), which the category ranks above plain added coverage.)
Tightens tapscript test vectors and enhances the script asset test runner to verify exact failure error codes rather than generic failure. The author discovered that prior tests allowed "a third-party implementation which executed OP_CHECKMULTISIG in tapscript and passed every case" because failure was inadvertently masked by DER signature errors rather than the tapscript opcode ban.
Membership: Labeled Tests; modifies test runner verification in src/test/script_assets_tests.cpp and tapscript test generation in test/functional/feature_taproot.py
Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 0