refactor(levm): moved retdata and state backup to callframe #2666

SDartayet · 2025-05-05T16:32:25Z

Motivation

Simplifying the data structures that make up the VM.

Description

Retdata and backup are always popped when callframe is, so both were made into a part of call_frame to simplify the code.

Resolves issue #2571

Depends on PR #2645

This reverts commit 739598f.

This reverts commit 0d172c9.

github-actions · 2025-05-05T16:33:30Z

Lines of code report

Total lines added: 79
Total lines removed: 210
Total lines changed: 289

Detailed view

+------------------------------------------------------+-------+------+
| File                                                 | Lines | Diff |
+------------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner/levm_runner.rs      | 382   | -1   |
+------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                             | 332   | +1   |
+------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs                    | 374   | -5   |
+------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/utils.rs                           | 118   | -10  |
+------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex_l2/src/commands/stack.rs           | 403   | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/bench/src/main.rs            | 96    | +1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/bench/src/rpc/db.rs          | 488   | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_watcher.rs             | 281   | -20  |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/discv4/server.rs        | 681   | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/network.rs              | 173   | -4   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection.rs      | 535   | -5   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/handshake.rs       | 424   | -2   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/p2p.rs             | 237   | -3   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/admin/mod.rs            | 57    | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/clients/eth/mod.rs      | 1076  | -23  |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/max_priority_fee.rs | 129   | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/lib.rs                  | 16    | +1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs                  | 643   | -4   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/utils.rs                | 411   | -2   |
+------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/web3/mod.rs             | 6     | +6   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                | 638   | -2   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/call_frame.rs              | 135   | +7   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/environment.rs             | 28    | -1   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/errors.rs                  | 228   | -2   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/execution_handlers.rs      | 222   | -11  |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/default_hook.rs      | 264   | -75  |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/l2_hook.rs           | 194   | +61  |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/mod.rs               | 3     | -4   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs  | 844   | +2   |
+------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                      | 355   | -31  |
+------------------------------------------------------+-------+------+

github-actions · 2025-05-05T16:44:08Z

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Factorial`	238.2 ± 12.3	232.4	272.6	1.00
`levm_Factorial`	872.7 ± 4.5	868.1	883.0	3.66 ± 0.19

Benchmark Results: Factorial - Recursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_FactorialRecursive`	1.443 ± 0.081	1.336	1.563	1.00
`levm_FactorialRecursive`	13.266 ± 0.250	12.975	13.747	9.19 ± 0.54

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Fibonacci`	215.8 ± 3.5	211.7	221.5	1.00
`levm_Fibonacci`	887.6 ± 38.0	863.3	991.3	4.11 ± 0.19

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ManyHashes`	8.7 ± 0.2	8.5	9.1	1.00
`levm_ManyHashes`	17.5 ± 0.3	17.2	18.4	2.03 ± 0.06

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_BubbleSort`	3.245 ± 0.011	3.231	3.265	1.00
`levm_BubbleSort`	5.729 ± 0.031	5.677	5.772	1.77 ± 0.01

Benchmark Results: ERC20 - Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Transfer`	254.6 ± 4.7	251.0	263.8	1.00
`levm_ERC20Transfer`	505.7 ± 7.3	499.1	520.8	1.99 ± 0.05

Benchmark Results: ERC20 - Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Mint`	143.4 ± 0.6	142.3	144.4	1.00
`levm_ERC20Mint`	319.7 ± 4.7	316.1	332.2	2.23 ± 0.03

Benchmark Results: ERC20 - Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_ERC20Approval`	1.052 ± 0.013	1.038	1.074	1.00
`levm_ERC20Approval`	1.914 ± 0.013	1.896	1.929	1.82 ± 0.03

Main Results

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Factorial`	239.7 ± 1.8	236.8	244.1	1.00
`levm_Factorial`	875.1 ± 3.6	871.6	882.9	3.65 ± 0.03

Benchmark Results: Factorial - Recursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_FactorialRecursive`	1.419 ± 0.104	1.323	1.631	1.00
`levm_FactorialRecursive`	13.347 ± 0.640	13.013	15.116	9.40 ± 0.82

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Fibonacci`	216.2 ± 1.7	212.2	218.2	1.00
`levm_Fibonacci`	877.2 ± 5.7	869.8	887.0	4.06 ± 0.04

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ManyHashes`	8.9 ± 0.1	8.8	9.0	1.00
`levm_ManyHashes`	18.2 ± 0.3	17.8	18.8	2.05 ± 0.04

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_BubbleSort`	3.275 ± 0.020	3.251	3.310	1.00
`levm_BubbleSort`	5.834 ± 0.035	5.799	5.920	1.78 ± 0.02

Benchmark Results: ERC20 - Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Transfer`	254.5 ± 3.4	251.2	263.2	1.00
`levm_ERC20Transfer`	504.2 ± 3.6	499.0	509.2	1.98 ± 0.03

Benchmark Results: ERC20 - Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Mint`	144.6 ± 1.2	142.6	147.1	1.00
`levm_ERC20Mint`	324.6 ± 4.7	319.2	334.2	2.24 ± 0.04

Benchmark Results: ERC20 - Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_ERC20Approval`	1.065 ± 0.011	1.054	1.088	1.00
`levm_ERC20Approval`	1.927 ± 0.016	1.911	1.962	1.81 ± 0.02

github-actions · 2025-05-05T16:52:56Z

EF Tests Comparison

Test Name	MAIN	PR	DIFF
Summary:	32725/38385 (85.25%)	30438/38385 (79.30%)	⬇️️ -2287
Prague:	5202/5202 (100.00%)	4809/5202 (92.45%)	⬇️️ -393
Cancun:	7608/7608 (100.00%)	7046/7608 (92.61%)	⬇️️ -562
Shanghai:	3214/3214 (100.00%)	2938/3214 (91.41%)	⬇️️ -276
Paris:	2886/2886 (100.00%)	2633/2886 (91.23%)	⬇️️ -253
London:	2870/2915 (98.46%)	2618/2915 (89.81%)	⬇️️ -252
Berlin:	215/2755 (7.80%)	215/2755 (7.80%)	➖️
Istanbul:	238/2709 (8.79%)	238/2709 (8.79%)	➖️
Petersburg:	2490/2564 (97.11%)	2362/2564 (92.12%)	⬇️️ -128
Constantinople:	2312/2428 (95.22%)	2190/2428 (90.20%)	⬇️️ -122
Byzantium:	2429/2492 (97.47%)	2326/2492 (93.34%)	⬇️️ -103
SpuriousDragon:	567/598 (94.82%)	523/598 (87.46%)	⬇️️ -44
Tangerine:	570/669 (85.20%)	525/669 (78.48%)	⬇️️ -45
Homestead:	1354/1465 (92.42%)	1291/1465 (88.12%)	⬇️️ -63
Frontier:	770/880 (87.50%)	724/880 (82.27%)	⬇️️ -46

fkrause98 · 2025-05-05T20:14:33Z

crates/vm/levm/src/vm.rs

        self.restore_cache_state(call_frame_backup)?;
+        let backup = self.current_call_frame()?.state_backup.clone();


We didn't need this clone before, let's see how we can avoid it.

JereSalo · 2025-05-06T13:22:54Z

crates/l2/prover/zkvm/interface/pico/src/main.rs

@@ -48,7 +48,7 @@ pub fn main() {
        let mut vm = Evm::from_execution_db(db.clone());
        let result = vm.execute_block(&block).expect("failed to execute block");
        let receipts = result.receipts;
-        let account_updates = vm.get_state_transitions(fork)?;
+        let account_updates = vm.get_state_transitions()?;


Maybe this shouldn't have changed in this PR

JereSalo

I know it is tedious but it would've been better if this PR was done with main as base because the other one was kind of experimental and we don't really know if it's going to be merged. But I understand the reason why it was done this way though. We have to see what to do with the other PR before tackling this one.

This reverts commit 1ea4d0b.

SDartayet added 13 commits April 30, 2025 12:20

Started refactor from scratch to see why tests are failing

3791861

Ran cargo fmt

5e71108

Applied the refactor to handle_opcode_result

d02ca55

Moved call frame pop to return handlers

6419eb5

Removed unused import and renamed variables

c6d65dd

Removed unnecessary imports

739598f

Small bug fix in instruction order

13f251b

Revert "Removed unnecessary imports"

0d172c9

This reverts commit 739598f.

Revert "Revert "Removed unnecessary imports""

ba6aaad

This reverts commit 0d172c9.

Undoing previous changes

6ff11af

Fixed bug with refund handling

7062c3c

Ran cargo fmt

2e6b940

Refactored backups and retdata to be a part of callframe

7fd6881

SDartayet added 2 commits May 5, 2025 13:35

Removed unused variables and imports

f25ee1e

Ran cargo fmt

1384c12

SDartayet marked this pull request as ready for review May 5, 2025 19:34

SDartayet requested a review from a team as a code owner May 5, 2025 19:34

fkrause98 requested changes May 5, 2025

View reviewed changes

SDartayet added 3 commits May 5, 2025 17:44

Removed unnecessary clone

64114a3

Fixed unnecessary clone

511f297

Fixed lints

1ea4d0b

JereSalo reviewed May 6, 2025

View reviewed changes

Revert "Fixed lints"

b2785d2

This reverts commit 1ea4d0b.

SDartayet closed this May 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(levm): moved retdata and state backup to callframe #2666

refactor(levm): moved retdata and state backup to callframe #2666

SDartayet commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

fkrause98 May 5, 2025

JereSalo May 6, 2025

JereSalo left a comment

		self.restore_cache_state(call_frame_backup)?;
		let backup = self.current_call_frame()?.state_backup.clone();

refactor(levm): moved retdata and state backup to callframe #2666

refactor(levm): moved retdata and state backup to callframe #2666

Conversation

SDartayet commented May 5, 2025 • edited Loading

github-actions bot commented May 5, 2025 • edited Loading

Lines of code report

github-actions bot commented May 5, 2025 • edited Loading

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Benchmark Results: Factorial - Recursive

Benchmark Results: Fibonacci

Benchmark Results: ManyHashes

Benchmark Results: BubbleSort

Benchmark Results: ERC20 - Transfer

Benchmark Results: ERC20 - Mint

Benchmark Results: ERC20 - Approval

Main Results

Benchmark Results: Factorial

Benchmark Results: Factorial - Recursive

Benchmark Results: Fibonacci

Benchmark Results: ManyHashes

Benchmark Results: BubbleSort

Benchmark Results: ERC20 - Transfer

Benchmark Results: ERC20 - Mint

Benchmark Results: ERC20 - Approval

github-actions bot commented May 5, 2025 • edited Loading

EF Tests Comparison

fkrause98 May 5, 2025

Choose a reason for hiding this comment

JereSalo May 6, 2025

Choose a reason for hiding this comment

JereSalo left a comment

Choose a reason for hiding this comment

SDartayet commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading