Skip to content

refactor(levm): decluttering vm.rs #2733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
May 13, 2025
Merged

refactor(levm): decluttering vm.rs #2733

merged 14 commits into from
May 13, 2025

Conversation

SDartayet
Copy link
Contributor

@SDartayet SDartayet commented May 9, 2025 •
edited by JereSalo
Loading

Motivation

Making the code of the vm.rs file cleaner, since it's a bit cluttered right now.

Description

The code of vm.rs is a bit messy right now. This PR moves EVM config to environment, moves a few attributes from environment to substate that make more sense there, and removes the StateBackup struct since it's made unnecessary by this change.

Closes #2731, Closes #2717
Resolves most of #2718

@SDartayet SDartayet requested a review from a team as a code owner May 9, 2025 18:53
@github-actions GitHub Actions
Copy link

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

Lines of code report

Total lines added: 65
Total lines removed: 94
Total lines changed: 159

Detailed view 
+------------------------------------------------------------------------+-------+------+
| File                                                                   | Lines | Diff |
+------------------------------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner/levm_runner.rs                        | 380   | -3   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                                  | 639   | -4   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/environment.rs                               | 91    | +62  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs | 272   | +2   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs                    | 771   | -8   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/utils.rs                                     | 412   | +1   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                                        | 336   | -79  |
+------------------------------------------------------------------------+-------+------+

@github-actions GitHub Actions
Copy link

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

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 241.5 ± 0.7 240.8 242.5 1.00
levm_Factorial 812.7 ± 6.6 804.3 822.4 3.37 ± 0.03

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.511 ± 0.053 1.368 1.557 1.00
levm_FactorialRecursive 12.965 ± 0.062 12.855 13.057 8.58 ± 0.30

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 212.4 ± 0.5 211.9 213.2 1.00
levm_Fibonacci 820.2 ± 9.9 809.0 840.9 3.86 ± 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 16.9 ± 0.1 16.8 17.1 1.95 ± 0.02

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.213 ± 0.004 3.207 3.220 1.00
levm_BubbleSort 5.500 ± 0.038 5.458 5.564 1.71 ± 0.01

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 251.3 ± 2.0 248.1 254.1 1.00
levm_ERC20Transfer 497.3 ± 2.0 495.4 501.6 1.98 ± 0.02

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 141.9 ± 1.5 140.6 145.9 1.00
levm_ERC20Mint 322.1 ± 4.4 316.4 327.3 2.27 ± 0.04

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.041 ± 0.010 1.031 1.061 1.00
levm_ERC20Approval 1.872 ± 0.007 1.866 1.889 1.80 ± 0.02

Main Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 234.6 ± 2.9 232.3 242.1 1.00
levm_Factorial 816.1 ± 4.5 811.0 823.3 3.48 ± 0.05

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.437 ± 0.082 1.319 1.545 1.00
levm_FactorialRecursive 13.291 ± 0.320 12.995 13.967 9.25 ± 0.57

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 207.9 ± 0.8 206.9 209.6 1.00
levm_Fibonacci 818.8 ± 12.1 807.2 846.0 3.94 ± 0.06

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 8.7 ± 0.1 8.5 8.9 1.00
levm_ManyHashes 17.1 ± 0.1 17.0 17.3 1.97 ± 0.02

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.221 ± 0.034 3.193 3.287 1.00
levm_BubbleSort 5.490 ± 0.025 5.461 5.524 1.70 ± 0.02

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 246.2 ± 2.4 243.9 251.1 1.00
levm_ERC20Transfer 495.2 ± 4.3 490.6 504.0 2.01 ± 0.03

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 141.1 ± 1.2 139.6 144.1 1.00
levm_ERC20Mint 315.1 ± 1.3 313.2 317.8 2.23 ± 0.02

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.030 ± 0.013 1.017 1.055 1.00
levm_ERC20Approval 1.869 ± 0.014 1.847 1.895 1.82 ± 0.03

JereSalo
JereSalo approved these changes May 12, 2025
View reviewed changes
Copy link
Contributor

@JereSalo JereSalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good :)

@JereSalo JereSalo enabled auto-merge May 13, 2025 15:39
@JereSalo JereSalo mentioned this pull request May 13, 2025
Open
rodrigo-o
rodrigo-o approved these changes May 13, 2025
View reviewed changes
Copy link
Collaborator

@rodrigo-o rodrigo-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@JereSalo JereSalo added this pull request to the merge queue May 13, 2025
Merged via the queue into main with commit a169985 May 13, 2025
36 checks passed
@JereSalo JereSalo deleted the vm.rs-declutter branch May 13, 2025 18:55
fmoletta pushed a commit that referenced this pull request May 15, 2025
@SDartayet @JereSalo @fmoletta
refactor(levm): decluttering vm.rs (#2733)
b6f24a6 
**Motivation**

Making the code of the vm.rs file cleaner, since it's a bit cluttered
right now.

**Description**

The code of vm.rs is a bit messy right now. This PR moves EVM config to
environment, moves a few attributes from environment to substate that
make more sense there, and removes the StateBackup struct since it's
made unnecessary by this change.

Closes #2731, Closes #2717 
Resolves most of #2718

---------

Co-authored-by: JereSalo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpaulucci mpaulucci mpaulucci left review comments

@JereSalo JereSalo JereSalo approved these changes

@fmoletta fmoletta fmoletta left review comments

@rodrigo-o rodrigo-o rodrigo-o approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move EVMConfig to vm.rs LEVM: Move refunded_gas and transient_storage to Substate
5 participants
@SDartayet @mpaulucci @rodrigo-o @JereSalo @fmoletta