From dc1ce1324d3c9906b586d90fc023d61021fdde1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 29 May 2026 18:10:12 +0200 Subject: [PATCH] test: Match blockchain test block exceptions by reason The blockchain test runner treated block validity as a bool: for a block expected to be invalid it accepted ANY rejection and never checked that the reason matched the fixture's `expectException`. So a block rejected for the wrong reason still passed. Make `validate_block` return a `std::error_code` instead of `bool`, with new block-level entries appended (at the back, keeping existing values stable) to `state::ErrorCode`. Each entry's message is the matching execution-spec-tests `BlockException` constant (INVALID_GASLIMIT, INVALID_BASEFEE_PER_GAS, INCORRECT_EXCESS_BLOB_GAS, RLP_BLOCK_LIMIT_EXCEEDED, INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT, INVALID_BLOCK_NUMBER, INCORRECT_BLOCK_FORMAT for the rest), except the missing-parent check which uses evmone's own INVALID_BLOCK_PARENT (a broader condition than EEST's UNKNOWN_PARENT). The loader captures the `expectException` string; the runner substring-matches the actual error's constant against it, which also handles the fixtures' `|`-separated alternatives. Legacy ethereum/tests fixtures use older, sometimes finer-grained exception names (e.g. uncles after Paris, a number gap, a wrong base fee). The loader maps those (many-to-one) onto the `BlockException` constant evmone reports, so they match by reason as well. --- test/blockchaintest/blockchaintest_runner.cpp | 86 +++++++++++-------- test/state/errors.hpp | 26 ++++++ test/utils/blockchaintest.hpp | 2 +- test/utils/blockchaintest_loader.cpp | 31 ++++++- 4 files changed, 108 insertions(+), 37 deletions(-) diff --git a/test/blockchaintest/blockchaintest_runner.cpp b/test/blockchaintest/blockchaintest_runner.cpp index ef99051f8b..6a80fc78ec 100644 --- a/test/blockchaintest/blockchaintest_runner.cpp +++ b/test/blockchaintest/blockchaintest_runner.cpp @@ -4,6 +4,7 @@ #include "blockchaintest_runner.hpp" #include +#include #include #include #include @@ -125,67 +126,69 @@ TransitionResult apply_block(const TestState& state, evmc::VM& vm, const state:: bloom, blob_gas_left, std::move(block_state)}; } -bool validate_block(evmc_revision rev, state::BlobParams blob_params, const TestBlock& test_block, - const BlockHeader* parent_header, bool parent_has_ommers) noexcept +/// Validates block-level validity unrelated to individual transactions. +/// +/// Returns an empty error_code if the block is valid, otherwise the specific validation error. +std::error_code validate_block(evmc_revision rev, state::BlobParams blob_params, + const TestBlock& test_block, const BlockHeader* parent_header, bool parent_has_ommers) noexcept { - // NOTE: includes only block validity unrelated to individual txs. See `apply_block`. + using namespace state; - // Fail if parent header was not found. + // Fail if parent header was not found: the block references a parent that is neither the + // genesis nor any previously-accepted block (an unknown or rejected parent). if (parent_header == nullptr) - return false; + return make_error_code(INVALID_BLOCK_PARENT); if (test_block.block_info.number != parent_header->block_number + 1) - return false; + return make_error_code(INVALID_BLOCK_NUMBER); if (test_block.block_info.gas_used > test_block.block_info.gas_limit) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); // Some tests have gas limit at INT64_MAX, so we cast to uint64_t to avoid overflow. const auto parent_header_gas_limit_u64 = static_cast(parent_header->gas_limit); const auto test_block_gas_limit_u64 = static_cast(test_block.block_info.gas_limit); if (test_block_gas_limit_u64 >= parent_header_gas_limit_u64 + parent_header_gas_limit_u64 / 1024) - return false; + return make_error_code(INVALID_GASLIMIT); if (test_block_gas_limit_u64 <= parent_header_gas_limit_u64 - parent_header_gas_limit_u64 / 1024) - return false; + return make_error_code(INVALID_GASLIMIT); // Block gas limit minimum from Yellow Paper. if (test_block.block_info.gas_limit < 5000) - return false; + return make_error_code(INVALID_GASLIMIT); // FIXME: Some tests have timestamp not fitting into int64_t, type has to be uint64_t. if (static_cast(test_block.block_info.timestamp) <= static_cast(parent_header->timestamp)) - return false; + return make_error_code(INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT); - if (test_block.block_info.difficulty != state::calculate_difficulty(parent_header->difficulty, - parent_has_ommers, parent_header->timestamp, - test_block.block_info.timestamp, - test_block.block_info.number, rev)) - return false; + if (test_block.block_info.difficulty != + calculate_difficulty(parent_header->difficulty, parent_has_ommers, parent_header->timestamp, + test_block.block_info.timestamp, test_block.block_info.number, rev)) + return make_error_code(INCORRECT_BLOCK_FORMAT); if (rev >= EVMC_PARIS && !test_block.block_info.ommers.empty()) - return false; - + return make_error_code(INCORRECT_BLOCK_FORMAT); for (const auto& ommer : test_block.block_info.ommers) { // Check that ommer block number difference with current block is within allowed range. // https://github.com/ethereum/execution-specs/blob/ee73be5c4d83a2e3c358bd14990878002e52ba9e/src/ethereum/gray_glacier/fork.py#L623 if (ommer.delta < 1 || ommer.delta > 6) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); } if (test_block.block_info.extra_data.size() > 32) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); if (rev >= EVMC_LONDON) { - const auto calculated_base_fee = state::calc_base_fee( + const auto calculated_base_fee = calc_base_fee( parent_header->gas_limit, parent_header->gas_used, parent_header->base_fee_per_gas); if (test_block.block_info.base_fee != calculated_base_fee) - return false; + return make_error_code(INVALID_BASEFEE_PER_GAS); } if (rev >= EVMC_CANCUN) @@ -193,34 +196,34 @@ bool validate_block(evmc_revision rev, state::BlobParams blob_params, const Test // `excess_blob_gas` and `blob_gas_used` mandatory after Cancun and invalid before. if (!test_block.block_info.excess_blob_gas.has_value() || !test_block.block_info.blob_gas_used.has_value()) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); // Check that the excess blob gas was updated correctly. // According to EIP-7918 current blocks params (`rev`) should be used for parent base fee // calculation. const auto parent_blob_base_fee = - state::compute_blob_gas_price(blob_params, parent_header->excess_blob_gas.value_or(0)); + compute_blob_gas_price(blob_params, parent_header->excess_blob_gas.value_or(0)); if (*test_block.block_info.excess_blob_gas != - state::calc_excess_blob_gas(rev, blob_params, parent_header->blob_gas_used.value_or(0), + calc_excess_blob_gas(rev, blob_params, parent_header->blob_gas_used.value_or(0), parent_header->excess_blob_gas.value_or(0), parent_header->base_fee_per_gas, parent_blob_base_fee)) - return false; + return make_error_code(INCORRECT_EXCESS_BLOB_GAS); } else { if (test_block.block_info.excess_blob_gas.has_value() || test_block.block_info.blob_gas_used.has_value()) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); } // Block is invalid if some of the withdrawal fields failed to be parsed. if (!test_block.withdrawals_parse_success) - return false; + return make_error_code(INCORRECT_BLOCK_FORMAT); if (rev >= EVMC_OSAKA && test_block.rlp_size > MAX_RLP_BLOCK_SIZE) - return false; + return make_error_code(RLP_BLOCK_LIMIT_EXCEEDED); - return true; + return {}; } std::optional mining_reward(evmc_revision rev) noexcept @@ -315,11 +318,13 @@ void run_blockchain_tests(std::span tests, evmc::VM& vm) SCOPED_TRACE(std::string{evmc::to_string(rev)} + '/' + std::to_string(case_index) + '/' + c.name + '/' + std::to_string(test_block.block_info.number)); - if (test_block.valid) + const auto block_error = + validate_block(rev, blob_params, test_block, parent_header, parent_has_ommers); + + if (test_block.expected_exception.empty()) { - ASSERT_TRUE( - validate_block(rev, blob_params, test_block, parent_header, parent_has_ommers)) - << "Expected block to be valid (validate_block)"; + ASSERT_FALSE(block_error) + << "Expected block to be valid (validate_block): " << block_error.message(); // Block being valid guarantees its parent was found. assert(parent_data_it != block_data.end()); @@ -377,8 +382,19 @@ void run_blockchain_tests(std::span tests, evmc::VM& vm) } else { - if (!validate_block(rev, blob_params, test_block, parent_header, parent_has_ommers)) + 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) + << "Block invalidity reason mismatch: got " << block_error.message() + << ", expected " << test_block.expected_exception; continue; + } // Block being valid guarantees its parent was found. assert(parent_data_it != block_data.end()); diff --git a/test/state/errors.hpp b/test/state/errors.hpp index e42e60b999..352c3d3239 100644 --- a/test/state/errors.hpp +++ b/test/state/errors.hpp @@ -32,6 +32,16 @@ enum ErrorCode : int // NOLINT(*-use-enum-class) EMPTY_AUTHORIZATION_LIST, MAX_GAS_LIMIT_EXCEEDED, UNKNOWN_ERROR, + + // Block-level validation. + INCORRECT_BLOCK_FORMAT, + INVALID_GASLIMIT, + INVALID_BASEFEE_PER_GAS, + INCORRECT_EXCESS_BLOB_GAS, + RLP_BLOCK_LIMIT_EXCEEDED, + INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT, + INVALID_BLOCK_PARENT, + INVALID_BLOCK_NUMBER, }; /// Obtains a reference to the static error category object for evmone errors. @@ -87,6 +97,22 @@ inline const std::error_category& evmone_category() noexcept return "max gas limit exceeded"; case UNKNOWN_ERROR: return "Unknown error"; + case INCORRECT_BLOCK_FORMAT: + return "BlockException.INCORRECT_BLOCK_FORMAT"; + case INVALID_GASLIMIT: + return "BlockException.INVALID_GASLIMIT"; + case INVALID_BASEFEE_PER_GAS: + return "BlockException.INVALID_BASEFEE_PER_GAS"; + case INCORRECT_EXCESS_BLOB_GAS: + return "BlockException.INCORRECT_EXCESS_BLOB_GAS"; + case RLP_BLOCK_LIMIT_EXCEEDED: + return "BlockException.RLP_BLOCK_LIMIT_EXCEEDED"; + case INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT: + return "BlockException.INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT"; + case INVALID_BLOCK_PARENT: + return "BlockException.INVALID_BLOCK_PARENT"; + case INVALID_BLOCK_NUMBER: + return "BlockException.INVALID_BLOCK_NUMBER"; default: assert(false); return "Wrong error code"; diff --git a/test/utils/blockchaintest.hpp b/test/utils/blockchaintest.hpp index cc5c63882d..aeb081709d 100644 --- a/test/utils/blockchaintest.hpp +++ b/test/utils/blockchaintest.hpp @@ -51,7 +51,7 @@ struct TestBlock std::vector transactions; size_t rlp_size = 0; bool withdrawals_parse_success = true; - bool valid = true; + std::string expected_exception; ///< Empty for valid blocks. BlockHeader expected_block_header; }; diff --git a/test/utils/blockchaintest_loader.cpp b/test/utils/blockchaintest_loader.cpp index 792cac8fd7..1fca6d5cb3 100644 --- a/test/utils/blockchaintest_loader.cpp +++ b/test/utils/blockchaintest_loader.cpp @@ -5,6 +5,7 @@ #include "blockchaintest.hpp" #include "statetest.hpp" #include "utils.hpp" +#include namespace evmone::test { @@ -136,6 +137,34 @@ static TestBlock load_test_block( namespace { +/// Maps a legacy "expectException" value to modern EEST-style exception name. +std::string map_legacy_block_exception(std::string_view expected_exception) +{ + using enum state::ErrorCode; + using Entry = std::pair; + + static constexpr Entry LEGACY_MAP[]{ + // ethereum/tests (EEST-format): + {"BlockException.IMPORT_IMPOSSIBLE_UNCLES_OVER_PARIS", INCORRECT_BLOCK_FORMAT}, + {"BlockException.GAS_USED_OVERFLOW", INCORRECT_BLOCK_FORMAT}, + {"BlockException.RLP_STRUCTURES_ENCODING|BlockException.RLP_INVALID_FIELD_OVERFLOW_64", + INCORRECT_BLOCK_FORMAT}, + // ethereum/legacytests (pre-EEST): + {"PostParisUncleHashIsNotEmpty", INCORRECT_BLOCK_FORMAT}, + {"3675PreParis1559BlockRejected", INCORRECT_BLOCK_FORMAT}, + {"InvalidNumber", INCORRECT_BLOCK_FORMAT}, + {"InvalidTimestampOlderParent", INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT}, + {"TooMuchGasUsed", INCORRECT_BLOCK_FORMAT}, + {"UncleParentIsNotAncestor", INCORRECT_BLOCK_FORMAT}, + {"InvalidGasLimit2", INVALID_GASLIMIT}, + {"1559BlockImportImpossible_BaseFeeWrong", INVALID_BASEFEE_PER_GAS}, + }; + + const auto it = std::ranges::find(LEGACY_MAP, expected_exception, &Entry::first); + return (it != std::end(LEGACY_MAP)) ? state::make_error_code(it->second).message() : + std::string{expected_exception}; +} + BlockchainTest load_blockchain_test_case(const std::string& name, const json::json& j) { using namespace state; @@ -165,7 +194,7 @@ BlockchainTest load_blockchain_test_case(const std::string& name, const json::js "tests with invalidly rlp-encoded blocks are not supported"); auto test_block = load_test_block(el.at("rlp_decoded"), bt.network, bt.blob_schedule); - test_block.valid = false; + test_block.expected_exception = map_legacy_block_exception(it->get()); test_block.rlp_size = from_json(el.at("rlp")).size(); bt.test_blocks.emplace_back(test_block); }