Skip to content

fix(l1): fix failing eip 7623 hive tests #4700

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
SDartayet merged 5 commits into from
Oct 1, 2025
Merged

fix(l1): fix failing eip 7623 hive tests #4700

SDartayet merged 5 commits into from
Oct 1, 2025

Conversation

@SDartayet
Copy link
Contributor

@SDartayet SDartayet commented Sep 29, 2025 •
edited
Loading

Motivation

Passing failing Hive tests

Description

This PR adds a new error type, that's returned specifically when the gas limit is below floor cost for the tokens. The gas limit is now also checked both against the intrinsic gas and the token gas cost floor because some tests were failing when both were equal (in which case the error returned should be over the intrinsic gas; plus it saves us from doing some extra calculations when the gas limit is below the intrinsic gas anyways). Additionally the error message for the IntrinsicGasTooLow error was improved.

For these changes to work a PR needs to also be merged into the EEST repo, specifically changing this file to map the errors correctly. A PR doing so is currently open on our fork of the repo

Closes #4682

@github-actions github-actions bot added the L1 Ethereum client label Sep 29, 2025
@SDartayet SDartayet mentioned this pull request Sep 29, 2025
Merged
@github-actions
Copy link

github-actions bot commented Sep 29, 2025 •
edited
Loading

Lines of code report

Total lines added: 17
Total lines removed: 0
Total lines changed: 17

Detailed view 
+--------------------------------------------------------------+-------+------+
| File                                                         | Lines | Diff |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/errors.rs                          | 231   | +2   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/default_hook.rs              | 347   | +1   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state/deserialize.rs                 | 416   | +3   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state/runner/levm_runner.rs          | 455   | +3   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state/types.rs                       | 267   | +1   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state_v2/src/modules/deserialize.rs  | 152   | +3   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state_v2/src/modules/result_check.rs | 294   | +3   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state_v2/src/modules/types.rs        | 478   | +1   |
+--------------------------------------------------------------+-------+------+

@github-actions
Copy link

github-actions bot commented Sep 29, 2025 •
edited
Loading

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.783 ± 0.018 4.761 4.824 1.03 ± 0.01
main_levm_BubbleSort 4.662 ± 0.011 4.646 4.676 1.00 ± 0.01
pr_revm_BubbleSort 4.797 ± 0.024 4.774 4.845 1.03 ± 0.01
pr_levm_BubbleSort 4.646 ± 0.030 4.624 4.728 1.00

Benchmark Results: ERC20Approval

Command Mean [s] Min [s] Max [s] Relative
main_revm_ERC20Approval 1.552 ± 0.017 1.540 1.591 1.00 ± 0.01
main_levm_ERC20Approval 1.649 ± 0.003 1.644 1.655 1.07 ± 0.01
pr_revm_ERC20Approval 1.547 ± 0.009 1.538 1.567 1.00
pr_levm_ERC20Approval 1.651 ± 0.004 1.647 1.659 1.07 ± 0.01

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 184.6 ± 1.0 183.7 186.5 1.00
main_levm_ERC20Mint 200.9 ± 0.6 199.9 202.0 1.09 ± 0.01
pr_revm_ERC20Mint 184.8 ± 0.4 184.3 185.5 1.00 ± 0.01
pr_levm_ERC20Mint 200.9 ± 1.5 198.6 203.5 1.09 ± 0.01

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 350.6 ± 2.2 348.6 355.8 1.00
main_levm_ERC20Transfer 388.7 ± 4.4 385.0 397.2 1.11 ± 0.01
pr_revm_ERC20Transfer 352.7 ± 1.7 350.5 355.4 1.01 ± 0.01
pr_levm_ERC20Transfer 388.4 ± 1.5 386.6 392.1 1.11 ± 0.01

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 235.8 ± 1.6 234.8 239.9 1.00
main_levm_Factorial 286.0 ± 0.7 285.0 287.1 1.21 ± 0.01
pr_revm_Factorial 236.4 ± 1.3 235.2 238.9 1.00 ± 0.01
pr_levm_Factorial 285.6 ± 1.0 284.1 287.4 1.21 ± 0.01

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.665 ± 0.021 1.634 1.700 1.00
main_levm_FactorialRecursive 8.730 ± 0.052 8.649 8.822 5.24 ± 0.07
pr_revm_FactorialRecursive 1.667 ± 0.028 1.619 1.715 1.00 ± 0.02
pr_levm_FactorialRecursive 8.732 ± 0.089 8.630 8.899 5.24 ± 0.09

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 213.7 ± 0.8 211.7 214.6 1.00
main_levm_Fibonacci 280.8 ± 4.5 276.6 290.5 1.31 ± 0.02
pr_revm_Fibonacci 214.5 ± 1.9 213.0 219.3 1.00 ± 0.01
pr_levm_Fibonacci 277.3 ± 4.1 268.7 282.6 1.30 ± 0.02

Benchmark Results: FibonacciRecursive

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_FibonacciRecursive 899.8 ± 9.4 887.6 913.9 1.00
main_levm_FibonacciRecursive 1051.0 ± 16.8 1036.4 1092.8 1.17 ± 0.02
pr_revm_FibonacciRecursive 911.9 ± 11.7 899.5 939.4 1.01 ± 0.02
pr_levm_FibonacciRecursive 1043.7 ± 7.6 1030.6 1059.1 1.16 ± 0.01

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 12.3 ± 0.1 12.3 12.5 1.00 ± 0.01
main_levm_ManyHashes 13.6 ± 0.1 13.5 13.7 1.11 ± 0.01
pr_revm_ManyHashes 12.3 ± 0.1 12.3 12.5 1.00
pr_levm_ManyHashes 13.6 ± 0.1 13.5 13.7 1.10 ± 0.01

Benchmark Results: MstoreBench

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_MstoreBench 266.3 ± 2.8 263.6 273.4 1.00
main_levm_MstoreBench 758.6 ± 3.5 753.8 764.5 2.85 ± 0.03
pr_revm_MstoreBench 267.2 ± 3.3 264.3 272.4 1.00 ± 0.02
pr_levm_MstoreBench 760.2 ± 4.9 754.5 771.2 2.86 ± 0.03

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 298.2 ± 0.8 296.7 299.5 1.00
main_levm_Push 844.2 ± 1.6 840.6 845.8 2.83 ± 0.01
pr_revm_Push 300.1 ± 0.7 298.9 301.3 1.01 ± 0.00
pr_levm_Push 835.1 ± 3.8 832.4 845.7 2.80 ± 0.02

Benchmark Results: SstoreBench_no_opt

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_SstoreBench_no_opt 220.5 ± 3.8 218.3 231.1 2.47 ± 0.05
main_levm_SstoreBench_no_opt 90.7 ± 5.1 87.8 104.9 1.02 ± 0.06
pr_revm_SstoreBench_no_opt 221.6 ± 1.6 219.9 224.8 2.48 ± 0.03
pr_levm_SstoreBench_no_opt 89.2 ± 0.7 88.0 90.0 1.00

@SDartayet SDartayet marked this pull request as ready for review September 30, 2025 14:32
@SDartayet SDartayet requested a review from a team as a code owner September 30, 2025 14:32
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Sep 30, 2025
@SDartayet SDartayet added this pull request to the merge queue Oct 1, 2025
Merged via the queue into main with commit db246ce Oct 1, 2025
55 checks passed
@SDartayet SDartayet deleted the fix-eip7623-hive-tests branch October 1, 2025 19:36
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Oct 1, 2025
Peponks9 pushed a commit to Peponks9/ethrex that referenced this pull request Oct 7, 2025
@SDartayet
fix(l1): fix failing eip 7623 hive tests (lambdaclass#4700)
38a1dc4 
**Motivation**

Passing failing Hive tests

**Description**

This PR adds a new error type, that's returned specifically when the gas
limit is below floor cost for the tokens. The gas limit is now also
checked both against the intrinsic gas and the token gas cost floor
because some tests were failing when both were equal (in which case the
error returned should be over the intrinsic gas; plus it saves us from
doing some extra calculations when the gas limit is below the intrinsic
gas anyways). Additionally the error message for the IntrinsicGasTooLow
error was improved.

For these changes to work a PR needs to also be merged into the EEST
repo, specifically changing [this
file](https://github.com/ethereum/execution-spec-tests/blob/main/src/ethereum_clis/clis/ethrex.py)
to map the errors correctly. [A PR doing
so](ethereum/execution-spec-tests#2226) is
currently open on our fork of the repo

Closes lambdaclass#4682
@SDartayet SDartayet mentioned this pull request Oct 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fmoletta fmoletta fmoletta approved these changes

+1 more reviewer

@cdiielsi cdiielsi cdiielsi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@SDartayet SDartayet

Labels

L1 Ethereum client

Projects

ethrex_l1
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix EEST tests for EIP-7623

4 participants

@SDartayet @cdiielsi @fmoletta