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.

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.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt
Approach ACK	sedited

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

CreateValidMempoolTransaction(..., false) in src/test/validation_block_contextual_tests.cpp

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.

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.

test: Add block validation unit tests

- Add a set of block validation tests structured around the Bitcoin Core
  ivalidation pipeline (header, stateless, contextual, and stateful
   validation).

- Use the test cases to test TestBlockValidity, the goal is to
  simulate/call other block validation path as well, in other to
  prevent regression due to changes to validation code.

5ae4c0cc39

ismaelsadeeq force-pushed on Apr 23, 2026

DrahtBot added the label CI failed on Apr 23, 2026