Merge bitcoin/bitcoin#35835: test: add CONST_SCRIPTCODE failure-path vectors to script_tests.json
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#35835: test: add CONST_SCRIPTCODE failure-path vectors to script_tests.json
829e44a1151fa432f293554f25832f19bde209ec test: add CONST_SCRIPTCODE failure-path vectors to script_tests.json (JP)
Pull request description:
Follow-up to #35664.
Going through which script error codes `script_tests.json` covers, I found that neither error gated behind `SCRIPT_VERIFY_CONST_SCRIPTCODE` is asserted anywhere. `SCRIPT_ERR_SIG_FINDANDDELETE` is not asserted by any test; `SCRIPT_ERR_OP_CODESEPARATOR` appears only as a mempool reject string in `invalid_txs.py`, never at the script level. `tx_invalid.json` does have a `CONST_SCRIPTCODE` section, but those vectors can only say a transaction is invalid, not which error made it fail; `script_tests.json` is the harness that pins error codes, and it has no vector using the flag.
This adds six vectors. Four fail with the flag set:
- `OP_CODESEPARATOR` in an executed pre-segwit script.
- `OP_CODESEPARATOR` in an unexecuted `IF` branch. The check in `EvalScript` runs ahead of the `fExec` guard, so the opcode is rejected even though it never executes — the rule with no error-level coverage before.
- A signature push that also appears in the scriptPubKey, against `CHECKSIG`.
- The same against `CHECKMULTISIG`, which calls `FindAndDelete` in a separate loop.
The other two are controls with the flag off, one per error. Both checks run before signature verification, so the vectors can use a dummy signature.
To confirm the expected errors are the ones that fire, I added the vectors expecting `OK` first and let the harness report the actual error for each.
Tested with:
```
build/bin/test_bitcoin --run_test=script_tests/script_json_test
```
ACKs for top commit:
fametrano:
ACK 829e44a1151fa432f293554f25832f19bde209ec
sedited:
ACK 829e44a1151fa432f293554f25832f19bde209ec
Tree-SHA512: b8d52028b5c7e0ec555b4df3749740337b6921b0357b5c9b156daf2a74203a5b95e3bcfa225ebd5a26e0b4cd90290732e4440c0d2b2bfe6f629f51cd319c4415
What changed, and why it matters
This commit only adds new test cases to a Bitcoin Core test data file. It does not change any production code, consensus rules, or network behavior. The tests verify that two specific script error conditions are correctly detected when a particular validation flag is enabled. There is no security vulnerability being fixed here—this is purely additional test coverage.
No action required. This is a benign test-only addition. Reviewers may optionally verify the test vectors match the documented behavior of SCRIPT_VERIFY_CONST_SCRIPTCODE.
Security signals we found
Test-only change
Adds regression/edge-case coverage for existing script validation flags
No production code modified
Evidence from the diff
The commit adds six vectors to src/test/data/script_tests.json to exercise SCRIPT_VERIFY_CONST_SCRIPTCODE failure paths: OP_CODESEPARATOR rejection (including in unexecuted IF branches) and SIG_FINDANDDELETE rejection for CHECKSIG/CHECKMULTISIG. Two control vectors with the flag off expect OK. No C++ code, script interpreter, or consensus logic is modified.
Changed components
src/test/data/script_tests.jsonInspect captured patch +11 / −0
### src/test/data/script_tests.json
@@ -2797,5 +2797,16 @@
["0 0x09 0x300602010102010101 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG", "OK", "BIP66-compliant but not NULLFAIL-compliant"],
["0 0x09 0x300602010102010101 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG,NULLFAIL", "NULLFAIL", "BIP66-compliant but not NULLFAIL-compliant"],
+["CONST_SCRIPTCODE tests"],
+["OP_CODESEPARATOR in a pre-segwit script and a signature push found in the scriptCode"],
+["are rejected with their own error codes when the CONST_SCRIPTCODE flag is set. Both"],
+["checks run before any signature verification, so the signatures never have to be valid."],
+["1", "CODESEPARATOR", "CONST_SCRIPTCODE", "OP_CODESEPARATOR", "OP_CODESEPARATOR in an executed pre-segwit script"],
+["0", "IF CODESEPARATOR ENDIF 1", "CONST_SCRIPTCODE", "OP_CODESEPARATOR", "OP_CODESEPARATOR is rejected even in an unexecuted branch"],
+["0", "IF CODESEPARATOR ENDIF 1", "", "OK", "Without CONST_SCRIPTCODE, OP_CODESEPARATOR in an unexecuted branch is allowed"],
+["0x02 0x0001", "0x02 0x0001 DROP 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG", "CONST_SCRIPTCODE", "SIG_FINDANDDELETE", "CHECKSIG fails when the signature push is found in the scriptCode"],
+["0 0x02 0x0001", "0x02 0x0001 DROP 1 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 1 CHECKMULTISIG", "CONST_SCRIPTCODE", "SIG_FINDANDDELETE", "CHECKMULTISIG fails when a signature push is found in the scriptCode"],
+["0x02 0x0001", "0x02 0x0001 DROP 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG NOT", "", "OK", "Without CONST_SCRIPTCODE, FindAndDelete silently drops the signature push from the scriptCode"],
+
["The End"]
]Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.