refactor(levm,l2): hooks #2508

ilitteri · 2025-04-21T18:35:09Z

Motivation

Remove duplicated code between DefaultHook and L2Hook implementations.
Use the L2Hook for every tx (regular ETH txs and L2's privilege txs) when running the L2.

Description

This PR:

Generates abstractions to be used in DefaultHook and L2Hook to remove repeated code.
Adds the is_privilege field to LEVM's Environment only compiled under the l2 feature flag.
L2Hook now supports executing every tx (before only privileged).

github-actions · 2025-04-21T18:36:12Z

Lines of code report

Total lines added: 99
Total lines removed: 61
Total lines changed: 160

Detailed view

+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner/levm_runner.rs | 383   | +1   |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs           | 640   | +2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/environment.rs        | 29    | +1   |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/errors.rs             | 230   | +2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/default_hook.rs | 339   | +75  |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/l2_hook.rs      | 133   | -61  |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/mod.rs          | 7     | +4   |
+-------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                 | 386   | +14  |
+-------------------------------------------------+-------+------+

github-actions · 2025-04-21T18:47:02Z

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Factorial`	239.3 ± 1.2	238.0	242.4	1.00
`levm_Factorial`	879.1 ± 4.0	873.0	886.7	3.67 ± 0.02

Benchmark Results: Factorial - Recursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_FactorialRecursive`	1.437 ± 0.073	1.352	1.540	1.00
`levm_FactorialRecursive`	13.302 ± 0.134	13.066	13.451	9.26 ± 0.48

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Fibonacci`	209.3 ± 0.6	207.9	210.2	1.00
`levm_Fibonacci`	879.0 ± 8.3	870.2	897.0	4.20 ± 0.04

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ManyHashes`	8.8 ± 0.1	8.7	9.0	1.00
`levm_ManyHashes`	17.6 ± 0.3	17.3	18.3	2.01 ± 0.04

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_BubbleSort`	3.243 ± 0.033	3.210	3.306	1.00
`levm_BubbleSort`	5.734 ± 0.028	5.702	5.780	1.77 ± 0.02

Benchmark Results: ERC20 - Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Transfer`	252.0 ± 3.9	247.7	257.9	1.00
`levm_ERC20Transfer`	507.9 ± 7.1	499.9	522.9	2.02 ± 0.04

Benchmark Results: ERC20 - Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Mint`	142.6 ± 0.6	141.7	143.6	1.00
`levm_ERC20Mint`	322.2 ± 4.5	316.1	329.8	2.26 ± 0.03

Benchmark Results: ERC20 - Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_ERC20Approval`	1.040 ± 0.007	1.031	1.051	1.00
`levm_ERC20Approval`	1.922 ± 0.040	1.894	2.032	1.85 ± 0.04

Main Results

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Factorial`	237.9 ± 1.9	233.0	240.4	1.00
`levm_Factorial`	885.5 ± 16.1	874.0	916.8	3.72 ± 0.07

Benchmark Results: Factorial - Recursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_FactorialRecursive`	1.577 ± 0.086	1.412	1.675	1.00
`levm_FactorialRecursive`	13.203 ± 0.201	12.986	13.430	8.37 ± 0.47

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_Fibonacci`	207.5 ± 2.1	205.9	213.3	1.00
`levm_Fibonacci`	874.9 ± 3.3	871.7	881.5	4.22 ± 0.05

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ManyHashes`	8.7 ± 0.1	8.6	8.8	1.00
`levm_ManyHashes`	17.6 ± 0.3	17.4	18.4	2.01 ± 0.04

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_BubbleSort`	3.259 ± 0.013	3.245	3.283	1.00
`levm_BubbleSort`	5.795 ± 0.065	5.724	5.889	1.78 ± 0.02

Benchmark Results: ERC20 - Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Transfer`	252.4 ± 0.5	251.2	252.9	1.00
`levm_ERC20Transfer`	511.4 ± 8.9	501.8	533.5	2.03 ± 0.04

Benchmark Results: ERC20 - Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`revm_ERC20Mint`	145.2 ± 3.5	143.3	155.0	1.00
`levm_ERC20Mint`	318.8 ± 2.0	316.1	322.0	2.20 ± 0.05

Benchmark Results: ERC20 - Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`revm_ERC20Approval`	1.039 ± 0.002	1.037	1.042	1.00
`levm_ERC20Approval`	1.925 ± 0.015	1.901	1.943	1.85 ± 0.01

crates/vm/levm/src/vm.rs

…l2_hook

This reverts commit d3eae00.

ManuelBilbao · 2025-04-30T17:54:27Z

cmd/ef_tests/state/runner/levm_runner.rs

@@ -188,6 +188,8 @@ pub fn prepare_vm_for_tx<'a>(
            tx_nonce: test_tx.nonce,
            block_gas_limit: test.env.current_gas_limit,
            transient_storage: HashMap::new(),
+            #[cfg(feature = "l2")]
+            is_privilege: false,


Should this be is_privileged?

Done in e60b7ee.

ManuelBilbao · 2025-04-30T18:00:18Z

crates/vm/levm/src/hooks/default_hook.rs

+    Ok(())
+}
+
+pub fn validate_type_3_tx(vm: &mut VM<'_>) -> Result<(), VMError> {


Something like validate_4844_tx might be easier to read

Done in 8941381.

ManuelBilbao · 2025-04-30T18:00:26Z

crates/vm/levm/src/hooks/default_hook.rs

+    Ok(())
+}
+
+pub fn validate_type_4_tx(vm: &mut VM<'_>) -> Result<(), VMError> {


Done in 8941381.

Refactor coinbase fee payment

41226c9

ilitteri added tech debt Refactors, cleanups, etc L2 Rollup client labels Apr 21, 2025

ilitteri self-assigned this Apr 21, 2025

Refactor self destruct set deletion

b4dff2b

ilitteri and others added 23 commits April 21, 2025 15:59

Move abstraction to default hook

439d8da

Refactor min gas limit check

d5d0577

Refactor max fee per blob gas check

51c5a6b

Refactor init code size validation

d68ad1a

Refactor intrinsic gas addition

766c062

Merge branch 'main' into generalize_l2_hook

634b4e6

Refactor sufficient max fee per gas validation

27f82ef

Refactor type 3 tx validation

346a595

Refactor type 4 tx validation

894c356

Validate nonce in non-privilege txs in L2Hook

740a53c

Transfer value to receiver in non-privilege txs in L2Hook

0ab0ee2

Validate sender in non-privilege txs in L2Hook

cf42b6a

Refactor sender validation

6a65d2f

Refactor gas allowance validation

eb034ce

Undo value transfer for non-privilege txs

efd6912

Refactor (1), (2), and (3) in DefaultHook

257e666

Add (1) and (3) to L2Hook for non-privilege txs

76dc885

Minor refactors

2738bb4

Add TODO

78db422

Final refactor to DefaultHook::finalize_execution

3a2d0d0

Final refactor to L2Hook::finalize_execution

1ea9b7f

Leftovers

6b0b0d8

Final refactor to L2Hook::prepare_execution

d502542

ilitteri added 3 commits April 22, 2025 14:08

Fix linter

b1f42c8

Fix linter

8dc0129

More refactors

5a35d11

ilitteri marked this pull request as ready for review April 22, 2025 19:15

ilitteri requested a review from a team as a code owner April 22, 2025 19:15

ilitteri changed the title ~~refactor(l2): generalize L2 hook~~ refactor(levm,l2): hooks Apr 22, 2025

Merge branch 'main' into generalize_l2_hook

4e69c41

tomip01 reviewed Apr 24, 2025

View reviewed changes

crates/vm/levm/src/vm.rs Outdated Show resolved Hide resolved

ilitteri and others added 2 commits April 24, 2025 17:23

Merge branch 'main' into generalize_l2_hook

d49ffb3

Fix recipient in L2Hook

fe211c6

ilitteri requested a review from tomip01 April 24, 2025 16:37

tomip01 approved these changes Apr 24, 2025

View reviewed changes

ilitteri and others added 8 commits April 24, 2025 22:35

Merge branch 'main' into generalize_l2_hook

d5736a8

Merge branch 'main' of github.com:lambdaclass/ethrex into generalize_…

e9fb0bc

…l2_hook

Fix merge errors

0921677

Lint

d4a2acc

Fix linter

d3eae00

Revert "Fix linter"

eeb03f9

This reverts commit d3eae00.

Add ethrex-levm/l2

a29b5ce

Fix lint

36b8751

ManuelBilbao reviewed Apr 30, 2025

View reviewed changes

ilitteri and others added 4 commits April 30, 2025 17:15

Fix lint

e60b7ee

Rename field

ee8a8c2

Improve fn name

8941381

Merge branch 'main' into generalize_l2_hook

db18d75

jrchatruc approved these changes Apr 30, 2025

View reviewed changes

ilitteri added this pull request to the merge queue Apr 30, 2025

Merged via the queue into main with commit bb8cece Apr 30, 2025
35 checks passed

ilitteri deleted the generalize_l2_hook branch April 30, 2025 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(levm,l2): hooks #2508

refactor(levm,l2): hooks #2508

Uh oh!

ilitteri commented Apr 21, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 21, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 21, 2025 •

edited

Loading

Uh oh!

Uh oh!

ManuelBilbao Apr 30, 2025

Uh oh!

ilitteri Apr 30, 2025

Uh oh!

ManuelBilbao Apr 30, 2025

Uh oh!

ilitteri Apr 30, 2025

Uh oh!

ManuelBilbao Apr 30, 2025

Uh oh!

ilitteri Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

refactor(levm,l2): hooks #2508

refactor(levm,l2): hooks #2508

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Lines of code report

Uh oh!

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

Uh oh!

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

Uh oh!

Uh oh!

ManuelBilbao Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ilitteri Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ManuelBilbao Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ilitteri Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ManuelBilbao Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ilitteri Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ilitteri commented Apr 21, 2025 •

edited

Loading

github-actions bot commented Apr 21, 2025 •

edited

Loading

github-actions bot commented Apr 21, 2025 •

edited

Loading