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