#36686 test: cover cumulative input value range in CheckTxInput tests.

ReviewabilityReviewsAgreementSize
#36686 KY-U test: cover cumulative input value range in CheckTxInput tests.Ready1Strong0 + 18 tests
  • Ensure consensus tests verify transactions exceeding total money limits across multiple inputs are rejected
  • Prevents mutations or logic bugs in cumulative input value validation from going undetected
  • Ready to review
  • Support verified by confirming the surviving mutation is now killed (mercie-ux)
  • Open suggestion to add a test case covering the per-coin check as well (mercie-ux)
  • 0 lines added or modified outside tests
  • 18 lines added or modified in tests
  • 9 lines removed in total
  • 1 files, 2 commits
Test infrastructure#34 of 54
P3 · test coverage
  • P3 because it closes a consensus unit test gap identified by mutation testing
  • Catches bugs where cumulative input limits could be bypassed or incorrectly evaluated
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

Extends the `checktxinputs_invalid_transactions_test` helper in `transaction_tests.cpp` to support transactions with multiple inputs. Adds a test case where two inputs are individually valid under `MoneyRange` but their sum exceeds `MAX_MONEY`. This verifies that `Consensus::CheckTxInputs` rejects transactions whose cumulative input values exceed consensus limits.

Problem: Existing unit tests for `Consensus::CheckTxInputs` only tested single-input transactions where `coin.out.nValue == nValueIn`. Consequently, mutations or bugs in the cumulative value check `!MoneyRange(nValueIn)` (such as switching `||` to `&&` in the out-of-range condition) passed unit tests undetected.

Evidence of importance:

  • PR description points to MutantHub mutant #275 showing `if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn))` mutated to `&&` survived the test suite.

Risk of a mistake: low (tests). Touches only unit tests in transaction_tests.cpp.

Changed since the previous assessment: No changes; diff matches the previously described implementation.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: mercie-ux's optional suggestion regarding adding a case for the per-coin check 2026-11-01

Agreement

Strong

substantive support, no open objection (mercie-ux)

Thread notes: Added support ACK and suggestion for per-coin check coverage from mercie-ux.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ValeraFinebits (none)suggestion, approachuse output_value = 1 so the mutant passes the fee check and incorrectly returns true, catching erroneous acceptance directlyanswered · author adopted the suggestion to change output_value to 1answeredyes2026-10-29: 'Optional: use `output_value = 1` here so the `||` -> `&&` mutant passes the fee check and incorrectly returns `true`.'
Status from: 2026-10-29: KY-U: 'Changed the output value to 1 as suggested in the first comment.'
ValeraFinebits (none)suggestion, approachkeep the zero-output case and add a separate case with output_value = 1 to verify error precedenceanswered · author preferred modifying the existing case to keep new tests minimalansweredyes2026-10-29: 'Or we could keep the zero-output case and add another with `output_value = 1`.'
Status from: 2026-10-29: KY-U: 'I'd prefer to change the existing case rather than add another one'
mercie-ux (contributor)suggestion, approachadd a test case for the per-coin checkopenansweredno2026-11-01: 'Could add a case for the per-coin check too, removing it currently doesn't break any test.'

Support:

  • mercie-ux (contributor) (ACK, 2026-11-01) [tested]: Verified the mutant is killed

Participants:

  • ValeraFinebits (none, neutral, 2 statements 2026-10-29 to 2026-10-29): suggested setting output_value = 1 or adding a second case to check erroneous acceptance directly
  • mercie-ux (contributor, support, 2 statements 2026-11-01 to 2026-11-01): ACKed after verifying the mutant is killed; optionally suggested covering per-coin check

Files

27 lines under test/bench/ci.

  • src/test/transaction_tests.cpp +18/-9

Card

The short text the ranking pass compares across PRs.

This PR updates CheckTxInputs unit tests to support multiple inputs and adds a test case where individually valid input amounts exceed MAX_MONEY when summed. Previously, unit tests only checked single-input transactions, leaving a mutation that changes the out-of-range check from logical OR to logical AND undetected. The change is restricted to transaction unit tests.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-02T01:11, confidence high, input hash a9119627f87e4726
thread read through 2026-11-01 (5 statements), head 133714d169
labels: Tests

Categories

Test infrastructure

P3 · test coverage

Extends unit test coverage for `Consensus::CheckTxInputs` to ensure transactions with cumulative input values exceeding `MAX_MONEY` are rejected. As noted in the description, mutant analysis revealed that changing `||` to `&&` in the `MoneyRange` check survived the test suite, making this a worthwhile addition to consensus test coverage.

Membership: Touches `src/test/transaction_tests.cpp` with the `Tests` label, extending a test helper and unit test coverage for consensus validation.

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