test: Add block validation unit tests #35000

ismaelsadeeq commented at 1:42 AM on April 4, 2026: member

Motivation

Recent work on exposing the Bitcoin kernel C header API, the usage of API, has increased demand for certain changes to block validation, e,g an ongoing effort to modularise block validation #32317. However, validation, especially block, is very critical and changes to it demand the utmost scrutiny and a high bar. Which is most of the time slow and careful code review and testing. As noticeable during review of #32317, a subtle issue (https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2819423976) went unnoticed for some time, which suggests that the current test coverage around block validation paths is not sufficiently comprehensive. We have a comprehensive block validation functional test but that is not enough because Block validation today is exercised through multiple entry points, such as TestBlockValidity, ProcessNewBlock, ConnectTip, VerifyDB, so having a structured test suite that targets malformed blocks across those validation paths/simulated ones, and testing invariants to ensure subtle regressions do not slip through review is important.

This PR aims to improve confidence in validation changes by introducing a structured set of unit tests that target specific consensus failure modes, The goal is not to fully de-risk validation changes, but to significantly improve the likelihood of catching subtle bugs early in CI.

Changes

This pr introduces block validation unit tests that are split into header, stateless, contextual, and stateful validation cases.
The header tests exercise failures related purely to block headers by constructing otherwise valid blocks and mutating header fields, ensuring that header-level checks fail with the expected validation results and reject reasons.
The stateless tests include malformed block structures such as missing or multiple coinbase transactions, invalid merkle roots, and duplicate transactions. They also cover transaction-level structural issues such as empty inputs or outputs, invalid output values including negative or overflowing amounts, and duplicate inputs within a transaction. Each test constructs a minimally valid block and then applies targeted mutations to trigger specific consensus failures, verifying both the validation result and the exact reject reason string.
The contextual tests cover rules that depend on chain context. These include enforcing correct coinbase height encoding, detecting coinbase overpayment, preventing premature spending of coinbase outputs, and checking transaction finality conditions. These tests rely on the active chain state and ensure that contextual validation correctly rejects blocks that violate consensus rules tied to block height or historical state.
The stateful tests construct blocks that spend from UTXO sets and then introduce failures such as missing or already spent inputs, input value overflows, and cases where total inputs are less than total outputs. Script validation failures are also covered by creating real spends that fail script execution under consensus rules. These tests verify that stateful validation correctly enforces value conservation and script correctness.
To support these tests, reusable helpers are introduced in test/util/block_validity.h. These helpers centralize the invocation of TestBlockValidity and standardize assertions on validation results, reject reasons, and debug messages. They also provide utilities for constructing spendable UTXOs, which simplifies the creation of stateful test scenarios and improves readability and consistency across tests. The aim is to also add other block validation paths in this util that can be easily invoked by this mutated blocks.
These mutated blocks are passed to TestBlockValidity, ProcessNewBlock and ConnectBlock

Open questions

The unit tests added here cover a substantial portion of consensus rules and some soft fork logic, they are not exhaustive. What is considered sufficient?.

DrahtBot added the label Tests on Apr 4, 2026

DrahtBot commented at 1:43 AM on April 4, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35000.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	sedited
Stale ACK	w0xlt

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

SignatureHash(multisig, mtx, 0, SIGHASH_ALL, 1 * COIN, SigVersion::BASE) in src/test/validation_block_stateful_tests.cpp
SignatureHash(p2wpkh_script, mtx, 0, SIGHASH_ALL, wrong_value, SigVersion::WITNESS_V0) in src/test/validation_block_stateful_tests.cpp

2026-06-05 16:54:23

stratospher commented at 6:55 AM on April 4, 2026: contributor

have you seen #34651 btw? from a quick skim, this fuzz test looks similar. it uses a wrapper around ConnectBlock instead of calling it directly. or am I missing something?

ismaelsadeeq commented at 1:57 PM on April 7, 2026: member

have you seen #34651 btw? from a quick skim, this fuzz test looks similar. it uses a wrapper around ConnectBlock instead of calling it directly. Or am I missing something?

Thanks for pointing that out. I wasn’t aware of it. #34651 and the second commit in this PR, e9115735a45289a783b561fe482ab1540b03d076 have a lot in common: both generate a valid block and then mutate its contents.

However, there are some differences. My focus here is on e2e block validity testing, whereas that PR only tests ConnectBlock, which skips ContextualCheckBlock and ContextualCheckBlockHeader.

If we expose something via the kernel (e.g., block validation with external coins), we already have a framework in place for A/B testing. i.e, if TestBlockValidityWithSpentTxOuts is given the same coins and uses a valid known chain index, the output should correlate; otherwise, they may diverge, that kind of thing.

But it would also be useful to have shared utilities for some of the things we have in common, so that we don't repeat ourselves.

Bortlesboat commented at 5:34 PM on April 7, 2026: none

Built and ran all four new test suites locally (header, stateless, contextual, stateful), 49/49 pass. Two things inline.

in src/test/validation_block_stateful_tests.cpp:133 in e9115735a4

 128 | +    // The sum of input values in a transaction cannot exceed the maximum possible supply (overflow check).
 129 | +    CBlock block = MakeBlock();
 130 | +    CScript script = CScript() << OP_TRUE;
 131 | +    auto outpoint1 = AddCoin(script, MAX_MONEY);
 132 | +    auto outpoint2 = AddCoin(script, MAX_MONEY);
 133 | +    if (!outpoint1.has_value() && !outpoint2.has_value()) return;

Bortlesboat commented at 5:34 PM on April 7, 2026:

This uses && so it only bails if both AddCoin calls fail. If just one fails you get a bad_optional_access from .value(). The rest of the file uses if (!outpoint) return; for each one individually. Should this be ||?

ismaelsadeeq commented at 5:50 PM on April 7, 2026:

Indeed, I was using if (outpoint1.has_value() && outpoint2.has_value()) {

And then, executing the test code inside that scope, I changed my mind and just bailed out, but didn't update the && to |.

Good catch, I will update.

in src/test/fuzz/validation_block_validity.cpp:1 in e9115735a4

   0 | @@ -0,0 +1,245 @@
   1 | +// Copyright (c) present The Bitcoin Core developers

Bortlesboat commented at 5:34 PM on April 7, 2026:

nit: "present" left over from the old copyright format. The other four files have the right header.

ismaelsadeeq force-pushed on Apr 7, 2026

ismaelsadeeq commented at 7:20 PM on April 7, 2026: member

Forced pushed from e9115735a45289a783b561fe482ab1540b03d076 to f1d2302ecc9abe72f15197f5ce40ed0ed46686a9 compare

Fixed #35000 (review)
Fixed #35000 (review)
use emplace back instead of pushbacks

ismaelsadeeq renamed this:
~~test: Add block validation unit tests + fuzz target for `TestBlockValidity`~~
test: Add block validation unit tests + fuzz harness for `TestBlockValidity`
on Apr 10, 2026

marcofleon commented at 2:24 PM on April 10, 2026: contributor

I agree that the fuzz test looks a bit redundant with #34651. And I'm not sure it's worth having this PR's harness only as a way to cover the contextual checks.

Here is some validation coverage to compare:

validation_block_validity from this PR

connect_block from 34651

full coverage from all fuzz tests on master

Looking at the coverage, connect_block reaches a lot of the same lines as validation_block_validity does wrt to the ConnectBlock path and it also provides more diverse inputs. And then the contextual checks seem already pretty well covered by our existing fuzz tests (assuming we have strong oracles scattered throughout validation). I think it could make sense to drop the fuzz target here and focus on the unit tests, which look well-written to me.

ismaelsadeeq commented at 3:28 PM on April 10, 2026: member

Thanks for taking a look at this @marcofleon To reiterate, when I pushed this, I did know about #34651. My rationale is explained in the description and this comment #35000 (comment)

But I agree, it will be better to spend more effort on #34651 since it has received a couple of review rounds already. I will review that PR and make some suggestions that will make #35000 (comment) achievable there.

In the meantime, I will remove the fuzz commit here, even if we move forward in this direction, you can argue that the fuzz test deserve it's own PR.

ismaelsadeeq force-pushed on Apr 10, 2026

ismaelsadeeq renamed this:
~~test: Add block validation unit tests + fuzz harness for `TestBlockValidity`~~
test: Add block validation unit tests
on Apr 10, 2026

ismaelsadeeq commented at 4:14 PM on April 10, 2026: member

Forced pushed from https://github.com/bitcoin/bitcoin/commit/f1d2302ecc9abe72f15197f5ce40ed0ed46686a9 to https://github.com/bitcoin/bitcoin/commit/26b1e0b554dcb9b6b28ef33e69eefc9c4e34fbdb

Pruned the fuzz harness added here in favour of the one in #34651

w0xlt commented at 6:57 PM on April 12, 2026: contributor

Concept ACK

in src/test/validation_block_header_tests.cpp:26 in 26b1e0b554

  21 | +{
  22 | +    BOOST_CHECK_MESSAGE(state.IsInvalid(), "Expected block to be invalid but it is valid");
  23 | +    BOOST_CHECK(state.GetResult() == expected_result);
  24 | +    BOOST_CHECK_EQUAL(state.GetRejectReason(), expected_reason);
  25 | +    BOOST_CHECK_EQUAL(state.GetDebugMessage(), expected_debug_message);
  26 | +}

sedited commented at 10:01 AM on April 22, 2026:

Why not put these into the new block_validity util module?

ismaelsadeeq commented at 12:36 PM on April 23, 2026:

The first attempt used that approach, but I was using the same setup in the fuzz harness, which does not want boost headers included. Now that the fuzz harness is gone and we will likely use setup in #34651 for that it's okay to move the util there.

ismaelsadeeq commented at 6:56 PM on April 28, 2026:

The error I initially saw persists because the fuzz binary includes the test util library, which in turn includes this block_validity.cpp which includes the boost headers. As a result, CI fails https://github.com/bitcoin/bitcoin/actions/runs/24834952407/job/72771891921.

I contemplated extracting CheckBlockValid and CheckBlockInvalid from the block_validity util, but not sure where it belongs, common?

In the meantime, I fixed this by not adding the include to the util library and instead including it in test_bitcoin. However, I’m not happy with this fix. It feels off that a file inside the util directory is not part of the test util library.

wdyt?

sedited commented at 7:10 PM on April 28, 2026:

Oh, right. Yeah, in that case duplication seems like the only sane way to do this. Or just consolidate all the tests into a single module.

sedited commented at 10:26 AM on April 22, 2026: contributor

Approach ACK

This looks like a good place to start - having all these cases is valuable. I think the current scope is fine, but I would call these TestBlockValidity unit tests, and not "block validation". These skip some checks that run in the Accept* functions and don't necessarily reflect the validation steps executed by the actual hot validation path. As far as I can tell the tests here don't exercise BLOCK_MISSING_PREV and BLOCK_INVALID_PREV.

Can we add the same checks through ProcessNewBlock by subscribing to the BlockChecked validation interface event?

As an addition I would also be interested in calling ConnectBlock directly with these invalid blocks. I think that would require some re-plumbing though to catch the various errors and validation states.

The distinction into the separate validation classes makes sense to me. Instead of splitting them up into separate tests I am wondering if we could tag them with an enum and then pass them to various entry points of validation (i.e. TestBlockValidity, ProcessNewBlock, ConnectBlock). The tests can then react to their failures based on their assigned validation variant, i.e. for contextual checks ConnectBlock does not expect failures, but other validation functions do.

ismaelsadeeq force-pushed on Apr 23, 2026

DrahtBot added the label CI failed on Apr 23, 2026

ismaelsadeeq force-pushed on Apr 28, 2026

ismaelsadeeq commented at 6:57 PM on April 28, 2026: member

I appreciate the review so far. I will spend some time to complete this by incorporating more tests beyond TestBlockValidity. In the meantime, I will move this to draft.

ismaelsadeeq marked this as a draft on Apr 28, 2026

DrahtBot removed the label CI failed on Apr 28, 2026

ismaelsadeeq force-pushed on May 27, 2026

DrahtBot added the label CI failed on May 27, 2026

DrahtBot commented at 3:26 PM on May 27, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/26516809572/job/78098993428 LLM reason (✨ experimental): CI failed due to a C++ build error in test_bitcoin (block_validity.h/cpp): node::BlockAssembler has no Options type (leading to multiple compiler errors).

<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>

ismaelsadeeq force-pushed on May 28, 2026

DrahtBot removed the label CI failed on May 28, 2026

ismaelsadeeq marked this as ready for review on May 28, 2026

ismaelsadeeq commented at 5:01 PM on May 28, 2026: member

I forced pushed from 8e556cdf59db04bb604554e7fb9b8a2ce345f2a3 to c9d6eb5d9d1db6d6c0ffc9096932d40b121b2875 compare_diff

Rebased to fix merge conflict
#35000 (review) fixed now

These skip some checks that run in the Accept* functions and don't necessarily reflect the validation steps executed by the actual hot validation path. As far as I can tell the tests here don't exercise BLOCK_MISSING_PREV and BLOCK_INVALID_PREV.

These tests are added now

Can we add the same checks through ProcessNewBlock by subscribing to the BlockChecked validation interface event?

Done in the recent push.

Instead of splitting them up into separate tests I am wondering if we could tag them with an enum and then pass them to various entry points of validation (i.e. TestBlockValidity, ProcessNewBlock, ConnectBlock). The tests can then react to their failures based on their assigned validation variant, i.e. for contextual checks ConnectBlock does not expect failures, but other validation functions do.

Hmm, won't this be more involved? I would like to keep it simple as it is currently, if you don't feel strongly about this.

Also, I limit this to just TestBlockValidity, ProcessNewBlock and ConnectBlock. I think this suffices for now; The block validation parts VerifyDB and ConnectTip just call ConnectBlock anyway, so independent calls of connectBlock added recently due to @sedited suggestion should cover them.

in src/test/validation_block_contextual_tests.cpp:74 in c9d6eb5d9d outdated

  69 | +
  70 | +BOOST_AUTO_TEST_CASE(tbv_bad_txns_nonfinal)
  71 | +{
  72 | +    // Transactions with time-locks (nLockTime) that are not yet satisfied are rejected.
  73 | +    CBlock block = MakeBlock();
  74 | +    const auto outpoint = AddCoin(CScript() << OP_TRUE, 50 * COIN);

w0xlt commented at 12:20 AM on June 4, 2026:

nit: BOOST_REQUIRE(outpoint) seems to be more appropriate than if (!outpoint) return because the outpoint is required for the rest of the test. If helper setup fails, the test will fail immediately instead of silently passing by returning early.

diff --git a/src/test/validation_block_contextual_tests.cpp b/src/test/validation_block_contextual_tests.cpp
index 380218b00e..f4c41a9464 100644
--- a/src/test/validation_block_contextual_tests.cpp
+++ b/src/test/validation_block_contextual_tests.cpp
@@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_nonfinal)
     // Transactions with time-locks (nLockTime) that are not yet satisfied are rejected.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE, 50 * COIN);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip113_locktime_uses_mtp)
     CBlock block = MakeBlock();
     const int64_t mtp = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip()->GetMedianTimePast());
     const auto outpoint = AddCoin(CScript() << OP_TRUE, 50 * COIN);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(tbv_bad_blk_weight)
     const int num_overweight_txns{4};
     for (int tx_idx = 0; tx_idx < num_overweight_txns; ++tx_idx) {
         const auto outpoint = AddCoin(p2tr, 50 * COIN);
-        if (!outpoint) return;
+        BOOST_REQUIRE(outpoint);
         CMutableTransaction mtx;
         mtx.vin.resize(1);
         mtx.vin[0].prevout = *outpoint;
diff --git a/src/test/validation_block_stateful_tests.cpp b/src/test/validation_block_stateful_tests.cpp
index 95eb6285e2..13a42dad92 100644
--- a/src/test/validation_block_stateful_tests.cpp
+++ b/src/test/validation_block_stateful_tests.cpp
@@ -24,7 +24,8 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_accumulated_fee_outofrange)
     CBlock block = MakeBlock();
     const auto outpoint1 = AddCoin(CScript(), MAX_MONEY);
     const auto outpoint2 = AddCoin(CScript(), 1);
-    if (!outpoint1 || !outpoint2) return;
+    BOOST_REQUIRE(outpoint1);
+    BOOST_REQUIRE(outpoint2);
     CMutableTransaction mtx, mtx2;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint1;
@@ -115,7 +116,8 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_inputs_sum_overflow)
     CScript script = CScript() << OP_TRUE;
     auto outpoint1 = AddCoin(script, MAX_MONEY);
     auto outpoint2 = AddCoin(script, MAX_MONEY);
-    if (!outpoint1 || !outpoint2) return;
+    BOOST_REQUIRE(outpoint1);
+    BOOST_REQUIRE(outpoint2);
     CMutableTransaction mtx;
     mtx.vin.resize(2);
     mtx.vin[0].prevout = outpoint1.value();
@@ -138,7 +140,7 @@ BOOST_AUTO_TEST_CASE(tbv_block_script_verify_flag_failed)
     CBlock block = MakeBlock();
     const CScript p2pk = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
     const auto outpoint = AddCoin(p2pk, 50 * COIN);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -160,7 +162,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip147_null_dummy)
     CBlock block = MakeBlock();
     CScript multisig = CScript() << 1 << ToByteVector(coinbaseKey.GetPubKey()) << 1 << OP_CHECKMULTISIG;
     const auto outpoint = AddCoin(multisig);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -186,7 +188,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip66_non_der_sig)
     CBlock block = MakeBlock();
     CScript p2pkh = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));
     const auto outpoint = AddCoin(p2pkh);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -218,7 +220,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip65_cltv_violation)
     const int lock_height = WITH_LOCK(cs_main, return m_chainstate.m_chain.Height()) + cltv_height_offset;
     const CScript cltv_script = CScript() << lock_height << OP_CHECKLOCKTIMEVERIFY << OP_DROP << OP_TRUE;
     const auto outpoint = AddCoin(cltv_script);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -245,7 +247,7 @@ BOOST_AUTO_TEST_CASE(tbv_taproot_invalid_sig)
     WitnessV1Taproot taproot = builder.GetOutput();
     CScript p2tr_script = GetScriptForDestination(taproot);
     const auto outpoint = AddCoin(p2tr_script);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -278,7 +280,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip112_csv_violation)
     uint32_t csv_block_offset = 10;
     const CScript csv_script = CScript() << csv_block_offset << OP_CHECKSEQUENCEVERIFY << OP_DROP << OP_TRUE;
     const auto outpoint = AddCoin(csv_script);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.version = 2; // CSV requires nVersion >= 2
     mtx.vin.resize(1);
@@ -303,7 +305,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip16_p2sh_invalid_redeem_script)
     const CScript redeem_script = CScript() << OP_FALSE;
     const CScript p2sh = GetScriptForDestination(ScriptHash(redeem_script));
     const auto outpoint = AddCoin(p2sh);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -326,7 +328,7 @@ BOOST_AUTO_TEST_CASE(tbv_segwit_v0_invalid_witness_script)
     const CScript witness_script = CScript() << OP_FALSE;
     const CScript p2wsh = GetScriptForDestination(WitnessV0ScriptHash(witness_script));
     const auto outpoint = AddCoin(p2wsh);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -357,7 +359,7 @@ BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
     WitnessV1Taproot taproot = builder.GetOutput();
     const CScript p2tr_script = GetScriptForDestination(taproot);
     const auto outpoint = AddCoin(p2tr_script);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     // Build the control block for the script path
     const auto spend_data = builder.GetSpendData();
     const auto& control_block = *spend_data.scripts.at(
@@ -390,7 +392,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip143_wrong_amount_in_sighash)
     const CScript p2wpkh = GetScriptForDestination(WitnessV0KeyHash(coinbaseKey.GetPubKey()));
     const CAmount actual_value = 1 * COIN;
     const auto outpoint = AddCoin(p2wpkh, actual_value);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -418,7 +420,7 @@ BOOST_AUTO_TEST_CASE(tbv_empty_scriptsig)
     // A non-coinbase transaction with an empty scriptSig spending OP_TRUE is valid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
 
     CMutableTransaction mtx;
     mtx.vin.resize(1);
@@ -439,7 +441,7 @@ BOOST_AUTO_TEST_CASE(tbv_scriptsig_non_push)
     // A scriptSig with non-push opcodes is valid in a block (consensus) even if non-standard.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE);
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
@@ -459,7 +461,7 @@ BOOST_AUTO_TEST_CASE(tbv_double_spend_same_block)
     // A block containing two transactions spending the same UTXO is invalid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin();
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     for (int tx_idx = 0; tx_idx < 2; ++tx_idx) {
         CMutableTransaction mtx;
         mtx.vin.resize(1);
@@ -483,7 +485,7 @@ BOOST_AUTO_TEST_CASE(tbv_zero_value_output)
     // A transaction with a zero-value output is consensus-valid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin();
-    if (!outpoint) return;
+    BOOST_REQUIRE(outpoint);
     CMutableTransaction mtx;
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;

</details>

Alternatively, this can be centralized in AddCoin() itself by making it return COutPoint directly and fail internally if no unused outpoint can be found.

diff --git a/src/test/util/block_validity.cpp b/src/test/util/block_validity.cpp
index 289339d81c..dfc314b4ae 100644
--- a/src/test/util/block_validity.cpp
+++ b/src/test/util/block_validity.cpp
@@ -29,7 +29,7 @@ BlockValidationState ValidationBlockValidityTestingSetup::TestValidity(CBlock& b
     return TestBlockValidity(m_chainstate, block, check_pow, check_merkle);
 }
 
-std::optional<COutPoint> ValidationBlockValidityTestingSetup::AddCoin(const CScript& script_pub_key, CAmount amount)
+COutPoint ValidationBlockValidityTestingSetup::AddCoin(const CScript& script_pub_key, CAmount amount)
 {
     // Max trials to find a unique random outpoint
     const int max_trials{5};
@@ -41,7 +41,8 @@ std::optional<COutPoint> ValidationBlockValidityTestingSetup::AddCoin(const CScr
             return outpoint;
         }
     }
-    return std::nullopt;
+    BOOST_REQUIRE_MESSAGE(false, "AddCoin: failed to find an unused outpoint");
+    return {};
 }
 
 void ValidationBlockValidityTestingSetup::SolveBlockPoW(CBlock& block)
diff --git a/src/test/util/block_validity.h b/src/test/util/block_validity.h
index 90b1aaf763..db6d5c6f8b 100644
--- a/src/test/util/block_validity.h
+++ b/src/test/util/block_validity.h
@@ -10,8 +10,6 @@
 #include <validation.h>
 #include <validationinterface.h>
 
-#include <optional>
-
 /**
  * A CValidationInterface subscriber that records the BlockValidationState from
  * each BlockChecked notification. Use ClearCheckedBlockStates() to reset
@@ -63,7 +61,7 @@ struct ValidationBlockValidityTestingSetup : public TestChain100Setup {
     BlockValidationState ConnectBlock(CBlock& block);
 
     /** Helper to add a spendable coin to the UTXO set for testing. */
-    std::optional<COutPoint> AddCoin(const CScript& script_pub_key = CScript() << OP_TRUE, CAmount amount = 1 * COIN);
+    COutPoint AddCoin(const CScript& script_pub_key = CScript() << OP_TRUE, CAmount amount = 1 * COIN);
     /**
      * Submit a block via ProcessNewBlock, drain the validation signal queue,
      * and return the single BlockChecked notification state.
diff --git a/src/test/validation_block_contextual_tests.cpp b/src/test/validation_block_contextual_tests.cpp
index 380218b00e..5f59193870 100644
--- a/src/test/validation_block_contextual_tests.cpp
+++ b/src/test/validation_block_contextual_tests.cpp
@@ -72,10 +72,9 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_nonfinal)
     // Transactions with time-locks (nLockTime) that are not yet satisfied are rejected.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE, 50 * COIN);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].nSequence = 0;
     mtx.vout.resize(1);
     mtx.vout[0] = CTxOut(49 * COIN, CScript() << OP_TRUE);
@@ -98,10 +97,9 @@ BOOST_AUTO_TEST_CASE(tbv_bip113_locktime_uses_mtp)
     CBlock block = MakeBlock();
     const int64_t mtp = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip()->GetMedianTimePast());
     const auto outpoint = AddCoin(CScript() << OP_TRUE, 50 * COIN);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 49 * COIN;
     // nLockTime exactly equal to MTP: non-final because IsFinalTx requires strictly less than MTP
@@ -139,10 +137,9 @@ BOOST_AUTO_TEST_CASE(tbv_bad_blk_weight)
     const int num_overweight_txns{4};
     for (int tx_idx = 0; tx_idx < num_overweight_txns; ++tx_idx) {
         const auto outpoint = AddCoin(p2tr, 50 * COIN);
-        if (!outpoint) return;
         CMutableTransaction mtx;
         mtx.vin.resize(1);
-        mtx.vin[0].prevout = *outpoint;
+        mtx.vin[0].prevout = outpoint;
         mtx.vout.resize(1);
         mtx.vout[0].nValue = 49 * COIN;
         // 1MB witness item, allowed by OP_SUCCESS bypassing the 520-byte limit
diff --git a/src/test/validation_block_stateful_tests.cpp b/src/test/validation_block_stateful_tests.cpp
index 95eb6285e2..cd36ee377f 100644
--- a/src/test/validation_block_stateful_tests.cpp
+++ b/src/test/validation_block_stateful_tests.cpp
@@ -24,14 +24,13 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_accumulated_fee_outofrange)
     CBlock block = MakeBlock();
     const auto outpoint1 = AddCoin(CScript(), MAX_MONEY);
     const auto outpoint2 = AddCoin(CScript(), 1);
-    if (!outpoint1 || !outpoint2) return;
     CMutableTransaction mtx, mtx2;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint1;
+    mtx.vin[0].prevout = outpoint1;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0;
     mtx2.vin.resize(1);
-    mtx2.vin[0].prevout = *outpoint2;
+    mtx2.vin[0].prevout = outpoint2;
     mtx2.vout.resize(1);
     mtx2.vout[0].nValue = 0;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
@@ -115,11 +114,10 @@ BOOST_AUTO_TEST_CASE(tbv_bad_txns_inputs_sum_overflow)
     CScript script = CScript() << OP_TRUE;
     auto outpoint1 = AddCoin(script, MAX_MONEY);
     auto outpoint2 = AddCoin(script, MAX_MONEY);
-    if (!outpoint1 || !outpoint2) return;
     CMutableTransaction mtx;
     mtx.vin.resize(2);
-    mtx.vin[0].prevout = outpoint1.value();
-    mtx.vin[1].prevout = outpoint2.value();
+    mtx.vin[0].prevout = outpoint1;
+    mtx.vin[1].prevout = outpoint2;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = MAX_MONEY;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
@@ -138,20 +136,19 @@ BOOST_AUTO_TEST_CASE(tbv_block_script_verify_flag_failed)
     CBlock block = MakeBlock();
     const CScript p2pk = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
     const auto outpoint = AddCoin(p2pk, 50 * COIN);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].scriptSig = CScript() << OP_0;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 49 * COIN;
     block.vtx.emplace_back(MakeTransactionRef(std::move(mtx)));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip147_null_dummy)
@@ -160,10 +157,9 @@ BOOST_AUTO_TEST_CASE(tbv_bip147_null_dummy)
     CBlock block = MakeBlock();
     CScript multisig = CScript() << 1 << ToByteVector(coinbaseKey.GetPubKey()) << 1 << OP_CHECKMULTISIG;
     const auto outpoint = AddCoin(multisig);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     std::vector<unsigned char> sig;
@@ -174,10 +170,10 @@ BOOST_AUTO_TEST_CASE(tbv_bip147_null_dummy)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Dummy CHECKMULTISIG argument must be zero)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip66_non_der_sig)
@@ -186,10 +182,9 @@ BOOST_AUTO_TEST_CASE(tbv_bip66_non_der_sig)
     CBlock block = MakeBlock();
     CScript p2pkh = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));
     const auto outpoint = AddCoin(p2pkh);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     uint256 hash = SignatureHash(p2pkh, mtx, 0, SIGHASH_ALL, 1 * COIN, SigVersion::BASE);
@@ -202,10 +197,10 @@ BOOST_AUTO_TEST_CASE(tbv_bip66_non_der_sig)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Non-canonical DER signature)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip65_cltv_violation)
@@ -218,10 +213,9 @@ BOOST_AUTO_TEST_CASE(tbv_bip65_cltv_violation)
     const int lock_height = WITH_LOCK(cs_main, return m_chainstate.m_chain.Height()) + cltv_height_offset;
     const CScript cltv_script = CScript() << lock_height << OP_CHECKLOCKTIMEVERIFY << OP_DROP << OP_TRUE;
     const auto outpoint = AddCoin(cltv_script);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; // Makes IsFinalTx pass, but disables CLTV
     mtx.nLockTime = 0;
     mtx.vout.resize(1);
@@ -229,10 +223,10 @@ BOOST_AUTO_TEST_CASE(tbv_bip65_cltv_violation)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Locktime requirement not satisfied)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_taproot_invalid_sig)
@@ -245,10 +239,9 @@ BOOST_AUTO_TEST_CASE(tbv_taproot_invalid_sig)
     WitnessV1Taproot taproot = builder.GetOutput();
     CScript p2tr_script = GetScriptForDestination(taproot);
     const auto outpoint = AddCoin(p2tr_script);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     // Create the Taproot signature (Keypath spend)
@@ -264,10 +257,10 @@ BOOST_AUTO_TEST_CASE(tbv_taproot_invalid_sig)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Invalid Schnorr signature)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip112_csv_violation)
@@ -278,21 +271,20 @@ BOOST_AUTO_TEST_CASE(tbv_bip112_csv_violation)
     uint32_t csv_block_offset = 10;
     const CScript csv_script = CScript() << csv_block_offset << OP_CHECKSEQUENCEVERIFY << OP_DROP << OP_TRUE;
     const auto outpoint = AddCoin(csv_script);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.version = 2; // CSV requires nVersion >= 2
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].nSequence = csv_block_offset - 1; // One block short of the requirement
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Locktime requirement not satisfied)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip16_p2sh_invalid_redeem_script)
@@ -303,20 +295,19 @@ BOOST_AUTO_TEST_CASE(tbv_bip16_p2sh_invalid_redeem_script)
     const CScript redeem_script = CScript() << OP_FALSE;
     const CScript p2sh = GetScriptForDestination(ScriptHash(redeem_script));
     const auto outpoint = AddCoin(p2sh);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(redeem_script.begin(), redeem_script.end());
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_segwit_v0_invalid_witness_script)
@@ -326,10 +317,9 @@ BOOST_AUTO_TEST_CASE(tbv_segwit_v0_invalid_witness_script)
     const CScript witness_script = CScript() << OP_FALSE;
     const CScript p2wsh = GetScriptForDestination(WitnessV0ScriptHash(witness_script));
     const auto outpoint = AddCoin(p2wsh);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     // Witness: <witness_script>
     mtx.vin[0].scriptWitness.stack = {
         std::vector<unsigned char>(witness_script.begin(), witness_script.end())};
@@ -338,10 +328,10 @@ BOOST_AUTO_TEST_CASE(tbv_segwit_v0_invalid_witness_script)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
@@ -357,7 +347,6 @@ BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
     WitnessV1Taproot taproot = builder.GetOutput();
     const CScript p2tr_script = GetScriptForDestination(taproot);
     const auto outpoint = AddCoin(p2tr_script);
-    if (!outpoint) return;
     // Build the control block for the script path
     const auto spend_data = builder.GetSpendData();
     const auto& control_block = *spend_data.scripts.at(
@@ -365,7 +354,7 @@ BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
                                      .begin();
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     // Witness stack for script path: <leaf_script> <control_block> (OP_FALSE consumes no args)
     mtx.vin[0].scriptWitness.stack = {
         std::vector<unsigned char>(leaf_script.begin(), leaf_script.end()),
@@ -375,10 +364,10 @@ BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_bip143_wrong_amount_in_sighash)
@@ -390,10 +379,9 @@ BOOST_AUTO_TEST_CASE(tbv_bip143_wrong_amount_in_sighash)
     const CScript p2wpkh = GetScriptForDestination(WitnessV0KeyHash(coinbaseKey.GetPubKey()));
     const CAmount actual_value = 1 * COIN;
     const auto outpoint = AddCoin(p2wpkh, actual_value);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0.9 * COIN;
     // Sign over a wrong amount — this is the BIP143-specific failure
@@ -407,10 +395,10 @@ BOOST_AUTO_TEST_CASE(tbv_bip143_wrong_amount_in_sighash)
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
-    CheckScriptViolation(TestValidity(block), block, *outpoint, reason);
-    CheckScriptViolation(ConnectBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(TestValidity(block), block, outpoint, reason);
+    CheckScriptViolation(ConnectBlock(block), block, outpoint, reason);
     SolveBlockPoW(block);
-    CheckScriptViolation(ProcessNewBlock(block), block, *outpoint, reason);
+    CheckScriptViolation(ProcessNewBlock(block), block, outpoint, reason);
 }
 
 BOOST_AUTO_TEST_CASE(tbv_empty_scriptsig)
@@ -418,11 +406,10 @@ BOOST_AUTO_TEST_CASE(tbv_empty_scriptsig)
     // A non-coinbase transaction with an empty scriptSig spending OP_TRUE is valid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE);
-    if (!outpoint) return;
 
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].scriptSig = CScript();
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 1;
@@ -439,10 +426,9 @@ BOOST_AUTO_TEST_CASE(tbv_scriptsig_non_push)
     // A scriptSig with non-push opcodes is valid in a block (consensus) even if non-standard.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin(CScript() << OP_TRUE);
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].scriptSig = CScript() << OP_1 << OP_DROP;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 1;
@@ -459,11 +445,10 @@ BOOST_AUTO_TEST_CASE(tbv_double_spend_same_block)
     // A block containing two transactions spending the same UTXO is invalid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin();
-    if (!outpoint) return;
     for (int tx_idx = 0; tx_idx < 2; ++tx_idx) {
         CMutableTransaction mtx;
         mtx.vin.resize(1);
-        mtx.vin[0].prevout = *outpoint;
+        mtx.vin[0].prevout = outpoint;
         mtx.vin[0].scriptSig = CScript() << OP_TRUE;
         mtx.vout.resize(1);
         mtx.vout[0].nValue = 1;
@@ -483,10 +468,9 @@ BOOST_AUTO_TEST_CASE(tbv_zero_value_output)
     // A transaction with a zero-value output is consensus-valid.
     CBlock block = MakeBlock();
     const auto outpoint = AddCoin();
-    if (!outpoint) return;
     CMutableTransaction mtx;
     mtx.vin.resize(1);
-    mtx.vin[0].prevout = *outpoint;
+    mtx.vin[0].prevout = outpoint;
     mtx.vin[0].scriptSig = CScript() << OP_TRUE;
     mtx.vout.resize(1);
     mtx.vout[0].nValue = 0;

</details>

in src/test/validation_block_stateful_tests.cpp:168 in c9d6eb5d9d

 163 | +    if (!outpoint) return;
 164 | +    CMutableTransaction mtx;
 165 | +    mtx.vin.resize(1);
 166 | +    mtx.vin[0].prevout = *outpoint;
 167 | +    mtx.vout.resize(1);
 168 | +    mtx.vout[0].nValue = 0.9 * COIN;

w0xlt commented at 1:09 AM on June 4, 2026:

micro-nit:

0.9 * COIN uses floating-point arithmetic.

COIN * 9 / 10 is evaluated as (COIN * 9) / 10, using integer arithmetic throughout, which fits CAmount better.

diff --git a/src/test/validation_block_stateful_tests.cpp b/src/test/validation_block_stateful_tests.cpp
index 95eb6285e2..71a59df65b 100644
--- a/src/test/validation_block_stateful_tests.cpp
+++ b/src/test/validation_block_stateful_tests.cpp
@@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip147_null_dummy)
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     std::vector<unsigned char> sig;
     uint256 hash = SignatureHash(multisig, mtx, 0, SIGHASH_ALL, 1 * COIN, SigVersion::BASE);
     BOOST_REQUIRE(coinbaseKey.Sign(hash, sig));
@@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip66_non_der_sig)
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     uint256 hash = SignatureHash(p2pkh, mtx, 0, SIGHASH_ALL, 1 * COIN, SigVersion::BASE);
     std::vector<unsigned char> sig;
     BOOST_REQUIRE(coinbaseKey.Sign(hash, sig));
@@ -225,7 +225,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip65_cltv_violation)
     mtx.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; // Makes IsFinalTx pass, but disables CLTV
     mtx.nLockTime = 0;
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Locktime requirement not satisfied)";
@@ -250,7 +250,7 @@ BOOST_AUTO_TEST_CASE(tbv_taproot_invalid_sig)
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     // Create the Taproot signature (Keypath spend)
     FlatSigningProvider provider;
     provider.keys[coinbaseKey.GetPubKey().GetID()] = coinbaseKey;
@@ -285,7 +285,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip112_csv_violation)
     mtx.vin[0].prevout = *outpoint;
     mtx.vin[0].nSequence = csv_block_offset - 1; // One block short of the requirement
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Locktime requirement not satisfied)";
@@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip16_p2sh_invalid_redeem_script)
     mtx.vin[0].prevout = *outpoint;
     mtx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(redeem_script.begin(), redeem_script.end());
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
@@ -334,7 +334,7 @@ BOOST_AUTO_TEST_CASE(tbv_segwit_v0_invalid_witness_script)
     mtx.vin[0].scriptWitness.stack = {
         std::vector<unsigned char>(witness_script.begin(), witness_script.end())};
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
@@ -371,7 +371,7 @@ BOOST_AUTO_TEST_CASE(tbv_tapscript_invalid_script_path)
         std::vector<unsigned char>(leaf_script.begin(), leaf_script.end()),
         control_block};
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     block.vtx.emplace_back(MakeTransactionRef(mtx));
     RegenerateCommitments(block, *m_node.chainman);
     const auto reason = "block-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)";
@@ -395,7 +395,7 @@ BOOST_AUTO_TEST_CASE(tbv_bip143_wrong_amount_in_sighash)
     mtx.vin.resize(1);
     mtx.vin[0].prevout = *outpoint;
     mtx.vout.resize(1);
-    mtx.vout[0].nValue = 0.9 * COIN;
+    mtx.vout[0].nValue = COIN * 9 / 10;
     // Sign over a wrong amount — this is the BIP143-specific failure
     const CAmount wrong_value = actual_value + 1;
     const CScript p2wpkh_script = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));

</details>

w0xlt commented at 1:09 AM on June 4, 2026: contributor

ACK c9d6eb5d9d1db6d6c0ffc9096932d40b121b2875

Some non-blocking nits:

DrahtBot requested review from sedited on Jun 4, 2026

test: Add block validity test setup and TestBlockValidity tests c79663e45e

test: Add ProcessNewBlock block validation path unit tests 8527fda363

test: Add ConnectBlock block validation path unit tests ccfc34482d

ismaelsadeeq force-pushed on Jun 5, 2026

ismaelsadeeq commented at 4:57 PM on June 5, 2026: member

Forced pushed from c9d6eb5d9d1db6d6c0ffc9096932d40b121b2875 to ccfc34482d1120aa05d65c83d3e41f8dbee851b3 compare diff

#35000 (review) took the second approach, it's more DRY
#35000 (review) taken