refactor(levm): rewrite of state EF tests runner first iteration #3642

sofiazcoaga · 2025-07-15T16:41:43Z

Motivation

Related issue: #3496.

The idea is to incrementally develop a new EF Test runner (for state tests) that can eventually replace the current one. The main goal of the new runner is to be easy to understand and as straightforward as possible, also making it possible to easily add any new requirement.

How to run
A target in the makefile was included. You can, then, from ethrex/cmd/ef_tests/state/ run make run-new-runner. If no specific path is passed, it will parse anything in the ./vectors folder. Otherwise you can do, for example:
make run-new-runner TESTS_PATH=./vectors/GeneralStateTests/Cancun to specify a path.

This command assumes you have the vectors directory downloaded, if not run make download-evm-ef-tests previously.

Considerations

The main changes are:

The new Test and TestCase structures in types.
The runner and parser simplified flows.

Files that should not be reviewed as they are full or partial copies of the original files:

runner_v2/deserialize.rs
runner_v2/utils.rs

This iteration excludes report-related code, option flags and other possible test case errors to be considered that will be included later. Checks are performed only on exceptions and root hash.

github-actions · 2025-07-15T16:43:41Z

Lines of code report

Total lines added: 1087
Total lines removed: 0
Total lines changed: 1087

Detailed view

+-----------------------------------------------------+-------+------+
| File                                                | Lines | Diff |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/lib.rs                    | 7     | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/deserialize.rs  | 294   | +294 |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/error.rs        | 11    | +11  |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/mod.rs          | 7     | +7   |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/parser.rs       | 48    | +48  |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/result_check.rs | 166   | +166 |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/run.rs          | 13    | +13  |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/runner.rs       | 104   | +104 |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/types.rs        | 406   | +406 |
+-----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner_v2/utils.rs        | 37    | +37  |
+-----------------------------------------------------+-------+------+

cdiielsi

Really nice work! I like how you distributed the modules, having runner separated from result_check makes things feel less crowded and easier to navigate. I made a couple of comments, one you can consider moving on (it's about writing reports) the rest are rather subjective and totally skippable.

cmd/ef_tests/state/runner_v2/error.rs

cmd/ef_tests/state/runner_v2/result_check.rs

JereSalo · 2025-07-16T21:06:50Z

Suggestion, leave in PR description the necessary steps for running the tests

JereSalo · 2025-07-16T21:09:28Z

Parsing test files...
Error: FailedToParseTestFile("./vectors/LegacyTests/Cancun/GeneralStateTests/Cancun/stEIP1153-transientStorage/21_tstoreCannotBeDosdOOO.json", "Failed to deserialize post field in test 21_tstoreCannotBeDosdOOO. Serde error: missing field state")

JereSalo · 2025-07-16T21:12:14Z

Now that each test is one struct we can filter out repeated tests, a problem in the other runner that we have was that we were running some tests twice because some files were duplicated

sofiazcoaga · 2025-07-18T13:48:33Z

@JereSalo your suggestions where added here:

The parsing error was fixed by making state field optional here
Include the possibility to specify a path to the tests here
Makefile command to run the tests here
How to run the tests left in the PR description

The comment about repeated runs I consider better to leave for another PR. What do you think?

JereSalo

Some additional comments:

Leave rustdocs comments in top of functions/methods. Even the ones that are kinda self-explanatory. Usually one line is enough for some of the short functions.
Add runner_report.txt to .gitignore
Use unwrap and delete a lot of the errors, keep only the errors that are specific to the VM if possible. If something shouldn't happen (like failing to write a file) just unwrap. Abuse this as code clarity and size matters, we want to make it easier and faster to read.

JereSalo · 2025-07-18T17:36:12Z

cmd/ef_tests/state/runner_v2/runner.rs

+    // Remove previous report if it exists.
+    let _ = fs::remove_file("./runner_v2/runner_report.txt").await;


Can't we simply overwrite instead of manually removing?

The thing is, every test report is written as soon as it finishes executing, so I have to open the file in append mode. To do that, I need to make sure that I'm appending to a file that belongs only to this run and no other.

cmd/ef_tests/state/runner_v2/run.rs

JereSalo · 2025-07-18T17:41:21Z

cmd/ef_tests/state/runner_v2/error.rs

+pub enum RunnerError {
+    RootMismatch,
+    FailedToGetAccountsUpdates,
+    VMExecutionError(String),
+    CurrentBaseFeeMissing,
+    MaxPriorityFeePerGasMissing,
+    MaxFeePerGasMissing,
+    TxSucceededAndExceptionWasExpected,
+    DifferentExceptionWasExpected,
+    EIP7702ShouldNotBeCreateType,
+    FailedToReadDirectory(PathBuf, String),
+    FailedToConvertPath,
+    FailedToGetFileType(String),
+    FailedToParseTestFile(PathBuf, String),
+    FailedToOpenFile(String),
+    FailedToWriteReport(String),
+    MissingJsonField(String),
+    FailedToDeserializeField(String),
+    FailedToCreateReportFile(String),
+    FailedToGetIndexValue(String),
+}


This needs to be tidy, it's too cluttered. It's okay for this iteration to be pretty basic but it has to be the simplest it can possibly be and this enum is overwhelming.

I removed a lot of errors as I mention in this comment, but I will certainly keep in mind to make this file tidier in later iterations. Thank you!

cmd/ef_tests/state/runner_v2/parser.rs

cmd/ef_tests/state/runner_v2/runner.rs

cmd/ef_tests/state/runner_v2/utils.rs

cmd/ef_tests/state/runner_v2/result_check.rs

cmd/ef_tests/state/Makefile

JereSalo · 2025-07-18T20:27:43Z

@JereSalo your suggestions where added here:

The parsing error was fixed by making state field optional here

Include the possibility to specify a path to the tests here

Makefile command to run the tests here

How to run the tests left in the PR description

The comment about repeated runs I consider better to leave for another PR. What do you think?

That's fine, I just thought this was important because it's a thing that we should consider for the structure of the tests. But I agree it should be tackle in another PR, it is a priority though.

sofiazcoaga · 2025-07-21T19:02:59Z

Some additional comments:

Leave rustdocs comments in top of functions/methods. Even the ones that are kinda self-explanatory. Usually one line is enough for some of the short functions.

Add runner_report.txt to .gitignore

Use unwrap and delete a lot of the errors, keep only the errors that are specific to the VM if possible. If something shouldn't happen (like failing to write a file) just unwrap. Abuse this as code clarity and size matters, we want to make it easier and faster to read.

I removed a lot of custom errors for situations external to VM problems in this commit, replacing with unwrap() as you suggested. This made the error.rs file smaller as well.

I added the report file to .gitignore here.

Also, I added some comments on the functions purposes here.

JereSalo

good job, I think the comment on the type of the transaction is wrong though but the pr is okay

sofiazcoaga added 3 commits July 15, 2025 13:24

add first iteration for new ef state tests runner

5b56dd0

include new runner execution in Cargo.toml tests

0dc1cdf

add new runner module in lib.rs

be2b62e

sofiazcoaga requested a review from a team as a code owner July 15, 2025 16:41

github-actions bot added the levm Lambda EVM implementation label Jul 15, 2025

github-project-automation bot added this to ethrex_l1 Jul 15, 2025

github-actions bot assigned sofiazcoaga Jul 15, 2025

sofiazcoaga marked this pull request as draft July 15, 2025 16:41

apply clippy suggestions

646233a

sofiazcoaga changed the title ~~refactor(levm): refactor of state EF tests runner~~ refactor(levm): rewrite of state EF tests runner first iteration Jul 15, 2025

sofiazcoaga marked this pull request as ready for review July 15, 2025 17:24

Merge branch 'main' into ef_tests/refactor-runner

e96db06

cdiielsi approved these changes Jul 16, 2025

View reviewed changes

cmd/ef_tests/state/runner_v2/error.rs Show resolved Hide resolved

cmd/ef_tests/state/runner_v2/result_check.rs Show resolved Hide resolved

cmd/ef_tests/state/runner_v2/result_check.rs Show resolved Hide resolved

cmd/ef_tests/state/runner_v2/result_check.rs Outdated Show resolved Hide resolved

sofiazcoaga mentioned this pull request Jul 16, 2025

refactor(levm): rewrite of EF state tests runner second iteration #3666

Open

sofiazcoaga added 2 commits July 16, 2025 17:00

remove unneeded unwrap() by using if let some() in result checks

2e5c7e5

rename exception_is_expected() to exception_matches_expected()

dd73b5c

mpaulucci moved this to In Progress in ethrex_l1 Jul 17, 2025

sofiazcoaga added 3 commits July 17, 2025 17:54

turn state field in Post an optional field

49715ae

obtain test path from cargo run parameters

f9df85c

add make command to run new runner

6d790a6

sofiazcoaga added 2 commits July 18, 2025 12:47

add release flag to new runner execution

d6f03f0

add skipped tests from the old runner

aa1431b

sofiazcoaga requested a review from JereSalo July 18, 2025 15:54

JereSalo requested changes Jul 18, 2025

View reviewed changes

JereSalo force-pushed the ef_tests/refactor-runner branch from 95eec0f to aa1431b Compare July 18, 2025 20:05

sofiazcoaga added 5 commits July 21, 2025 15:29

add new ef state tests runner report to gitignore

ca2faba

change default path for new tests runner in makefile

ab66cbc

replace custom errors with unwraps for non-VM problems

5b43fb7

include explanatory comments for tx construction

cd5f438

change runner terminal output

64fc746

add doc comments to runner functions

0805383

JereSalo approved these changes Jul 21, 2025

View reviewed changes

github-project-automation bot moved this from In Progress to In Review in ethrex_l1 Jul 21, 2025

sofiazcoaga and others added 4 commits July 22, 2025 11:47

fix comment on transaction types in the runner

50c12ab

Merge branch 'main' into ef_tests/refactor-runner

9ff3a12

handle VM creation result in new runner

2b5a26c

remove unneeded closure in VM creation error handling

ab70faf

sofiazcoaga added this pull request to the merge queue Jul 22, 2025

Merged via the queue into main with commit 2ce46bf Jul 22, 2025
37 checks passed

sofiazcoaga deleted the ef_tests/refactor-runner branch July 22, 2025 18:42

github-project-automation bot moved this from In Review to Done in ethrex_l1 Jul 22, 2025

		// Remove previous report if it exists.
		let _ = fs::remove_file("./runner_v2/runner_report.txt").await;

refactor(levm): rewrite of state EF tests runner first iteration #3642

refactor(levm): rewrite of state EF tests runner first iteration #3642

Uh oh!

Conversation

sofiazcoaga commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lines of code report

Uh oh!

cdiielsi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JereSalo commented Jul 16, 2025

Uh oh!

JereSalo commented Jul 16, 2025

Uh oh!

JereSalo commented Jul 16, 2025

Uh oh!

sofiazcoaga commented Jul 18, 2025

Uh oh!

JereSalo left a comment

Choose a reason for hiding this comment

Uh oh!

JereSalo Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

sofiazcoaga Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JereSalo Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

sofiazcoaga Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JereSalo commented Jul 18, 2025

Uh oh!

sofiazcoaga commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JereSalo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sofiazcoaga commented Jul 15, 2025 •

edited

Loading

github-actions bot commented Jul 15, 2025 •

edited

Loading

sofiazcoaga commented Jul 21, 2025 •

edited

Loading