Export state tests with invalid txs (#858)

Author: pdobacz

test/statetest/statetest_runner.cpp

@@ -50,11 +50,18 @@ void run_state_test(const StateTransitionTest& test, evmc::VM& vm, bool trace_su
                 std::clog << R"("stateRoot":"0x)" << hex(state_root) << "\"}\n";
             }
 
-            if (holds_alternative<state::TransactionReceipt>(res))
-                EXPECT_EQ(logs_hash(get<state::TransactionReceipt>(res).logs), expected.logs_hash);
+            if (expected.exception)
+            {
+                ASSERT_FALSE(holds_alternative<state::TransactionReceipt>(res))
+                    << "unexpected valid transaction";
+                EXPECT_EQ(logs_hash(std::vector<state::Log>()), expected.logs_hash);
+            }
             else
-                EXPECT_TRUE(expected.exception)
+            {
+                ASSERT_TRUE(holds_alternative<state::TransactionReceipt>(res))
                     << "unexpected invalid transaction: " << get<std::error_code>(res).message();
+                EXPECT_EQ(logs_hash(get<state::TransactionReceipt>(res).logs), expected.logs_hash);
+            }
 
             EXPECT_EQ(state_root, expected.state_hash);
         }

test/unittests/state_transition.cpp

@@ -15,8 +15,7 @@ void state_transition::SetUp()
 {
     pre.insert(tx.sender, {.nonce = 1, .balance = tx.gas_limit * tx.max_gas_price + tx.value + 1});
 
-    // Default expectations.
-    expect.post[Coinbase].exists = true;
+    // Default expectation (coinbase is added later for valid txs only).
     expect.post[tx.sender].exists = true;
 }
 
@@ -69,20 +68,28 @@ void state_transition::TearDown()
         EXPECT_EQ(tx_error, expected_error)
             << tx_error.message() << " vs " << expected_error.message();
 
-        // TODO: Compare states carefully, they should be identical.
         EXPECT_EQ(state.get_accounts().size(), pre.get_accounts().size());
         for (const auto& [addr, acc] : state.get_accounts())
         {
             EXPECT_TRUE(pre.get_accounts().contains(addr)) << "unexpected account " << addr;
         }
-
-        // TODO: Export also tests with invalid transactions.
-        return;  // Do not check anything else.
     }
+    else
+    {
+        ASSERT_TRUE(holds_alternative<TransactionReceipt>(res))
+            << std::get<std::error_code>(res).message();
+        const auto& receipt = std::get<TransactionReceipt>(res);
 
-    ASSERT_TRUE(holds_alternative<TransactionReceipt>(res))
-        << std::get<std::error_code>(res).message();
-    const auto& receipt = std::get<TransactionReceipt>(res);
+        EXPECT_EQ(receipt.status, expect.status);
+        if (expect.gas_used.has_value())
+        {
+            EXPECT_EQ(receipt.gas_used, *expect.gas_used);
+        }
+        // Update default expectations - valid transaction means coinbase exists unless explicitly
+        // requested otherwise
+        if (expect.post.find(Coinbase) == expect.post.end())
+            expect.post[Coinbase].exists = true;
+    }
     state::finalize(state, rev, block.coinbase, block_reward, block.ommers, block.withdrawals);
 
     if (trace)
@@ -92,12 +99,6 @@ void state_transition::TearDown()
         EXPECT_EQ(trace_capture->get_capture(), expect.trace);
     }
 
-    EXPECT_EQ(receipt.status, expect.status);
-    if (expect.gas_used.has_value())
-    {
-        EXPECT_EQ(receipt.gas_used, *expect.gas_used);
-    }
-
     for (const auto& [addr, expected_acc] : expect.post)
     {
         const auto acc = state.find(addr);
@@ -146,7 +147,7 @@ void state_transition::TearDown()
     }
 
     if (!export_file_path.empty())
-        export_state_test(receipt, state);
+        export_state_test(res, state);
 }
 
 namespace
@@ -166,7 +167,8 @@ std::string_view to_test_fork_name(evmc_revision rev) noexcept
 }
 }  // namespace
 
-void state_transition::export_state_test(const TransactionReceipt& receipt, const State& post)
+void state_transition::export_state_test(
+    const std::variant<TransactionReceipt, std::error_code>& res, const State& post)
 {
     json::json j;
     auto& jt = j[export_test_name];
@@ -225,7 +227,16 @@ void state_transition::export_state_test(const TransactionReceipt& receipt, cons
     auto& jpost = jt["post"][to_test_fork_name(rev)][0];
     jpost["indexes"] = {{"data", 0}, {"gas", 0}, {"value", 0}};
     jpost["hash"] = hex0x(mpt_hash(post.get_accounts()));
-    jpost["logs"] = hex0x(logs_hash(receipt.logs));
+
+    if (holds_alternative<std::error_code>(res))
+    {
+        jpost["expectException"] = std::get<std::error_code>(res).message();
+        jpost["logs"] = hex0x(logs_hash(std::vector<Log>()));
+    }
+    else
+    {
+        jpost["logs"] = hex0x(logs_hash(std::get<TransactionReceipt>(res).logs));
+    }
 
     std::ofstream{export_file_path} << std::setw(2) << j;
 }

test/unittests/state_transition.hpp

@@ -92,7 +92,8 @@ class state_transition : public ExportableFixture
     void TearDown() override;
 
     /// Exports the test in the JSON State Test format to ExportableFixture::export_out.
-    void export_state_test(const TransactionReceipt& receipt, const State& post);
+    void export_state_test(
+        const std::variant<TransactionReceipt, std::error_code>& res, const State& post);
 };
 
 }  // namespace evmone::test

test/unittests/state_transition_eof_create_test.cpp

@@ -1243,6 +1243,7 @@ TEST_F(state_transition, txcreate_initcontainer_empty)
     pre.insert(*tx.to, {.nonce = 1, .code = factory_container});
 
     expect.tx_error = INIT_CODE_EMPTY;
+    expect.post[*tx.to].exists = true;
 }
 
 TEST_F(state_transition, txcreate_no_initcontainer)
@@ -1258,6 +1259,7 @@ TEST_F(state_transition, txcreate_no_initcontainer)
     pre.insert(*tx.to, {.nonce = 1, .code = factory_container});
 
     expect.tx_error = INIT_CODE_COUNT_ZERO;
+    expect.post[*tx.to].exists = true;
 }
 
 TEST_F(state_transition, txcreate_initcontainer_too_large)
@@ -1287,6 +1289,7 @@ TEST_F(state_transition, txcreate_initcontainer_too_large)
     pre.insert(*tx.to, {.nonce = 1, .code = factory_container});
 
     expect.tx_error = INIT_CODE_SIZE_LIMIT_EXCEEDED;
+    expect.post[*tx.to].exists = true;
 }
 
 TEST_F(state_transition, txcreate_too_many_initcontainers)
@@ -1312,6 +1315,7 @@ TEST_F(state_transition, txcreate_too_many_initcontainers)
     pre.insert(*tx.to, {.nonce = 1, .code = factory_container});
 
     expect.tx_error = INIT_CODE_COUNT_LIMIT_EXCEEDED;
+    expect.post[*tx.to].exists = true;
 }
 
 TEST_F(state_transition, initcode_transaction_before_prague)

test/unittests/state_transition_tx_test.cpp

@@ -29,7 +29,7 @@ TEST_F(state_transition, tx_non_existing_sender)
 
     expect.status = EVMC_SUCCESS;
     expect.post.at(Sender).nonce = 1;
-    expect.post.at(Coinbase).exists = false;
+    expect.post[Coinbase].exists = false;
 }
 
 TEST_F(state_transition, invalid_tx_non_existing_sender)
@@ -43,6 +43,7 @@ TEST_F(state_transition, invalid_tx_non_existing_sender)
     pre.get_accounts().erase(Sender);
 
     expect.tx_error = INSUFFICIENT_FUNDS;
+    expect.post[Sender].exists = false;
 }
 
 TEST_F(state_transition, tx_blob_gas_price)
@@ -60,7 +61,7 @@ TEST_F(state_transition, tx_blob_gas_price)
 
     pre.get(tx.sender).balance = 0x20000 + tx.gas_limit * tx.max_gas_price;
 
-    expect.post.at(Coinbase).exists = false;  // all gas is burned, Coinbase gets nothing
+    expect.post[Coinbase].exists = false;  // all gas is burned, Coinbase gets nothing
     expect.status = EVMC_SUCCESS;
 }