refactor(l1): refactor chainconfig #5233

SDartayet · 2025-11-07T15:31:18Z

Motivation

Improve the maintainability of our code with regards to future forks.

Description

Currently we have a lot of match statements spread throughout various functions to get the current fork, blobschedule, etc; and separate functions for checking the activation of each fork. This creates serious maintainability issues down the line. This PR centralized the match statements in two functions: one to get the activation timestamp/block for a fork, and one to get the blob schedule for a fork, and rewrites the rest of the functions accordingly. All the funcitons to check fork activation were also aggregated into a single is_fork_activated function that takes the fork as a parameter and checks its activation. The implementation of get_blob_schedule_for_time also fetches the most recent active blob schedule if the current fork specifies none, which is part of tackling #4849.

Additionally, some code related to pre-Merge logic was removed (checks for EIP 155 and EIP 2028)

Closes #4720

github-actions · 2025-11-07T15:34:02Z

Lines of code report

Total lines added: 8
Total lines removed: 94
Total lines changed: 102

Detailed view

+--------------------------------------------------------------+-------+------+
| File                                                         | Lines | Diff |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                       | 1299  | +1   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/constants.rs                        | 13    | -1   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/mempool.rs                          | 707   | -26  |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs                          | 672   | -4   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/common/types/block.rs                          | 884   | +4   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/common/types/genesis.rs                        | 896   | -56  |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer/payload_builder.rs | 188   | -7   |
+--------------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/blobs.rs                 | 139   | +2   |
+--------------------------------------------------------------+-------+------+
| ethrex/tooling/ef_tests/state/deserialize.rs                 | 417   | +1   |
+--------------------------------------------------------------+-------+------+

github-actions · 2025-11-07T15:46:55Z

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`	2.978 ± 0.021	2.957	3.019	1.00 ± 0.01
`main_levm_BubbleSort`	3.113 ± 0.028	3.080	3.171	1.05 ± 0.01
`pr_revm_BubbleSort`	2.974 ± 0.023	2.953	3.021	1.00
`pr_levm_BubbleSort`	3.093 ± 0.024	3.061	3.135	1.04 ± 0.01

Benchmark Results: ERC20Approval

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Approval`	979.4 ± 15.9	968.2	1021.4	1.00
`main_levm_ERC20Approval`	1103.4 ± 30.9	1085.1	1188.1	1.13 ± 0.04
`pr_revm_ERC20Approval`	993.3 ± 69.5	955.0	1188.7	1.01 ± 0.07
`pr_levm_ERC20Approval`	1085.9 ± 3.5	1082.1	1092.5	1.11 ± 0.02

Benchmark Results: ERC20Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Mint`	134.6 ± 0.8	133.1	135.5	1.01 ± 0.01
`main_levm_ERC20Mint`	164.0 ± 1.1	162.9	166.2	1.23 ± 0.01
`pr_revm_ERC20Mint`	133.8 ± 1.0	132.6	135.9	1.00
`pr_levm_ERC20Mint`	165.2 ± 3.8	162.2	175.7	1.23 ± 0.03

Benchmark Results: ERC20Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Transfer`	231.1 ± 1.7	229.1	234.2	1.00 ± 0.01
`main_levm_ERC20Transfer`	281.6 ± 1.8	279.2	285.5	1.22 ± 0.01
`pr_revm_ERC20Transfer`	230.3 ± 1.5	228.3	232.8	1.00
`pr_levm_ERC20Transfer`	280.2 ± 4.8	275.7	290.9	1.22 ± 0.02

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Factorial`	232.1 ± 10.2	226.2	260.9	1.03 ± 0.05
`main_levm_Factorial`	264.7 ± 4.7	262.3	277.8	1.17 ± 0.02
`pr_revm_Factorial`	226.1 ± 0.5	225.2	226.8	1.00
`pr_levm_Factorial`	267.9 ± 4.1	264.3	278.1	1.18 ± 0.02

Benchmark Results: FactorialRecursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_FactorialRecursive`	1.659 ± 0.048	1.551	1.702	1.01 ± 0.04
`main_levm_FactorialRecursive`	8.472 ± 0.113	8.374	8.771	5.17 ± 0.12
`pr_revm_FactorialRecursive`	1.639 ± 0.032	1.593	1.707	1.00
`pr_levm_FactorialRecursive`	8.471 ± 0.072	8.367	8.562	5.17 ± 0.11

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Fibonacci`	209.2 ± 5.9	206.2	226.0	1.01 ± 0.03
`main_levm_Fibonacci`	260.5 ± 6.8	250.6	272.3	1.26 ± 0.03
`pr_revm_Fibonacci`	206.5 ± 1.3	205.3	209.5	1.00
`pr_levm_Fibonacci`	255.4 ± 5.8	251.1	270.9	1.24 ± 0.03

Benchmark Results: FibonacciRecursive

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_FibonacciRecursive`	866.1 ± 8.0	854.2	879.5	1.14 ± 0.02
`main_levm_FibonacciRecursive`	762.6 ± 11.2	755.9	792.9	1.00
`pr_revm_FibonacciRecursive`	855.4 ± 16.4	840.5	895.6	1.12 ± 0.03
`pr_levm_FibonacciRecursive`	773.1 ± 17.1	755.9	810.8	1.01 ± 0.03

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ManyHashes`	8.3 ± 0.0	8.2	8.3	1.00
`main_levm_ManyHashes`	9.1 ± 0.1	9.0	9.1	1.10 ± 0.01
`pr_revm_ManyHashes`	8.3 ± 0.1	8.2	8.6	1.00 ± 0.02
`pr_levm_ManyHashes`	9.2 ± 0.1	9.0	9.3	1.11 ± 0.01

Benchmark Results: MstoreBench

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_MstoreBench`	268.5 ± 2.9	265.8	275.1	1.11 ± 0.02
`main_levm_MstoreBench`	244.7 ± 10.5	239.7	274.4	1.01 ± 0.04
`pr_revm_MstoreBench`	268.6 ± 3.3	266.2	277.4	1.11 ± 0.02
`pr_levm_MstoreBench`	242.1 ± 2.0	239.2	245.8	1.00

Benchmark Results: Push

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Push`	296.1 ± 1.9	294.6	300.6	1.00
`main_levm_Push`	297.6 ± 0.9	296.5	299.6	1.01 ± 0.01
`pr_revm_Push`	299.2 ± 12.1	293.8	333.5	1.01 ± 0.04
`pr_levm_Push`	296.8 ± 1.8	294.5	300.5	1.00 ± 0.01

Benchmark Results: SstoreBench_no_opt

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_SstoreBench_no_opt`	174.9 ± 5.0	168.1	186.6	1.94 ± 0.06
`main_levm_SstoreBench_no_opt`	90.0 ± 1.6	87.9	91.9	1.00
`pr_revm_SstoreBench_no_opt`	171.9 ± 5.5	167.7	184.3	1.91 ± 0.07
`pr_levm_SstoreBench_no_opt`	91.6 ± 3.8	87.0	99.6	1.02 ± 0.05

mpaulucci · 2025-11-07T16:46:52Z

crates/common/types/genesis.rs

-    GrayGlacier = 14,
-    Paris = 15,
-    Shanghai = 16,
+    Homestead = 0,


why is this change of names?

I changed the enum so the forks would be the ones mentioned in labels for the activation block numbers and timestamps. This was necessary ƒor the match statements in fork_activation_time_or_block and is_fork_activated to work properly. While I could have made it a separate enum, I think it makes more sense for the fork enums to reflect the forks as laid out in the chain config files (and in practice only the chain config struct methods and the tests use the fork enum, so it shouldn't create broader problems)

mpaulucci · 2025-11-07T16:47:01Z

crates/common/types/genesis.rs

+    GrayGlacier = 13,
+    Paris = 14,
+    Shanghai = 15,
    #[default]


we probably shouldn't have defaults

I tried to do this but it breaks other structs that have fork as a field and implement default, like parts of the LEVM runner or EVMConfig. Unless we want to remove those defaults too it might be better to leave this as is (so fork defaults aren't spread out throughout the code)

it kinda sucks, but if you have to keep it, at least choose the (soon to be) current one, Osaka

mpaulucci · 2025-11-07T16:50:10Z

crates/vm/levm/src/environment.rs


        let blob_schedule = chain_config
-            .get_fork_blob_schedule(block_header.timestamp)
+            .get_current_blob_schedule(block_header.timestamp)


current is deceiving if its calculated based on a timestamp

Sure. Addressed here (alongside some fixes for other things that were failing)

crates/blockchain/constants.rs

mpaulucci · 2025-11-10T18:16:51Z

crates/blockchain/payload.rs

            // TODO: maybe fetch hash too when filtering mempool so we don't have to compute it here (we can do this in the same refactor as adding timestamp)
            let tx_hash = head_tx.tx.hash();

-            // Check whether the tx is replay-protected


state tests don't use this?

They aren't failing on the CI and they aren't failing locally either, so preseumably not.

fmoletta · 2025-11-12T15:27:49Z

crates/common/types/genesis.rs

+    pub fn get_blob_schedule_for_time(&self, block_timestamp: u64) -> Option<ForkBlobSchedule> {
+        if let Some(fork_with_current_blob_schedule) = FORKS.into_iter().rfind(|fork| {
+            self.get_blob_schedule_for_fork(*fork).is_some()
+                && self.is_fork_activated(*fork, block_timestamp)
+        }) {
+            self.get_blob_schedule_for_fork(fork_with_current_blob_schedule)


Do we need to iterate all forks? Can't we just fetch the blob schedule for the active fork?

Ah, it's a part of tackling #4849. Recently it was decided that if a fork doesn't change the blob schedule, it won't have a blob schedule field in the genesis file, and the blob schedule from the most recent active fork should be used instead. This implements that logic; rfind short circuits as soon as it finds an element of the array that fulfills the closure conditions so this shouldn't iterate through too much of the array in practice

mpaulucci · 2025-11-13T21:32:43Z

crates/common/types/genesis.rs

    BPO5 = 24,
 }

+pub const FORKS: [Fork; 25] = [


Why do you need this? Fork already has a number so you should be able to use that for ordering?

We do, but it helps with either iterating through forks (when, for example, we want the latest forks that fulfills a certain condition or other; we use this to get the latest scheduled fork, the current one, etc)

you should be able to implement iterator on the other structure? I don't think we need this

Not really. You can't iterate through enum variants by default. Iterating through numbers and converting them to forks wouldn't work either without implementing a from function that matches numbers to forks, but I could do that if you think it's better

mpaulucci · 2025-11-13T21:35:03Z

crates/common/types/genesis.rs


-    pub fn is_eip155_activated(&self, block_number: BlockNumber) -> bool {
-        self.eip155_block.is_some_and(|num| num <= block_number)
+    pub fn is_fork_activated(&self, fork: Fork, timestamp_or_block: u64) -> bool {


why do we need to support blocks here? Is this ever called with a block?

I don't think it ever should be but it's more of a syntax thing if anything since we want to cover the block number based forks in the match arms too (and it doesn't change functionality). If you find it better I could just make it return true for all forks pre-Merge

Actually I realized there is an edge case: if a call to the eth_config RPC endpoint happens before the node has synced to head, we need to check which is the currently active fork to know whether to respond or not (since the response format isn't defined for forks pre-cancun), and that requires iterating through the forks and checking which is the most recent one that's active

current implementation. is error prone. You should either only accept timestamps or create a type BlockNumberOrTimestamp, or something like that and deal accordingly

Refactored chainconfig

3e54808

github-actions bot assigned SDartayet Nov 7, 2025

github-actions bot added the L1 Ethereum client label Nov 7, 2025

github-project-automation bot added this to ethrex_l1 Nov 7, 2025

mpaulucci reviewed Nov 7, 2025

View reviewed changes

SDartayet added 5 commits November 7, 2025 15:09

Fixed tests and addressed comment

f2a880d

Remove leftover removed fork

8340a23

Added missing fork

89f8911

fixing compile errors

0c5036f

Cargo fmt

1fd404a

SDartayet marked this pull request as ready for review November 10, 2025 17:58

SDartayet requested a review from a team as a code owner November 10, 2025 17:58

SDartayet requested a review from mpaulucci November 10, 2025 17:58

ethrex-project-sync bot moved this to In Review in ethrex_l1 Nov 10, 2025

mpaulucci reviewed Nov 10, 2025

View reviewed changes

crates/blockchain/constants.rs Show resolved Hide resolved

mpaulucci reviewed Nov 10, 2025

View reviewed changes

SDartayet requested a review from mpaulucci November 10, 2025 18:50

fmoletta reviewed Nov 12, 2025

View reviewed changes

SDartayet requested a review from fmoletta November 12, 2025 15:50

Merge branch 'main' into refactor-chainconfig-2

b3261bf

mpaulucci reviewed Nov 13, 2025

View reviewed changes

Merge branch 'main' into refactor-chainconfig-2

e4a8602

SDartayet requested a review from mpaulucci November 13, 2025 21:41

SDartayet added 2 commits November 13, 2025 18:54

fix compile issue

4b5da78

Fixed lint

2868af4

refactor(l1): refactor chainconfig #5233

Are you sure you want to change the base?

refactor(l1): refactor chainconfig #5233

Uh oh!

Conversation

SDartayet commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 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 Nov 7, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SDartayet commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading