feat(l1): process blocks in batches when syncing #2174
Conversation
Lines of code reportTotal lines added: Detailed view |
| @@ -110,6 +110,63 @@ impl Blockchain { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| pub fn add_blocks_in_batch( | |||
There was a problem hiding this comment.
Can't we change the implementation of add_block to just call this function?
There was a problem hiding this comment.
Yes, I planned to refactor it after having some benches.
| @@ -110,6 +110,63 @@ impl Blockchain { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| pub fn add_blocks_in_batch( | |||
| &self, | |||
| parent_hash: BlockHash, | |||
There was a problem hiding this comment.
why do you need to explicitly pass parent_hash? Couldn't you just get it from blocks[0].parent_hash?
There was a problem hiding this comment.
Yes you are right, it was a param inherited from add_block.
crates/blockchain/blockchain.rs
Outdated
| for (i, block) in blocks.iter().enumerate() { | ||
| let block_hash = block.header.compute_block_hash(); | ||
| // Validate if it can be the new head and find the parent | ||
| let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { |
There was a problem hiding this comment.
wait, shouldn't it be a precondition of the function that block n + 1 has block n as parent?
There was a problem hiding this comment.
Sure we could add that pre-condition to return earlier. Thou, I don't know if its actually really necessary as in the next iteration you are going to verify that block n-1 is indeed the parent of n.
crates/blockchain/blockchain.rs
Outdated
| return Err(ChainError::ParentNotFound); | ||
| }; | ||
|
|
||
| let chain_config: ChainConfig = self.storage.get_chain_config()?; |
There was a problem hiding this comment.
this is inmutable so it could be passed to Blockchain on new, or called in the constructor.
There was a problem hiding this comment.
You are right, we would avoid a db read as well.
| // todo only execute transactions | ||
| // batch account updates to merge the repeated accounts | ||
| self.storage | ||
| .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; |
There was a problem hiding this comment.
Ahh, I understand now. I was hoping we could just call: https://github.com/lambdaclass/lambda_ethereum_rust/blob/0acc5e28b861f88c30cebb6cbfe0230970df25ed/crates/vm/backends/revm.rs#L96 get_state_transitions only once.
We would need to add a execute_blocks inside vm.
There was a problem hiding this comment.
we can discuss this later.
There was a problem hiding this comment.
I've tried this approach but it won't work without making larger modifications to the vm backend.
we were missing to set the cannonical block hash for the number
Motivation
Accelerate syncing!
Description
This PR introduces block batching during full sync:
nblocks instead of one per block (we'll need less storage also).Considerations:
levmbackend we need it to cache the executions state and persist it between them, as we don't store anything until the final of the batch.Closes None