Skip to content

refactor(levm): extract common gas calculation to auxiliar function #3401

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.

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

Closed
wants to merge 3 commits into from

Conversation

sofiazcoaga
Copy link
Contributor

This PR addresses issue #3095.

Opcodes CALL, CALLCODE, DELEGATECALL and STATICCALL had a lot of the gas consumption process in common and they presented repeated code.

Now a new function calculate_call_gas_costs() that returns the sum of all different costs exists. It calculates memory expansion and address access costs (that is done the same way for the four opcodes). Also includes in the sum the values of positive_value_cost and value_to_empty_account (that are optional and each opcode handles differently). The opcodes that do not have this value pass None which translates to adding 0.

@sofiazcoaga sofiazcoaga requested a review from a team as a code owner June 30, 2025 15:20
Copy link

github-actions bot commented Jun 30, 2025

Lines of code report

Total lines added: 31
Total lines removed: 6
Total lines changed: 37

Detailed view
+-----------------------------------------------------+-------+------+
| File                                                | Lines | Diff |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/gas_cost.rs               | 819   | -6   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs | 767   | +31  |
+-----------------------------------------------------+-------+------+

Copy link

No significant difference was registered for any benchmark run.

Detailed Results

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
main_revm_BubbleSort 3.170 ± 0.029 3.150 3.229 1.00
main_levm_BubbleSort 4.506 ± 0.063 4.455 4.661 1.42 ± 0.02
pr_revm_BubbleSort 3.170 ± 0.071 3.131 3.360 1.00 ± 0.02
pr_levm_BubbleSort 4.436 ± 0.037 4.402 4.529 1.40 ± 0.02

Benchmark Results: ERC20Approval

Command Mean [s] Min [s] Max [s] Relative
main_revm_ERC20Approval 1.046 ± 0.007 1.037 1.062 1.01 ± 0.01
main_levm_ERC20Approval 1.541 ± 0.028 1.522 1.617 1.49 ± 0.03
pr_revm_ERC20Approval 1.037 ± 0.006 1.031 1.052 1.00
pr_levm_ERC20Approval 1.538 ± 0.038 1.517 1.644 1.48 ± 0.04

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 138.0 ± 0.9 137.1 139.9 1.00
main_levm_ERC20Mint 260.4 ± 1.5 259.0 263.5 1.89 ± 0.02
pr_revm_ERC20Mint 139.1 ± 0.8 137.1 139.9 1.01 ± 0.01
pr_levm_ERC20Mint 255.7 ± 2.7 253.2 262.4 1.85 ± 0.02

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 248.7 ± 2.6 246.1 253.0 1.03 ± 0.01
main_levm_ERC20Transfer 405.3 ± 2.7 403.0 412.5 1.67 ± 0.01
pr_revm_ERC20Transfer 242.5 ± 1.0 241.5 244.5 1.00
pr_levm_ERC20Transfer 400.6 ± 6.0 397.0 417.1 1.65 ± 0.03

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 234.3 ± 3.6 226.7 240.2 1.01 ± 0.02
main_levm_Factorial 441.2 ± 2.0 439.2 446.0 1.90 ± 0.02
pr_revm_Factorial 232.5 ± 1.5 231.0 235.1 1.00
pr_levm_Factorial 438.7 ± 1.5 436.4 440.7 1.89 ± 0.01

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.634 ± 0.016 1.611 1.663 1.01 ± 0.02
main_levm_FactorialRecursive 2.748 ± 0.043 2.696 2.844 1.69 ± 0.04
pr_revm_FactorialRecursive 1.622 ± 0.027 1.572 1.644 1.00
pr_levm_FactorialRecursive 2.733 ± 0.042 2.681 2.797 1.68 ± 0.04

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 206.9 ± 1.6 205.5 210.4 1.00
main_levm_Fibonacci 442.5 ± 13.2 430.8 463.1 2.14 ± 0.07
pr_revm_Fibonacci 207.9 ± 0.5 207.2 209.0 1.00 ± 0.01
pr_levm_Fibonacci 430.2 ± 4.2 424.3 437.6 2.08 ± 0.03

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 8.7 ± 0.0 8.7 8.8 1.01 ± 0.01
main_levm_ManyHashes 13.1 ± 0.1 13.0 13.3 1.51 ± 0.01
pr_revm_ManyHashes 8.7 ± 0.0 8.6 8.7 1.00
pr_levm_ManyHashes 12.9 ± 0.1 12.8 13.1 1.49 ± 0.01

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 294.1 ± 0.5 293.4 294.7 1.00
main_levm_Push 1154.9 ± 8.6 1141.8 1171.7 3.93 ± 0.03
pr_revm_Push 294.6 ± 0.8 293.3 295.7 1.00 ± 0.00
pr_levm_Push 1104.6 ± 11.8 1082.7 1127.6 3.76 ± 0.04

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.

Why are there no changes to system.rs? The goal was to simplify that part. Look at those opcodes and find the // GAS comment, that behavior is repeated in different calls.

@sofiazcoaga sofiazcoaga marked this pull request as draft June 30, 2025 15:37
Copy link

No significant difference was registered for any benchmark run.

Detailed Results

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
main_revm_BubbleSort 3.188 ± 0.030 3.160 3.264 1.01 ± 0.01
main_levm_BubbleSort 4.419 ± 0.045 4.374 4.515 1.40 ± 0.02
pr_revm_BubbleSort 3.146 ± 0.015 3.127 3.180 1.00
pr_levm_BubbleSort 4.446 ± 0.039 4.415 4.536 1.41 ± 0.01

Benchmark Results: ERC20Approval

Command Mean [s] Min [s] Max [s] Relative
main_revm_ERC20Approval 1.068 ± 0.005 1.063 1.077 1.00 ± 0.01
main_levm_ERC20Approval 1.539 ± 0.036 1.523 1.640 1.45 ± 0.03
pr_revm_ERC20Approval 1.064 ± 0.006 1.056 1.074 1.00
pr_levm_ERC20Approval 1.538 ± 0.035 1.517 1.629 1.45 ± 0.03

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 141.1 ± 0.6 140.4 142.1 1.01 ± 0.01
main_levm_ERC20Mint 262.7 ± 3.6 259.5 271.8 1.88 ± 0.03
pr_revm_ERC20Mint 139.8 ± 1.6 136.8 141.4 1.00
pr_levm_ERC20Mint 263.2 ± 13.7 255.4 299.3 1.88 ± 0.10

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 248.3 ± 1.4 247.0 251.8 1.00 ± 0.01
main_levm_ERC20Transfer 400.7 ± 1.9 397.7 403.5 1.62 ± 0.01
pr_revm_ERC20Transfer 247.9 ± 0.8 247.2 249.9 1.00
pr_levm_ERC20Transfer 400.2 ± 1.6 397.8 402.3 1.61 ± 0.01

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 237.8 ± 12.7 232.1 273.7 1.03 ± 0.06
main_levm_Factorial 468.4 ± 45.4 449.7 596.6 2.03 ± 0.20
pr_revm_Factorial 231.1 ± 1.0 230.2 232.7 1.00
pr_levm_Factorial 479.7 ± 71.0 440.6 657.0 2.08 ± 0.31

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.614 ± 0.036 1.546 1.645 1.00
main_levm_FactorialRecursive 2.803 ± 0.033 2.777 2.887 1.74 ± 0.04
pr_revm_FactorialRecursive 1.627 ± 0.057 1.471 1.666 1.01 ± 0.04
pr_levm_FactorialRecursive 2.754 ± 0.039 2.713 2.810 1.71 ± 0.05

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 206.8 ± 1.6 205.8 210.8 1.00 ± 0.01
main_levm_Fibonacci 440.2 ± 14.5 430.2 469.4 2.14 ± 0.07
pr_revm_Fibonacci 206.1 ± 1.3 205.0 209.1 1.00
pr_levm_Fibonacci 434.7 ± 9.8 426.0 460.7 2.11 ± 0.05

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 8.9 ± 0.0 8.8 9.0 1.00 ± 0.01
main_levm_ManyHashes 14.2 ± 0.1 14.0 14.3 1.60 ± 0.02
pr_revm_ManyHashes 8.9 ± 0.1 8.8 9.0 1.00
pr_levm_ManyHashes 13.4 ± 0.1 13.3 13.5 1.51 ± 0.02

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 296.8 ± 1.5 295.2 299.9 1.00
main_levm_Push 1043.7 ± 10.8 1036.2 1071.7 3.52 ± 0.04
pr_revm_Push 296.9 ± 1.2 295.7 299.6 1.00 ± 0.01
pr_levm_Push 1067.7 ± 12.2 1058.4 1101.3 3.60 ± 0.04

@sofiazcoaga sofiazcoaga closed this Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants