fix(l1): added error for case where system contract code is empty #4731

SDartayet · 2025-10-01T19:04:52Z

Description

This PR adds a check for the case where a system contract code is empty. This is required to pass a few Hive tests recently added that ask to return an error in such a case.

github-actions · 2025-10-01T19:09:23Z

Lines of code report

Total lines added: 13
Total lines removed: 0
Total lines changed: 13

Detailed view

+---------------------------------------+-------+------+
| File                                  | Lines | Diff |
+---------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs | 470   | +9   |
+---------------------------------------+-------+------+
| ethrex/crates/vm/system_contracts.rs  | 66    | +4   |
+---------------------------------------+-------+------+

github-actions · 2025-10-01T21:17:44Z

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.758 ± 0.005	4.748	4.763	1.02 ± 0.00
`main_levm_BubbleSort`	4.643 ± 0.018	4.622	4.677	1.00
`pr_revm_BubbleSort`	4.796 ± 0.095	4.756	5.064	1.03 ± 0.02
`pr_levm_BubbleSort`	4.680 ± 0.066	4.638	4.849	1.01 ± 0.01

Benchmark Results: ERC20Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_ERC20Approval`	1.548 ± 0.005	1.543	1.559	1.00
`main_levm_ERC20Approval`	1.634 ± 0.005	1.627	1.642	1.06 ± 0.00
`pr_revm_ERC20Approval`	1.575 ± 0.025	1.536	1.618	1.02 ± 0.02
`pr_levm_ERC20Approval`	1.641 ± 0.003	1.638	1.648	1.06 ± 0.00

Benchmark Results: ERC20Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Mint`	185.3 ± 6.1	182.7	202.6	1.01 ± 0.03
`main_levm_ERC20Mint`	197.0 ± 0.5	196.5	197.9	1.08 ± 0.01
`pr_revm_ERC20Mint`	183.2 ± 1.5	181.6	186.5	1.00
`pr_levm_ERC20Mint`	197.7 ± 0.5	197.0	198.4	1.08 ± 0.01

Benchmark Results: ERC20Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Transfer`	351.1 ± 2.4	349.4	357.3	1.00 ± 0.01
`main_levm_ERC20Transfer`	384.5 ± 1.3	382.8	387.0	1.10 ± 0.01
`pr_revm_ERC20Transfer`	350.9 ± 3.8	347.8	361.3	1.00
`pr_levm_ERC20Transfer`	390.3 ± 3.2	387.1	398.2	1.11 ± 0.02

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Factorial`	235.4 ± 2.9	233.9	243.4	1.00
`main_levm_Factorial`	281.6 ± 8.6	276.3	302.4	1.20 ± 0.04
`pr_revm_Factorial`	236.0 ± 3.3	234.1	244.7	1.00 ± 0.02
`pr_levm_Factorial`	279.2 ± 2.5	277.0	285.4	1.19 ± 0.02

Benchmark Results: FactorialRecursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_FactorialRecursive`	1.672 ± 0.032	1.606	1.713	1.00
`main_levm_FactorialRecursive`	8.738 ± 0.089	8.614	8.873	5.23 ± 0.11
`pr_revm_FactorialRecursive`	1.692 ± 0.030	1.666	1.766	1.01 ± 0.03
`pr_levm_FactorialRecursive`	8.778 ± 0.051	8.718	8.904	5.25 ± 0.10

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Fibonacci`	213.5 ± 2.6	210.7	219.9	1.00
`main_levm_Fibonacci`	261.5 ± 1.2	259.9	263.9	1.22 ± 0.02
`pr_revm_Fibonacci`	213.8 ± 1.8	212.7	218.8	1.00 ± 0.01
`pr_levm_Fibonacci`	262.5 ± 3.7	260.4	272.7	1.23 ± 0.02

Benchmark Results: FibonacciRecursive

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_FibonacciRecursive`	923.4 ± 23.2	882.0	957.3	1.01 ± 0.03
`main_levm_FibonacciRecursive`	1054.6 ± 4.6	1048.7	1062.0	1.16 ± 0.01
`pr_revm_FibonacciRecursive`	911.6 ± 6.6	902.6	921.7	1.00
`pr_levm_FibonacciRecursive`	1050.6 ± 6.4	1042.0	1065.3	1.15 ± 0.01

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ManyHashes`	12.3 ± 0.1	12.3	12.4	1.00
`main_levm_ManyHashes`	13.6 ± 0.1	13.6	13.8	1.10 ± 0.01
`pr_revm_ManyHashes`	12.4 ± 0.1	12.3	12.4	1.00 ± 0.01
`pr_levm_ManyHashes`	13.7 ± 0.1	13.6	14.1	1.11 ± 0.01

Benchmark Results: MstoreBench

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_MstoreBench`	264.3 ± 0.7	263.2	265.4	1.00
`main_levm_MstoreBench`	757.0 ± 1.5	754.0	758.7	2.86 ± 0.01
`pr_revm_MstoreBench`	266.5 ± 2.9	264.0	272.9	1.01 ± 0.01
`pr_levm_MstoreBench`	758.0 ± 2.2	755.4	763.2	2.87 ± 0.01

Benchmark Results: Push

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Push`	298.2 ± 6.0	295.6	315.2	1.01 ± 0.02
`main_levm_Push`	830.2 ± 2.4	828.0	836.1	2.80 ± 0.01
`pr_revm_Push`	296.5 ± 0.7	295.9	298.3	1.00
`pr_levm_Push`	839.1 ± 8.1	832.6	857.9	2.83 ± 0.03

Benchmark Results: SstoreBench_no_opt

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_SstoreBench_no_opt`	218.3 ± 2.2	216.7	224.4	2.40 ± 0.04
`main_levm_SstoreBench_no_opt`	90.9 ± 1.3	89.3	93.0	1.00
`pr_revm_SstoreBench_no_opt`	217.7 ± 0.9	216.6	219.7	2.40 ± 0.04
`pr_levm_SstoreBench_no_opt`	91.7 ± 1.4	89.6	94.2	1.01 ± 0.02

mpaulucci · 2025-10-06T15:56:22Z

crates/vm/backends/levm/mod.rs

        ..Default::default()
    };

+    if !(contract_address == HISTORY_STORAGE_ADDRESS.address


would be better to have a vec of system contracts somewhere? Otherwise it will be easy to miss this when a new system contract is added?

We do have a vec of system contracts. However this case is more specific; it's not any given system contract but those that have code (the function in which this check is in is already only reached when a system contract is called). We could build a vec specifically for the system contracts that do have code though (or add a has_code attribute to the existing ones)

mpaulucci · 2025-10-06T15:57:02Z

crates/vm/levm/src/hooks/default_hook.rs

-    if vm.current_call_frame.gas_limit < min_gas_limit {
-        return Err(TxValidationError::IntrinsicGasTooLow.into());
+    if vm.current_call_frame.gas_limit < floor_cost_by_tokens {
+        return Err(TxValidationError::IntrinsicGasBelowFloorGasCost.into());


which ones does this fix? https://hive.ethpandaops.io/#/test/generic/1759727493-afbe365eb26fd5a231ad1aa9e83cd55d

JereSalo · 2025-10-08T16:48:21Z

crates/vm/backends/levm/mod.rs

        ..Default::default()
    };

+    if !(contract_address == HISTORY_STORAGE_ADDRESS.address


I'd personally add a comment that we have this code in order to pass the tests and that it's impossible in real scenarios. I am not in favor of adding the check but if we want it I'd just add that comment.

Sure, done here. Let me know if you think the comment is adequate

It's great, thanks

JereSalo · 2025-10-08T18:18:15Z

crates/vm/levm/src/hooks/default_hook.rs

Is it possible that the changes in this file are somehow unrelated to the PR title? Like the PR is for when the code is empty in system contract calls, this is for other scenario, isn't it?

Also did you add this because you saw a test that made it necessary or what?

Those changes were from a separate PR I made before this one. That one already got merged though, so I'm not sure why those are showing up in the diff for this one

Those are from this PR

Ok there, merging the latest version of main into the branch fixed it, the files changed should show up properly now

JereSalo

Does the deposit contract belong to the const of the contracts? Like I know the other two belong here because of EIP-2935 but I'm not sure about the other one. It does no harm anyway but I think it doesn't belong here.

Also, make sure to link the corresponding EIP for each contract that says that it should fail if there's no code
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7251.md
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7002.md#empty-code-failure

JereSalo · 2025-10-09T14:32:37Z

crates/vm/system_contracts.rs

        .filter(move |system_contract| system_contract.active_since_fork <= fork)
 }
+
+pub const SYSTEM_CONTRACTS_WITH_CODE: [SystemContract; 3] = [


I don't think this is an accurate name. The contracts that should be here are just 2, the EIP 7002 and EIP 7251 contracts. Idk if we can call them PRAGUE_SYSTEM_CONTRACTS or any name representative of that

For example, the beacon root contract is a system contract with code but doesn't belong to the check we are trying to make. Correct me if I'm wrong

Fair, I didn't know the details of the beacon roots contract. As for the rest, I'll fix it in a sec. I thought the deposits contract should be included too but I guess thinking about it now it needn't

JereSalo · 2025-10-09T20:57:05Z

Also, make sure to link the corresponding EIP for each contract that says that it should fail if there's no code https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7251.md#empty-code-failure https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7002.md#empty-code-failure

this is still missing

SDartayet · 2025-10-09T21:33:06Z

Adding links to EIPs

Sorry, my bad, idk how I missed that comment. Fixed it here

SDartayet and others added 6 commits September 29, 2025 20:14

Added error to pass hive tests and improved error messages

aaa9f76

Removed unused import

ab8ab37

Updating message in blockchain tests

1aa1d80

Merge branch 'main' into fix-eip7623-hive-tests

e35843e

Merge branch 'main' into fix-eip7623-hive-tests

5675249

Added error for case where system contract code is empty

ebed22a

github-actions bot assigned SDartayet Oct 1, 2025

github-actions bot added the L1 Ethereum client label Oct 1, 2025

github-project-automation bot added this to ethrex_l1 Oct 1, 2025

SDartayet added 3 commits October 1, 2025 16:10

Merge branch 'main' into fix-hive-tests

aa57c47

Merge branch 'fix-eip7623-hive-tests' into fix-hive-tests

fa6c430

Fixed failing test

2080e2f

SDartayet added 2 commits October 2, 2025 10:39

fix lint issues

b9fe38f

fixing previous commit

2749d4d

SDartayet marked this pull request as ready for review October 6, 2025 13:00

SDartayet requested a review from a team as a code owner October 6, 2025 13:00

ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 6, 2025

mpaulucci reviewed Oct 6, 2025

View reviewed changes

JereSalo reviewed Oct 8, 2025

View reviewed changes

Addressing review comments

84ba906

SDartayet requested review from JereSalo and mpaulucci October 8, 2025 17:56

JereSalo reviewed Oct 8, 2025

View reviewed changes

Merge branch 'main' into fix-hive-tests

9f9c021

mpaulucci approved these changes Oct 9, 2025

View reviewed changes

SDartayet requested a review from JereSalo October 9, 2025 13:29

JereSalo reviewed Oct 9, 2025

View reviewed changes

Åddressing review comments

67cc166

SDartayet requested a review from JereSalo October 9, 2025 16:15

Adding links to EIPs

d9e8ef7

fix(l1): added error for case where system contract code is empty #4731

Are you sure you want to change the base?

fix(l1): added error for case where system contract code is empty #4731

Uh oh!

Conversation

SDartayet commented Oct 1, 2025

Uh oh!

github-actions bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lines of code report

Uh oh!

github-actions bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark Results Comparison

Benchmark Results: BubbleSort

Benchmark Results: ERC20Approval

Benchmark Results: ERC20Mint

Benchmark Results: ERC20Transfer

Benchmark Results: Factorial

Benchmark Results: FactorialRecursive

Benchmark Results: Fibonacci

Benchmark Results: FibonacciRecursive

Benchmark Results: ManyHashes

Benchmark Results: MstoreBench

Benchmark Results: Push

Benchmark Results: SstoreBench_no_opt

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JereSalo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JereSalo commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SDartayet commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Oct 1, 2025 •

edited

Loading

github-actions bot commented Oct 1, 2025 •

edited

Loading

JereSalo left a comment •

edited

Loading

JereSalo commented Oct 9, 2025 •

edited

Loading