From 04fdfcb09238099c48036c8cbec779ee1a9d2ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Sun, 9 Aug 2026 17:04:07 +0200 Subject: [PATCH] test: Check why a state test transaction was rejected A state test with an `expectException` only checked that the transaction was rejected, not that it was rejected for the stated reason, so an implementation failing a rule for the wrong cause passed. Compare the rejection against the fixture's exception. Where the specs draw a distinction evmone does not, or draw it elsewhere, the alternative names are listed in one table in test/utils/error_matching.cpp. The retesteth vocabulary of the ethereum/tests fixtures is mapped to the spec names there too, moved out of the blockchain-test loader and extended with the transaction-level spellings, so both runners compare one vocabulary. --- test/statetest/statetest_runner.cpp | 11 +- test/utils/CMakeLists.txt | 2 + test/utils/blockchaintest_loader.cpp | 31 +----- test/utils/error_matching.cpp | 144 +++++++++++++++++++++++++++ test/utils/error_matching.hpp | 32 ++++++ test/utils/statetest.hpp | 7 +- test/utils/statetest_loader.cpp | 4 +- 7 files changed, 198 insertions(+), 33 deletions(-) create mode 100644 test/utils/error_matching.cpp create mode 100644 test/utils/error_matching.hpp diff --git a/test/statetest/statetest_runner.cpp b/test/statetest/statetest_runner.cpp index 954af0c721..8d2ea43c5f 100644 --- a/test/statetest/statetest_runner.cpp +++ b/test/statetest/statetest_runner.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -84,10 +85,18 @@ void run_state_test(const StateTransitionTest& test, evmc::VM& vm, bool trace_su std::clog << R"("stateRoot":"0x)" << hex(state_root) << "\"}\n"; } - if (expected.exception) + if (!expected.exception.empty()) { ASSERT_FALSE(holds_alternative(res)) << "unexpected valid transaction"; + + // The transaction must be rejected for the reason the fixture states, not merely + // rejected: a wrong reason is a wrong implementation of the rule being tested. + const auto& reason = get(res); + EXPECT_TRUE(is_expected_tx_exception(reason, expected.exception)) + << "transaction rejected as \"" << reason.message() << "\", expected " + << expected.exception; + EXPECT_EQ(logs_hash(std::vector()), expected.logs_hash); } else diff --git a/test/utils/CMakeLists.txt b/test/utils/CMakeLists.txt index 14c354a866..3ed2a3cc65 100644 --- a/test/utils/CMakeLists.txt +++ b/test/utils/CMakeLists.txt @@ -21,6 +21,8 @@ target_sources( blockchaintest.hpp blockchaintest_loader.cpp bytecode.hpp + error_matching.hpp + error_matching.cpp mpt.hpp mpt.cpp mpt_hash.hpp diff --git a/test/utils/blockchaintest_loader.cpp b/test/utils/blockchaintest_loader.cpp index dbab3c6ece..aed3203db2 100644 --- a/test/utils/blockchaintest_loader.cpp +++ b/test/utils/blockchaintest_loader.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "blockchaintest.hpp" +#include "error_matching.hpp" #include "statetest.hpp" #include "utils.hpp" #include @@ -119,34 +120,6 @@ 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; @@ -177,7 +150,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.expected_exception = map_legacy_block_exception(it->get()); + test_block.expected_exception = map_legacy_exception(it->get()); test_block.rlp = from_json(el.at("rlp")); bt.test_blocks.emplace_back(test_block); } diff --git a/test/utils/error_matching.cpp b/test/utils/error_matching.cpp new file mode 100644 index 0000000000..e4c37c0774 --- /dev/null +++ b/test/utils/error_matching.cpp @@ -0,0 +1,144 @@ +// evmone: Fast Ethereum Virtual Machine implementation +// Copyright 2026 The evmone Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include "error_matching.hpp" +#include +#include + +namespace evmone::test +{ +namespace +{ +/// The exceptions a fixture may name for a transaction evmone rejects 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. + std::string_view names; ///< The other names the fixtures use for it, `|`-separated. +}; + +/// Where the execution specs draw more distinctions than evmone does, or draw one of them in a +/// different place. Both refuse the same transactions, so every name listed here is accepted. +constexpr AlternativeExceptions ALTERNATIVE_TX_EXCEPTIONS[]{ + // The specs make the floor cost (EIP-7623) a rule of its own; evmone folds it into the + // intrinsic gas. + {state::INTRINSIC_GAS_TOO_LOW, "TransactionException.INTRINSIC_GAS_BELOW_FLOOR_GAS_COST"}, + + // The specs name the transaction type that arrived before its fork; evmone has one rule. + {state::TYPE_NOT_SUPPORTED, + "TransactionException.TYPE_1_TX_PRE_FORK|" + "TransactionException.TYPE_2_TX_PRE_FORK|" + "TransactionException.TYPE_3_TX_PRE_FORK|" + "TransactionException.TYPE_4_TX_PRE_FORK"}, + + // The specs separate the transaction's own blob count from the block's blob gas allowance. + {state::BLOB_GAS_LIMIT_EXCEEDED, + "TransactionException.TYPE_3_TX_MAX_BLOB_GAS_ALLOWANCE_EXCEEDED"}, + + // evmone bounds the signature v while decoding the transaction, because the domain of v is + // what tells a legacy transaction from a typed one and carries the chain id (EIP-155). The + // execution specs read v as a plain integer and bound it with the rest of the signature. + // decode_transaction() reports one code for every malformed encoding, so this accepts more + // than the v rule; narrowing it needs the decoder to report the v domain separately. + {state::INVALID_ENCODING, "TransactionException.INVALID_SIGNATURE_VRS"}, +}; + +/// 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 +{ + std::string_view name; + state::ErrorCode errc; + state::ErrorCode alt = state::SUCCESS; +}; + +constexpr LegacyException LEGACY_EXCEPTIONS[]{ + // Transaction-level, ethereum/tests and ethereum/legacytests. + {"TR_IntrinsicGas", state::INTRINSIC_GAS_TOO_LOW}, + {"IntrinsicGas", state::INTRINSIC_GAS_TOO_LOW}, + {"TR_TypeNotSupported", state::TYPE_NOT_SUPPORTED}, + {"TR_NoFunds", state::INSUFFICIENT_ACCOUNT_FUNDS}, + {"TR_NoFundsX", state::INSUFFICIENT_ACCOUNT_FUNDS}, + {"TR_NoFundsOrGas", state::INSUFFICIENT_ACCOUNT_FUNDS, state::INTRINSIC_GAS_TOO_LOW}, + {"SenderNotEOA", state::SENDER_NOT_EOA}, + {"SenderNotEOAorNoCASH", state::SENDER_NOT_EOA, state::INSUFFICIENT_ACCOUNT_FUNDS}, + {"TR_GasLimitReached", state::GAS_ALLOWANCE_EXCEEDED}, + {"TR_TipGtFeeCap", state::PRIORITY_GREATER_THAN_MAX_FEE_PER_GAS}, + {"TR_FeeCapLessThanBlocks", state::INSUFFICIENT_MAX_FEE_PER_GAS}, + {"TR_FeeCapLessThanBlocksORNoFunds", state::INSUFFICIENT_MAX_FEE_PER_GAS, + state::INSUFFICIENT_ACCOUNT_FUNDS}, + {"TR_FeeCapLessThanBlocksORGasLimitReached", state::INSUFFICIENT_MAX_FEE_PER_GAS, + state::GAS_ALLOWANCE_EXCEEDED}, + {"TR_NonceHasMaxValue", state::NONCE_IS_MAX}, + {"TR_NonceTooLow", state::NONCE_TOO_LOW}, + {"TR_NonceTooHigh", state::NONCE_TOO_HIGH}, + {"TR_RLP_WRONGVALUE", state::INVALID_ENCODING}, + {"TR_InitCodeLimitExceeded", state::INITCODE_SIZE_EXCEEDED}, + {"TR_BLOBCREATE", state::CREATE_BLOB_TX}, + {"TR_EMPTYBLOB", state::EMPTY_BLOB_HASHES_LIST}, + {"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}, + {"PostParisUncleHashIsNotEmpty", state::INCORRECT_BLOCK_FORMAT}, + {"3675PreParis1559BlockRejected", state::INCORRECT_BLOCK_FORMAT}, + {"InvalidNumber", state::INCORRECT_BLOCK_FORMAT}, + {"InvalidTimestampOlderParent", state::INVALID_BLOCK_TIMESTAMP_OLDER_THAN_PARENT}, + {"TooMuchGasUsed", state::INCORRECT_BLOCK_FORMAT}, + {"UncleParentIsNotAncestor", state::INCORRECT_BLOCK_FORMAT}, + {"InvalidGasLimit2", state::INVALID_GASLIMIT}, + {"1559BlockImportImpossible_BaseFeeWrong", state::INVALID_BASEFEE_PER_GAS}, +}; + +/// Takes the next `|`-separated name off @p list, which is left pointing past it. +std::string_view take_name(std::string_view& list) noexcept +{ + const auto end = std::min(list.find('|'), list.size()); + const auto name = list.substr(0, end); + list.remove_prefix(std::min(end + 1, list.size())); + return name; +} +} // namespace + +std::string map_legacy_exception(std::string_view expected) +{ + const auto it = std::ranges::find(LEGACY_EXCEPTIONS, expected, &LegacyException::name); + if (it == std::end(LEGACY_EXCEPTIONS)) + return std::string{expected}; + + auto names = make_error_code(it->errc).message(); + if (it->alt != state::SUCCESS) + names += '|' + make_error_code(it->alt).message(); + return names; +} + +bool contains_any(std::string_view expected, std::string_view names) noexcept +{ + while (!names.empty()) + { + const auto name = take_name(names); + for (auto rest = expected; !rest.empty();) + { + if (take_name(rest) == name) + return true; + } + } + return false; +} + +bool is_expected_tx_exception(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 make_error_code(a.errc) == ec && contains_any(expected, a.names); + }); +} +} // namespace evmone::test diff --git a/test/utils/error_matching.hpp b/test/utils/error_matching.hpp new file mode 100644 index 0000000000..d936440026 --- /dev/null +++ b/test/utils/error_matching.hpp @@ -0,0 +1,32 @@ +// evmone: Fast Ethereum Virtual Machine implementation +// Copyright 2026 The evmone Authors. +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include + +namespace evmone::test +{ +/// Rewrites a fixture's `expectException` value to the execution-spec-tests names evmone reports, +/// so both test runners compare one vocabulary. Covers the retesteth vocabulary of ethereum/tests +/// (TR_NoFunds, InvalidGasLimit2, ...) and the few block-level spec names evmone does not tell +/// apart. Anything else is returned unchanged. +[[nodiscard]] std::string map_legacy_exception(std::string_view expected); + +/// Whether any of the `|`-separated @p names is one of the `|`-separated @p expected, a fixture's +/// `expectException` value listing the exceptions it accepts. Names are compared whole: several +/// are a prefix of another (BlockException.UNKNOWN_PARENT and BlockException.UNKNOWN_PARENT_ZERO), +/// so a substring search would accept a rejection for a different rule. +/// +/// TODO(C++23): both sides become std::views::split ranges. In C++20 that view is the lazy one: +/// it yields forward ranges, not the contiguous ones std::string_view can be built from. +[[nodiscard]] bool contains_any(std::string_view expected, std::string_view names) noexcept; + +/// Whether the transaction validation error @p ec is one of the exceptions @p expected, the +/// fixture's `expectException` value. The canonical name is the error's own message; where the +/// 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; +} // namespace evmone::test diff --git a/test/utils/statetest.hpp b/test/utils/statetest.hpp index 72639e73fc..c9d071e34c 100644 --- a/test/utils/statetest.hpp +++ b/test/utils/statetest.hpp @@ -51,7 +51,11 @@ struct StateTransitionTest TestMultiTransaction::Indexes indexes; hash256 state_hash; hash256 logs_hash = EmptyListHash; - bool exception = false; + + /// The exception the transaction is expected to be rejected with, empty if it is + /// expected to be valid. Lists `|`-separated alternatives, see + /// is_expected_tx_exception() in error_matching.hpp. + std::string exception; /// The full encoded transaction for this case. Not always available. std::optional txbytes; @@ -145,7 +149,6 @@ json::json to_state_test(std::string_view test_name, const state::BlockInfo& blo state::Transaction& tx, const TestState& pre, evmc_revision rev, const std::variant& res, const TestState& post); - std::vector load_state_tests(std::istream& input); /// Validates the invariants of the Ethereum state (e.g. no zero-value storage entries). diff --git a/test/utils/statetest_loader.cpp b/test/utils/statetest_loader.cpp index 5d98b42929..118fc9f683 100644 --- a/test/utils/statetest_loader.cpp +++ b/test/utils/statetest_loader.cpp @@ -2,6 +2,7 @@ // Copyright 2022 The evmone Authors. // SPDX-License-Identifier: Apache-2.0 +#include "error_matching.hpp" #include "statetest.hpp" #include "stdx/utility.hpp" #include "utils.hpp" @@ -456,7 +457,8 @@ static void from_json(const json::json& j, StateTransitionTest::Case::Expectatio o.indexes = j.at("indexes").get(); o.state_hash = from_json(j.at("hash")); o.logs_hash = from_json(j.at("logs")); - o.exception = j.contains("expectException"); + if (const auto it = j.find("expectException"); it != j.end()) + o.exception = map_legacy_exception(it->get()); o.txbytes = load_optional(j, "txbytes"); }