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

pull KY-U wants to merge 2 commits into bitcoin:master from KY-U:2026-10-checktxinputs changing 1 files +18 −9
  1. KY-U commented at 5:45 PM on October 28, 2026: contributor

    In Consensus::CheckTxInputs, the inputs are invalid if, for every input coin, either coin.out.nValue or nValueIn is out of range. However, the current checktxinputs_invalid_transactions_test only covers cases with one input, so the sum nValueIn is always equal to coin.out.nValue, leaving the following mutant alive.

    --- a/src/consensus/tx_verify.cpp
    +++ b/src/consensus/tx_verify.cpp
    @@ -189,7 +189,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state,
     
             // Check for negative or overflow input values
             nValueIn += coin.out.nValue;
    -        if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) {
    +        if (!MoneyRange(coin.out.nValue) && !MoneyRange(nValueIn)) {
                 return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputvalues-outofrange");
             }
         }
    

    https://github.com/bitcoin/bitcoin/pull/36686/commits/f8c6cd84e08043f206ca338d0ba7e4dc8bef5214 adds support for multiple inputs in the test helper.

    https://github.com/bitcoin/bitcoin/pull/36686/commits/133714d1695bc40277b213afd252c3e320cac73e adds a test case with multiple valid inputs, whose sum exceed MAX_MONEY, while also having nValueIn != coin.out.nValue, killing the mutant.

  2. DrahtBot added the label Tests on Oct 28, 2026
  3. DrahtBot commented at 5:45 PM on October 28, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. test: support multiple inputs in CheckTxInputs test helper
    Accept a list of input values and create a distinct coin and prevout for each one. Keep the existing test cases as single-input transactions with no change in behaviour.
    f8c6cd84e0
  5. KY-U force-pushed on Oct 28, 2026
  6. DrahtBot added the label CI failed on Oct 28, 2026
  7. DrahtBot commented at 6:16 PM on October 28, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/40738712606/job/122994163824</sub> <sub>LLM reason (✨ experimental): CI failed the commit_msg lint check because both commits lack a blank line between the subject and body.</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  8. DrahtBot removed the label CI failed on Oct 28, 2026
  9. test: cover cumulative input value range in CheckTxInputs
    Add invalid test case with two inputs that are individually within range, but exceed MAX_MONEY when summed.
    133714d169
  10. in src/test/transaction_tests.cpp:1178 in 5d93087c6a
    1174 | +                  /*coinbase=*/false,
    1175 | +                  /*spend_height=*/2,
    1176 | +                  TxValidationResult::TX_CONSENSUS, /*expected_reason=*/"bad-txns-inputvalues-outofrange");
    1177 | +
    1178 | +    check_invalid(/*input_values=*/{MAX_MONEY, 1},
    1179 |                    /*output_value=*/0,
    


    ValeraFinebits commented at 1:30 PM on October 29, 2026:

    Optional: use output_value = 1 here so the || -> && mutant passes the fee check and incorrectly returns true. The current case already catches the mutant via the reject reason, this would catch erroneous acceptance directly.

                      /*output_value=*/1,
    

    ValeraFinebits commented at 1:35 PM on October 29, 2026:

    Or we could keep the zero-output case and add another with output_value = 1. The existing case also checks that the input value error takes precedence over the fee error.


    KY-U commented at 3:14 PM on October 29, 2026:

    Changed the output value to 1 as suggested in the first comment. I'd prefer to change the existing case rather than add another one in order to keep new test cases to a minimum while exercising the erroneous acceptance and killing the mutant. Thanks for the suggestions!

  11. KY-U force-pushed on Oct 29, 2026
Labels

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-30 09:40 UTC