From aa87aae14f9eee79e3a0fb9c0f5ff3eaa97433e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 31 Jan 2026 21:02:19 +0100 Subject: [PATCH 1/4] consensus/test: add `MoneyRange` unit tests for `CheckTxInputs` Add minimal unit tests exercising `Consensus::CheckTxInputs` reject reasons for coinbase maturity (`bad-txns-premature-spend-of-coinbase`), input value range failures (`bad-txns-inputvalues-outofrange`), and for `nValueIn < value_out` (`bad-txns-in-belowout`). Inspired by b-c-cov coverage reports: * "bad-txns-premature-spend-of-coinbase" - https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L180 * "bad-txns-inputvalues-outofrange" - https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L187 * "bad-txns-in-belowout" - https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L193 --- src/test/transaction_tests.cpp | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 3919f226d69..0f175c7d6f3 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -1114,6 +1115,46 @@ 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"); +} + /** Sanity check the return value of SpendsNonAnchorWitnessProg for various output types. */ BOOST_AUTO_TEST_CASE(spends_witness_prog) { From 232a2bce90a96720f5c8d31413f1d14b4c9d90f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 31 Jan 2026 21:03:16 +0100 Subject: [PATCH 2/4] consensus/test: add out-of-range output unit tests for `CTransaction::GetValueOut` Inspired by b-c-cov coverage reports: * "GetValueOut: value out of range" - https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/primitives/transaction.cpp.gcov.html#L103 --- src/test/transaction_tests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 0f175c7d6f3..a7a46e4f07c 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -1155,6 +1155,15 @@ BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test) 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) { From 82ef92c8d006b3f5c3baaf00e5f8200d289d85d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 31 Jan 2026 19:20:20 +0100 Subject: [PATCH 3/4] consensus/doc: explain unreachable `bad-txns-fee-outofrange` check After the previous conditions were validated, document why `bad-txns-fee-outofrange` is unreachable once `nValueIn` and `value_out` are `MoneyRange` and `nValueIn >= value_out`. Although unreachable, keep the check itself in place (instead of removing) as it's part of consensus-critical code; the comment serves as a proof for future refactors. Inspired by b-c-cov coverage reports: * "bad-txns-fee-outofrange" - https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L200 --- src/consensus/tx_verify.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 00022a33dd3..ec612a55e6f 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -197,6 +197,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"); } From 8c03318387f6b3d1520a539f426b300bae316fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 2 Feb 2026 19:31:28 +0100 Subject: [PATCH 4/4] consensus/doc: explain `GetValueOut()` precondition `Consensus::CheckTxInputs` calls `tx.GetValueOut()` and assumes output-range checks already ran. Document the mempool and block validation call paths where this is guaranteed. Co-authored-by: Antoine Poinsot --- src/consensus/tx_verify.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index ec612a55e6f..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",