From 8068e296bacd8fd3e0d7b355e123110b29bdcd0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 10 Aug 2026 01:38:54 +0200 Subject: [PATCH] test: Check why a blockchain test block was rejected A block expected to be invalid only had to differ from its fixture somewhere: every header mismatch counted as "correctly rejected", so a block that failed the state root of a test about the receipts root passed just as well. A rejected transaction only had to be rejected for some reason, with "TransactionException." appearing anywhere in the expectation. Each header mismatch is now matched against the BlockException it is the symptom of, and the transaction reason goes through is_expected_tx_exception(), the same mapping the state test runner uses. That one takes the error code rather than its message, so apply_block() reports the code. Two rules evmone does not implement keep their old verdict, each marked with a TODO: transaction senders come from the fixture instead of being recovered from the signature, and ommer headers are not validated. --- test/blockchaintest/blockchaintest_runner.cpp | 97 +++++++++++++++---- test/utils/block_transition.cpp | 3 +- test/utils/block_transition.hpp | 6 +- test/utils/error_matching.cpp | 45 +++++++-- test/utils/error_matching.hpp | 4 + test/utils/t8n.cpp | 2 +- 6 files changed, 122 insertions(+), 35 deletions(-) diff --git a/test/blockchaintest/blockchaintest_runner.cpp b/test/blockchaintest/blockchaintest_runner.cpp index c72701fc06..c328e57368 100644 --- a/test/blockchaintest/blockchaintest_runner.cpp +++ b/test/blockchaintest/blockchaintest_runner.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -328,12 +329,9 @@ void run_blockchain_tests(std::span tests, evmc::VM& vm) if (block_error) { // Block correctly rejected at validation; verify the reason matches the - // fixture's expected exception. The error message is the `BlockException` - // constant; `expected_exception` may list `|`-separated alternatives, so a - // substring search suffices as long as no constant name is a substring of - // another (true for the constants evmone produces). - EXPECT_NE(test_block.expected_exception.find(block_error.message()), - std::string::npos) + // fixture's expected exception. + EXPECT_TRUE( + is_expected_block_exception(block_error, test_block.expected_exception)) << "Block invalidity reason mismatch: got " << block_error.message() << ", expected " << test_block.expected_exception; continue; @@ -343,58 +341,119 @@ void run_blockchain_tests(std::span tests, evmc::VM& vm) assert(parent_data_it != block_data.end()); const auto& pre_state = parent_data_it->second.post_state; + // Legacy fixtures name the broken rule in vocabulary evmone does not speak + // (InvalidStateRoot, TooManyUncles); only the spec names can be compared. + const auto names_spec_exception = + test_block.expected_exception.find("Exception.") != std::string::npos; + + // TODO: The transaction senders come from the fixture instead of being recovered + // from the signatures, so evmone never sees the signature the test broke. Such a + // transaction executes as the sender the fixture names and the block is rejected + // by whatever rule that sender happens to break, or by its state root alone. + const auto sender_not_recovered = contains_any( + test_block.expected_exception, "TransactionException.INVALID_SIGNATURE_VRS"); + const auto res = apply_block(pre_state, vm, bi, block_hashes, test_block.transactions, rev, blob_gas_limit, {.block_reward = mining_reward(rev)}); if (!res.rejected.empty()) { - // Check if EEST expects transaction-level exception (ignore "legacy" names). - // `expected_exception` may list `|`-separated alternatives (and a tx-level - // alternative can appear after a block-level one), so search for a - // `TransactionException.` anywhere rather than only at the start. - if (test_block.expected_exception.find("Exception.") != std::string::npos) + // A transaction was rejected: the fixture must name that reason, not merely + // some rejection. + const auto& rejected = res.rejected.front(); + if (names_spec_exception && !sender_not_recovered) { - EXPECT_NE(test_block.expected_exception.find("TransactionException."), - std::string::npos) - << "Transaction-level invalidity mismatch: got " - << res.rejected.front().message << ", expected " + EXPECT_TRUE( + is_expected_tx_exception(rejected.error, test_block.expected_exception)) + << "Transaction-level invalidity mismatch: got \"" + << rejected.error.message() << "\", expected " << test_block.expected_exception; } continue; } if (res.requests_error) { - // Requests collection failure; verify the reason matches (same - // `BlockException.*` substring match as the block validation errors above). - EXPECT_NE(test_block.expected_exception.find(res.requests_error.message()), - std::string::npos) + // Requests collection failure; verify the reason the same way. + EXPECT_TRUE(is_expected_block_exception( + res.requests_error, test_block.expected_exception)) << "Block invalidity reason mismatch: got " << res.requests_error.message() << ", expected " << test_block.expected_exception; continue; } + // The block executed, so it is invalid only if it computes something other than + // its header claims. Each difference below is the symptom of one BlockException: + // a block failing a check other than the one the fixture names breaks a different + // rule than the test is about. + // TODO: Of the ommers only the count and the distance to their nephew are + // validated, not the ommer headers themselves, so a fixture that breaks an + // ommer's gas limit, number or timestamp reaches execution and lands here. + const auto ommers_not_validated = !test_block.block_info.ommers.empty(); + + // Asserts the fixture names one of @p names, the exceptions the check that just + // fired is the symptom of. Silent where the reason cannot be compared. + const auto expect_fixture_names = [&](std::string_view names) { + if (!names_spec_exception || ommers_not_validated) + return; + EXPECT_TRUE(contains_any(test_block.expected_exception, names)) + << "Block invalidity reason mismatch: the block failed the check for " + << names << ", expected " << test_block.expected_exception; + }; + if (blob_gas_limit - res.blob_gas_left != static_cast(bi.blob_gas_used.value_or(0))) + { + expect_fixture_names( + "BlockException.INCORRECT_BLOB_GAS_USED|" + "BlockException.BLOB_GAS_USED_ABOVE_LIMIT"); continue; + } if (state::mpt_hash(res.block_state) != test_block.expected_block_header.state_root) + { + // The state root is also where a sender that was not recovered surfaces, + // see the TODO above. + expect_fixture_names( + "BlockException.INVALID_STATE_ROOT|" + "TransactionException.INVALID_SIGNATURE_VRS"); continue; + } if (rev >= EVMC_SHANGHAI && state::mpt_hash(test_block.block_info.withdrawals) != test_block.expected_block_header.withdrawal_root) + { + expect_fixture_names("BlockException.INVALID_WITHDRAWALS_ROOT"); continue; + } if (state::mpt_hash(test_block.transactions) != test_block.expected_block_header.transactions_root) + { + expect_fixture_names("BlockException.INVALID_TRANSACTIONS_ROOT"); continue; + } if (state::mpt_hash(res.receipts) != test_block.expected_block_header.receipts_root) + { + expect_fixture_names("BlockException.INVALID_RECEIPTS_ROOT"); continue; + } if (rev >= EVMC_PRAGUE && calculate_requests_hash(res.requests) != test_block.expected_block_header.requests_hash) + { + expect_fixture_names("BlockException.INVALID_REQUESTS"); continue; + } if (res.gas_used != test_block.expected_block_header.gas_used) + { + expect_fixture_names( + "BlockException.INVALID_GAS_USED|" + "BlockException.GAS_USED_OVERFLOW"); continue; + } if (bytes_view{res.bloom} != bytes_view{test_block.expected_block_header.logs_bloom}) + { + expect_fixture_names("BlockException.INVALID_LOG_BLOOM"); continue; + } EXPECT_TRUE(false) << "Expected block to be invalid but resulted valid"; } diff --git a/test/utils/block_transition.cpp b/test/utils/block_transition.cpp index 50070897d5..c8a36a733b 100644 --- a/test/utils/block_transition.cpp +++ b/test/utils/block_transition.cpp @@ -67,8 +67,7 @@ TransitionResult apply_block(const TestState& state, evmc::VM& vm, const state:: if (holds_alternative(res)) { - const auto ec = std::get(res); - rejected_txs.push_back({computed_tx_hash, i, ec.message()}); + rejected_txs.push_back({computed_tx_hash, i, std::get(res)}); } else { diff --git a/test/utils/block_transition.hpp b/test/utils/block_transition.hpp index 9bc156e5c2..c95628ac47 100644 --- a/test/utils/block_transition.hpp +++ b/test/utils/block_transition.hpp @@ -24,9 +24,9 @@ namespace evmone::test /// A transaction rejected during block application. struct RejectedTransaction { - hash256 hash; ///< keccak256 of the transaction's RLP encoding. - size_t index; ///< Position in the input transaction list. - std::string message; + hash256 hash; ///< keccak256 of the transaction's RLP encoding. + size_t index; ///< Position in the input transaction list. + std::error_code error; ///< Why the transaction was rejected. }; /// Options for apply_block(). Defaults match block-validation (full) behavior. diff --git a/test/utils/error_matching.cpp b/test/utils/error_matching.cpp index e4c37c0774..b239ad0fa3 100644 --- a/test/utils/error_matching.cpp +++ b/test/utils/error_matching.cpp @@ -5,16 +5,17 @@ #include "error_matching.hpp" #include #include +#include namespace evmone::test { namespace { -/// The exceptions a fixture may name for a transaction evmone rejects with this error, on top of +/// The exceptions a fixture may name for a rejection evmone reports with this error, on top of /// the canonical name its error message carries. struct AlternativeExceptions { - state::ErrorCode errc; ///< The code evmone rejects the transaction with. + state::ErrorCode errc; ///< The code evmone rejects with. std::string_view names; ///< The other names the fixtures use for it, `|`-separated. }; @@ -44,6 +45,21 @@ constexpr AlternativeExceptions ALTERNATIVE_TX_EXCEPTIONS[]{ {state::INVALID_ENCODING, "TransactionException.INVALID_SIGNATURE_VRS"}, }; +/// The same, for the rules evmone checks on the block rather than the transaction. +constexpr AlternativeExceptions ALTERNATIVE_BLOCK_EXCEPTIONS[]{ + // A parent that is absent and one whose hash is zero are the same lookup miss to evmone. + {state::UNKNOWN_PARENT, "BlockException.UNKNOWN_PARENT_ZERO"}, + + // evmone reports one malformed-header error where the specs name the individual rule. Nine + // validate_block() branches share that code, so these names are interchangeable to it; + // separating them needs a distinct code per branch. + {state::INCORRECT_BLOCK_FORMAT, + "BlockException.GAS_USED_OVERFLOW|" + "BlockException.IMPORT_IMPOSSIBLE_UNCLES_OVER_PARIS|" + "BlockException.RLP_STRUCTURES_ENCODING|" + "BlockException.RLP_INVALID_FIELD_OVERFLOW_64"}, +}; + /// A retesteth `expectException` value and the evmone rejection(s) it stands for. Some legacy /// names cover two rules at once, hence the second code. struct LegacyException @@ -80,12 +96,7 @@ constexpr LegacyException LEGACY_EXCEPTIONS[]{ {"TR_BLOBVERSION_INVALID", state::INVALID_BLOB_HASH_VERSION}, {"TR_BLOBLIST_OVERSIZE", state::BLOB_GAS_LIMIT_EXCEEDED}, - // Block-level. The first three are spec names, not retesteth ones: they name rules evmone - // does not tell apart, so the fixture's value is replaced with the one evmone reports. - {"BlockException.IMPORT_IMPOSSIBLE_UNCLES_OVER_PARIS", state::INCORRECT_BLOCK_FORMAT}, - {"BlockException.GAS_USED_OVERFLOW", state::INCORRECT_BLOCK_FORMAT}, - {"BlockException.RLP_STRUCTURES_ENCODING|BlockException.RLP_INVALID_FIELD_OVERFLOW_64", - state::INCORRECT_BLOCK_FORMAT}, + // Block-level. {"PostParisUncleHashIsNotEmpty", state::INCORRECT_BLOCK_FORMAT}, {"3675PreParis1559BlockRejected", state::INCORRECT_BLOCK_FORMAT}, {"InvalidNumber", state::INCORRECT_BLOCK_FORMAT}, @@ -132,13 +143,27 @@ bool contains_any(std::string_view expected, std::string_view names) noexcept return false; } -bool is_expected_tx_exception(const std::error_code& ec, std::string_view expected) noexcept +namespace +{ +bool is_expected_exception(std::span alternatives, + const std::error_code& ec, std::string_view expected) noexcept { if (contains_any(expected, ec.message())) // The message is the canonical exception name. return true; - return std::ranges::any_of(ALTERNATIVE_TX_EXCEPTIONS, [&](const AlternativeExceptions& a) { + return std::ranges::any_of(alternatives, [&](const AlternativeExceptions& a) { return make_error_code(a.errc) == ec && contains_any(expected, a.names); }); } +} // namespace + +bool is_expected_tx_exception(const std::error_code& ec, std::string_view expected) noexcept +{ + return is_expected_exception(ALTERNATIVE_TX_EXCEPTIONS, ec, expected); +} + +bool is_expected_block_exception(const std::error_code& ec, std::string_view expected) noexcept +{ + return is_expected_exception(ALTERNATIVE_BLOCK_EXCEPTIONS, ec, expected); +} } // namespace evmone::test diff --git a/test/utils/error_matching.hpp b/test/utils/error_matching.hpp index d936440026..921c9c6ebe 100644 --- a/test/utils/error_matching.hpp +++ b/test/utils/error_matching.hpp @@ -29,4 +29,8 @@ namespace evmone::test /// specs name more exceptions for the same rule, those are accepted too. [[nodiscard]] bool is_expected_tx_exception( const std::error_code& ec, std::string_view expected) noexcept; + +/// The same for a block validation error. +[[nodiscard]] bool is_expected_block_exception( + const std::error_code& ec, std::string_view expected) noexcept; } // namespace evmone::test diff --git a/test/utils/t8n.cpp b/test/utils/t8n.cpp index 1f0d7ac1b2..dbf8a0888d 100644 --- a/test/utils/t8n.cpp +++ b/test/utils/t8n.cpp @@ -133,7 +133,7 @@ void t8n(evmc::VM& vm, const T8NArgs& args) JSON j_rejected_tx; j_rejected_tx["hash"] = hex0x(rejected_it->hash); j_rejected_tx["index"] = i; - j_rejected_tx["error"] = rejected_it->message; + j_rejected_tx["error"] = rejected_it->error.message(); j_result["rejected"].push_back(j_rejected_tx); ++rejected_it; }