feat(l2): add l2 block pipelined #5527
Open
+92
−115
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gianbelinche
requested review from
a team,
ManuelBilbao,
avilagaston9 and
ilitteri
as code owners
gianbelinche
changed the title
Lines of code reportTotal lines added: Detailed view |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces L2 block pipelining to improve performance when processing L2 blocks, similar to the existing L1 block pipelining optimization. The implementation adds a new add_l2_block_pipeline function that executes and merkleizes blocks concurrently in separate threads, leveraging a pre-initialized VM for re-executing already validated blocks from checkpoints.
Key Changes
- Added
execute_l2_block_pipelineandadd_l2_block_pipelinefunctions to the blockchain that take a pre-initialized VM and parent header for efficient block re-execution - Refactored checkpoint regeneration in L1 committer to use the new pipelined approach instead of manual execution and merkleization
- Updated block processing in P2P connections and command-line operations to use the pipelined variant for performance gains
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/blockchain/blockchain.rs | Adds execute_l2_block_pipeline and add_l2_block_pipeline functions that implement concurrent execution and merkleization for L2 blocks using a pre-initialized VM |
| crates/l2/sequencer/l1_committer.rs | Refactors checkpoint generation and batch production to use add_l2_block_pipeline instead of manual block execution and state updates, simplifying the code and improving performance |
| crates/networking/p2p/rlpx/l2/l2_connection.rs | Updates block processing to use add_block_pipeline instead of add_block for performance improvement |
| cmd/ethrex/l2/command.rs | Changes checkpoint import to use add_block_pipeline instead of add_block for faster block execution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/blockchain/blockchain.rs
Outdated
Comment on lines
1145
to
1162
| pub fn add_l2_block_pipeline( | ||
| &self, | ||
| block: Block, | ||
| parent_header: &BlockHeader, | ||
| vm: &mut Evm, | ||
| ) -> Result<(), ChainError> { | ||
| let (res, account_updates_list) = | ||
| self.execute_l2_block_pipeline(&block, parent_header, vm)?; | ||
|
|
||
| self.store_block( | ||
| block, | ||
| account_updates_list, | ||
| BlockExecutionResult { | ||
| receipts: res.receipts, | ||
| requests: vec![], | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Missing documentation for this public function. Consider adding a doc comment that explains:
- The purpose of this L2-specific pipeline variant
- Why it takes a pre-initialized VM and parent_header as parameters (unlike
add_block_pipeline) - When it should be used (e.g., for checkpoint regeneration/re-execution of already validated blocks)
crates/blockchain/blockchain.rs
Outdated
Comment on lines
191
to
241
| fn execute_l2_block_pipeline( | ||
| &self, | ||
| block: &Block, | ||
| parent_header: &BlockHeader, | ||
| vm: &mut Evm, | ||
| ) -> Result<(BlockExecutionResult, AccountUpdatesList), ChainError> { | ||
| let queue_length = AtomicUsize::new(0); | ||
| let queue_length_ref = &queue_length; | ||
| let mut max_queue_length = 0; | ||
| let (execution_result, account_updates_list) = std::thread::scope(|s| { | ||
| let max_queue_length_ref = &mut max_queue_length; | ||
| let (tx, rx) = channel(); | ||
| let execution_handle = std::thread::Builder::new() | ||
| .name("block_executor_execution".to_string()) | ||
| .spawn_scoped(s, move || -> Result<_, ChainError> { | ||
| let execution_result = | ||
| vm.execute_block_pipeline(block, tx, queue_length_ref)?; | ||
|
|
||
| Ok(execution_result) | ||
| }) | ||
| .expect("Failed to spawn block_executor exec thread"); | ||
| let parent_header_ref = &parent_header; // Avoid moving to thread | ||
| let merkleize_handle = std::thread::Builder::new() | ||
| .name("block_executor_merkleizer".to_string()) | ||
| .spawn_scoped(s, move || -> Result<_, StoreError> { | ||
| let account_updates_list = self.handle_merkleization( | ||
| s, | ||
| rx, | ||
| parent_header_ref, | ||
| queue_length_ref, | ||
| max_queue_length_ref, | ||
| )?; | ||
| Ok(account_updates_list) | ||
| }) | ||
| .expect("Failed to spawn block_executor merkleizer thread"); | ||
| ( | ||
| execution_handle.join().unwrap_or_else(|_| { | ||
| Err(ChainError::Custom("execution thread panicked".to_string())) | ||
| }), | ||
| merkleize_handle.join().unwrap_or_else(|_| { | ||
| Err(StoreError::Custom( | ||
| "merklization thread panicked".to_string(), | ||
| )) | ||
| }), | ||
| ) | ||
| }); | ||
| let execution_result = execution_result?; | ||
| let account_updates_list = account_updates_list?; | ||
|
|
||
| Ok((execution_result, account_updates_list)) | ||
| } |
There was a problem hiding this comment.
Missing documentation for this private function. Consider adding a doc comment explaining:
- The purpose of this L2-specific execution pipeline
- Why it differs from
execute_block_pipeline(no validation, takes pre-initialized VM) - The threading/pipelining strategy being employed
crates/blockchain/blockchain.rs
Outdated
Comment on lines
1157
to
1160
| BlockExecutionResult { | ||
| receipts: res.receipts, | ||
| requests: vec![], | ||
| }, |
There was a problem hiding this comment.
The requests from the execution result (res.requests) are being discarded and replaced with an empty vector. While this maintains the existing behavior from the refactored code, consider:
- Using
resdirectly (likeadd_block_pipelinedoes on line 1175) if requests should be preserved - Adding a comment explaining why requests are intentionally discarded for L2 blocks if this is the intended behavior
Suggested change
| BlockExecutionResult { | |
| receipts: res.receipts, | |
| requests: vec![], | |
| }, | |
| res, |
ilitteri
approved these changes
Dec 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We want to add the performance improvement done in L1 with the block pipelining also for the L2.
Description
Adds the
add_l2_block_pipelinefunction to the blockchain and uses it for regenerating the blockchain from checkpoints.