Extend script/tx unit test coverage

[Naviabheeman]

Tapyrus Script Coverage & Risk Audit

Scope: src/test/data/{script_tests,tx_valid,tx_invalid}.json, src/script/interpreter.cpp, src/script/standard.cpp, src/validation.cpp
Opcodes checked: 119

A pass over the JSON-driven script test suites, the opcode interpreter, the consensus verify flags, and the ECDSA/Schnorr/compressed-key signature paths — what's exercised, what's thin, and what the interpreter permits that's worth knowing about.

Table of contents

1. Opcode test coverage
2. Risky-but-permitted combinations
3. Script verify flags
4. Compressed keys, Schnorr & ECDSA

---

1. Opcode test coverage

Every opcode defined in script.h was matched (mnemonic, with or without the OP_ prefix) against the three JSON test corpora. Counts are occurrences, not test cases — they indicate whether an opcode is exercised at all, not how thoroughly.

Opcodes defined	119
Exercised ≥1×	114
Thin coverage	2
Never exercised	1

The gaps

Opcode	Status	Note
OP_INVALIDOPCODE	never hit	The 0xff sentinel is never pushed by name. The unassigned range around it (0xbd–0xfe, including 0xff) is covered by raw hex literals in script_tests.json (all correctly return BAD_OPCODE), so the default: switch arm is exercised — only the named constant itself is untested.
OP_CHECKSIGVERIFY	thin	Zero hits in script_tests.json (the pure-interpreter suite); covered only via tx_valid.json/tx_invalid.json (20 total). The VERIFY-suffix pop-on-success/fail-on-false path has no dedicated script-level test.
OP_CODESEPARATOR	thin	Only 1 hit in script_tests.json (a bare no-op check); the interaction with FindAndDelete, SCRIPT_VERIFY_CONST_SCRIPTCODE, and multiple separators per script is exercised mainly through tx_invalid.json (36 hits).
OP_FALSE / OP_TRUE	non-issue	Pure C++ aliases for OP_0/OP_1 (script.h:53,60). Test JSON always spells these 0/1, so a zero hit on the alias name is expected, not a gap.
OP_NOP2 / OP_NOP3	non-issue	Aliases for OP_CHECKLOCKTIMEVERIFY (38 hits) and OP_CHECKSEQUENCEVERIFY (107 hits) — the opcode byte is thoroughly tested under its modern name.

Best-covered families

Flow control (IF/ELSE/ENDIF 270–330 hits each), stack comparison (EQUAL 401, EQUALVERIFY 99), and multisig (CHECKMULTISIG 564, CHECKMULTISIGVERIFY 451) dominate — unsurprising, since script_tests.json is built around exhaustive flow-control and signature-check permutations. All 15 disabled opcodes (CAT, SUBSTR, LEFT, RIGHT, INVERT, AND, OR, XOR, 2MUL, 2DIV, MUL, DIV, MOD, LSHIFT, RSHIFT) have at least 2 hits confirming they still hard-fail with DISABLED_OPCODE.

Tapyrus-specific opcodes

OP_COLOR (5 hits), OP_CHECKDATASIG (53) and OP_CHECKDATASIGVERIFY (47) are Chaintope additions on top of upstream Bitcoin script. All three are exercised only in script_tests.json — zero hits in tx_valid.json/tx_invalid.json. That means their pure opcode semantics are tested, but there's no full-transaction (fee, sighash-in-context, multi-input) test exercising them end to end in the data-driven suites.

---

2. Risky-but-permitted combinations

None of these are bugs in the sense of "the code does the wrong thing" — each behaves exactly as written. They're flagged because the combination opens more room than the canonical script templates use, and a future change elsewhere in the stack could turn that room into a real issue.

[by design] Zero-of-N CHECKMULTISIG is a trivial pass

In interpreter.cpp:1030–1114, when the scriptPubKey encodes num_of_signatures = 0, the verification while loop never executes and fSuccess stays true — the output is spendable by anyone, regardless of how many (or which) pubkeys are listed.

Scenario: a wallet or script-builder that constructs an M-of-N multisig output but miscomputes M as 0 (off-by-one in threshold logic, or a template that defaults M before the user sets it) silently produces an anyone-can-spend output. This mirrors long-standing Bitcoin Core behavior, not a Tapyrus-introduced defect — it's listed because it's easy to reproduce by accident and the interpreter gives no warning.

[mitigated upstream] OP_COLOR can read a color ID assembled by prior stack ops

The OP_COLOR case (interpreter.cpp:1150–1187) only checks: a non-null colorId pointer (not inside a P2SH redeem script), not already set, not inside an OP_IF branch, and that the top stack item is a well-formed 33-byte identifier. It does not require that value to be an immediate push — <a> <b> SWAP DROP COLOR or a PICK/ROLL that pulls the identifier from deeper in the stack (including scriptSig-supplied data) is accepted by the raw interpreter exactly like a direct push.

Scenario: this is currently closed off one layer up. CheckColorIdentifierValidity (validation.cpp:442) requires colored outputs/inputs to match the exact CColorKeyID/CColorScriptID template (standard.cpp:349,355: <colorid> COLOR ... as a fixed prefix) once SCRIPT_VERIFY_CP2SH_COLORED is active for the block height — anything else is rejected with bad-txns-nonstandard-opcolor. Before that soft fork activates for a given chain/height, a non-template colored script is silently continue'd past rather than rejected (validation.cpp:457–459, 478–482), so the interpreter-level flexibility is live during that window. Worth an explicit test asserting the interpreter's own permissiveness here, independent of the validation-layer backstop.

[implementer risk] OP_CHECKDATASIG messages have no built-in replay binding

The signed message for CHECKDATASIG/CHECKDATASIGVERIFY (interpreter.cpp:983–1028) is arbitrary stack data, SHA-256'd and verified with no hashtype byte and no implicit link to the spending transaction (no txid, sequence, or amount is mixed in automatically).

Scenario: an oracle-attestation script that doesn't itself hash in transaction-specific data (e.g. the prevout or a nonce) alongside the attested message can have that same signature/message pair replayed to satisfy any other output using the same script template — this is inherent to how data signatures work everywhere (BCH, this codebase), so it's a note for script authors, not a defect.

[by design] ECDSA and Schnorr can be freely mixed outside CHECKMULTISIG

CHECKSIG/CHECKSIGVERIFY pick their scheme per call, purely from signature length (65 bytes incl. hashtype → Schnorr, else ECDSA/DER; interpreter.cpp:958–960). CHECKMULTISIG is the only opcode that enforces a single scheme across all its signatures (SCRIPT_ERR_MIXED_SCHEME_MULTISIG, interpreter.cpp:1076,1095–1097) — a hand-rolled threshold script built from separate CHECKSIG calls (e.g. an OR-of-two-sigs pattern) has no such constraint and can mix schemes freely between branches.

[mitigated upstream] Time-locks are only excluded from colored scripts at the validation layer

OP_CHECKLOCKTIMEVERIFY/OP_CHECKSEQUENCEVERIFY have no awareness of colorId in EvalScript — nothing in the interpreter stops a script from combining <colorid> COLOR with a CLTV/CSV check ahead of it. The restriction to CP2PKH/CP2SH/burn-only templates (CLTV/CSV must live inside a CP2SH redeem script, never in a direct colored scriptPubKey) is enforced entirely by CheckColorIdentifierValidity's template match, the same consensus gate discussed above.

Confirmed non-issue: disabled opcodes (CAT, SUBSTR, bitwise/arithmetic-shift family) are rejected unconditionally at interpreter.cpp:356–371 — before the executed-branch check, so they fail even sitting inside a never-taken OP_IF arm. That's the safer of the two designs historically seen in Bitcoin-derived interpreters.

---

3. Script verify flags

Tapyrus folds several flags Bitcoin Core still treats as optional (P2SH, STRICTENC, DERSIG, LOW_S, NULLDUMMY, MINIMALDATA, CLTV, CSV) into unconditional consensus behavior — MANDATORY_SCRIPT_VERIFY_FLAGS is literally 0 because there's nothing left to gate (interpreter.h:98–111). That leaves 10 real flag bits.

Flag	Standard set	script_tests.json flags column	Flag-invariance loop
SIGPUSHONLY	yes	used 6×	included
DISCOURAGE_UPGRADABLE_NOPS	yes	used 12×	included
CLEANSTACK	yes	used 6×	included
WITNESS	—	used 23×	commented out
DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM	yes	0×	included
MINIMALIF	yes	used 22×	included
NULLFAIL	yes	used 2×	included
WITNESS_PUBKEYTYPE	yes	0×	included
CONST_SCRIPTCODE	yes	0×	included
CP2SH_COLORED	yes	unnameable	excluded

Note: the CP2SH_COLORED row reflects the state of this codebase at the time of the original audit. It has since been wired into mapFlagNames and test_flags_list (see the unitTestScriptExtension branch) — it's no longer unnameable/excluded.

The CP2SH_COLORED gap was structural, not accidental

mapFlagNames in transaction_tests.cpp:40–49 — the string-to-bit table every JSON test's flags column is parsed through — simply had no entry for CP2SH_COLORED. It could not be named from script_tests.json, tx_valid.json, or tx_invalid.json even if a test author wanted to. Separately, DoTest's flag-mutation invariance loop (script_tests.cpp:185–204, which re-runs every test with each flag toggled to confirm monotonic soft-fork behavior) also omitted it from test_flags_list.

This wasn't a real coverage hole overall — the flag has purpose-built coverage in test/functional/feature_cp2sh_softfork.py and dedicated cases in script_tests.cpp/txvalidation_tests.cpp/federationparams_tests.cpp. But it meant it could never appear in, or be cross-checked against, the three general JSON corpora the way every other flag can.

WITNESS-family flags

Tapyrus doesn't use segwit (per the source comment at interpreter.h:66–68, the code is "left unchanged until we can cleanup all segwit code"). DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM and WITNESS_PUBKEYTYPE never appear in the flags column of script_tests.json even though they're both in the standard set and in the invariance loop — low-priority given witness scripts aren't reachable in practice, but they are dead weight in the standard flag bitmask if truly unreachable.

---

4. Compressed keys, Schnorr & ECDSA

Tapyrus dispatches signature scheme purely by byte length — there is no explicit version tag. Coverage of the resulting matrix is thorough.

Pubkey encoding (interpreter.cpp:65–97)

Prefix	Length	Accepted?	Note
0x02 / 0x03	33 B	yes	Compressed. Only type mandatory when WITNESS_PUBKEYTYPE + SigVersion::WITNESS_V0 apply — unreachable in practice (no segwit).
0x04	65 B	yes	Uncompressed. Valid everywhere outside the (unreachable) witness-pubkeytype gate.
0x06 / 0x07	65 B	no	Hybrid encoding — always PUBKEYTYPE error. Explicitly tested ("...with hybrid pubkey" cases, both schemes, both CHECKSIG and CHECKDATASIG).

Signature scheme dispatch

Opcode	Schnorr trigger	Else	Uniformity rule
CHECKSIG / CHECKSIGVERIFY	len == 65	DER + hashtype (ECDSA)	None — independent per call
CHECKMULTISIG(VERIFY)	len == 65	DER + hashtype (ECDSA)	SCRIPT_ERR_MIXED_SCHEME_MULTISIG if signatures disagree
CHECKDATASIG(VERIFY)	len == 64	DER, no hashtype (ECDSA)	Single signature, not applicable

The final dispatch (GenericTransactionSignatureChecker::VerifySignature, interpreter.cpp:1435–1441) is a one-line size() == 64 ? Verify_Schnorr : Verify_ECDSA. Malformed 64-byte data destined for Schnorr isn't pre-validated by a shape check the way ECDSA's DER encoding is (CheckSchnorrSignatureEncoding is skipped in the CHECKDATASIG path when length already equals 64) — but secp256k1_schnorr_verify simply returns false on malformed input, so this is a defense-in-depth asymmetry, not an exploitable gap.

Test coverage

Both schemes are paired across essentially every relevant construct in script_tests.json: P2PK, P2PKH, P2SH-wrapped P2PK/P2PKH, 2-of-3 multisig (including an explicit "MIXED" negative case), NULLFAIL on/off, DER malleability (high-S, R/S padding), undefined hashtypes, and hybrid pubkeys — each combination present for both ECDSA and Schnorr, for both CHECKSIG and CHECKDATASIG(VERIFY). This is the most thoroughly covered corner of the four areas in this audit.

---

PR changes:

• Add script-level valid/invalid OP_COLOR CP2PKH and CP2SH(P2PKH) test vectors (ECDSA and Schnorr) to script_tests.json, tx_valid.json, and tx_invalid.json.
• Add script_risky_combinations_tests.cpp covering risky-but-permitted interpreter behaviors (0-of-n CHECKMULTISIG, OP_COLOR via stack manipulation, CHECKDATASIG replay, mixed ECDSA/Schnorr branches, OP_COLOR + CLTV), each verified with a control case.
• Add matching mempool-level tests in txvalidation_tests.cpp proving these behaviors hold (or are rejected) at the transaction-acceptance layer, not just the raw interpreter.
• Wire SCRIPT_VERIFY_CP2SH_COLORED into mapFlagNames and test_flags_list so it's nameable from JSON test flags and exercised by the flag-invariance loop.
• Add duplicate_nft_issuance_after_burn_rejected to chainstate_tests.cpp, proving that burning an NFT does not erase its colorId from g_colorid_state, so re-issuance is still rejected.
• Fix ColorIdentifier being shared across JSON test entries in transaction_tests.cpp (now fresh per input, matching CScriptCheck).

[azuchi]

Overall — this is a solid test-coverage PR: test-only changes (all 9 files under src/test/), no production code touched, and the new suite passes cleanly in CI. The five "risky-but-permitted" combinations documented in the PR body are each exercised with a positive case + control, so future regressions (widening or narrowing the interpreter's behavior around these edges) will surface loudly instead of silently. The ColorIdentifier sharing fix in transaction_tests.cpp is a genuine bug fix on top of the coverage work. No blockers from me — a few small hardening suggestions below.

Verified claims from the audit

Spot-checked the load-bearing claims in the PR body against the interpreter/consensus sources on master — all check out:

• OP_INVALIDOPCODE (0xFF) never appears by name in the three JSON corpora (grep -c OP_INVALIDOPCODE = 0 across all three), while 0x…ff hex literals do appear (21 total). The default: switch arm at interpreter.cpp:1191 is exercised; only the named constant is untested — as the body correctly notes, this isn't a real gap.
• SCRIPT_ERR_MIXED_SCHEME_MULTISIG exists (script_error.h:39) and is set at interpreter.cpp:1096, matching risk_mixed_signature_schemes case 4c.
• OP_COLOR's interpreter logic at interpreter.cpp:1150–1187 really does only check (null colorId, already-set, !vfExec.empty(), 33-byte encoding) — no template-shape check — so risk_opcolor_stack_manipulation and risk_opcolor_with_timelock are accurate about what the raw interpreter permits.
• OP_CHECKDATASIG(V) at interpreter.cpp:983–1028 uses CSHA256().Write(msg).Finalize() on the raw message (no sighash, no tx binding) — so risk_checkdatasig_replay is a legitimate replay demonstration, not a test artefact.
• CScriptCheck at scriptcheck.h holds colorid as a per-instance field, default-constructed (or from coloridIn). The transaction_tests.cpp fix — moving ColorIdentifier colorId; inside the per-input loop — matches production semantics.

🟢 What's well done

Test + control pattern. Every risk has at least one positive case demonstrating the behavior and one control case demonstrating where the behavior doesn't apply — 1d (1-of-2 with no sig fails), 2c (canonical CP2PKH template is recognized), 3b (CHECKSIG doesn't replay across nLockTime), 4c (CHECKMULTISIG rejects mixed schemes with the dedicated error), 5b (locktime enforcement isn't swallowed by OP_COLOR). This is exactly the shape that catches a future change that "fixes" one of these into stricter behavior without updating the audit.

Belt-and-suspenders between layers. script_risky_combinations_tests.cpp proves the risky behavior at the raw-interpreter layer; txvalidation_tests.cpp proves the same behavior propagates through mempool + CreateAndProcessBlock end-to-end. That answers the "yes but does the consensus layer catch it?" question up front — for 0-of-N CHECKMULTISIG and CHECKDATASIG replay, no; for OP_COLOR non-templates, yes (via CheckColorIdentifierValidity). The split is worth its extra ~180 lines.

SCRIPT_VERIFY_CP2SH_COLORED wiring is a structural improvement, not just cosmetic. Adding the flag to mapFlagNames (nameable from JSON) and to test_flags_list in script_tests.cpp (exercised by the flag-invariance loop) closes a real hole — the audit noted the flag "could not be named from script_tests.json, tx_valid.json, or tx_invalid.json even if a test author wanted to." The two new JSON cases at the end of script_tests.json (flag off → outer HASH160 check only; flag on → redeem script's CHECKSIG genuinely evaluated) are a good demonstration of what the flag actually gates.

transaction_tests.cpp ColorIdentifier fix. The old code had a single ColorIdentifier colorId declared outside both loops in tx_valid and tx_invalid. That's a bug: once any test's script executed OP_COLOR, colorId->type != TokenTypes::NONE was true for every subsequent input in every subsequent test, and any later OP_COLOR would fail with SCRIPT_ERR_OP_COLORMULTIPLE. Moving the declaration inside the per-input loop matches CScriptCheck in production. The failure mode is order-dependent (a colored test earlier in the file poisons later ones), which is exactly the shape of bug that hides for a long time.

duplicate_nft_issuance_after_burn_rejected is well-designed. Verifies the real invariant: burning doesn't call CIssuedColorIds::Erase() (that's reorg-only), so g_colorid_state->IsIssued(colorid) stays true after burn, and a re-issuance attempt against a synthetically-restored UTXO view is correctly rejected with bad-txns-colorid-already-issued. The synthetic CCoinsViewCache is the right way to exercise the reorg-shaped condition without actually needing to construct a SHA-256 collision on the defining outpoint.

🟡 Small hardening suggestions (all optional)

1. A few control cases only check !ok without asserting the specific error. For example risk_checkmultisig_zero_of_n case 1d:

   BOOST_CHECK(!ok);
   // no BOOST_CHECK_EQUAL(err, SCRIPT_ERR_...)

   The point of the control is that "1-of-2 with no signature" fails for the multisig-verification reason, not for some unrelated stack-bookkeeping reason. Asserting the specific error (SCRIPT_ERR_CHECKMULTISIGVERIFY here) would make the control tighter — a future change that starts failing this case with the wrong error would still be caught. Same applies to a few other !ok sites (the STANDARD-flag-set fail paths).

2. duplicate_nft_issuance_after_burn_rejected's block-numbered comments couple to TestChainSetup.

   // Block 6: spend coinbase[1] into a plain TPC P2PKH output.
   // Block 7: issue an NFT (nValue must be exactly 1) from the P2PKH UTXO.
   // Block 8: burn the NFT

   The "6/7/8" numbering assumes TestChainSetup produces exactly 5 initial blocks. If someone bumps that count for another test's sake (e.g. m_coinbase_txns.size() grows), these comments silently rot. Referring to them by role (// Block N: spend the defining UTXO into…) rather than absolute index would age better. The test logic is fine — this is a comment-rot concern.

3. vchKeyBytes[32] in the same test uses 3, 0, ... with no explanation. A one-line note ("arbitrary non-coinbase key; low byte chosen to avoid clashing with any TestChainSetup pre-defined key") would answer the obvious "why 3?" question without a reader having to look up whether it's load-bearing.

4. Consider a brief note on STANDARD_SCRIPT_VERIFY_FLAGS semantics in risk_opcolor_stack_manipulation. Since this PR wires SCRIPT_VERIFY_CP2SH_COLORED into the standard set, and this test asserts that the SWAP-derived / ALTSTACK-derived scripts still pass under STANDARD flags, a one-line comment clarifying "the CP2SH_COLORED flag gates redeem-script evaluation for CP2SH shapes, not template shape at the interpreter layer — the non-canonical shape is still accepted by VerifyScript here" would head off a reader wondering whether the STANDARD flag choice is significant.

Summary

Test-only, no production risk, all new suite cases pass in CI, the risky-combinations audit is faithfully backed by runnable proofs, and the ColorIdentifier sharing fix is a real bug fix. The suggestions above are quality-of-life tightening — nothing blocks landing.

[azuchi]

Follow-up observation, not something this PR needs to address — flagging so we can decide whether to open a separate issue.

While cross-checking the audit against upstream, I noticed several 2024–2026 additions to Bitcoin Core's script_tests.json / tx_valid.json / tx_invalid.json for shared functionality (not segwit/taproot) that Tapyrus hasn't picked up. The audit in this PR is thorough on Tapyrus's own coverage, but by comparison to upstream master these gaps exist:

Actual bug fix with matching test change (code + data)

• Fix 11-year-old mis-categorized error code in OP_IF evaluation bitcoin/bitcoin#32143 (2026-01) — 11-year-old mis-categorised error code in OP_IF evaluation. Upstream now returns SCRIPT_ERR_INVALID_STACK_OPERATION when OP_IF/OP_NOTIF sees an empty stack; Tapyrus's interpreter.cpp:500 still returns SCRIPT_ERR_UNBALANCED_CONDITIONAL. The corresponding test entries in script_tests.json:1050-1051 ("NOP", "IF 1 ENDIF" and "NOP", "NOTIF 1 ENDIF") still assert UNBALANCED_CONDITIONAL too. Consensus-neutral, but it's a real classification bug we're inheriting.

Test-only additions (data catch-up, no code change)

• test: Add unit test for OP_NUMEQUALVERIFY bitcoin/bitcoin#34145 (2026-01) — dedicated OP_NUMEQUALVERIFY exercise. One-line addition: ["2 0", "NUMEQUALVERIFY", "P2SH,STRICTENC", "NUMEQUALVERIFY"]. SCRIPT_ERR_NUMEQUALVERIFY exists in script_error.h:30 but never appears in any JSON test case in Tapyrus (grep -c '"NUMEQUALVERIFY"' src/test/data/*.json = 0).
• test: Improve STRICTENC/DERSIG unit tests bitcoin/bitcoin#34295 (2026-02) — DERSIG variants of CHECKMULTISIG early-exit tests. The "distinguish exit-early on invalid sig vs invalid pubkey" cases are extended to also cover DERSIG alone. Note: Tapyrus folds DERSIG into MANDATORY_SCRIPT_VERIFY_FLAGS = 0 (per the interpreter.h:98–111 audit note in this PR's body), so the DERSIG-flag semantics differ from Bitcoin Core; but the underlying "1-valid-sig + 1-invalid-sig" positions in the multisig ordering are still Tapyrus-relevant and worth adding under whatever flag combination makes sense here.
• bitcoin/bitcoin PR (2025-07-08, 5fa81e239a) — minimum-sized (8-byte DER) ECDSA signature valid-tx case. Directly Tapyrus-relevant, ECDSA path is shared.
• bitcoin/bitcoin PR (2025-12-18, 6bb66fcccb) — improved coverage for pubkey checks. Shared code.
• bitcoin/bitcoin PR (2025-07-23, b94c6356a2) — OP_2ROT proper-behavior test. Basic stack opcode.

Larger port (new error category + tests)

• script: return proper error for CScriptNum errors bitcoin/bitcoin#34381 (2026-01) — SCRIPT_ERR_SCRIPTNUM for CScriptNum errors. Splits CScriptNum failures out of the current catch-all error and adds JSON cases for them. SCRIPT_ERR_SCRIPTNUM doesn't exist in Tapyrus at all (grep -c SCRIPT_ERR_SCRIPTNUM src/script/script_error.h = 0). Bigger port — needs the error-code addition first.

Explicitly not Tapyrus-relevant

For completeness, I did not count taproot/tapscript additions (SCRIPT_ERR_TAPSCRIPT_EMPTY_PUBKEY, "improves tapscript unit tests") or the segwit-only Add missing test for empty P2WSH redeem — these don't apply given interpreter.h:66-68 ("left unchanged until we can cleanup all segwit code") and the absence of taproot.

Suggestion

None of this is in scope for this PR. But given the coverage focus, I'd be inclined to open a separate follow-up issue tracking "Bitcoin Core script/tx test data catch-up (shared functionality only)" and pick these off one at a time — the OP_IF fix in particular is a small, clean port that closes a real (if consensus-neutral) misclassification.

Happy to file the issue if that direction sounds right.