#35882 test: cover Taproot standardness rules in mempool_accept

ReviewabilityReviewsAgreementSize
#35882 jeanpablojp test: cover Taproot standardness rules in mempool_acceptReady0Crickets0 + 17 tests
  • Explicitly test Taproot standardness rejections for oversized stack items and empty control blocks
  • Prevents regressions where missing checks cause node segfaults undetected by existing test suites
  • Ready to review
  • No review verdicts recorded
  • No formal reviews or approvals yet
  • Questioned whether existing taproot tests already covered these mutant cases (sedited)
  • Author clarified coverage gaps and dropped a redundant test case
  • 0 lines added or modified outside tests
  • 17 lines added or modified in tests
  • 0 lines removed in total
  • 1 files, 1 commits
Mempool and policy#11 of 14
P3 · test coverage
  • P3 because it adds targeted functional test coverage without changing node behavior
  • Prevents silent regressions in control block checks that previously risked node crashes
Ranking notes
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 functional test cases in mempool_accept.py for two Taproot standardness policy rules checked in IsWitnessStandard: tapscript stack items exceeding 80 bytes and empty control blocks. The test mutates a reference P2TR script-path spend and asserts that testmempoolaccept rejects the transactions with bad-witness-nonstandard.

Problem: Taproot standardness rejection rules in IsWitnessStandard lacked explicit assertions for bad-witness-nonstandard. Removing the empty control block check caused no failures in existing tests because no test runner generated an empty control block, while removing the 80-byte check only intermittently caused test failures depending on test seeds.

Evidence of importance:

  • Author notes in PR description: 'With the 80-byte check removed, feature_taproot.py failed on only 6 of the ten seeds I tried, and with the empty control block check removed it can't fail at all, since none of its spenders has an empty control block. Without that check the node segfaults reading control_block[0].'

Risk of a mistake: low (tests). Only adds test assertions to a Python functional test script.

Changed since the previous assessment: Dropped the test case for spends with an annex, retaining only the test cases for tapscript stack elements over 80 bytes and empty control blocks.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: sedited to review the author's explanation 2026-10-27
  • reviewer: sedited to respond to author's explanation 2026-10-27

Agreement

Crickets

no substantive comment either way

Thread notes: Re-classified sedited's usefulness objection as a suggestion per current rules. The author answered by explaining the partial/missing coverage in feature_taproot.py and dropping the redundant annex case. / Re-classified sedited's item from objection to suggestion per schema rules; marked answered by author explanation in c:6453470823.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
sedited (member)suggestion, usefulnessexplain why these tests are needed if the mutants are already killed by feature_taproot.pyanswered · author explained the coverage gaps for the 80-byte and empty control block checks and dropped the annex caseansweredyes2026-09-20: 'These mutants already seem to be killed by feature_taproot.py. Why add these here?'
Status from: 2026-10-27: jeanpablojp: 'You're right about the annex. `feature_taproot.py` kills that mutant on all ten seeds I tried, but the 80-byte one on only 6'

Participants:

  • sedited (member, question, 1 statements 2026-09-20 to 2026-09-20): asked why tests are being added if mutants are already killed in feature_taproot.py

Files

17 lines under test/bench/ci.

  • test/functional/mempool_accept.py +17/-0

Card

The short text the ranking pass compares across PRs.

Adds functional test cases in mempool_accept.py verifying that transactions with Taproot stack elements over 80 bytes or empty control blocks are rejected with bad-witness-nonstandard. Previously, no test verified the specific reject reason for either rule, and empty control blocks were not exercised at all. This ensures regressions in mempool standardness policy checks for Taproot are caught deterministically in CI.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-29T01:22, confidence high, input hash ceefa9850d3f5ba8
thread read through 2026-10-27 (2 statements), head b46c4baefa
labels: Tests

Categories

Mempool and policy

P3 · test coverage

Adds explicit test coverage for two Taproot standardness checks in IsWitnessStandard that previously lacked regression coverage. As the author notes, removing the empty control block check previously resulted in no test failures even though 'Without that check the node segfaults reading control_block[0].'

Membership: Adds functional test cases in test/functional/mempool_accept.py verifying IsWitnessStandard Taproot policy rejections.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0