feat(stateless): Run EEST tests in stateless block validator & bug fixes (#18140)

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

.config/nextest.toml

@@ -6,6 +6,10 @@ slow-timeout = { period = "30s", terminate-after = 4 }
 filter = "test(general_state_tests)"
 slow-timeout = { period = "1m", terminate-after = 10 }
 
+[[profile.default.overrides]]
+filter = "test(eest_fixtures)"
+slow-timeout = { period = "2m", terminate-after = 10 }
+
 # E2E tests using the testsuite framework from crates/e2e-test-utils
 # These tests are located in tests/e2e-testsuite/ directories across various crates
 [[profile.default.overrides]]

.github/workflows/unit.yml

@@ -81,6 +81,15 @@ jobs:
           path: testing/ef-tests/ethereum-tests
           submodules: recursive
           fetch-depth: 1
+      - name: Download & extract EEST fixtures (public)
+        shell: bash
+        env:
+          EEST_TESTS_TAG: v4.5.0
+        run: |
+          set -euo pipefail
+          mkdir -p testing/ef-tests/execution-spec-tests
+          URL="https://github.com/ethereum/execution-spec-tests/releases/download/${EEST_TESTS_TAG}/fixtures_stable.tar.gz"
+          curl -L "$URL" | tar -xz --strip-components=1 -C testing/ef-tests/execution-spec-tests
       - uses: rui314/setup-mold@v1
       - uses: dtolnay/rust-toolchain@stable
       - uses: taiki-e/install-action@nextest

Cargo.lock

@@ -3095,6 +3095,14 @@ dependencies = [
  "syn 2.0.106",
 ]
 
+[[package]]
+name = "ef-test-runner"
+version = "1.7.0"
+dependencies = [
+ "clap",
+ "ef-tests",
+]
+
 [[package]]
 name = "ef-tests"
 version = "1.7.0"

Cargo.toml

@@ -172,6 +172,7 @@ members = [
     "examples/custom-beacon-withdrawals",
     "testing/ef-tests/",
     "testing/testing-utils",
+    "testing/runner",
     "crates/tracing-otlp",
 ]
 default-members = ["bin/reth"]

Makefile

@@ -30,6 +30,11 @@ EF_TESTS_TAG := v17.0
 EF_TESTS_URL := https://github.com/ethereum/tests/archive/refs/tags/$(EF_TESTS_TAG).tar.gz
 EF_TESTS_DIR := ./testing/ef-tests/ethereum-tests
 
+# The release tag of https://github.com/ethereum/execution-spec-tests to use for EEST tests
+EEST_TESTS_TAG := v4.5.0
+EEST_TESTS_URL := https://github.com/ethereum/execution-spec-tests/releases/download/$(EEST_TESTS_TAG)/fixtures_stable.tar.gz
+EEST_TESTS_DIR := ./testing/ef-tests/execution-spec-tests
+
 # The docker image name
 DOCKER_IMAGE_NAME ?= ghcr.io/paradigmxyz/reth
 
@@ -202,9 +207,18 @@ $(EF_TESTS_DIR):
 	tar -xzf ethereum-tests.tar.gz --strip-components=1 -C $(EF_TESTS_DIR)
 	rm ethereum-tests.tar.gz
 
+# Downloads and unpacks EEST tests in the `$(EEST_TESTS_DIR)` directory.
+#
+# Requires `wget` and `tar`
+$(EEST_TESTS_DIR):
+	mkdir $(EEST_TESTS_DIR)
+	wget $(EEST_TESTS_URL) -O execution-spec-tests.tar.gz
+	tar -xzf execution-spec-tests.tar.gz --strip-components=1 -C $(EEST_TESTS_DIR)
+	rm execution-spec-tests.tar.gz
+
 .PHONY: ef-tests
-ef-tests: $(EF_TESTS_DIR) ## Runs Ethereum Foundation tests.
+ef-tests: $(EF_TESTS_DIR) $(EEST_TESTS_DIR) ## Runs Legacy and EEST tests.
-	cargo nextest run -p ef-tests --features ef-tests
+	cargo nextest run -p ef-tests --release --features ef-tests
 
 ##@ reth-bench
 

crates/chainspec/src/spec.rs

@@ -787,6 +787,12 @@ impl ChainSpecBuilder {
         self
     }
 
+    /// Resets any existing hardforks from the builder.
+    pub fn reset(mut self) -> Self {
+        self.hardforks = ChainHardforks::default();
+        self
+    }
+
     /// Set the genesis block.
     pub fn genesis(mut self, genesis: Genesis) -> Self {
         self.genesis = Some(genesis);

crates/stateless/src/trie.rs

@@ -286,7 +286,6 @@ fn calculate_state_root(
         state.accounts.into_iter().sorted_unstable_by_key(|(addr, _)| *addr)
     {
         let nibbles = Nibbles::unpack(hashed_address);
-        let account = account.unwrap_or_default();
 
         // Determine which storage root should be used for this account
         let storage_root = if let Some(storage_trie) = trie.storage_trie_mut(&hashed_address) {
@@ -298,12 +297,12 @@ fn calculate_state_root(
         };
 
         // Decide whether to remove or update the account leaf
-        if account.is_empty() && storage_root == EMPTY_ROOT_HASH {
+        if let Some(account) = account {
-            trie.remove_account_leaf(&nibbles, &provider_factory)?;
-        } else {
             account_rlp_buf.clear();
             account.into_trie_account(storage_root).encode(&mut account_rlp_buf);
             trie.update_account_leaf(nibbles, account_rlp_buf.clone(), &provider_factory)?;
+        } else {
+            trie.remove_account_leaf(&nibbles, &provider_factory)?;
         }
     }
 

crates/trie/db/src/witness.rs

@@ -44,6 +44,7 @@ impl<'a, TX: DbTx> DatabaseTrieWitness<'a, TX>
                 &state_sorted,
             ))
             .with_prefix_sets_mut(input.prefix_sets)
+            .always_include_root_node()
             .compute(target)
     }
 }

testing/ef-tests/.gitignore

@@ -1 +1,2 @@
-ethereum-tests
+ethereum-tests
+execution-spec-tests

testing/ef-tests/src/cases/blockchain_test.rs

@@ -23,26 +23,31 @@ use reth_revm::{database::StateProviderDatabase, witness::ExecutionWitnessRecord
 use reth_stateless::{validation::stateless_validation, ExecutionWitness};
 use reth_trie::{HashedPostState, KeccakKeyHasher, StateRoot};
 use reth_trie_db::DatabaseStateRoot;
-use std::{collections::BTreeMap, fs, path::Path, sync::Arc};
+use std::{
+    collections::BTreeMap,
+    fs,
+    path::{Path, PathBuf},
+    sync::Arc,
+};
 
 /// A handler for the blockchain test suite.
 #[derive(Debug)]
 pub struct BlockchainTests {
-    suite: String,
+    suite_path: PathBuf,
 }
 
 impl BlockchainTests {
-    /// Create a new handler for a subset of the blockchain test suite.
+    /// Create a new suite for tests with blockchain tests format.
-    pub const fn new(suite: String) -> Self {
+    pub const fn new(suite_path: PathBuf) -> Self {
-        Self { suite }
+        Self { suite_path }
     }
 }
 
 impl Suite for BlockchainTests {
     type Case = BlockchainTestCase;
 
-    fn suite_name(&self) -> String {
+    fn suite_path(&self) -> &Path {
-        format!("BlockchainTests/{}", self.suite)
+        &self.suite_path
     }
 }
 
@@ -157,7 +162,7 @@ impl Case for BlockchainTestCase {
     fn run(&self) -> Result<(), Error> {
         // If the test is marked for skipping, return a Skipped error immediately.
         if self.skip {
-            return Err(Error::Skipped)
+            return Err(Error::Skipped);
         }
 
         // Iterate through test cases, filtering by the network type to exclude specific forks.
@@ -306,18 +311,25 @@ fn run_case(case: &BlockchainTest) -> Result<(), Error> {
         parent = block.clone()
     }
 
-    // Validate the post-state for the test case.
+    match &case.post_state {
-    //
+        Some(expected_post_state) => {
-    // If we get here then it means that the post-state root checks
+            // Validate the post-state for the test case.
-    // made after we execute each block was successful.
+            //
-    //
+            // If we get here then it means that the post-state root checks
-    // If an error occurs here, then it is:
+            // made after we execute each block was successful.
-    // - Either an issue with the test setup
+            //
-    // - Possibly an error in the test case where the post-state root in the last block does not
+            // If an error occurs here, then it is:
-    //   match the post-state values.
+            // - Either an issue with the test setup
-    let expected_post_state = case.post_state.as_ref().ok_or(Error::MissingPostState)?;
+            // - Possibly an error in the test case where the post-state root in the last block does
-    for (&address, account) in expected_post_state {
+            //   not match the post-state values.
-        account.assert_db(address, provider.tx_ref())?;
+            for (address, account) in expected_post_state {
+                account.assert_db(*address, provider.tx_ref())?;
+            }
+        }
+        None => {
+            // Some test may not have post-state (e.g., state-heavy benchmark tests).
+            // In this case, we can skip the post-state validation.
+        }
     }
 
     // Now validate using the stateless client if everything else passes

testing/ef-tests/src/models.rs

@@ -5,7 +5,7 @@ use alloy_consensus::Header as RethHeader;
 use alloy_eips::eip4895::Withdrawals;
 use alloy_genesis::GenesisAccount;
 use alloy_primitives::{keccak256, Address, Bloom, Bytes, B256, B64, U256};
-use reth_chainspec::{ChainSpec, ChainSpecBuilder};
+use reth_chainspec::{ChainSpec, ChainSpecBuilder, EthereumHardfork, ForkCondition};
 use reth_db_api::{cursor::DbDupCursorRO, tables, transaction::DbTx};
 use reth_primitives_traits::SealedHeader;
 use serde::Deserialize;
@@ -294,9 +294,14 @@ pub enum ForkSpec {
     /// London
     London,
     /// Paris aka The Merge
+    #[serde(alias = "Paris")]
     Merge,
+    /// Paris to Shanghai at time 15k
+    ParisToShanghaiAtTime15k,
     /// Shanghai
     Shanghai,
+    /// Shanghai to Cancun at time 15k
+    ShanghaiToCancunAtTime15k,
     /// Merge EOF test
     #[serde(alias = "Merge+3540+3670")]
     MergeEOF,
@@ -308,39 +313,63 @@ pub enum ForkSpec {
     MergePush0,
     /// Cancun
     Cancun,
+    /// Cancun to Prague at time 15k
+    CancunToPragueAtTime15k,
     /// Prague
     Prague,
 }
 
 impl From<ForkSpec> for ChainSpec {
     fn from(fork_spec: ForkSpec) -> Self {
-        let spec_builder = ChainSpecBuilder::mainnet();
+        let spec_builder = ChainSpecBuilder::mainnet().reset();
 
         match fork_spec {
             ForkSpec::Frontier => spec_builder.frontier_activated(),
-            ForkSpec::Homestead | ForkSpec::FrontierToHomesteadAt5 => {
+            ForkSpec::FrontierToHomesteadAt5 => spec_builder
-                spec_builder.homestead_activated()
+                .frontier_activated()
-            }
+                .with_fork(EthereumHardfork::Homestead, ForkCondition::Block(5)),
-            ForkSpec::EIP150 | ForkSpec::HomesteadToDaoAt5 | ForkSpec::HomesteadToEIP150At5 => {
+            ForkSpec::Homestead => spec_builder.homestead_activated(),
-                spec_builder.tangerine_whistle_activated()
+            ForkSpec::HomesteadToDaoAt5 => spec_builder
-            }
+                .homestead_activated()
+                .with_fork(EthereumHardfork::Dao, ForkCondition::Block(5)),
+            ForkSpec::HomesteadToEIP150At5 => spec_builder
+                .homestead_activated()
+                .with_fork(EthereumHardfork::Tangerine, ForkCondition::Block(5)),
+            ForkSpec::EIP150 => spec_builder.tangerine_whistle_activated(),
             ForkSpec::EIP158 => spec_builder.spurious_dragon_activated(),
-            ForkSpec::Byzantium |
+            ForkSpec::EIP158ToByzantiumAt5 => spec_builder
-            ForkSpec::EIP158ToByzantiumAt5 |
+                .spurious_dragon_activated()
-            ForkSpec::ConstantinopleFix |
+                .with_fork(EthereumHardfork::Byzantium, ForkCondition::Block(5)),
-            ForkSpec::ByzantiumToConstantinopleFixAt5 => spec_builder.byzantium_activated(),
+            ForkSpec::Byzantium => spec_builder.byzantium_activated(),
+            ForkSpec::ByzantiumToConstantinopleAt5 => spec_builder
+                .byzantium_activated()
+                .with_fork(EthereumHardfork::Constantinople, ForkCondition::Block(5)),
+            ForkSpec::ByzantiumToConstantinopleFixAt5 => spec_builder
+                .byzantium_activated()
+                .with_fork(EthereumHardfork::Petersburg, ForkCondition::Block(5)),
+            ForkSpec::Constantinople => spec_builder.constantinople_activated(),
+            ForkSpec::ConstantinopleFix => spec_builder.petersburg_activated(),
             ForkSpec::Istanbul => spec_builder.istanbul_activated(),
             ForkSpec::Berlin => spec_builder.berlin_activated(),
-            ForkSpec::London | ForkSpec::BerlinToLondonAt5 => spec_builder.london_activated(),
+            ForkSpec::BerlinToLondonAt5 => spec_builder
+                .berlin_activated()
+                .with_fork(EthereumHardfork::London, ForkCondition::Block(5)),
+            ForkSpec::London => spec_builder.london_activated(),
             ForkSpec::Merge |
             ForkSpec::MergeEOF |
             ForkSpec::MergeMeterInitCode |
             ForkSpec::MergePush0 => spec_builder.paris_activated(),
+            ForkSpec::ParisToShanghaiAtTime15k => spec_builder
+                .paris_activated()
+                .with_fork(EthereumHardfork::Shanghai, ForkCondition::Timestamp(15_000)),
             ForkSpec::Shanghai => spec_builder.shanghai_activated(),
+            ForkSpec::ShanghaiToCancunAtTime15k => spec_builder
+                .shanghai_activated()
+                .with_fork(EthereumHardfork::Cancun, ForkCondition::Timestamp(15_000)),
             ForkSpec::Cancun => spec_builder.cancun_activated(),
-            ForkSpec::ByzantiumToConstantinopleAt5 | ForkSpec::Constantinople => {
+            ForkSpec::CancunToPragueAtTime15k => spec_builder
-                panic!("Overridden with PETERSBURG")
+                .cancun_activated()
-            }
+                .with_fork(EthereumHardfork::Prague, ForkCondition::Timestamp(15_000)),
             ForkSpec::Prague => spec_builder.prague_activated(),
         }
         .build()

testing/ef-tests/src/result.rs

@@ -17,9 +17,6 @@ pub enum Error {
     /// The test was skipped
     #[error("test was skipped")]
     Skipped,
-    /// No post state found in test
-    #[error("no post state found for validation")]
-    MissingPostState,
     /// Block processing failed
     /// Note: This includes but is not limited to execution.
     /// For example, the header number could be incorrect.

testing/ef-tests/src/suite.rs

@@ -12,25 +12,28 @@ pub trait Suite {
     /// The type of test cases in this suite.
     type Case: Case;
 
-    /// The name of the test suite used to locate the individual test cases.
+    /// The path to the test suite directory.
-    ///
+    fn suite_path(&self) -> &Path;
-    /// # Example
-    ///
-    /// - `GeneralStateTests`
-    /// - `BlockchainTests/InvalidBlocks`
-    /// - `BlockchainTests/TransitionTests`
-    fn suite_name(&self) -> String;
 
-    /// Load and run each contained test case.
+    /// Run all test cases in the suite.
+    fn run(&self) {
+        let suite_path = self.suite_path();
+        for entry in WalkDir::new(suite_path).min_depth(1).max_depth(1) {
+            let entry = entry.expect("Failed to read directory");
+            if entry.file_type().is_dir() {
+                self.run_only(entry.file_name().to_string_lossy().as_ref());
+            }
+        }
+    }
+
+    /// Load and run each contained test case for the provided sub-folder.
     ///
     /// # Note
     ///
     /// This recursively finds every test description in the resulting path.
-    fn run(&self) {
+    fn run_only(&self, name: &str) {
         // Build the path to the test suite directory
-        let suite_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        let suite_path = self.suite_path().join(name);
-            .join("ethereum-tests")
-            .join(self.suite_name());
 
         // Verify that the path exists
         assert!(suite_path.exists(), "Test suite path does not exist: {suite_path:?}");
@@ -48,7 +51,7 @@ pub trait Suite {
         let results = Cases { test_cases }.run();
 
         // Assert that all tests in the suite pass
-        assert_tests_pass(&self.suite_name(), &suite_path, &results);
+        assert_tests_pass(name, &suite_path, &results);
     }
 }
 

testing/ef-tests/tests/tests.rs

@@ -2,13 +2,19 @@
 #![cfg(feature = "ef-tests")]
 
 use ef_tests::{cases::blockchain_test::BlockchainTests, suite::Suite};
+use std::path::PathBuf;
 
 macro_rules! general_state_test {
     ($test_name:ident, $dir:ident) => {
         #[test]
         fn $test_name() {
             reth_tracing::init_test_tracing();
-            BlockchainTests::new(format!("GeneralStateTests/{}", stringify!($dir))).run();
+            let suite_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+                .join("ethereum-tests")
+                .join("BlockchainTests");
+
+            BlockchainTests::new(suite_path)
+                .run_only(&format!("GeneralStateTests/{}", stringify!($dir)));
         }
     };
 }
@@ -83,10 +89,24 @@ macro_rules! blockchain_test {
         #[test]
         fn $test_name() {
             reth_tracing::init_test_tracing();
-            BlockchainTests::new(format!("{}", stringify!($dir))).run();
+            let suite_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+                .join("ethereum-tests")
+                .join("BlockchainTests");
+
+            BlockchainTests::new(suite_path).run_only(&format!("{}", stringify!($dir)));
         }
     };
 }
 
 blockchain_test!(valid_blocks, ValidBlocks);
 blockchain_test!(invalid_blocks, InvalidBlocks);
+
+#[test]
+fn eest_fixtures() {
+    reth_tracing::init_test_tracing();
+    let suite_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        .join("execution-spec-tests")
+        .join("blockchain_tests");
+
+    BlockchainTests::new(suite_path).run();
+}

testing/runner/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "ef-test-runner"
+version.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+license.workspace = true
+homepage.workspace = true
+repository.workspace = true
+exclude.workspace = true
+
+[dependencies]
+clap = { workspace = true, features = ["derive"] }
+ef-tests.path = "../ef-tests"
+
+[lints]
+workspace = true

testing/runner/src/main.rs

@@ -0,0 +1,17 @@
+//! Command-line interface for running tests.
+use std::path::PathBuf;
+
+use clap::Parser;
+use ef_tests::{cases::blockchain_test::BlockchainTests, Suite};
+
+/// Command-line arguments for the test runner.
+#[derive(Debug, Parser)]
+pub struct TestRunnerCommand {
+    /// Path to the test suite
+    suite_path: PathBuf,
+}
+
+fn main() {
+    let cmd = TestRunnerCommand::parse();
+    BlockchainTests::new(cmd.suite_path.join("blockchain_tests")).run();
+}