test: Check why a blockchain test block was rejected

test/blockchaintest/blockchaintest_runner.cpp

@@ -9,6 +9,7 @@
 #include <test/state/requests.hpp>
 #include <test/state/rlp_decode.hpp>
 #include <test/utils/block_transition.hpp>
+#include <test/utils/error_matching.hpp>
 #include <test/utils/mpt_hash.hpp>
 #include <test/utils/rlp.hpp>
 #include <test/utils/rlp_encode.hpp>
@@ -328,12 +329,9 @@ void run_blockchain_tests(std::span<const BlockchainTest> 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<const BlockchainTest> 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<int64_t>(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";
             }

test/utils/block_transition.cpp

@@ -67,8 +67,7 @@ TransitionResult apply_block(const TestState& state, evmc::VM& vm, const state::
 
         if (holds_alternative<std::error_code>(res))
         {
-            const auto ec = std::get<std::error_code>(res);
-            rejected_txs.push_back({computed_tx_hash, i, ec.message()});
+            rejected_txs.push_back({computed_tx_hash, i, std::get<std::error_code>(res)});
         }
         else
         {

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.

test/utils/error_matching.cpp

@@ -5,16 +5,17 @@
 #include "error_matching.hpp"
 #include <test/state/errors.hpp>
 #include <algorithm>
+#include <span>
 
 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<const AlternativeExceptions> 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

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

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