refactor(levm,l1,l2): split block execution and update generation by iovoid · Pull Request #2519 · lambdaclass/ethrex

iovoid · 2025-04-22T12:35:23Z

Motivation

Currently during batch processing, the state transitions are calculated for every block and then merged, when it would be more performant to calculate them once at the end.

Description

This PR removes the account updates from the execution result and makes every consumer manually request them.

Closes #2504

github-actions · 2025-04-22T12:36:31Z

Lines of code report

Total lines added: 15
Total lines removed: 11
Total lines changed: 26

Detailed view

+----------------------------------------------------------+-------+------+
| File                                                     | Lines | Diff |
+----------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                   | 494   | +9   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/zkvm/interface/risc0/src/main.rs | 46    | +1   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/zkvm/interface/sp1/src/main.rs   | 43    | +1   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_committer.rs               | 387   | +4   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                    | 637   | -5   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/mod.rs                         | 241   | -1   |
+----------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/revm/mod.rs                    | 607   | -5   |
+----------------------------------------------------------+-------+------+

mpaulucci · 2025-04-22T13:24:18Z

+            if chain_config.fork(block.header.timestamp) != fork {
+                return Err((
+                    ChainError::Custom("Crossing fork boundary in bulk mode".into()),
+                    Some(BatchBlockProcessingFailure {


I think is a bit confusing, since the problem is not that the block n is invalid, but that the caller made a mistake by calling this functions with blocks from different forks.

Maybe a new error variant?

Sounds good.

We should handle this (if we cannot avoid it) in caller code (aka full sync) as crossing fork boundaries in a batch is something that will eventually happen when syncing networks. This probably exceeds the scope of this PR but we should create an issue for it

agreed with @fmoletta, we should create a ticket and handle this from the caller. But I think it's better to also check this from inside and return an error.

See conversation in slack. This shouldn't be a problem since we're only dealing with post merge networks and we should be able to do batch block processing across fork boundaries.

mpaulucci · 2025-04-22T13:26:53Z

+            Ok(v) => v,
+            Err(err) => return Err((ChainError::EvmError(err), None)),
+        };
+        for account_update in account_updates {


not sure what this does, but it seems weird do do account updates processing in the blockchain crate. From a design perspective, it seems this should be done inside the vm crate and that blockchain crate should just get results and update the db?

Actually now that a single get_state_transition call is made, this merge isn't needed. Fixed.

Yep, we also don't need to use all_account_updates nor the for loop. We can just send account_updates directly to the apply_account_updates method.

mpaulucci · 2025-04-22T13:27:31Z

-    let account_updates = result.account_updates;
+    let _result = vm.execute_block(&block)?;
+    // let receipts = result.receipts;
+    let account_updates = vm.get_state_transitions(fork)?;


hmm, weird to pass the current fork to the get_state_transitions function. From the outside, it doesn't make much sense.

It's needed because because SpuriousDragon changed the rules on remotion from the trie

But we don't support SpuriousDragon, so maybe we can avoid this?

Yes, I wanted to remove that in the past but the thing is that the State EF Tests test for Spurious Dragon and we use this same method. As per my understanding LEVM will ideally support all forks, but if we change opinion on that then it's ok to remove the fork.

The vm should know which fork the block was executed from, so when calling get_state_transitions it should have that information internally. It should not be passed explicitly. Otherwise I could execute a Cacun block and then ask for the state transitions for SpuriousDragon

Yes! The VM knows the fork but the thing is that get_state_transitions is not a method from the actual VM. It is just a function of the LEVM module in the intersection between L1 and the VM. Maybe it should be a VM method though.

Adding to this. Our VM just exists in the context of a transaction, after executing one we discard the VM and the only thing that's left is the modified state, so we lose that VM instance that knew which fork it was executing. I agree that it's not nice to pass the fork, for now we can just disable SpuriousDragon checks

The LEVM DB doesn't currently keep track of which blocks it executed.

github-actions · 2025-04-22T14:20:34Z

Benchmark Block Execution Results Comparison Against Main

Command	Mean [s]	Min [s]	Max [s]	Relative
`base`	180.069 ± 1.017	178.719	181.440	1.00
`head`	180.115 ± 0.956	178.538	181.982	1.00 ± 0.01

JereSalo · 2025-04-23T16:52:31Z

+        let account_updates = match vm.get_state_transitions(fork) {
+            Ok(v) => v,
+            Err(err) => return Err((ChainError::EvmError(err), None)),
+        };


Suggested change

let account_updates = match vm.get_state_transitions(fork) {

Ok(v) => v,

Err(err) => return Err((ChainError::EvmError(err), None)),

};

let account_updates = vm.get_state_transitions(fork).map_err(|err| (ChainError::EvmError(err), None))?;

nit

JereSalo · 2025-04-23T17:03:16Z

        panic!("invalid database")
    };

+    let fork = db.chain_config.fork(block.header.timestamp);


This is not necessary when running with REVM

JereSalo

I think it's good!

iovoid

Updated

JulianVentura

LGTM

…mbdaclass#2519) **Motivation** Currently during batch processing, the state transitions are calculated for every block and then merged, when it would be more performant to calculate them once at the end. **Description** This PR removes the account updates from the execution result and makes every consumer manually request them.  Closes lambdaclass#2504

split block execution and update generation

47153d2

iovoid requested a review from a team as a code owner April 22, 2025 12:35

fail if trying to cross fork boundary

be88392

iovoid added performance Block execution throughput and performance in general levm Lambda EVM implementation L2 Rollup client L1 Ethereum client labels Apr 22, 2025

iovoid added 3 commits April 22, 2025 09:43

update perf changelog

8356a50

fix prover execution

3b8b7e0

fix sp1

f622473

mpaulucci reviewed Apr 22, 2025

View reviewed changes

fix for pico and risc0

ced1ab7

mpaulucci reviewed Apr 22, 2025

View reviewed changes

iovoid added 2 commits April 22, 2025 10:42

shorten update processing since it's non-overlapping now

1b6f5bb

fix pico

f6ebca4

fmoletta mentioned this pull request Apr 22, 2025

fix(levm): bug when collecting storage updates affecting batch execute #2515

Closed

iovoid added 3 commits April 23, 2025 09:10

remove unneeded expect

19792c7

Merge branch 'main' into refactor/separate-state-transitions

e8b38bf

Merge branch 'main' into refactor/separate-state-transitions

6ce8634

JereSalo reviewed Apr 23, 2025

View reviewed changes

JereSalo approved these changes Apr 23, 2025

View reviewed changes

iovoid added 3 commits April 23, 2025 14:35

style fixes

8df2381

only bail when crossing spuriousdragon

e307876

remove unneeded copying

e63fadf

iovoid commented Apr 23, 2025

View reviewed changes

JulianVentura approved these changes Apr 23, 2025

View reviewed changes

iovoid added this pull request to the merge queue Apr 23, 2025

Merged via the queue into main with commit 5a7d759 Apr 23, 2025

iovoid deleted the refactor/separate-state-transitions branch April 23, 2025 19:29

Conversation

iovoid commented Apr 22, 2025

Uh oh!

github-actions Bot commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lines of code report

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!

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!

github-actions Bot commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark Block Execution Results Comparison Against Main

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

Choose a reason for hiding this comment

Uh oh!

iovoid left a comment

Choose a reason for hiding this comment

Uh oh!

JulianVentura left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented Apr 22, 2025 •

edited

Loading

github-actions Bot commented Apr 22, 2025 •

edited

Loading