Skip to content

Conversation

pablodeymo
Copy link
Contributor

Motivation

This PR improves readibility of ecrecover of levm.

Description

Closes #issue_number

@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 21:57
@pablodeymo pablodeymo requested a review from a team as a code owner September 23, 2025 21:57
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ecrecover precompile function to improve code readability by consolidating variable declarations with error handling and removing unnecessary intermediate variables.

  • Combines variable extraction with validation using let-else patterns
  • Eliminates intermediate variables and unnecessary match statements
  • Removes a clippy allow annotation that is no longer needed

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions github-actions bot added the levm Lambda EVM implementation label Sep 23, 2025
Copy link

github-actions bot commented Sep 23, 2025

Lines of code report

Total lines added: 0
Total lines removed: 4
Total lines changed: 4

Detailed view
+------------------------------------------+-------+------+
| File                                     | Lines | Diff |
+------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/precompiles.rs | 1474  | -4   |
+------------------------------------------+-------+------+

Copy link
Collaborator

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

Looks good

@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Sep 23, 2025
Copy link

github-actions bot commented Sep 23, 2025

Benchmark Results Comparison

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 4.750 ± 0.090 4.702 5.005 1.03 ± 0.02
main_levm_BubbleSort 4.599 ± 0.014 4.581 4.620 1.00
pr_revm_BubbleSort 4.742 ± 0.032 4.715 4.824 1.03 ± 0.01
pr_levm_BubbleSort 4.762 ± 0.013 4.745 4.788 1.04 ± 0.00

Benchmark Results: ERC20Approval

Command Mean [s] Min [s] Max [s] Relative
main_revm_ERC20Approval 1.522 ± 0.007 1.513 1.535 1.00 ± 0.01
main_levm_ERC20Approval 1.623 ± 0.005 1.612 1.629 1.07 ± 0.01
pr_revm_ERC20Approval 1.521 ± 0.007 1.512 1.539 1.00
pr_levm_ERC20Approval 1.661 ± 0.097 1.623 1.934 1.09 ± 0.06

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 179.8 ± 1.1 178.6 182.8 1.00
main_levm_ERC20Mint 195.2 ± 0.6 193.9 195.9 1.09 ± 0.01
pr_revm_ERC20Mint 180.3 ± 1.1 179.4 182.6 1.00 ± 0.01
pr_levm_ERC20Mint 195.0 ± 0.7 194.1 196.4 1.08 ± 0.01

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 346.0 ± 1.8 343.8 349.2 1.00
main_levm_ERC20Transfer 382.4 ± 2.9 379.0 387.3 1.10 ± 0.01
pr_revm_ERC20Transfer 346.1 ± 0.9 344.8 347.6 1.00 ± 0.01
pr_levm_ERC20Transfer 381.1 ± 1.3 379.3 383.8 1.10 ± 0.01

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 233.0 ± 0.4 232.5 233.8 1.00 ± 0.00
main_levm_Factorial 294.6 ± 4.4 290.9 305.8 1.27 ± 0.02
pr_revm_Factorial 232.3 ± 0.6 231.6 233.1 1.00
pr_levm_Factorial 293.9 ± 1.9 292.6 299.1 1.27 ± 0.01

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.636 ± 0.035 1.556 1.671 1.00
main_levm_FactorialRecursive 8.075 ± 0.113 7.886 8.242 4.94 ± 0.13
pr_revm_FactorialRecursive 1.641 ± 0.032 1.598 1.708 1.00 ± 0.03
pr_levm_FactorialRecursive 7.984 ± 0.127 7.859 8.256 4.88 ± 0.13

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 211.8 ± 1.8 210.8 217.0 1.00 ± 0.01
main_levm_Fibonacci 284.9 ± 2.1 281.6 289.9 1.35 ± 0.01
pr_revm_Fibonacci 210.8 ± 0.8 210.3 212.8 1.00
pr_levm_Fibonacci 288.6 ± 7.0 283.2 302.0 1.37 ± 0.03

Benchmark Results: FibonacciRecursive

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_FibonacciRecursive 881.8 ± 7.4 867.4 894.7 1.01 ± 0.01
main_levm_FibonacciRecursive 1058.7 ± 4.2 1052.8 1067.1 1.21 ± 0.01
pr_revm_FibonacciRecursive 876.9 ± 6.9 866.7 889.9 1.00
pr_levm_FibonacciRecursive 1031.2 ± 4.4 1026.1 1038.4 1.18 ± 0.01

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 12.1 ± 0.1 12.0 12.3 1.00 ± 0.01
main_levm_ManyHashes 13.6 ± 0.1 13.5 13.8 1.13 ± 0.01
pr_revm_ManyHashes 12.1 ± 0.0 12.1 12.2 1.00
pr_levm_ManyHashes 14.1 ± 0.1 14.0 14.3 1.17 ± 0.01

Benchmark Results: MstoreBench

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_MstoreBench 267.7 ± 4.5 263.1 277.3 1.00 ± 0.02
main_levm_MstoreBench 766.4 ± 6.6 760.5 783.3 2.87 ± 0.04
pr_revm_MstoreBench 267.2 ± 3.0 263.7 271.9 1.00
pr_levm_MstoreBench 778.3 ± 56.0 757.1 937.6 2.91 ± 0.21

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 289.1 ± 1.2 287.6 291.4 1.00
main_levm_Push 869.7 ± 3.4 863.5 872.7 3.01 ± 0.02
pr_revm_Push 289.3 ± 1.2 288.0 292.4 1.00 ± 0.01
pr_levm_Push 870.4 ± 4.2 866.3 879.3 3.01 ± 0.02

Benchmark Results: SstoreBench_no_opt

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_SstoreBench_no_opt 219.0 ± 1.1 217.8 221.2 2.67 ± 0.08
main_levm_SstoreBench_no_opt 82.0 ± 2.4 80.0 88.4 1.00
pr_revm_SstoreBench_no_opt 218.3 ± 0.8 217.3 219.1 2.66 ± 0.08
pr_levm_SstoreBench_no_opt 82.1 ± 2.2 80.5 88.0 1.00 ± 0.04

@pablodeymo pablodeymo added this pull request to the merge queue Sep 24, 2025
Merged via the queue into main with commit 1d4ae8f Sep 24, 2025
70 of 72 checks passed
@pablodeymo pablodeymo deleted the improve_ecrecove_style branch September 24, 2025 13:54
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
levm Lambda EVM implementation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants