diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 00022a33dd3..4efed70fd41 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -188,6 +188,10 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, } } + // `tx.GetValueOut()` won't throw in validation paths because output-range checks run first + // (`bad-txns-vout-negative`, `bad-txns-vout-toolarge`, `bad-txns-txouttotal-toolarge`): + // * `MemPoolAccept::PreChecks`: `CheckTransaction()` is called before this method; + // * `Chainstate::ConnectBlock`: `CheckTransaction()` is called via `CheckBlock()` before this method. const CAmount value_out = tx.GetValueOut(); if (nValueIn < value_out) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-in-belowout", @@ -197,6 +201,11 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, // Tally transaction fees const CAmount txfee_aux = nValueIn - value_out; if (!MoneyRange(txfee_aux)) { + // Unreachable, given the following preconditions: + // * `value_out` comes from `tx.GetValueOut()`, which throws unless `MoneyRange(value_out)` and asserts `MoneyRange(nValueOut)` on return. + // * `MoneyRange(nValueIn)` was enforced in the input loop. + // * `nValueIn < value_out` was handled above, so `nValueIn >= value_out` here (and `txfee_aux >= 0`). + // Therefore `0 <= txfee_aux = nValueIn - value_out <= nValueIn <= MAX_MONEY`. return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-fee-outofrange"); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index d0be15a00db..e722e724c9f 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -1113,6 +1114,55 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); } +BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test) +{ + auto check_invalid{[](CAmount input_value, CAmount output_value, bool coinbase, int spend_height, TxValidationResult expected_result, std::string_view expected_reason) { + CCoinsView coins_dummy; + CCoinsViewCache inputs(&coins_dummy); + + const COutPoint prevout{Txid::FromUint256(uint256::ONE), 0}; + inputs.AddCoin(prevout, Coin{{input_value, CScript() << OP_TRUE}, /*nHeightIn=*/1, coinbase}, /*possible_overwrite=*/false); + + CMutableTransaction mtx; + mtx.vin.emplace_back(prevout); + mtx.vout.emplace_back(output_value, CScript() << OP_TRUE); + + TxValidationState state; + CAmount txfee{0}; + BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction{mtx}, state, inputs, spend_height, txfee)); + BOOST_CHECK(state.IsInvalid()); + BOOST_CHECK_EQUAL(state.GetResult(), expected_result); + BOOST_CHECK_EQUAL(state.GetRejectReason(), expected_reason); + }}; + + check_invalid(/*input_value=*/MAX_MONEY + 1, + /*output_value=*/0, + /*coinbase=*/false, + /*spend_height=*/2, + TxValidationResult::TX_CONSENSUS, /*expected_reason=*/"bad-txns-inputvalues-outofrange"); + + check_invalid(/*input_value=*/1 * COIN, + /*output_value=*/2 * COIN, + /*coinbase=*/false, + /*spend_height=*/2, + TxValidationResult::TX_CONSENSUS, /*expected_reason=*/"bad-txns-in-belowout"); + + check_invalid(/*input_value=*/1 * COIN, + /*output_value=*/0, + /*coinbase=*/true, + /*spend_height=*/COINBASE_MATURITY, + TxValidationResult::TX_PREMATURE_SPEND, /*expected_reason=*/"bad-txns-premature-spend-of-coinbase"); +} + +BOOST_AUTO_TEST_CASE(getvalueout_out_of_range_throws) +{ + CMutableTransaction mtx; + mtx.vout.emplace_back(MAX_MONEY + 1, CScript() << OP_TRUE); + + const CTransaction tx{mtx}; + BOOST_CHECK_EXCEPTION(tx.GetValueOut(), std::runtime_error, HasReason("GetValueOut: value out of range")); +} + /** Sanity check the return value of SpendsNonAnchorWitnessProg for various output types. */ BOOST_AUTO_TEST_CASE(spends_witness_prog) {