Skip to content

chore(levm, l1, l2): remove code specific to unsupported forks #2670

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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

SDartayet
Copy link
Contributor

Motivation

Keep the codebase clean and as simple as possible by removing code we don't really need.

Description

All the code that was only relevant to forks prior to Paris was removed. This includes constants, ifs, etc.

Resolves issue #2659

Copy link

github-actions bot commented May 5, 2025

Lines of code report

Total lines added: 8
Total lines removed: 373
Total lines changed: 381

Detailed view
+------------------------------------------------------------------------+-------+------+
| File                                                                   | Lines | Diff |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                                 | 493   | -1   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs                                    | 550   | -2   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/src/backends/exec.rs                           | 80    | -1   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer/payload_builder.rs           | 225   | -2   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_committer.rs                             | 505   | -4   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/l2/utils/prover/save_state.rs                            | 424   | -1   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                                  | 639   | -4   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/db/gen_db.rs                                 | 178   | -4   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/execution_handlers.rs                        | 231   | -2   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/gas_cost.rs                                  | 902   | -226 |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/default_hook.rs                        | 347   | +8   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/arithmetic.rs                | 217   | -1   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/bitwise_comparison.rs        | 289   | -12  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/block.rs                     | 131   | -9   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/environment.rs               | 304   | -11  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs | 270   | -17  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs                    | 800   | -31  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/precompiles.rs                               | 1367  | -40  |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/utils.rs                                     | 411   | -3   |
+------------------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                                        | 384   | -2   |
+------------------------------------------------------------------------+-------+------+

Copy link

github-actions bot commented May 5, 2025

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 242.4 ± 2.9 240.3 247.6 1.00
levm_Factorial 885.5 ± 3.6 880.7 891.4 3.65 ± 0.05

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.570 ± 0.066 1.473 1.644 1.00
levm_FactorialRecursive 13.272 ± 0.268 13.014 13.626 8.45 ± 0.40

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 214.0 ± 1.1 212.7 216.1 1.00
levm_Fibonacci 882.3 ± 5.8 873.2 890.5 4.12 ± 0.03

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 18.4 ± 0.2 18.2 18.8 2.09 ± 0.02

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.259 ± 0.029 3.231 3.314 1.00
levm_BubbleSort 6.047 ± 0.026 6.013 6.096 1.86 ± 0.02

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 253.7 ± 5.8 250.2 269.5 1.00
levm_ERC20Transfer 522.1 ± 4.2 518.5 532.5 2.06 ± 0.05

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 143.8 ± 1.3 141.9 146.4 1.00
levm_ERC20Mint 327.5 ± 1.4 325.8 329.6 2.28 ± 0.02

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.036 ± 0.008 1.015 1.046 1.00
levm_ERC20Approval 1.982 ± 0.017 1.967 2.026 1.91 ± 0.02

Main Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 240.4 ± 2.0 238.1 243.4 1.00
levm_Factorial 876.7 ± 8.1 867.8 891.3 3.65 ± 0.05

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.512 ± 0.086 1.407 1.624 1.00
levm_FactorialRecursive 13.169 ± 0.220 12.947 13.433 8.71 ± 0.52

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 221.6 ± 23.3 212.6 287.9 1.00
levm_Fibonacci 880.7 ± 15.3 863.4 915.5 3.97 ± 0.42

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 8.7 ± 0.1 8.7 8.9 1.00
levm_ManyHashes 18.1 ± 0.2 17.8 18.6 2.07 ± 0.03

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.252 ± 0.025 3.222 3.296 1.00
levm_BubbleSort 5.752 ± 0.067 5.694 5.917 1.77 ± 0.02

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 251.9 ± 3.2 249.2 259.4 1.00
levm_ERC20Transfer 507.0 ± 7.4 500.3 526.2 2.01 ± 0.04

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 144.1 ± 1.4 141.2 145.8 1.00
levm_ERC20Mint 323.1 ± 4.4 318.4 334.7 2.24 ± 0.04

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.041 ± 0.011 1.029 1.057 1.00
levm_ERC20Approval 1.935 ± 0.015 1.905 1.967 1.86 ± 0.02

Copy link

github-actions bot commented May 5, 2025

EF Tests Comparison

Same results between main branch and the current PR.

@SDartayet SDartayet changed the title chore(levm): remove code specific to unsupported forks chore(levm, l1, l2): remove code specific to unsupported forks May 6, 2025
@SDartayet SDartayet marked this pull request as ready for review May 6, 2025 19:06
@SDartayet SDartayet requested a review from a team as a code owner May 6, 2025 19:06
@JereSalo
Copy link
Contributor

JereSalo commented May 7, 2025

There is an unnecessary check of Paris fork in src/opcode_handlers/block.rs. We can remove that one too

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.

Amazing job. We can merge after making the requested changes :)

Comment on lines +35 to +56
// check for gas limit is grater or equal than the minimum required
let calldata = vm.current_call_frame()?.calldata.clone();
let intrinsic_gas: u64 = vm.get_intrinsic_gas()?;

// calldata_cost = tokens_in_calldata * 4
let calldata_cost: u64 = gas_cost::tx_calldata(&calldata).map_err(VMError::OutOfGas)?;

// same as calculated in gas_used()
let tokens_in_calldata: u64 = calldata_cost
.checked_div(STANDARD_TOKEN_COST)
.ok_or(VMError::Internal(InternalError::DivisionError))?;

// floor_cost_by_tokens = TX_BASE_COST + TOTAL_COST_FLOOR_PER_TOKEN * tokens_in_calldata
let floor_cost_by_tokens = tokens_in_calldata
.checked_mul(TOTAL_COST_FLOOR_PER_TOKEN)
.ok_or(VMError::Internal(InternalError::GasOverflow))?
.checked_add(TX_BASE_COST)
.ok_or(VMError::Internal(InternalError::GasOverflow))?;

let min_gas_limit = max(intrinsic_gas, floor_cost_by_tokens);
if vm.current_call_frame()?.gas_limit < min_gas_limit {
return Err(VMError::TxValidation(TxValidationError::IntrinsicGasTooLow));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't all this the same as what's in validate_min_gas_limit?

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, we can remove it

@@ -131,11 +131,13 @@ impl Hook for L2Hook {
vm.increase_account_balance(vm.env.origin, vm.current_call_frame()?.msg_value)?;
}

// 2. Return unused gas + gas refunds to the sender.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add a comment in this PR?

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