From 394a53d7b0f594f1f9f19ff00a7b603023f053e7 Mon Sep 17 00:00:00 2001 From: Ignacio Hagopian Date: Tue, 9 Sep 2025 14:48:14 +0200 Subject: [PATCH] feat(stateless): Run EEST tests in stateless block validator & bug fixes (#18140) Signed-off-by: Ignacio Hagopian Co-authored-by: Matthias Seitz --- .config/nextest.toml | 4 ++ .github/workflows/unit.yml | 9 +++ Cargo.lock | 8 +++ Cargo.toml | 1 + Makefile | 18 +++++- crates/chainspec/src/spec.rs | 6 ++ crates/stateless/src/trie.rs | 7 +-- crates/trie/db/src/witness.rs | 1 + testing/ef-tests/.gitignore | 3 +- testing/ef-tests/src/cases/blockchain_test.rs | 52 ++++++++++------ testing/ef-tests/src/models.rs | 61 ++++++++++++++----- testing/ef-tests/src/result.rs | 3 - testing/ef-tests/src/suite.rs | 31 +++++----- testing/ef-tests/tests/tests.rs | 24 +++++++- testing/runner/Cargo.toml | 16 +++++ testing/runner/src/main.rs | 17 ++++++ 16 files changed, 199 insertions(+), 62 deletions(-) create mode 100644 testing/runner/Cargo.toml create mode 100644 testing/runner/src/main.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 94d55bf031..26b4a000b9 100644 --- a/.config/nextest.toml +++ b/.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]] diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml index 39aeebde21..d9aca93f21 100644 --- a/.github/workflows/unit.yml +++ b/.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 diff --git a/Cargo.lock b/Cargo.lock index 9ce08fa66d..40057e7335 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index eaa9225fc8..a4e39d05e9 100644 --- a/Cargo.toml +++ b/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"] diff --git a/Makefile b/Makefile index f14c3730cb..30f6b0aa47 100644 --- a/Makefile +++ b/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. - cargo nextest run -p ef-tests --features ef-tests +ef-tests: $(EF_TESTS_DIR) $(EEST_TESTS_DIR) ## Runs Legacy and EEST tests. + cargo nextest run -p ef-tests --release --features ef-tests ##@ reth-bench diff --git a/crates/chainspec/src/spec.rs b/crates/chainspec/src/spec.rs index 58aaf45c94..206ac3339b 100644 --- a/crates/chainspec/src/spec.rs +++ b/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); diff --git a/crates/stateless/src/trie.rs b/crates/stateless/src/trie.rs index 9fd2639249..49d1f6cf0f 100644 --- a/crates/stateless/src/trie.rs +++ b/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 { - trie.remove_account_leaf(&nibbles, &provider_factory)?; - } else { + if let Some(account) = account { 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)?; } } diff --git a/crates/trie/db/src/witness.rs b/crates/trie/db/src/witness.rs index a240734f8c..3afb8c340c 100644 --- a/crates/trie/db/src/witness.rs +++ b/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) } } diff --git a/testing/ef-tests/.gitignore b/testing/ef-tests/.gitignore index eae5bd973f..0bf9998816 100644 --- a/testing/ef-tests/.gitignore +++ b/testing/ef-tests/.gitignore @@ -1 +1,2 @@ -ethereum-tests \ No newline at end of file +ethereum-tests +execution-spec-tests \ No newline at end of file diff --git a/testing/ef-tests/src/cases/blockchain_test.rs b/testing/ef-tests/src/cases/blockchain_test.rs index a420296e91..9cc70ff5b4 100644 --- a/testing/ef-tests/src/cases/blockchain_test.rs +++ b/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. - pub const fn new(suite: String) -> Self { - Self { suite } + /// Create a new suite for tests with blockchain tests format. + pub const fn new(suite_path: PathBuf) -> Self { + Self { suite_path } } } impl Suite for BlockchainTests { type Case = BlockchainTestCase; - fn suite_name(&self) -> String { - format!("BlockchainTests/{}", self.suite) + fn suite_path(&self) -> &Path { + &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. - // - // If we get here then it means that the post-state root checks - // made after we execute each block was successful. - // - // If an error occurs here, then it is: - // - 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 - // match the post-state values. - let expected_post_state = case.post_state.as_ref().ok_or(Error::MissingPostState)?; - for (&address, account) in expected_post_state { - account.assert_db(address, provider.tx_ref())?; + match &case.post_state { + Some(expected_post_state) => { + // Validate the post-state for the test case. + // + // If we get here then it means that the post-state root checks + // made after we execute each block was successful. + // + // If an error occurs here, then it is: + // - 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 match the post-state values. + 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 diff --git a/testing/ef-tests/src/models.rs b/testing/ef-tests/src/models.rs index 6cad5331e5..49c49bf193 100644 --- a/testing/ef-tests/src/models.rs +++ b/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 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 => { - spec_builder.homestead_activated() - } - ForkSpec::EIP150 | ForkSpec::HomesteadToDaoAt5 | ForkSpec::HomesteadToEIP150At5 => { - spec_builder.tangerine_whistle_activated() - } + ForkSpec::FrontierToHomesteadAt5 => spec_builder + .frontier_activated() + .with_fork(EthereumHardfork::Homestead, ForkCondition::Block(5)), + ForkSpec::Homestead => spec_builder.homestead_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 | - ForkSpec::ConstantinopleFix | - ForkSpec::ByzantiumToConstantinopleFixAt5 => spec_builder.byzantium_activated(), + ForkSpec::EIP158ToByzantiumAt5 => spec_builder + .spurious_dragon_activated() + .with_fork(EthereumHardfork::Byzantium, ForkCondition::Block(5)), + 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 => { - panic!("Overridden with PETERSBURG") - } + ForkSpec::CancunToPragueAtTime15k => spec_builder + .cancun_activated() + .with_fork(EthereumHardfork::Prague, ForkCondition::Timestamp(15_000)), ForkSpec::Prague => spec_builder.prague_activated(), } .build() diff --git a/testing/ef-tests/src/result.rs b/testing/ef-tests/src/result.rs index f53a4fab25..0284e06da0 100644 --- a/testing/ef-tests/src/result.rs +++ b/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. diff --git a/testing/ef-tests/src/suite.rs b/testing/ef-tests/src/suite.rs index 237ca935ba..0b3ed447a2 100644 --- a/testing/ef-tests/src/suite.rs +++ b/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. - /// - /// # Example - /// - /// - `GeneralStateTests` - /// - `BlockchainTests/InvalidBlocks` - /// - `BlockchainTests/TransitionTests` - fn suite_name(&self) -> String; + /// The path to the test suite directory. + fn suite_path(&self) -> &Path; - /// 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")) - .join("ethereum-tests") - .join(self.suite_name()); + let suite_path = self.suite_path().join(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); } } diff --git a/testing/ef-tests/tests/tests.rs b/testing/ef-tests/tests/tests.rs index a1838d43e5..0961817e90 100644 --- a/testing/ef-tests/tests/tests.rs +++ b/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(); +} diff --git a/testing/runner/Cargo.toml b/testing/runner/Cargo.toml new file mode 100644 index 0000000000..0b6893fd8b --- /dev/null +++ b/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 diff --git a/testing/runner/src/main.rs b/testing/runner/src/main.rs new file mode 100644 index 0000000000..a36c443850 --- /dev/null +++ b/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(); +}