#36360 test: add tests in script test suites covering script.cpp live mutants

ReviewabilityReviewsAgreementSize
#36360 Yudis-bit test: add tests in script test suites covering script.cpp live mutantsReady0Mild0 + 159 tests
  • Add unit tests to catch boundary condition and parsing bugs in script validation
  • Ensure script logic checks and live mutants in core script handling are detected
  • Ready for review
  • No review verdicts recorded
  • Answered objection: truncated GetOp test does not kill the mutants it claims to (brunoerg)
  • Answered objection: LLM-generated text violates project AI policy (sedited)
  • Open suggestion: add a test covering an analogous witness script hash size mutant (fametrano)
  • 0 lines added or modified outside tests
  • 159 lines added or modified in tests
  • 0 lines removed in total
  • 4 files, 4 commits
Uncategorized15 PRs, unranked
1 rejected
  • Test infrastructure: not a member. Touches only unit tests in src/test/ (script_segwit_tests.cpp, script_tests.cpp, sigopcount_tests.cpp, script_standard_tests.cpp) targeting script behavior; the tests category covers testing machinery and explicitly excludes tests that pin the behavior of a specific area.
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 unit tests across four script test suites to cover boundary conditions and untested branches in src/script/script.cpp. The additions test CScript::IsPayToTaproot(), CScript::IsPayToAnchor(), CScript::GetSigOpCount(), CScript::HasValidOps(), CScriptWitness::ToString(), and truncated pushdata parsing in CScript::GetOp().

Problem: Boundary conditions, truncated input parsing, and specific logic checks in src/script/script.cpp were not tested by the existing test suite, leaving twelve live mutants undetected during mutation testing.

Evidence of importance:

  • PR description documents 12 specific mutation failures in src/script/script.cpp that now fail tests, including mutants 2346-2348 in IsPayToTaproot, mutant 2337 in IsPayToAnchor, and mutants in GetSigOpCount and GetScriptOp
  • Commit messages note that each mutation was applied separately against baseline and verified to fail assertions with the added tests

Scope: Only touches unit test files (+159 lines across 4 test suites).

Risk of a mistake: low (tests). Modifies only unit test suites and does not change any consensus or production script validation code.

Changed since the previous assessment: The patch ID changed from a rebase or minor edit, but the scope and target tests across all four test files remain the same.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: author response to fametrano's suggestion to add a P2WSH size check mutant test 2026-11-01

Agreement

Mild

objection that needs a change was answered, no reply from the objector (brunoerg: the script_GetOp_truncated test case does not kill any of the four GetOp mutants it claims to kill; sedited: comments and PR descriptions generated by LLMs violate the project AI policy and waste human review time; brunoerg: the PR description is not accurate regarding what the PR claims and does)

Thread notes: Added fametrano's suggestion in rc:4436813993 to cover the analogous surviving mutant in IsPayToWitnessScriptHash.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
brunoerg (member)objection, correctnessthe 16-of-16 P2SH test does not test the OP_16 boundaryfixed · fix pushed · author replaced the test to put OP_16 directly before the redeem-script pushchangedyes2026-09-28: 'the 16-of-16 P2SH test doesn't test the OP_16 boundary (b9ad09fe3)'
Status from: 2026-09-29: Yudis-bit: 'The replacement puts OP_16 directly before the redeem-script push.'
brunoerg (member)objection, correctnessthe script_GetOp_truncated test case does not kill any of the four GetOp mutants it claims to killanswered · author explained that three mutants are detected by the original tests with dataset patches and removed mutant 2372changedyes2026-09-28: 'your `script_GetOp_truncated` test case does not kill any of the four `GetOp` mutants it claims'
Status from: 2026-09-29: Yudis-bit: 'three of the four IDs are detected by the original tests'
sedited (member)objection, maintenancecomments and PR descriptions generated by LLMs violate the project AI policy and waste human review timeanswered · author acknowledged LLM usage, rewrote their explanation, and updated the PR descriptionchangedyes2026-10-08: 'Per the AI policy, we require contributors to write their own comments and PR descriptions.'
Status from: 2026-10-08: Yudis-bit: 'Fair point. I used an LLM earlier to draft the environment summary'
brunoerg (member)suggestion, maintenanceput the mutant diff in the description instead of mutant numbers to facilitate reproductionanswered · author updated description with reproduction diffsansweredyes2026-10-21: 'Please, instead of saying "killing surviving mutant 2383", could you put the diff in the description, it would facilitate the reproduction?'
Status from: 2026-10-22: Yudis-bit: 'Updated description with reproduction diffs and MutantHub links.'
brunoerg (member)objection, correctnessthe PR description is not accurate regarding what the PR claims and doesanswered · author updated description to replace generic claims with exact script.cpp boundary checkschangedyes2026-10-21: 'the description doesn't seem accurate on what's claiming and doing.'
Status from: 2026-10-22: Yudis-bit: 'Replaced the generic claims with the exact script.cpp boundary checks tested in each commit.'
brunoerg (member)objection, scopethe added test in sigopcount_tests is redundant because #36130 already covers itfixed · fix pushed · author dropped the redundant check in a push while keeping the OP_1 checkchangedyes2026-10-21: '#36130 already covers it.'
Status from: 2026-10-22: Yudis-bit: 'Dropped the redundant GetSigOpCount(false) check in 20d01e52a.'
fametrano (none)objection, correctnessthe IsPayToTaproot mutant changing size() == 34 to >= 34 is not killed by the new testsfixed · fix pushed · author added a 35-byte script test case that kills the mutantchangedyes2026-10-23: 'The `>= 34` mutant passes the new tests and the whole `test_bitcoin`.'
Status from: 2026-10-26: Yudis-bit: 'Added the 35-byte test case in 279d5cab3.'
fametrano (none)suggestion, correctnessadd a test case with a 35-byte script starting with OP_1 0x20 to catch the IsPayToTaproot mutantfixed · fix pushed · author added the requested 35-byte test caseansweredyes2026-10-23: 'This case catches the `>= 34` mutant: it passes unmutated and fails with the mutant.'
Status from: 2026-10-26: Yudis-bit: 'Added the 35-byte test case in 279d5cab3.'
fametrano (none)suggestion, scopeadd a test case for IsPayToWitnessScriptHash covering the >= 34 mutantopenansweredno2026-11-01: 'The P2WSH twin of this case is missing: `size() == 34` to `>= 34` in `IsPayToWitnessScriptHash` (script.cpp:236) survives the whole `test_bitcoin`.'

Participants:

  • fanquake (member, neutral, 1 statements 2026-09-27 to 2026-09-27): pinged brunoerg
  • brunoerg (member, objection, 3 statements 2026-09-28 to 2026-10-21): objected to inaccurate PR description, duplicate test coverage already in #36130, and ineffective mutant tests
  • sedited (member, objection, 1 statements 2026-10-08 to 2026-10-08): objected to AI-generated comments and PR description under project AI policy
  • fametrano (none, objection, 3 statements 2026-10-23 to 2026-11-01): objected that the IsPayToTaproot mutant was not killed and suggested adding a P2WSH mutant test

Files

159 lines under test/bench/ci.

  • src/test/script_segwit_tests.cpp +86/-0
  • src/test/script_tests.cpp +40/-0
  • src/test/sigopcount_tests.cpp +18/-0
  • src/test/script_standard_tests.cpp +15/-0

Card

The short text the ranking pass compares across PRs.

Adds unit tests across four script test suites to cover boundary conditions and kill 12 live mutants in src/script/script.cpp identified through mutation testing. The new test cases exercise Taproot and Anchor recognition edge cases, SigOp counting boundary opcodes, opcode limits in HasValidOps, incomplete pushdata byte lengths in GetOp, and witness stack string formatting. Only unit test files are touched.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-02T01:11, confidence high, input hash 1ab8e1c5ef285d51
thread read through 2026-11-01 (13 statements), head 76b4532b3f
labels: Tests