From 3653b85c910d9bc0beeb9a45c8637f5cb157e6d7 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 6 Mar 2025 17:58:50 -0300 Subject: [PATCH 01/60] feat: add_blocks_in_batch --- crates/blockchain/blockchain.rs | 76 +++++++++++++++++++++++++-------- crates/storage/api.rs | 10 +++++ crates/storage/store.rs | 63 +++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 17 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 43f0da98cd..6f2daf39f4 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -97,8 +97,8 @@ impl Blockchain { // Processes requests from receipts, computes the requests_hash and compares it against the header validate_requests_hash(&block.header, &chain_config, &requests)?; - store_block(&self.storage, block.clone())?; - store_receipts(&self.storage, receipts, block_hash)?; + self.storage.add_block(block.clone())?; + self.storage.add_receipts(block_hash, receipts)?; let interval = Instant::now().duration_since(since).as_millis(); if interval != 0 { @@ -110,6 +110,63 @@ impl Blockchain { Ok(()) } + pub fn add_blocks_in_batch( + &self, + parent_hash: BlockHash, + blocks: &[Block], + ) -> Result<(), ChainError> { + let Some(mut state_trie) = self.storage.state_trie(parent_hash)? else { + return Err(ChainError::ParentNotFound); + }; + + let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; + + 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 { + // If the parent is not present, we store it as pending. + self.storage.add_pending_block(block.clone())?; + return Err(ChainError::ParentNotFound); + }; + + let mut state = evm_state(self.storage.clone(), block.header.parent_hash); + let chain_config: ChainConfig = state.chain_config().map_err(ChainError::from)?; + + // Validate the block pre-execution + validate_block(block, &parent_header, &chain_config)?; + let BlockExecutionResult { + receipts, + requests, + account_updates, + } = self.vm.execute_block(block, &mut state)?; + + validate_gas_used(&receipts, &block.header)?; + + // todo only execute transactions + // batch account updates to merge the repeated accounts + state + .database() + .ok_or(ChainError::StoreError(StoreError::MissingStore))? + .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; + + // Validate state trie on last block only + if i >= blocks.len() - 1 { + let root_hash = state_trie.hash().map_err(StoreError::Trie)?; + validate_state_root(&block.header, root_hash)?; + validate_receipts_root(&block.header, &receipts)?; + validate_requests_hash(&block.header, &chain_config, &requests)?; + } + + all_receipts.push((block_hash, receipts)); + } + + self.storage.add_batch_of_blocks(blocks.into())?; + self.storage.add_batch_of_receipts(all_receipts)?; + + Ok(()) + } + //TODO: Forkchoice Update shouldn't be part of this function pub fn import_blocks(&self, blocks: &Vec) { let size = blocks.len(); @@ -342,21 +399,6 @@ pub fn validate_requests_hash( Ok(()) } -/// Stores block and header in the database -pub fn store_block(storage: &Store, block: Block) -> Result<(), ChainError> { - storage.add_block(block)?; - Ok(()) -} - -pub fn store_receipts( - storage: &Store, - receipts: Vec, - block_hash: BlockHash, -) -> Result<(), ChainError> { - storage.add_receipts(block_hash, receipts)?; - Ok(()) -} - /// Performs post-execution checks pub fn validate_state_root( block_header: &BlockHeader, diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 324a78f790..267575f68c 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -10,6 +10,10 @@ use crate::{error::StoreError, store::STATE_TRIE_SEGMENTS}; use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { + /// Add a batch of blocks in a single tx + /// This will store -> BlockHeader, BlockBody, BlockTransaction, BlockNumber + fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError>; + /// Add block header fn add_block_header( &self, @@ -97,6 +101,12 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { fn add_receipts(&self, block_hash: BlockHash, receipts: Vec) -> Result<(), StoreError>; + /// Adds a batch of receipts + fn add_batch_of_receipts( + &self, + blocks: Vec<(BlockHash, Vec)>, + ) -> Result<(), StoreError>; + /// Obtain receipt for a canonical block represented by the block number. fn get_receipt( &self, diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 62dd825e93..7a25da22d0 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -286,6 +286,7 @@ impl Store { let account_state = AccountState::decode(&encoded_state)?; self.get_account_code(account_state.code_hash) } + pub fn get_nonce_by_account_address( &self, block_number: BlockNumber, @@ -358,6 +359,56 @@ impl Store { Ok(Some(state_trie.hash()?)) } + /// Applies state transitions to the given trie and does not commit nor calculate root hash + pub fn apply_account_updates_to_trie( + &self, + account_updates: &[AccountUpdate], + state_trie: &mut Trie, + ) -> Result<(), StoreError> { + for update in account_updates.iter() { + let hashed_address = hash_address(&update.address); + if update.removed { + // Remove account from trie + state_trie.remove(hashed_address)?; + } else { + // Add or update AccountState in the trie + // Fetch current state or create a new state to be inserted + let mut account_state = match state_trie.get(&hashed_address)? { + Some(encoded_state) => AccountState::decode(&encoded_state)?, + None => AccountState::default(), + }; + if let Some(info) = &update.info { + account_state.nonce = info.nonce; + account_state.balance = info.balance; + account_state.code_hash = info.code_hash; + // Store updated code in DB + if let Some(code) = &update.code { + self.add_account_code(info.code_hash, code.clone())?; + } + } + // Store the added storage in the account's storage trie and compute its new root + if !update.added_storage.is_empty() { + let mut storage_trie = self.engine.open_storage_trie( + H256::from_slice(&hashed_address), + account_state.storage_root, + ); + for (storage_key, storage_value) in &update.added_storage { + let hashed_key = hash_key(storage_key); + if storage_value.is_zero() { + storage_trie.remove(hashed_key)?; + } else { + storage_trie.insert(hashed_key, storage_value.encode_to_vec())?; + } + } + account_state.storage_root = storage_trie.hash()?; + } + state_trie.insert(hashed_address, account_state.encode_to_vec())?; + } + } + + Ok(()) + } + /// Adds all genesis accounts and returns the genesis block's state_root pub fn setup_genesis_state_trie( &self, @@ -392,6 +443,7 @@ impl Store { Ok(genesis_state_trie.hash()?) } + /// Adds all genesis accounts and returns the genesis block's state_root pub fn add_receipt( &self, block_hash: BlockHash, @@ -409,6 +461,13 @@ impl Store { self.engine.add_receipts(block_hash, receipts) } + pub fn add_batch_of_receipts( + &self, + batch: Vec<(BlockHash, Vec)>, + ) -> Result<(), StoreError> { + self.engine.add_batch_of_receipts(batch) + } + pub fn get_receipt( &self, block_number: BlockNumber, @@ -428,6 +487,10 @@ impl Store { self.add_block_number(hash, number) } + pub fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + self.engine.add_batch_of_blocks(blocks) + } + pub fn add_initial_state(&self, genesis: Genesis) -> Result<(), StoreError> { info!("Setting initial sync status to false"); self.update_sync_status(false)?; From 6b8161dd7917dc8eb3798a61d409ba72e77c3f02 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 6 Mar 2025 19:03:46 -0300 Subject: [PATCH 02/60] feat: add_blocks_in_batch in full sync --- crates/blockchain/blockchain.rs | 4 +-- crates/networking/p2p/sync.rs | 60 ++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 6f2daf39f4..edf513bccc 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -113,7 +113,7 @@ impl Blockchain { pub fn add_blocks_in_batch( &self, parent_hash: BlockHash, - blocks: &[Block], + blocks: Vec, ) -> Result<(), ChainError> { let Some(mut state_trie) = self.storage.state_trie(parent_hash)? else { return Err(ChainError::ParentNotFound); @@ -161,7 +161,7 @@ impl Blockchain { all_receipts.push((block_hash, receipts)); } - self.storage.add_batch_of_blocks(blocks.into())?; + self.storage.add_batch_of_blocks(blocks)?; self.storage.add_batch_of_receipts(all_receipts)?; Ok(()) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index ceb552f91b..4999078d37 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -8,7 +8,7 @@ mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; use ethrex_blockchain::{error::ChainError, Blockchain}; use ethrex_common::{ - types::{Block, BlockHash}, + types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, }; use ethrex_rlp::error::RLPDecodeError; @@ -270,13 +270,17 @@ impl SyncManager { block_headers.remove(0); // Store headers and save hashes for full block retrieval all_block_hashes.extend_from_slice(&block_hashes[..]); - store.add_block_headers(block_hashes.clone(), block_headers)?; if self.sync_mode == SyncMode::Full { - self.download_and_run_blocks(&mut block_hashes, store.clone()) + self.download_and_run_blocks(&block_hashes, &block_headers, store.clone()) .await?; } + // in full sync mode, headers are added if after added after the execution and validation + if self.sync_mode == SyncMode::Snap { + store.add_block_headers(block_hashes.clone(), block_headers)?; + } + if sync_head_found { break; }; @@ -336,11 +340,10 @@ impl SyncManager { /// Returns an error if there was a problem while executing or validating the blocks async fn download_and_run_blocks( &mut self, - block_hashes: &mut [BlockHash], + block_hashes: &[BlockHash], + block_headers: &[BlockHeader], store: Store, ) -> Result<(), SyncError> { - let mut last_valid_hash = H256::default(); - let mut current_chunk_idx = 0; let chunks: Vec> = block_hashes .chunks(MAX_BLOCK_BODIES_TO_REQUEST) @@ -351,6 +354,8 @@ impl SyncManager { Some(res) => res.clone(), None => return Ok(()), }; + let mut headers_idx = 0; + let mut blocks: Vec = vec![]; loop { debug!("Requesting Block Bodies"); @@ -367,40 +372,41 @@ impl SyncManager { block_bodies_len, first_block_hash, first_block_header_number ); - // Execute and store blocks - for (hash, body) in chunk + // Push blocks + for ((_, body), header) in chunk .drain(..block_bodies_len) .zip(block_bodies.into_iter()) + .zip(block_headers[headers_idx..block_bodies_len].iter()) { - let header = store - .get_block_header_by_hash(hash)? - .ok_or(SyncError::CorruptDB)?; - let number = header.number; - let block = Block::new(header, body); - if let Err(error) = self.blockchain.add_block(&block) { - warn!("Failed to add block during FullSync: {error}"); - self.invalid_ancestors.insert(hash, last_valid_hash); - return Err(error.into()); - } - store.set_canonical_block(number, hash)?; - store.update_latest_block_number(number)?; - last_valid_hash = hash; - debug!( - "Executed and stored block number {} with hash {}", - number, hash - ); + let block = Block::new(header.clone(), body); + blocks.push(block); } - debug!("Executed & stored {} blocks", block_bodies_len); + + headers_idx += block_bodies_len; if chunk.is_empty() { current_chunk_idx += 1; chunk = match chunks.get(current_chunk_idx) { Some(res) => res.clone(), - None => return Ok(()), + None => break, }; }; } } + + debug!("Starting to execute and validate blocks in batch"); + let first_block = blocks.first().unwrap().clone(); + let last_block = blocks.last().unwrap().clone(); + let blocks_len = blocks.len(); + + self.blockchain + .add_blocks_in_batch(first_block.header.parent_hash, blocks)?; + + store.set_canonical_block(last_block.header.number, last_block.hash())?; + store.update_latest_block_number(last_block.header.number)?; + debug!("Executed & stored {} blocks", blocks_len); + + Ok(()) } } From 1220097266acbbf139765ffedeabbbe635da2f31 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 7 Mar 2025 08:08:34 -0300 Subject: [PATCH 03/60] feat: store batch of receipts in libmdbx and in-memory --- crates/storage/store_db/in_memory.rs | 19 +++++++++++++++++++ crates/storage/store_db/libmdbx.rs | 25 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 641f4c427b..485e34dacd 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,6 +143,10 @@ impl StoreEngine for Store { Ok(()) } + fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + Ok(()) + } + fn add_block_number( &self, block_hash: BlockHash, @@ -198,6 +202,21 @@ impl StoreEngine for Store { Ok(()) } + fn add_batch_of_receipts( + &self, + receipts: Vec<(BlockHash, Vec)>, + ) -> Result<(), StoreError> { + let mut store = self.inner(); + for (index, (block_hash, receipts)) in receipts.iter().enumerate() { + let entry = store.receipts.entry(*block_hash).or_default(); + for receipt in receipts { + entry.insert(index as u64, receipt.clone()); + } + } + + Ok(()) + } + fn get_receipt( &self, block_number: BlockNumber, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index c7d82fb089..a4949c7965 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -125,6 +125,10 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } + fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + Ok(()) + } + fn get_block_body(&self, block_number: BlockNumber) -> Result, StoreError> { if let Some(hash) = self.get_block_hash_by_block_number(block_number)? { self.get_block_body_by_hash(hash) @@ -472,6 +476,27 @@ impl StoreEngine for Store { self.write_batch::(key_values.into_iter()) } + fn add_batch_of_receipts( + &self, + blocks_receipts: Vec<(BlockHash, Vec)>, + ) -> std::result::Result<(), StoreError> { + let mut key_values = vec![]; + + for (block_hash, receipts) in blocks_receipts { + for (index, receipt) in receipts.into_iter().enumerate() { + let key = (block_hash, index as u64).into(); + let receipt_rlp = receipt.encode_to_vec(); + let Some(mut entries) = IndexedChunk::from::(key, &receipt_rlp) else { + continue; + }; + + key_values.append(&mut entries); + } + } + + self.write_batch::(key_values.into_iter()) + } + fn get_receipts_for_block(&self, block_hash: &BlockHash) -> Result, StoreError> { let mut receipts = vec![]; let mut receipt_index = 0; From f78db0d14b25acdfded1632ebf8e1c002a05231c Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 7 Mar 2025 08:53:09 -0300 Subject: [PATCH 04/60] feat: store batch of blocks in single tx for libmdbx and in-memory --- crates/storage/store_db/in_memory.rs | 17 +++++++++++ crates/storage/store_db/libmdbx.rs | 42 +++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 485e34dacd..81c44bf92c 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -144,6 +144,23 @@ impl StoreEngine for Store { } fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + for block in blocks { + let header = block.header; + let number = header.number; + let hash = header.compute_block_hash(); + let locations = block + .body + .transactions + .iter() + .enumerate() + .map(|(i, tx)| (tx.compute_hash(), number, hash, i as u64)); + + self.add_transaction_locations(locations.collect())?; + self.add_block_body(hash, block.body)?; + self.add_block_header(hash, header)?; + self.add_block_number(hash, number)?; + } + Ok(()) } diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index a4949c7965..787313eb2b 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -126,7 +126,47 @@ impl StoreEngine for Store { } fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { - Ok(()) + let tx = self + .db + .begin_readwrite() + .map_err(StoreError::LibmdbxError)?; + + for block in blocks { + let header = block.header; + let number = header.number; + let hash = header.compute_block_hash(); + + let mut cursor = tx + .cursor::() + .map_err(StoreError::LibmdbxError)?; + for (index, transaction) in block.body.transactions.iter().enumerate() { + cursor + .upsert( + transaction.compute_hash().into(), + (number, hash, index as u64).into(), + ) + .map_err(StoreError::LibmdbxError)?; + } + + let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; + cursor + .upsert(hash.into(), block.body.into()) + .map_err(StoreError::LibmdbxError)?; + + let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; + cursor + .upsert(hash.into(), header.into()) + .map_err(StoreError::LibmdbxError)?; + + let mut cursor = tx + .cursor::() + .map_err(StoreError::LibmdbxError)?; + cursor + .upsert(hash.into(), number.into()) + .map_err(StoreError::LibmdbxError)?; + } + + tx.commit().map_err(StoreError::LibmdbxError) } fn get_block_body(&self, block_number: BlockNumber) -> Result, StoreError> { From 0acc5e28b861f88c30cebb6cbfe0230970df25ed Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 7 Mar 2025 10:41:42 -0300 Subject: [PATCH 05/60] fix: merge with main --- crates/blockchain/blockchain.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index bfc5e6f822..a519d33dca 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -133,24 +133,27 @@ impl Blockchain { return Err(ChainError::ParentNotFound); }; - let mut state = evm_state(self.storage.clone(), block.header.parent_hash); - let chain_config: ChainConfig = state.chain_config().map_err(ChainError::from)?; + let chain_config: ChainConfig = self.storage.get_chain_config()?; // Validate the block pre-execution validate_block(block, &parent_header, &chain_config)?; + + let mut vm = Evm::new( + self.evm_engine, + self.storage.clone(), + block.header.parent_hash, + ); let BlockExecutionResult { receipts, requests, account_updates, - } = self.vm.execute_block(block, &mut state)?; + } = vm.execute_block(block)?; validate_gas_used(&receipts, &block.header)?; // todo only execute transactions // batch account updates to merge the repeated accounts - state - .database() - .ok_or(ChainError::StoreError(StoreError::MissingStore))? + self.storage .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; // Validate state trie on last block only From 80384ad12fe313badff619aaf478c48e23a3181f Mon Sep 17 00:00:00 2001 From: nicolau Date: Sat, 8 Mar 2025 18:24:48 -0300 Subject: [PATCH 06/60] fix: parent hash check and block execution state --- crates/blockchain/blockchain.rs | 26 ++++++++++++++------------ crates/storage/store.rs | 1 + crates/vm/backends/mod.rs | 12 ++++++++++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index a519d33dca..aae1a46041 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -123,31 +123,33 @@ impl Blockchain { }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; + let chain_config: ChainConfig = self.storage.get_chain_config()?; + let mut vm = Evm::new(self.evm_engine, self.storage.clone(), parent_hash); 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 { - // If the parent is not present, we store it as pending. - self.storage.add_pending_block(block.clone())?; - return Err(ChainError::ParentNotFound); + let parent_header = match i == 0 { + true => { + let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { + // If the parent is not present, we store it as pending. + self.storage.add_pending_block(block.clone())?; + return Err(ChainError::ParentNotFound); + }; + parent_header + } + false => blocks[i - 1].header.clone(), }; - let chain_config: ChainConfig = self.storage.get_chain_config()?; - // Validate the block pre-execution validate_block(block, &parent_header, &chain_config)?; - let mut vm = Evm::new( - self.evm_engine, - self.storage.clone(), - block.header.parent_hash, - ); let BlockExecutionResult { receipts, requests, account_updates, - } = vm.execute_block(block)?; + } = vm.execute_block_without_clearing_state(block)?; validate_gas_used(&receipts, &block.header)?; diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 4e3950ff6b..f4926d2750 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -400,6 +400,7 @@ impl Store { storage_trie.insert(hashed_key, storage_value.encode_to_vec())?; } } + // TODO: don't commit to db here to batch all the writes in a single tx account_state.storage_root = storage_trie.hash()?; } state_trie.insert(hashed_address, account_state.encode_to_vec())?; diff --git a/crates/vm/backends/mod.rs b/crates/vm/backends/mod.rs index 14ef72b25b..9a9202e174 100644 --- a/crates/vm/backends/mod.rs +++ b/crates/vm/backends/mod.rs @@ -89,6 +89,18 @@ impl Evm { } } + pub fn execute_block_without_clearing_state( + &mut self, + block: &Block, + ) -> Result { + match self { + Evm::REVM { state } => REVM::execute_block(block, state), + Evm::LEVM { store_wrapper, .. } => { + LEVM::execute_block(block, store_wrapper.store.clone()) + } + } + } + /// Wraps [REVM::execute_tx] and [LEVM::execute_tx]. /// The output is `(Receipt, u64)` == (transaction_receipt, gas_used). #[allow(clippy::too_many_arguments)] From 3746d40467fbb96606930642a2a0d9f67e5d39e4 Mon Sep 17 00:00:00 2001 From: nicolau Date: Sun, 9 Mar 2025 21:26:32 -0300 Subject: [PATCH 07/60] fix: withdrawal test regression we were missing to set the cannonical block hash for the number --- crates/blockchain/blockchain.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index aae1a46041..d5153cd8d3 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -165,8 +165,10 @@ impl Blockchain { validate_receipts_root(&block.header, &receipts)?; validate_requests_hash(&block.header, &chain_config, &requests)?; } - all_receipts.push((block_hash, receipts)); + + self.storage + .set_canonical_block(block.header.number, block_hash)?; } self.storage.add_batch_of_blocks(blocks)?; @@ -493,7 +495,10 @@ pub fn is_canonical( block_hash: BlockHash, ) -> Result { match store.get_canonical_block_hash(block_number)? { - Some(hash) if hash == block_hash => Ok(true), + Some(hash) => { + info!("CANONICAL HASH {} PROVIDED HASH {}", hash, block_hash); + Ok(hash == block_hash) + } _ => Ok(false), } } From 5d5a97c0a1311a24522c324ef2ea2ea216aa4f59 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 10 Mar 2025 08:49:09 -0300 Subject: [PATCH 08/60] feat: use blocks_in_batch in ethrex cmd import chain.rlp --- crates/blockchain/blockchain.rs | 36 ++++----------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index d5153cd8d3..dced7cc302 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -180,38 +180,10 @@ impl Blockchain { //TODO: Forkchoice Update shouldn't be part of this function pub fn import_blocks(&self, blocks: &Vec) { let size = blocks.len(); - for block in blocks { - let hash = block.hash(); - info!( - "Adding block {} with hash {:#x}.", - block.header.number, hash - ); - if let Err(error) = self.add_block(block) { - warn!( - "Failed to add block {} with hash {:#x}: {}.", - block.header.number, hash, error - ); - } - if self - .storage - .update_latest_block_number(block.header.number) - .is_err() - { - error!("Fatal: added block {} but could not update the block number -- aborting block import", block.header.number); - break; - }; - if self - .storage - .set_canonical_block(block.header.number, hash) - .is_err() - { - error!( - "Fatal: added block {} but could not set it as canonical -- aborting block import", - block.header.number - ); - break; - }; - } + let last_block = blocks.last().unwrap().clone(); + self.add_blocks_in_batch(blocks[0].header.parent_hash, blocks)?; + self.storage + .update_latest_block_number(last_block.header.number)?; if let Some(last_block) = blocks.last() { let hash = last_block.hash(); match self.evm_engine { From 56114b7e44a8b597d05565d01340092f39ac8e38 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 10 Mar 2025 09:06:18 -0300 Subject: [PATCH 09/60] fix: import blocks in batch --- cmd/ethrex/subcommands/import.rs | 2 +- crates/blockchain/blockchain.rs | 44 +++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index dcd879b24d..81cce00950 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -47,5 +47,5 @@ pub fn import_blocks_from_path( info!("Importing blocks from chain file: {}", path); utils::read_chain_file(path) }; - blockchain.import_blocks(&blocks); + blockchain.import_blocks(blocks); } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index dced7cc302..eeb561e9b0 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -178,22 +178,36 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks(&self, blocks: &Vec) { + pub fn import_blocks(&self, blocks: Vec) { let size = blocks.len(); - let last_block = blocks.last().unwrap().clone(); - self.add_blocks_in_batch(blocks[0].header.parent_hash, blocks)?; - self.storage - .update_latest_block_number(last_block.header.number)?; - if let Some(last_block) = blocks.last() { - let hash = last_block.hash(); - match self.evm_engine { - EvmEngine::LEVM => { - // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. - let _ = apply_fork_choice(&self.storage, hash, hash, hash); - } - EvmEngine::REVM => { - apply_fork_choice(&self.storage, hash, hash, hash).unwrap(); - } + let last_block = blocks.last().unwrap().header.clone(); + if let Err(err) = self.add_blocks_in_batch(blocks[0].header.parent_hash, blocks) { + warn!("Failed to add blocks: {:?}.", err); + }; + if let Err(err) = self.storage.update_latest_block_number(last_block.number) { + error!("Fatal: added block {} but could not update the block number, err {:?} -- aborting block import", last_block.number, err); + return; + }; + + let last_block_hash = last_block.compute_block_hash(); + match self.evm_engine { + EvmEngine::LEVM => { + // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. + let _ = apply_fork_choice( + &self.storage, + last_block_hash, + last_block_hash, + last_block_hash, + ); + } + EvmEngine::REVM => { + apply_fork_choice( + &self.storage, + last_block_hash, + last_block_hash, + last_block_hash, + ) + .unwrap(); } } info!("Added {size} blocks to blockchain"); From e1a86ed18ca31503ede647ca2c41e8556c6d35ef Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 10 Mar 2025 17:13:05 -0300 Subject: [PATCH 10/60] refactor: add_block call add_blocks_in_batch, take ownership to avoid cloning blocks --- cmd/ef_tests/blockchain/test_runner.rs | 2 +- crates/blockchain/blockchain.rs | 101 ++++++++---------------- crates/blockchain/smoke_test.rs | 50 ++++++------ crates/l2/prover/tests/perf_zkvm.rs | 2 +- crates/l2/utils/prover/save_state.rs | 2 +- crates/l2/utils/test_data_io.rs | 2 +- crates/networking/p2p/sync.rs | 12 ++- crates/networking/rpc/engine/payload.rs | 6 +- crates/storage/api.rs | 5 ++ crates/storage/store_db/in_memory.rs | 1 + crates/storage/store_db/libmdbx.rs | 7 ++ crates/vm/backends/mod.rs | 2 +- 12 files changed, 86 insertions(+), 106 deletions(-) diff --git a/cmd/ef_tests/blockchain/test_runner.rs b/cmd/ef_tests/blockchain/test_runner.rs index 63333b36ff..9e5d40f424 100644 --- a/cmd/ef_tests/blockchain/test_runner.rs +++ b/cmd/ef_tests/blockchain/test_runner.rs @@ -29,7 +29,7 @@ pub fn run_ef_test(test_key: &str, test: &TestUnit) { } // Won't panic because test has been validated - let block: &CoreBlock = &block_fixture.block().unwrap().clone().into(); + let block: CoreBlock = block_fixture.block().unwrap().clone().into(); let hash = block.hash(); // Attempt to add the block as the head of the chain diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index eeb561e9b0..e15f0fc1a3 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -55,82 +55,45 @@ impl Blockchain { } } - pub fn add_block(&self, block: &Block) -> Result<(), ChainError> { - let since = Instant::now(); - - let block_hash = block.header.compute_block_hash(); + pub fn add_block(&self, block: Block) -> Result<(), ChainError> { + self.add_blocks_in_batch(vec![block]) + } - // Validate if it can be the new head and find the parent - let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { - // If the parent is not present, we store it as pending. - self.storage.add_pending_block(block.clone())?; - return Err(ChainError::ParentNotFound); + pub fn add_blocks_in_batch(&self, blocks: Vec) -> Result<(), ChainError> { + let first_block_header = match blocks.first() { + Some(block) => block.header.clone(), + None => return Err(ChainError::Custom("First block not found".into())), + }; + let last_block_header = match blocks.first() { + Some(block) => block.header.clone(), + None => return Err(ChainError::Custom("Last block not found".into())), }; - let chain_config = self.storage.get_chain_config()?; - - // Validate the block pre-execution - validate_block(block, &parent_header, &chain_config)?; - - let mut vm = Evm::new( - self.evm_engine, - self.storage.clone(), - block.header.parent_hash, - ); - let BlockExecutionResult { - receipts, - requests, - account_updates, - } = vm.execute_block(block)?; - - validate_gas_used(&receipts, &block.header)?; - - // Apply the account updates over the last block's state and compute the new state root - let new_state_root = self - .storage - .apply_account_updates(block.header.parent_hash, &account_updates)? - .ok_or(ChainError::ParentStateNotFound)?; - - // Check state root matches the one in block header after execution - validate_state_root(&block.header, new_state_root)?; - - // Check receipts root matches the one in block header after execution - validate_receipts_root(&block.header, &receipts)?; - - // Processes requests from receipts, computes the requests_hash and compares it against the header - validate_requests_hash(&block.header, &chain_config, &requests)?; - - self.storage.add_block(block.clone())?; - self.storage.add_receipts(block_hash, receipts)?; - - let interval = Instant::now().duration_since(since).as_millis(); - if interval != 0 { - let as_gigas = (block.header.gas_used as f64).div(10_f64.powf(9_f64)); - let throughput = (as_gigas) / (interval as f64) * 1000_f64; - info!("[METRIC] BLOCK EXECUTION THROUGHPUT: {throughput} Gigagas/s TIME SPENT: {interval} msecs"); + if self.evm_engine == EvmEngine::LEVM { + panic!("LEVM engine is not supported for add blocks in batch"); } - Ok(()) - } - - pub fn add_blocks_in_batch( - &self, - parent_hash: BlockHash, - blocks: Vec, - ) -> Result<(), ChainError> { - let Some(mut state_trie) = self.storage.state_trie(parent_hash)? else { + let Some(mut state_trie) = self.storage.state_trie(first_block_header.parent_hash)? else { return Err(ChainError::ParentNotFound); }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; let chain_config: ChainConfig = self.storage.get_chain_config()?; - let mut vm = Evm::new(self.evm_engine, self.storage.clone(), parent_hash); + let mut vm = Evm::new( + self.evm_engine, + self.storage.clone(), + first_block_header.parent_hash, + ); for (i, block) in blocks.iter().enumerate() { + let is_first_block = i == 0; + let is_last_block = i == blocks.len() - 1; + let block_hash = block.header.compute_block_hash(); // Validate if it can be the new head and find the parent - let parent_header = match i == 0 { + let parent_header = match is_first_block { + // for the first block, we need to query the store true => { let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { // If the parent is not present, we store it as pending. @@ -139,6 +102,7 @@ impl Blockchain { }; parent_header } + // for the subsequent ones, the parent is the previous block false => blocks[i - 1].header.clone(), }; @@ -158,19 +122,18 @@ impl Blockchain { self.storage .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; - // Validate state trie on last block only - if i >= blocks.len() - 1 { - let root_hash = state_trie.hash().map_err(StoreError::Trie)?; - validate_state_root(&block.header, root_hash)?; + if is_last_block { validate_receipts_root(&block.header, &receipts)?; validate_requests_hash(&block.header, &chain_config, &requests)?; } - all_receipts.push((block_hash, receipts)); - self.storage - .set_canonical_block(block.header.number, block_hash)?; + all_receipts.push((block_hash, receipts)); } + // Compute state trie root hash, validate it and commit to db + let root_hash = state_trie.hash().map_err(StoreError::Trie)?; + validate_state_root(&last_block_header, root_hash)?; + self.storage.add_batch_of_blocks(blocks)?; self.storage.add_batch_of_receipts(all_receipts)?; @@ -181,7 +144,7 @@ impl Blockchain { pub fn import_blocks(&self, blocks: Vec) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); - if let Err(err) = self.add_blocks_in_batch(blocks[0].header.parent_hash, blocks) { + if let Err(err) = self.add_blocks_in_batch(blocks) { warn!("Failed to add blocks: {:?}.", err); }; if let Err(err) = self.storage.update_latest_block_number(last_block.number) { diff --git a/crates/blockchain/smoke_test.rs b/crates/blockchain/smoke_test.rs index 0d85ab7ea2..614589154c 100644 --- a/crates/blockchain/smoke_test.rs +++ b/crates/blockchain/smoke_test.rs @@ -28,19 +28,21 @@ mod blockchain_integration_test { // Add first block. We'll make it canonical. let block_1a = new_block(&store, &genesis_header); + let block_1a_header = block_1a.header.clone(); let hash_1a = block_1a.hash(); - blockchain.add_block(&block_1a).unwrap(); + blockchain.add_block(block_1a).unwrap(); store.set_canonical_block(1, hash_1a).unwrap(); let retrieved_1a = store.get_block_header(1).unwrap().unwrap(); - assert_eq!(retrieved_1a, block_1a.header); + assert_eq!(retrieved_1a, block_1a_header); assert!(is_canonical(&store, 1, hash_1a).unwrap()); // Add second block at height 1. Will not be canonical. let block_1b = new_block(&store, &genesis_header); + let block_1b_header = block_1b.header.clone(); let hash_1b = block_1b.hash(); blockchain - .add_block(&block_1b) + .add_block(block_1b) .expect("Could not add block 1b."); let retrieved_1b = store.get_block_header_by_hash(hash_1b).unwrap().unwrap(); @@ -48,10 +50,10 @@ mod blockchain_integration_test { assert!(!is_canonical(&store, 1, hash_1b).unwrap()); // Add a third block at height 2, child to the non canonical block. - let block_2 = new_block(&store, &block_1b.header); + let block_2 = new_block(&store, &block_1b_header); let hash_2 = block_2.hash(); blockchain - .add_block(&block_2) + .add_block(block_2) .expect("Could not add block 2."); let retrieved_2 = store.get_block_header_by_hash(hash_2).unwrap(); @@ -61,7 +63,7 @@ mod blockchain_integration_test { // Receive block 2 as new head. apply_fork_choice( &store, - block_2.hash(), + hash_2, genesis_header.compute_block_hash(), genesis_header.compute_block_hash(), ) @@ -84,15 +86,16 @@ mod blockchain_integration_test { // Build a single valid block. let block_1 = new_block(&store, &genesis_header); + let block_1_header = block_1.header.clone(); let hash_1 = block_1.header.compute_block_hash(); - blockchain.add_block(&block_1).unwrap(); + blockchain.add_block(block_1).unwrap(); apply_fork_choice(&store, hash_1, H256::zero(), H256::zero()).unwrap(); // Build a child, then change its parent, making it effectively a pending block. - let mut block_2 = new_block(&store, &block_1.header); + let mut block_2 = new_block(&store, &block_1_header); block_2.header.parent_hash = H256::random(); let hash_2 = block_2.header.compute_block_hash(); - let result = blockchain.add_block(&block_2); + let result = blockchain.add_block(block_2); assert!(matches!(result, Err(ChainError::ParentNotFound))); // block 2 should now be pending. @@ -118,30 +121,31 @@ mod blockchain_integration_test { // Add first block. Not canonical. let block_1a = new_block(&store, &genesis_header); let hash_1a = block_1a.hash(); - blockchain.add_block(&block_1a).unwrap(); + blockchain.add_block(block_1a).unwrap(); let retrieved_1a = store.get_block_header_by_hash(hash_1a).unwrap().unwrap(); assert!(!is_canonical(&store, 1, hash_1a).unwrap()); // Add second block at height 1. Canonical. let block_1b = new_block(&store, &genesis_header); + let block_1b_header = block_1b.header.clone(); let hash_1b = block_1b.hash(); blockchain - .add_block(&block_1b) + .add_block(block_1b) .expect("Could not add block 1b."); apply_fork_choice(&store, hash_1b, genesis_hash, genesis_hash).unwrap(); let retrieved_1b = store.get_block_header(1).unwrap().unwrap(); assert_ne!(retrieved_1a, retrieved_1b); - assert_eq!(retrieved_1b, block_1b.header); + assert_eq!(retrieved_1b, block_1b_header); assert!(is_canonical(&store, 1, hash_1b).unwrap()); assert_eq!(latest_canonical_block_hash(&store).unwrap(), hash_1b); // Add a third block at height 2, child to the canonical one. - let block_2 = new_block(&store, &block_1b.header); + let block_2 = new_block(&store, &block_1b_header); let hash_2 = block_2.hash(); blockchain - .add_block(&block_2) + .add_block(block_2) .expect("Could not add block 2."); apply_fork_choice(&store, hash_2, genesis_hash, genesis_hash).unwrap(); let retrieved_2 = store.get_block_header_by_hash(hash_2).unwrap(); @@ -154,7 +158,7 @@ mod blockchain_integration_test { // Receive block 1a as new head. apply_fork_choice( &store, - block_1a.hash(), + hash_1a, genesis_header.compute_block_hash(), genesis_header.compute_block_hash(), ) @@ -179,16 +183,17 @@ mod blockchain_integration_test { // Add block at height 1. let block_1 = new_block(&store, &genesis_header); + let block_1_header = block_1.header.clone(); let hash_1 = block_1.hash(); blockchain - .add_block(&block_1) + .add_block(block_1) .expect("Could not add block 1b."); // Add child at height 2. - let block_2 = new_block(&store, &block_1.header); + let block_2 = new_block(&store, &block_1_header); let hash_2 = block_2.hash(); blockchain - .add_block(&block_2) + .add_block(block_2) .expect("Could not add block 2."); assert!(!is_canonical(&store, 1, hash_1).unwrap()); @@ -228,15 +233,16 @@ mod blockchain_integration_test { // Add block at height 1. let block_1 = new_block(&store, &genesis_header); + let block_1_header = block_1.header.clone(); blockchain - .add_block(&block_1) + .add_block(block_1) .expect("Could not add block 1b."); // Add child at height 2. - let block_2 = new_block(&store, &block_1.header); + let block_2 = new_block(&store, &block_1_header); let hash_2 = block_2.hash(); blockchain - .add_block(&block_2) + .add_block(block_2) .expect("Could not add block 2."); assert_eq!(latest_canonical_block_hash(&store).unwrap(), genesis_hash); @@ -250,7 +256,7 @@ mod blockchain_integration_test { let block_1b = new_block(&store, &genesis_header); let hash_b = block_1b.hash(); blockchain - .add_block(&block_1b) + .add_block(block_1b) .expect("Could not add block b."); // The latest block should be the same. diff --git a/crates/l2/prover/tests/perf_zkvm.rs b/crates/l2/prover/tests/perf_zkvm.rs index f7a1c83064..56d4a6e063 100644 --- a/crates/l2/prover/tests/perf_zkvm.rs +++ b/crates/l2/prover/tests/perf_zkvm.rs @@ -81,7 +81,7 @@ async fn setup() -> (ProgramInput, Block) { block.body.transactions.len(), block.header.number ); - blockchain.add_block(block).unwrap(); + blockchain.add_block(block.clone()).unwrap(); } let block_to_prove = blocks.get(3).unwrap(); diff --git a/crates/l2/utils/prover/save_state.rs b/crates/l2/utils/prover/save_state.rs index 276206fdd9..a9adfad01c 100644 --- a/crates/l2/utils/prover/save_state.rs +++ b/crates/l2/utils/prover/save_state.rs @@ -422,7 +422,7 @@ mod tests { // create blockchain let blockchain = Blockchain::default_with_store(store.clone()); for block in &blocks { - blockchain.add_block(block).unwrap(); + blockchain.add_block(block.clone()).unwrap(); } let mut account_updates_vec: Vec> = Vec::new(); diff --git a/crates/l2/utils/test_data_io.rs b/crates/l2/utils/test_data_io.rs index 89a08e9611..8c8ca39808 100644 --- a/crates/l2/utils/test_data_io.rs +++ b/crates/l2/utils/test_data_io.rs @@ -74,7 +74,7 @@ pub fn generate_program_input( // create blockchain let blockchain = Blockchain::default_with_store(store.clone()); for block in chain { - blockchain.add_block(&block)?; + blockchain.add_block(block)?; } let parent_hash = block.header.parent_hash; diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 4999078d37..9e65cba72e 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -320,9 +320,10 @@ impl SyncManager { let block = store .get_block_by_hash(*hash)? .ok_or(SyncError::CorruptDB)?; - self.blockchain.add_block(&block)?; - store.set_canonical_block(block.header.number, *hash)?; - store.update_latest_block_number(block.header.number)?; + let block_number = block.header.number; + self.blockchain.add_block(block)?; + store.set_canonical_block(block_number, *hash)?; + store.update_latest_block_number(block_number)?; } self.last_snap_pivot = pivot_header.number; // Finished a sync cycle without aborting halfway, clear current checkpoint @@ -395,14 +396,11 @@ impl SyncManager { } debug!("Starting to execute and validate blocks in batch"); - let first_block = blocks.first().unwrap().clone(); let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); - self.blockchain - .add_blocks_in_batch(first_block.header.parent_hash, blocks)?; + self.blockchain.add_blocks_in_batch(blocks)?; - store.set_canonical_block(last_block.header.number, last_block.hash())?; store.update_latest_block_number(last_block.header.number)?; debug!("Executed & stored {} blocks", blocks_len); diff --git a/crates/networking/rpc/engine/payload.rs b/crates/networking/rpc/engine/payload.rs index 128cc387ed..289a03625d 100644 --- a/crates/networking/rpc/engine/payload.rs +++ b/crates/networking/rpc/engine/payload.rs @@ -544,7 +544,7 @@ fn handle_new_payload_v1_v2( } else if let Err(RpcErr::Internal(error_msg)) = validate_block_hash(payload, &block) { PayloadStatus::invalid_with_err(&error_msg) } else { - execute_payload(&block, &context)? + execute_payload(block, &context)? } } }; @@ -576,7 +576,7 @@ fn handle_new_payload_v3( "Invalid blob_versioned_hashes", )); } - execute_payload(&block, &context) + execute_payload(block, &context) } } } @@ -624,7 +624,7 @@ fn validate_block_hash(payload: &ExecutionPayload, block: &Block) -> Result<(), Ok(()) } -fn execute_payload(block: &Block, context: &RpcApiContext) -> Result { +fn execute_payload(block: Block, context: &RpcApiContext) -> Result { let block_hash = block.hash(); let storage = &context.storage; // Return the valid message directly if we have it. diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 4c2ac5b390..d06c7338b5 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -12,6 +12,11 @@ use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Add a batch of blocks in a single tx /// This will store -> BlockHeader, BlockBody, BlockTransaction, BlockNumber + /// + /// TODO: Currently, when adding each block, we assume it's part of the canonical chain + /// and set the canonical hash to the block number. This approach is used to optimize + /// for writing all blocks in a single transaction. However, this is a temporary measure, + /// and we should revisit this assumption when implementing sidechains. fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError>; /// Add block header diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index cc9fa2feef..4cde42215a 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -159,6 +159,7 @@ impl StoreEngine for Store { self.add_block_body(hash, block.body)?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; + self.set_canonical_block(number, hash)?; } Ok(()) diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index f16f8aa821..06ee6a9df1 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -164,6 +164,13 @@ impl StoreEngine for Store { cursor .upsert(hash.into(), number.into()) .map_err(StoreError::LibmdbxError)?; + + let mut cursor = tx + .cursor::() + .map_err(StoreError::LibmdbxError)?; + cursor + .upsert(number, hash.into()) + .map_err(StoreError::LibmdbxError)?; } tx.commit().map_err(StoreError::LibmdbxError) diff --git a/crates/vm/backends/mod.rs b/crates/vm/backends/mod.rs index 9a9202e174..be6ff999fd 100644 --- a/crates/vm/backends/mod.rs +++ b/crates/vm/backends/mod.rs @@ -14,7 +14,7 @@ use levm::LEVM; use revm::REVM; use std::sync::Arc; -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, PartialEq, Clone, Copy, Default)] pub enum EvmEngine { #[default] REVM, From 44c820a2f8511cd1b950b6d96da9975a084fe1e4 Mon Sep 17 00:00:00 2001 From: nicolau Date: Mon, 10 Mar 2025 22:04:15 -0300 Subject: [PATCH 11/60] fix: rpc-compat hive tests --- cmd/ethrex/subcommands/import.rs | 2 +- crates/blockchain/blockchain.rs | 36 +++++++++++++++++++------------- crates/networking/p2p/sync.rs | 2 +- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 81cce00950..10509a7889 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -47,5 +47,5 @@ pub fn import_blocks_from_path( info!("Importing blocks from chain file: {}", path); utils::read_chain_file(path) }; - blockchain.import_blocks(blocks); + blockchain.import_blocks(blocks, true); } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index e15f0fc1a3..c1e2f3e168 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -56,15 +56,19 @@ impl Blockchain { } pub fn add_block(&self, block: Block) -> Result<(), ChainError> { - self.add_blocks_in_batch(vec![block]) + self.add_blocks_in_batch(vec![block], false) } - pub fn add_blocks_in_batch(&self, blocks: Vec) -> Result<(), ChainError> { + pub fn add_blocks_in_batch( + &self, + blocks: Vec, + should_commit_intermediate_tries: bool, + ) -> Result<(), ChainError> { let first_block_header = match blocks.first() { Some(block) => block.header.clone(), None => return Err(ChainError::Custom("First block not found".into())), }; - let last_block_header = match blocks.first() { + let last_block_header = match blocks.last() { Some(block) => block.header.clone(), None => return Err(ChainError::Custom("Last block not found".into())), }; @@ -122,6 +126,11 @@ impl Blockchain { self.storage .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; + if should_commit_intermediate_tries { + let root_hash = state_trie.hash().map_err(StoreError::Trie)?; + validate_state_root(&block.header, root_hash)?; + } + if is_last_block { validate_receipts_root(&block.header, &receipts)?; validate_requests_hash(&block.header, &chain_config, &requests)?; @@ -130,9 +139,10 @@ impl Blockchain { all_receipts.push((block_hash, receipts)); } - // Compute state trie root hash, validate it and commit to db - let root_hash = state_trie.hash().map_err(StoreError::Trie)?; - validate_state_root(&last_block_header, root_hash)?; + if !should_commit_intermediate_tries { + let root_hash = state_trie.hash().map_err(StoreError::Trie)?; + validate_state_root(&last_block_header, root_hash)?; + } self.storage.add_batch_of_blocks(blocks)?; self.storage.add_batch_of_receipts(all_receipts)?; @@ -141,10 +151,10 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks(&self, blocks: Vec) { + pub fn import_blocks(&self, blocks: Vec, should_commit_intermediate_tries: bool) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); - if let Err(err) = self.add_blocks_in_batch(blocks) { + if let Err(err) = self.add_blocks_in_batch(blocks, should_commit_intermediate_tries) { warn!("Failed to add blocks: {:?}.", err); }; if let Err(err) = self.storage.update_latest_block_number(last_block.number) { @@ -164,13 +174,12 @@ impl Blockchain { ); } EvmEngine::REVM => { - apply_fork_choice( + let _ = apply_fork_choice( &self.storage, last_block_hash, last_block_hash, last_block_hash, - ) - .unwrap(); + ); } } info!("Added {size} blocks to blockchain"); @@ -444,10 +453,7 @@ pub fn is_canonical( block_hash: BlockHash, ) -> Result { match store.get_canonical_block_hash(block_number)? { - Some(hash) => { - info!("CANONICAL HASH {} PROVIDED HASH {}", hash, block_hash); - Ok(hash == block_hash) - } + Some(hash) => Ok(hash == block_hash), _ => Ok(false), } } diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 9e65cba72e..5eaf2a8d7d 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -399,7 +399,7 @@ impl SyncManager { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); - self.blockchain.add_blocks_in_batch(blocks)?; + self.blockchain.add_blocks_in_batch(blocks, false)?; store.update_latest_block_number(last_block.header.number)?; debug!("Executed & stored {} blocks", blocks_len); From 23bdd74313f187dad44b7d5fb3af37f1a913a09d Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 10:21:05 -0300 Subject: [PATCH 12/60] fix: add max tries to store in db --- cmd/ethrex/subcommands/import.rs | 13 ++++++++++++- crates/blockchain/blockchain.rs | 3 ++- crates/networking/p2p/sync.rs | 21 +++++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 10509a7889..a0ba66f9c4 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -2,6 +2,7 @@ use std::fs::{self, metadata}; use clap::ArgMatches; +use ethrex_blockchain::MAX_TRIES_IN_STORE; use ethrex_vm::backends::EvmEngine; use tracing::info; @@ -47,5 +48,15 @@ pub fn import_blocks_from_path( info!("Importing blocks from chain file: {}", path); utils::read_chain_file(path) }; - blockchain.import_blocks(blocks, true); + + if blocks.len() <= MAX_TRIES_IN_STORE { + blockchain.import_blocks(blocks, true); + } else { + let idx = blocks.len() - MAX_TRIES_IN_STORE; + let head = blocks[..idx].to_vec(); + let tail = blocks[idx..].to_vec(); + + blockchain.import_blocks(head, false); + blockchain.import_blocks(tail, true); + } } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index c1e2f3e168..6037dba885 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -20,7 +20,6 @@ use ethrex_common::types::{ use ethrex_common::{Address, H256}; use mempool::Mempool; -use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; use ethrex_storage::Store; @@ -28,6 +27,8 @@ use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; +pub const MAX_TRIES_IN_STORE: usize = 128; + //TODO: Implement a struct Chain or BlockChain to encapsulate //functionality and canonical chain state and config diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 5eaf2a8d7d..16679cfdbf 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -6,7 +6,7 @@ mod storage_healing; mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; -use ethrex_blockchain::{error::ChainError, Blockchain}; +use ethrex_blockchain::{error::ChainError, Blockchain, MAX_TRIES_IN_STORE}; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, @@ -399,7 +399,24 @@ impl SyncManager { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); - self.blockchain.add_blocks_in_batch(blocks, false)?; + let latest_block_number = store.get_latest_block_number()?; + if last_block.header.number.saturating_sub(latest_block_number) <= MAX_TRIES_IN_STORE as u64 + { + if blocks_len <= MAX_TRIES_IN_STORE { + self.blockchain.add_blocks_in_batch(blocks, true)?; + } else { + let idx = blocks_len - MAX_TRIES_IN_STORE; + // Using `split_off` avoids cloning the slice, which would be necessary if we used `[..idx]` and `[idx..]` directly. + // This is avoids cloning all blocks which might get expensive if they are large. + let tail = blocks.split_off(idx); + let head = std::mem::take(&mut blocks); + + self.blockchain.add_blocks_in_batch(head, false)?; + self.blockchain.add_blocks_in_batch(tail, true)?; + } + } else { + self.blockchain.add_blocks_in_batch(blocks, false)?; + } store.update_latest_block_number(last_block.header.number)?; debug!("Executed & stored {} blocks", blocks_len); From bc9ab4b867ff3d27eb5b588d383f0443105840fb Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 10:40:04 -0300 Subject: [PATCH 13/60] fix: merge --- crates/networking/rpc/engine/payload.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/networking/rpc/engine/payload.rs b/crates/networking/rpc/engine/payload.rs index 54b61a32d4..7b700cb83c 100644 --- a/crates/networking/rpc/engine/payload.rs +++ b/crates/networking/rpc/engine/payload.rs @@ -581,7 +581,7 @@ fn handle_new_payload_v1_v2( } // All checks passed, execute payload - let payload_status = execute_payload(&block, &context)?; + let payload_status = execute_payload(block, &context)?; serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) } @@ -623,7 +623,7 @@ fn handle_new_payload_v3( } // All checks passed, execute payload - execute_payload(&block, &context) + execute_payload(block, &context) } // Elements of the list MUST be ordered by request_type in ascending order. From 6e4dc5e4d89e7fa99fa91dfc41b35a30c9601bfd Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 13:42:59 -0300 Subject: [PATCH 14/60] feat: re-introduce ancestors --- crates/blockchain/blockchain.rs | 50 +++++++++++++++++++++++++-------- crates/networking/p2p/sync.rs | 37 +++++++++++++++++++++--- 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 51fcb9754d..97dbfb7f48 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -57,40 +57,52 @@ impl Blockchain { } pub fn add_block(&self, block: Block) -> Result<(), ChainError> { - self.add_blocks_in_batch(vec![block], false) + match self.add_blocks_in_batch(vec![block], false) { + Ok(_) => Ok(()), + Err((err, _)) => Err(err), + } } + /// Adds multiple blocks in a batch. + /// If an error occurs, returns a tuple containing the error type and the failing block (if the error was caused by block processing). pub fn add_blocks_in_batch( &self, blocks: Vec, should_commit_intermediate_tries: bool, - ) -> Result<(), ChainError> { + ) -> Result<(), (ChainError, Option)> { let first_block_header = match blocks.first() { Some(block) => block.header.clone(), - None => return Err(ChainError::Custom("First block not found".into())), + None => return Err((ChainError::Custom("First block not found".into()), None)), }; let last_block_header = match blocks.last() { Some(block) => block.header.clone(), - None => return Err(ChainError::Custom("Last block not found".into())), + None => return Err((ChainError::Custom("Last block not found".into()), None)), }; if self.evm_engine == EvmEngine::LEVM { panic!("LEVM engine is not supported for add blocks in batch"); } - let Some(mut state_trie) = self.storage.state_trie(first_block_header.parent_hash)? else { - return Err(ChainError::ParentNotFound); + let Some(mut state_trie) = self + .storage + .state_trie(first_block_header.parent_hash) + .map_err(|e| (e.into(), None))? + else { + return Err((ChainError::ParentNotFound, None)); }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; - let chain_config: ChainConfig = self.storage.get_chain_config()?; + let chain_config: ChainConfig = self + .storage + .get_chain_config() + .map_err(|e| (e.into(), None))?; let mut vm = Evm::new( self.evm_engine, self.storage.clone(), first_block_header.parent_hash, ); - for (i, block) in blocks.iter().enumerate() { + let mut add_block = |block: &Block, i: usize| -> Result<(), ChainError> { let is_first_block = i == 0; let is_last_block = i == blocks.len() - 1; @@ -138,15 +150,29 @@ impl Blockchain { } all_receipts.push((block_hash, receipts)); + + Ok(()) + }; + + for (i, block) in blocks.iter().enumerate() { + if let Err(err) = add_block(block, i) { + return Err((err, Some(block.clone()))); + }; } if !should_commit_intermediate_tries { - let root_hash = state_trie.hash().map_err(StoreError::Trie)?; - validate_state_root(&last_block_header, root_hash)?; + let root_hash = state_trie + .hash() + .map_err(|e| (ChainError::StoreError(e.into()), None))?; + validate_state_root(&last_block_header, root_hash).map_err(|e| (e.into(), None))?; } - self.storage.add_batch_of_blocks(blocks)?; - self.storage.add_batch_of_receipts(all_receipts)?; + self.storage + .add_batch_of_blocks(blocks) + .map_err(|e| (e.into(), None))?; + self.storage + .add_batch_of_receipts(all_receipts) + .map_err(|e| (e.into(), None))?; Ok(()) } diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 2b00e01fcc..b4f93fb826 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -417,7 +417,39 @@ impl SyncManager { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); - let latest_block_number = store.get_latest_block_number()?; + if let Err((error, block)) = self.add_blocks(blocks, store.clone()) { + warn!("Failed to add block during FullSync: {error}"); + if let Some(block) = block { + self.invalid_ancestors + .insert(block.hash(), block.header.parent_hash); + + // TODO(#2127): Just marking the failing ancestor and the sync head is enough + // to fix the Missing Ancestors hive test, we want to look at a more robust + // solution in the future if needed. + self.invalid_ancestors + .insert(sync_head, block.header.parent_hash); + } + + return Err(error.into()); + } + + store.update_latest_block_number(last_block.header.number)?; + debug!("Executed & stored {} blocks", blocks_len); + + Ok(()) + } + + fn add_blocks( + &self, + mut blocks: Vec, + store: Store, + ) -> Result<(), (ChainError, Option)> { + let last_block = blocks.last().unwrap().clone(); + let blocks_len = blocks.len(); + let latest_block_number = store + .get_latest_block_number() + .map_err(|e| (e.into(), None))?; + if last_block.header.number.saturating_sub(latest_block_number) <= MAX_TRIES_IN_STORE as u64 { if blocks_len <= MAX_TRIES_IN_STORE { @@ -436,9 +468,6 @@ impl SyncManager { self.blockchain.add_blocks_in_batch(blocks, false)?; } - store.update_latest_block_number(last_block.header.number)?; - debug!("Executed & stored {} blocks", blocks_len); - Ok(()) } } From 42a6af8ec251979ad5801da5b0934f4a242dc245 Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 14:11:58 -0300 Subject: [PATCH 15/60] refactor: don't panic on levm, instead store every state trie --- crates/blockchain/blockchain.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 97dbfb7f48..c965342038 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -68,7 +68,7 @@ impl Blockchain { pub fn add_blocks_in_batch( &self, blocks: Vec, - should_commit_intermediate_tries: bool, + mut should_commit_intermediate_tries: bool, ) -> Result<(), (ChainError, Option)> { let first_block_header = match blocks.first() { Some(block) => block.header.clone(), @@ -80,7 +80,8 @@ impl Blockchain { }; if self.evm_engine == EvmEngine::LEVM { - panic!("LEVM engine is not supported for add blocks in batch"); + // levm does not support batch block processing because it does not persist the state between block executions + should_commit_intermediate_tries = true; } let Some(mut state_trie) = self From d4097b6484297a274fce995d5140b2d5b2be67eb Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 17:07:46 -0300 Subject: [PATCH 16/60] fix: remaining hive tests --- crates/blockchain/blockchain.rs | 63 +++++++++++++++------------- crates/networking/p2p/sync.rs | 20 ++++----- crates/storage/api.rs | 14 +++---- crates/storage/store.rs | 8 +++- crates/storage/store_db/in_memory.rs | 10 ++++- crates/storage/store_db/libmdbx.rs | 20 +++++---- 6 files changed, 78 insertions(+), 57 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index c965342038..a87a75a54b 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -57,26 +57,40 @@ impl Blockchain { } pub fn add_block(&self, block: Block) -> Result<(), ChainError> { - match self.add_blocks_in_batch(vec![block], false) { + match self.add_blocks_in_batch(vec![block], false, false) { Ok(_) => Ok(()), - Err((err, _)) => Err(err), + Err((err, _, _)) => Err(err), } } /// Adds multiple blocks in a batch. - /// If an error occurs, returns a tuple containing the error type and the failing block (if the error was caused by block processing). + /// + /// If an error occurs, returns a tuple containing: + /// - The error type (`ChainError`). + /// - The failing block (if the error was caused by block processing). + /// - The last valid block hash (`H256`). + /// + /// `should_commit_intermediate_tries` determines whether the state tries for each block + /// should be stored in the database (the last one is always stored). + /// + /// `as_canonical` determines whether the block number should be set as canonical for the block hash when committing to the db. pub fn add_blocks_in_batch( &self, blocks: Vec, mut should_commit_intermediate_tries: bool, - ) -> Result<(), (ChainError, Option)> { + as_canonical: bool, + ) -> Result<(), (ChainError, Option, H256)> { + let mut last_valid_hash = H256::default(); + let first_block_header = match blocks.first() { Some(block) => block.header.clone(), - None => return Err((ChainError::Custom("First block not found".into()), None)), - }; - let last_block_header = match blocks.last() { - Some(block) => block.header.clone(), - None => return Err((ChainError::Custom("Last block not found".into()), None)), + None => { + return Err(( + ChainError::Custom("First block not found".into()), + None, + last_valid_hash, + )) + } }; if self.evm_engine == EvmEngine::LEVM { @@ -87,16 +101,17 @@ impl Blockchain { let Some(mut state_trie) = self .storage .state_trie(first_block_header.parent_hash) - .map_err(|e| (e.into(), None))? + .map_err(|e| (e.into(), None, last_valid_hash))? else { - return Err((ChainError::ParentNotFound, None)); + return Err((ChainError::ParentNotFound, None, last_valid_hash)); }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; let chain_config: ChainConfig = self .storage .get_chain_config() - .map_err(|e| (e.into(), None))?; + .map_err(|e| (e.into(), None, last_valid_hash))?; + let mut vm = Evm::new( self.evm_engine, self.storage.clone(), @@ -140,40 +155,32 @@ impl Blockchain { self.storage .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; - if should_commit_intermediate_tries { + if should_commit_intermediate_tries || is_last_block { let root_hash = state_trie.hash().map_err(StoreError::Trie)?; validate_state_root(&block.header, root_hash)?; - } - - if is_last_block { validate_receipts_root(&block.header, &receipts)?; validate_requests_hash(&block.header, &chain_config, &requests)?; } all_receipts.push((block_hash, receipts)); + last_valid_hash = block_hash; + Ok(()) }; for (i, block) in blocks.iter().enumerate() { if let Err(err) = add_block(block, i) { - return Err((err, Some(block.clone()))); + return Err((err, Some(block.clone()), last_valid_hash)); }; } - if !should_commit_intermediate_tries { - let root_hash = state_trie - .hash() - .map_err(|e| (ChainError::StoreError(e.into()), None))?; - validate_state_root(&last_block_header, root_hash).map_err(|e| (e.into(), None))?; - } - self.storage - .add_batch_of_blocks(blocks) - .map_err(|e| (e.into(), None))?; + .add_batch_of_blocks(blocks, as_canonical) + .map_err(|e| (e.into(), None, last_valid_hash))?; self.storage .add_batch_of_receipts(all_receipts) - .map_err(|e| (e.into(), None))?; + .map_err(|e| (e.into(), None, last_valid_hash))?; Ok(()) } @@ -182,7 +189,7 @@ impl Blockchain { pub fn import_blocks(&self, blocks: Vec, should_commit_intermediate_tries: bool) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); - if let Err(err) = self.add_blocks_in_batch(blocks, should_commit_intermediate_tries) { + if let Err(err) = self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) { warn!("Failed to add blocks: {:?}.", err); }; if let Err(err) = self.storage.update_latest_block_number(last_block.number) { diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index b4f93fb826..30e39735ce 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -417,17 +417,15 @@ impl SyncManager { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); - if let Err((error, block)) = self.add_blocks(blocks, store.clone()) { + if let Err((error, block, last_valid_hash)) = self.add_blocks(blocks, store.clone()) { warn!("Failed to add block during FullSync: {error}"); if let Some(block) = block { - self.invalid_ancestors - .insert(block.hash(), block.header.parent_hash); + self.invalid_ancestors.insert(block.hash(), last_valid_hash); // TODO(#2127): Just marking the failing ancestor and the sync head is enough // to fix the Missing Ancestors hive test, we want to look at a more robust // solution in the future if needed. - self.invalid_ancestors - .insert(sync_head, block.header.parent_hash); + self.invalid_ancestors.insert(sync_head, last_valid_hash); } return Err(error.into()); @@ -443,17 +441,17 @@ impl SyncManager { &self, mut blocks: Vec, store: Store, - ) -> Result<(), (ChainError, Option)> { + ) -> Result<(), (ChainError, Option, H256)> { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); let latest_block_number = store .get_latest_block_number() - .map_err(|e| (e.into(), None))?; + .map_err(|e| (e.into(), None, H256::default()))?; if last_block.header.number.saturating_sub(latest_block_number) <= MAX_TRIES_IN_STORE as u64 { if blocks_len <= MAX_TRIES_IN_STORE { - self.blockchain.add_blocks_in_batch(blocks, true)?; + self.blockchain.add_blocks_in_batch(blocks, true, true)?; } else { let idx = blocks_len - MAX_TRIES_IN_STORE; // Using `split_off` avoids cloning the slice, which would be necessary if we used `[..idx]` and `[idx..]` directly. @@ -461,11 +459,11 @@ impl SyncManager { let tail = blocks.split_off(idx); let head = std::mem::take(&mut blocks); - self.blockchain.add_blocks_in_batch(head, false)?; - self.blockchain.add_blocks_in_batch(tail, true)?; + self.blockchain.add_blocks_in_batch(head, false, true)?; + self.blockchain.add_blocks_in_batch(tail, true, true)?; } } else { - self.blockchain.add_blocks_in_batch(blocks, false)?; + self.blockchain.add_blocks_in_batch(blocks, false, true)?; } Ok(()) diff --git a/crates/storage/api.rs b/crates/storage/api.rs index d06c7338b5..3427a00df6 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -10,14 +10,14 @@ use crate::{error::StoreError, store::STATE_TRIE_SEGMENTS}; use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { - /// Add a batch of blocks in a single tx - /// This will store -> BlockHeader, BlockBody, BlockTransaction, BlockNumber + /// Add a batch of blocks in a single transaction. + /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. /// - /// TODO: Currently, when adding each block, we assume it's part of the canonical chain - /// and set the canonical hash to the block number. This approach is used to optimize - /// for writing all blocks in a single transaction. However, this is a temporary measure, - /// and we should revisit this assumption when implementing sidechains. - fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError>; + /// If `as_canonical` is true, each block is assumed to be part of the canonical chain, + /// and the canonical hash is set to the block number. This optimizes writes when + /// processing blocks in bulk. + fn add_batch_of_blocks(&self, blocks: Vec, as_canonical: bool) + -> Result<(), StoreError>; /// Add block header fn add_block_header( diff --git a/crates/storage/store.rs b/crates/storage/store.rs index f4926d2750..4ad1bcbc74 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -488,8 +488,12 @@ impl Store { self.add_block_number(hash, number) } - pub fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { - self.engine.add_batch_of_blocks(blocks) + pub fn add_batch_of_blocks( + &self, + blocks: Vec, + as_canonical: bool, + ) -> Result<(), StoreError> { + self.engine.add_batch_of_blocks(blocks, as_canonical) } pub fn add_initial_state(&self, genesis: Genesis) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 4cde42215a..2169195420 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,7 +143,11 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + fn add_batch_of_blocks( + &self, + blocks: Vec, + as_canonical: bool, + ) -> Result<(), StoreError> { for block in blocks { let header = block.header; let number = header.number; @@ -159,7 +163,9 @@ impl StoreEngine for Store { self.add_block_body(hash, block.body)?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; - self.set_canonical_block(number, hash)?; + if as_canonical { + self.set_canonical_block(number, hash)?; + } } Ok(()) diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 06ee6a9df1..41890f8cba 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -125,7 +125,11 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_batch_of_blocks(&self, blocks: Vec) -> Result<(), StoreError> { + fn add_batch_of_blocks( + &self, + blocks: Vec, + as_canonical: bool, + ) -> Result<(), StoreError> { let tx = self .db .begin_readwrite() @@ -165,12 +169,14 @@ impl StoreEngine for Store { .upsert(hash.into(), number.into()) .map_err(StoreError::LibmdbxError)?; - let mut cursor = tx - .cursor::() - .map_err(StoreError::LibmdbxError)?; - cursor - .upsert(number, hash.into()) - .map_err(StoreError::LibmdbxError)?; + if as_canonical { + let mut cursor = tx + .cursor::() + .map_err(StoreError::LibmdbxError)?; + cursor + .upsert(number, hash.into()) + .map_err(StoreError::LibmdbxError)?; + } } tx.commit().map_err(StoreError::LibmdbxError) From bf76b775e475aa8c086ac17f72458761419fa449 Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 17:33:46 -0300 Subject: [PATCH 17/60] fix: merge with main execute block + store block for the l2 --- crates/blockchain/blockchain.rs | 75 +++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index a87a75a54b..231e22be3e 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -56,11 +56,78 @@ impl Blockchain { } } - pub fn add_block(&self, block: Block) -> Result<(), ChainError> { - match self.add_blocks_in_batch(vec![block], false, false) { - Ok(_) => Ok(()), - Err((err, _, _)) => Err(err), + pub fn execute_block(&self, block: &Block) -> Result { + let since = Instant::now(); + + // Validate if it can be the new head and find the parent + let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { + // If the parent is not present, we store it as pending. + self.storage.add_pending_block(block.clone())?; + return Err(ChainError::ParentNotFound); + }; + let chain_config = self.storage.get_chain_config()?; + + // Validate the block pre-execution + validate_block(block, &parent_header, &chain_config)?; + + let mut vm = Evm::new( + self.evm_engine, + self.storage.clone(), + block.header.parent_hash, + ); + let execution_result = vm.execute_block(block)?; + + let interval = Instant::now().duration_since(since).as_millis(); + if interval != 0 { + let as_gigas = (block.header.gas_used as f64).div(10_f64.powf(9_f64)); + let throughput = (as_gigas) / (interval as f64) * 1000_f64; + info!("[METRIC] BLOCK EXECUTION THROUGHPUT: {throughput} Gigagas/s TIME SPENT: {interval} msecs"); } + + Ok(execution_result) + } + + pub fn store_block( + &self, + block: &Block, + execution_result: BlockExecutionResult, + ) -> Result<(), ChainError> { + // Assumes block is valid + let BlockExecutionResult { + receipts, + requests, + account_updates, + } = execution_result; + let chain_config = self.storage.get_chain_config()?; + + let block_hash = block.header.compute_block_hash(); + + validate_gas_used(&receipts, &block.header)?; + + // Apply the account updates over the last block's state and compute the new state root + let new_state_root = self + .storage + .apply_account_updates(block.header.parent_hash, &account_updates)? + .ok_or(ChainError::ParentStateNotFound)?; + + // Check state root matches the one in block header + validate_state_root(&block.header, new_state_root)?; + + // Check receipts root matches the one in block header + validate_receipts_root(&block.header, &receipts)?; + + // Processes requests from receipts, computes the requests_hash and compares it against the header + validate_requests_hash(&block.header, &chain_config, &requests)?; + + store_block(&self.storage, block.clone())?; + store_receipts(&self.storage, receipts, block_hash)?; + + Ok(()) + } + + pub fn add_block(&self, block: Block) -> Result<(), ChainError> { + self.execute_block(&block) + .and_then(|res| self.store_block(&block, res)) } /// Adds multiple blocks in a batch. From 88614dd883e14ef59c2790d4fbf2c52a89660be3 Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 17:38:38 -0300 Subject: [PATCH 18/60] fix: ethrex build --- crates/blockchain/blockchain.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 231e22be3e..ca7a275ead 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -5,6 +5,9 @@ pub mod mempool; pub mod payload; mod smoke_test; +use std::ops::Div; +use std::time::Instant; + use constants::MAX_INITCODE_SIZE; use error::MempoolError; use error::{ChainError, InvalidBlockError}; @@ -119,8 +122,8 @@ impl Blockchain { // Processes requests from receipts, computes the requests_hash and compares it against the header validate_requests_hash(&block.header, &chain_config, &requests)?; - store_block(&self.storage, block.clone())?; - store_receipts(&self.storage, receipts, block_hash)?; + self.storage.add_block(block.clone())?; + self.storage.add_receipts(block_hash, receipts)?; Ok(()) } From aa73736bf2847e45029b88299392f35173517adf Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 17:39:01 -0300 Subject: [PATCH 19/60] chore: add entry to perf changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 246a4321a9..84d5a95bea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,3 +5,7 @@ #### 2025-02-28 * Don't recompute transaction senders when building blocks [#2097](https://github.com/lambdaclass/ethrex/pull/2097) + +#### 2025-03-11 + +* Process blocks in batches when syncing and importing [#2174](https://github.com/lambdaclass/ethrex/pull/2174) \ No newline at end of file From 673d2b3d3ebbcbcd12e6bf151e03c0548daa39e1 Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 18:57:18 -0300 Subject: [PATCH 20/60] refactor: general --- crates/blockchain/blockchain.rs | 58 +++++++++++++++--------------- crates/storage/store.rs | 1 - crates/storage/store_db/libmdbx.rs | 34 +++++------------- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index ca7a275ead..5229e45a1f 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -5,9 +5,6 @@ pub mod mempool; pub mod payload; mod smoke_test; -use std::ops::Div; -use std::time::Instant; - use constants::MAX_INITCODE_SIZE; use error::MempoolError; use error::{ChainError, InvalidBlockError}; @@ -23,6 +20,7 @@ use ethrex_common::types::{ use ethrex_common::{Address, H256}; use mempool::Mempool; +use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; use ethrex_storage::Store; @@ -147,7 +145,7 @@ impl Blockchain { pub fn add_blocks_in_batch( &self, blocks: Vec, - mut should_commit_intermediate_tries: bool, + should_commit_intermediate_tries: bool, as_canonical: bool, ) -> Result<(), (ChainError, Option, H256)> { let mut last_valid_hash = H256::default(); @@ -163,10 +161,9 @@ impl Blockchain { } }; - if self.evm_engine == EvmEngine::LEVM { - // levm does not support batch block processing because it does not persist the state between block executions - should_commit_intermediate_tries = true; - } + // levm does not support batch block processing because it does not persist the state between block executions + // so we need to commit to the db after every block + let is_levm = self.evm_engine == EvmEngine::LEVM; let Some(mut state_trie) = self .storage @@ -212,27 +209,28 @@ impl Blockchain { // Validate the block pre-execution validate_block(block, &parent_header, &chain_config)?; - let BlockExecutionResult { - receipts, - requests, - account_updates, - } = vm.execute_block_without_clearing_state(block)?; + let execution_result = vm.execute_block_without_clearing_state(block)?; - validate_gas_used(&receipts, &block.header)?; + validate_gas_used(&execution_result.receipts, &block.header)?; - // todo only execute transactions - // batch account updates to merge the repeated accounts - self.storage - .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; + self.storage.apply_account_updates_to_trie( + &execution_result.account_updates, + &mut state_trie, + )?; - if should_commit_intermediate_tries || is_last_block { + if should_commit_intermediate_tries || is_last_block || is_levm { + //TODO hash_no_commit, validate and then commit let root_hash = state_trie.hash().map_err(StoreError::Trie)?; validate_state_root(&block.header, root_hash)?; - validate_receipts_root(&block.header, &receipts)?; - validate_requests_hash(&block.header, &chain_config, &requests)?; + validate_receipts_root(&block.header, &execution_result.receipts)?; + validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; } - all_receipts.push((block_hash, receipts)); + if is_levm { + self.store_block(block, execution_result)?; + } else { + all_receipts.push((block_hash, execution_result.receipts)); + } last_valid_hash = block_hash; @@ -245,12 +243,14 @@ impl Blockchain { }; } - self.storage - .add_batch_of_blocks(blocks, as_canonical) - .map_err(|e| (e.into(), None, last_valid_hash))?; - self.storage - .add_batch_of_receipts(all_receipts) - .map_err(|e| (e.into(), None, last_valid_hash))?; + if !is_levm { + self.storage + .add_batch_of_blocks(blocks, as_canonical) + .map_err(|e| (e.into(), None, last_valid_hash))?; + self.storage + .add_batch_of_receipts(all_receipts) + .map_err(|e| (e.into(), None, last_valid_hash))?; + } Ok(()) } @@ -558,7 +558,7 @@ pub fn is_canonical( block_hash: BlockHash, ) -> Result { match store.get_canonical_block_hash(block_number)? { - Some(hash) => Ok(hash == block_hash), + Some(hash) if hash == block_hash => Ok(true), _ => Ok(false), } } diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 4ad1bcbc74..038c57d848 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -444,7 +444,6 @@ impl Store { Ok(genesis_state_trie.hash()?) } - /// Adds all genesis accounts and returns the genesis block's state_root pub fn add_receipt( &self, block_hash: BlockHash, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 41890f8cba..a7d3406c0a 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -140,41 +140,25 @@ impl StoreEngine for Store { let number = header.number; let hash = header.compute_block_hash(); - let mut cursor = tx - .cursor::() - .map_err(StoreError::LibmdbxError)?; for (index, transaction) in block.body.transactions.iter().enumerate() { - cursor - .upsert( - transaction.compute_hash().into(), - (number, hash, index as u64).into(), - ) - .map_err(StoreError::LibmdbxError)?; + tx.upsert::( + transaction.compute_hash().into(), + (number, hash, index as u64).into(), + ) + .map_err(StoreError::LibmdbxError)?; } - let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; - cursor - .upsert(hash.into(), block.body.into()) + tx.upsert::(hash.into(), block.body.into()) .map_err(StoreError::LibmdbxError)?; - let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; - cursor - .upsert(hash.into(), header.into()) + tx.upsert::(hash.into(), header.into()) .map_err(StoreError::LibmdbxError)?; - let mut cursor = tx - .cursor::() - .map_err(StoreError::LibmdbxError)?; - cursor - .upsert(hash.into(), number.into()) + tx.upsert::(hash.into(), number.into()) .map_err(StoreError::LibmdbxError)?; if as_canonical { - let mut cursor = tx - .cursor::() - .map_err(StoreError::LibmdbxError)?; - cursor - .upsert(number, hash.into()) + tx.upsert::(number, hash.into()) .map_err(StoreError::LibmdbxError)?; } } From 1948a381d2a5d3f55b868e413f3c4a3ba281802b Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 23:03:49 -0300 Subject: [PATCH 21/60] fix: hive regression on cancun suite --- crates/blockchain/blockchain.rs | 25 ++++++++++++------------- crates/networking/p2p/sync.rs | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 5229e45a1f..0997c3e39c 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -191,19 +191,17 @@ impl Blockchain { let block_hash = block.header.compute_block_hash(); - // Validate if it can be the new head and find the parent - let parent_header = match is_first_block { + let parent_header = if is_first_block { // for the first block, we need to query the store - true => { - let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { - // If the parent is not present, we store it as pending. - self.storage.add_pending_block(block.clone())?; - return Err(ChainError::ParentNotFound); - }; - parent_header - } + let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { + // If the parent is not present, we store it as pending. + self.storage.add_pending_block(block.clone())?; + return Err(ChainError::ParentNotFound); + }; + parent_header + } else { // for the subsequent ones, the parent is the previous block - false => blocks[i - 1].header.clone(), + blocks[i - 1].header.clone() }; // Validate the block pre-execution @@ -219,9 +217,10 @@ impl Blockchain { )?; if should_commit_intermediate_tries || is_last_block || is_levm { - //TODO hash_no_commit, validate and then commit - let root_hash = state_trie.hash().map_err(StoreError::Trie)?; + let root_hash = state_trie.hash_no_commit(); validate_state_root(&block.header, root_hash)?; + // commit to db after validating the root + state_trie.hash().map_err(StoreError::Trie)?; validate_receipts_root(&block.header, &execution_result.receipts)?; validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; } diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 30e39735ce..1c2ac394f2 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -288,6 +288,8 @@ impl SyncManager { &block_hashes, &block_headers, sync_head, + &mut current_head, + &mut search_head, store.clone(), ) .await?; @@ -361,6 +363,8 @@ impl SyncManager { block_hashes: &[BlockHash], block_headers: &[BlockHeader], sync_head: BlockHash, + current_head: &mut H256, + search_head: &mut H256, store: Store, ) -> Result<(), SyncError> { let mut current_chunk_idx = 0; @@ -376,6 +380,9 @@ impl SyncManager { let mut headers_idx = 0; let mut blocks: Vec = vec![]; + let max_tries = 10; + let mut tries = 0; + loop { debug!("Requesting Block Bodies"); if let Some(block_bodies) = self.peers.request_block_bodies(chunk.clone()).await { @@ -410,6 +417,22 @@ impl SyncManager { None => break, }; }; + } else { + tries += 1; + // If we fail to retrieve block bodies, increment the retry counter. + // This failure could be due to missing bodies for some headers, possibly because: + // - Some headers belong to a sidechain, and not all peers have the corresponding bodies. + // - We are not verifying headers before requesting the bodies so they might be invalid + // + // To mitigate this, we update `current_head` and `search_head` to the last successfully processed block. + // This makes sure that the next header request starts from the last confirmed block. + // + // TODO: validate headers before downloading the bodies + if tries >= max_tries { + *current_head = blocks.last().unwrap().hash(); + *search_head = blocks.last().unwrap().hash(); + break; + } } } From 61ff4918292f6291416f4e69795eb097806b7b4b Mon Sep 17 00:00:00 2001 From: nicolau Date: Tue, 11 Mar 2025 23:15:18 -0300 Subject: [PATCH 22/60] refactor: remove unwraps --- crates/networking/p2p/sync.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 1c2ac394f2..d3743868c7 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -425,19 +425,22 @@ impl SyncManager { // - We are not verifying headers before requesting the bodies so they might be invalid // // To mitigate this, we update `current_head` and `search_head` to the last successfully processed block. - // This makes sure that the next header request starts from the last confirmed block. + // This makes sure that the next header request starts from the last confirmed block (see below). // // TODO: validate headers before downloading the bodies if tries >= max_tries { - *current_head = blocks.last().unwrap().hash(); - *search_head = blocks.last().unwrap().hash(); break; } } } debug!("Starting to execute and validate blocks in batch"); - let last_block = blocks.last().unwrap().clone(); + let Some(last_block) = blocks.last().cloned() else { + return Err(SyncError::BodiesNotFound); + }; + *current_head = last_block.hash(); + *search_head = last_block.hash(); + let blocks_len = blocks.len(); if let Err((error, block, last_valid_hash)) = self.add_blocks(blocks, store.clone()) { @@ -688,4 +691,6 @@ enum SyncError { JoinHandle(#[from] tokio::task::JoinError), #[error("Missing data from DB")] CorruptDB, + #[error("No bodies were found for the given headers")] + BodiesNotFound, } From d8cab934aa5e46bb56619bd4194cd21c7711e305 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 09:41:57 -0300 Subject: [PATCH 23/60] reafactor: add blocks in batch match engine this removes the ifs for levm inside the add blocks in batch --- crates/blockchain/blockchain.rs | 77 ++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 0997c3e39c..d48f4cc90d 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -147,6 +147,39 @@ impl Blockchain { blocks: Vec, should_commit_intermediate_tries: bool, as_canonical: bool, + ) -> Result<(), (ChainError, Option, H256)> { + match self.evm_engine { + // LEVM does not support batch block processing as it does not persist the state between block executions + // Therefore, we must commit to the db after each block + EvmEngine::LEVM => { + let mut last_valid_hash = H256::default(); + for block in blocks { + let block_hash = block.hash(); + let block_number = block.header.number; + if let Err(e) = self.add_block(block) { + return Err((e, Some(block), last_valid_hash)); + }; + last_valid_hash = block_hash; + if as_canonical { + self.storage.set_canonical_block(block_number, block_hash); + } + } + + Ok(()) + } + EvmEngine::REVM => self.add_blocks_in_batch_inner( + blocks, + should_commit_intermediate_tries, + as_canonical, + ), + } + } + + fn add_blocks_in_batch_inner( + &self, + blocks: Vec, + should_commit_intermediate_tries: bool, + as_canonical: bool, ) -> Result<(), (ChainError, Option, H256)> { let mut last_valid_hash = H256::default(); @@ -161,10 +194,6 @@ impl Blockchain { } }; - // levm does not support batch block processing because it does not persist the state between block executions - // so we need to commit to the db after every block - let is_levm = self.evm_engine == EvmEngine::LEVM; - let Some(mut state_trie) = self .storage .state_trie(first_block_header.parent_hash) @@ -207,29 +236,27 @@ impl Blockchain { // Validate the block pre-execution validate_block(block, &parent_header, &chain_config)?; - let execution_result = vm.execute_block_without_clearing_state(block)?; + let BlockExecutionResult { + account_updates, + receipts, + requests, + } = vm.execute_block_without_clearing_state(block)?; - validate_gas_used(&execution_result.receipts, &block.header)?; + validate_gas_used(&receipts, &block.header)?; - self.storage.apply_account_updates_to_trie( - &execution_result.account_updates, - &mut state_trie, - )?; + self.storage + .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; - if should_commit_intermediate_tries || is_last_block || is_levm { + if should_commit_intermediate_tries || is_last_block { let root_hash = state_trie.hash_no_commit(); validate_state_root(&block.header, root_hash)?; // commit to db after validating the root state_trie.hash().map_err(StoreError::Trie)?; - validate_receipts_root(&block.header, &execution_result.receipts)?; - validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; + validate_receipts_root(&block.header, &receipts)?; + validate_requests_hash(&block.header, &chain_config, &requests)?; } - if is_levm { - self.store_block(block, execution_result)?; - } else { - all_receipts.push((block_hash, execution_result.receipts)); - } + all_receipts.push((block_hash, receipts)); last_valid_hash = block_hash; @@ -242,14 +269,12 @@ impl Blockchain { }; } - if !is_levm { - self.storage - .add_batch_of_blocks(blocks, as_canonical) - .map_err(|e| (e.into(), None, last_valid_hash))?; - self.storage - .add_batch_of_receipts(all_receipts) - .map_err(|e| (e.into(), None, last_valid_hash))?; - } + self.storage + .add_batch_of_blocks(blocks, as_canonical) + .map_err(|e| (e.into(), None, last_valid_hash))?; + self.storage + .add_batch_of_receipts(all_receipts) + .map_err(|e| (e.into(), None, last_valid_hash))?; Ok(()) } From 0d1aed80e0686285bfd8dc235bde1f4faea24007 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 10:03:10 -0300 Subject: [PATCH 24/60] refactor: error return type in block in batch --- crates/blockchain/blockchain.rs | 61 +++++++++++++++++++++------------ crates/networking/p2p/sync.rs | 19 ++++++---- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index d48f4cc90d..ddd50b437d 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -40,6 +40,11 @@ pub struct Blockchain { pub mempool: Mempool, } +pub struct BatchBlockProcessingFailure { + pub last_valid_hash: H256, + pub failed_block_hash: H256, +} + impl Blockchain { pub fn new(evm_engine: EvmEngine, store: Store) -> Self { Self { @@ -134,9 +139,8 @@ impl Blockchain { /// Adds multiple blocks in a batch. /// /// If an error occurs, returns a tuple containing: - /// - The error type (`ChainError`). - /// - The failing block (if the error was caused by block processing). - /// - The last valid block hash (`H256`). + /// - The error type ([`ChainError`]). + /// - [`BatchProcessingFailure`] (if the error was caused by block processing). /// /// `should_commit_intermediate_tries` determines whether the state tries for each block /// should be stored in the database (the last one is always stored). @@ -147,7 +151,7 @@ impl Blockchain { blocks: Vec, should_commit_intermediate_tries: bool, as_canonical: bool, - ) -> Result<(), (ChainError, Option, H256)> { + ) -> Result<(), (ChainError, Option)> { match self.evm_engine { // LEVM does not support batch block processing as it does not persist the state between block executions // Therefore, we must commit to the db after each block @@ -156,13 +160,24 @@ impl Blockchain { for block in blocks { let block_hash = block.hash(); let block_number = block.header.number; + if let Err(e) = self.add_block(block) { - return Err((e, Some(block), last_valid_hash)); + return Err(( + e, + Some(BatchBlockProcessingFailure { + last_valid_hash, + failed_block_hash: block_hash, + }), + )); }; - last_valid_hash = block_hash; + if as_canonical { - self.storage.set_canonical_block(block_number, block_hash); + self.storage + .set_canonical_block(block_number, block_hash) + .map_err(|e| (e.into(), None))?; } + + last_valid_hash = block_hash; } Ok(()) @@ -180,33 +195,27 @@ impl Blockchain { blocks: Vec, should_commit_intermediate_tries: bool, as_canonical: bool, - ) -> Result<(), (ChainError, Option, H256)> { + ) -> Result<(), (ChainError, Option)> { let mut last_valid_hash = H256::default(); let first_block_header = match blocks.first() { Some(block) => block.header.clone(), - None => { - return Err(( - ChainError::Custom("First block not found".into()), - None, - last_valid_hash, - )) - } + None => return Err((ChainError::Custom("First block not found".into()), None)), }; let Some(mut state_trie) = self .storage .state_trie(first_block_header.parent_hash) - .map_err(|e| (e.into(), None, last_valid_hash))? + .map_err(|e| (e.into(), None))? else { - return Err((ChainError::ParentNotFound, None, last_valid_hash)); + return Err((ChainError::ParentNotFound, None)); }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; let chain_config: ChainConfig = self .storage .get_chain_config() - .map_err(|e| (e.into(), None, last_valid_hash))?; + .map_err(|e| (e.into(), None))?; let mut vm = Evm::new( self.evm_engine, @@ -265,16 +274,22 @@ impl Blockchain { for (i, block) in blocks.iter().enumerate() { if let Err(err) = add_block(block, i) { - return Err((err, Some(block.clone()), last_valid_hash)); + return Err(( + err, + Some(BatchBlockProcessingFailure { + failed_block_hash: block.hash(), + last_valid_hash, + }), + )); }; } self.storage .add_batch_of_blocks(blocks, as_canonical) - .map_err(|e| (e.into(), None, last_valid_hash))?; + .map_err(|e| (e.into(), None))?; self.storage .add_batch_of_receipts(all_receipts) - .map_err(|e| (e.into(), None, last_valid_hash))?; + .map_err(|e| (e.into(), None))?; Ok(()) } @@ -283,7 +298,9 @@ impl Blockchain { pub fn import_blocks(&self, blocks: Vec, should_commit_intermediate_tries: bool) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); - if let Err(err) = self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) { + if let Err((err, _)) = + self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) + { warn!("Failed to add blocks: {:?}.", err); }; if let Err(err) = self.storage.update_latest_block_number(last_block.number) { diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index d3743868c7..5965f20da7 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -6,7 +6,9 @@ mod storage_healing; mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; -use ethrex_blockchain::{error::ChainError, Blockchain, MAX_TRIES_IN_STORE}; +use ethrex_blockchain::{ + error::ChainError, BatchBlockProcessingFailure, Blockchain, MAX_TRIES_IN_STORE, +}; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, @@ -443,10 +445,15 @@ impl SyncManager { let blocks_len = blocks.len(); - if let Err((error, block, last_valid_hash)) = self.add_blocks(blocks, store.clone()) { + if let Err((error, failure)) = self.add_blocks(blocks, store.clone()) { warn!("Failed to add block during FullSync: {error}"); - if let Some(block) = block { - self.invalid_ancestors.insert(block.hash(), last_valid_hash); + if let Some(BatchBlockProcessingFailure { + failed_block_hash, + last_valid_hash, + }) = failure + { + self.invalid_ancestors + .insert(failed_block_hash, last_valid_hash); // TODO(#2127): Just marking the failing ancestor and the sync head is enough // to fix the Missing Ancestors hive test, we want to look at a more robust @@ -467,12 +474,12 @@ impl SyncManager { &self, mut blocks: Vec, store: Store, - ) -> Result<(), (ChainError, Option, H256)> { + ) -> Result<(), (ChainError, Option)> { let last_block = blocks.last().unwrap().clone(); let blocks_len = blocks.len(); let latest_block_number = store .get_latest_block_number() - .map_err(|e| (e.into(), None, H256::default()))?; + .map_err(|e| (e.into(), None))?; if last_block.header.number.saturating_sub(latest_block_number) <= MAX_TRIES_IN_STORE as u64 { From 9d328242f6438f1de124cf15eba594dcdaf57b31 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 15:59:14 -0300 Subject: [PATCH 25/60] refactor: take slice of blocks instead of vec --- cmd/ef_tests/blockchain/test_runner.rs | 2 +- cmd/ethrex/subcommands/import.rs | 15 +++----- crates/blockchain/blockchain.rs | 15 ++++---- crates/blockchain/smoke_test.rs | 50 +++++++++++-------------- crates/l2/prover/tests/perf_zkvm.rs | 2 +- crates/l2/utils/prover/save_state.rs | 2 +- crates/l2/utils/test_data_io.rs | 2 +- crates/networking/p2p/sync.rs | 30 +++++++-------- crates/networking/rpc/engine/payload.rs | 6 +-- crates/storage/api.rs | 5 +-- crates/storage/store.rs | 2 +- crates/storage/store_db/in_memory.rs | 16 +++----- crates/storage/store_db/libmdbx.rs | 27 ++++++------- 13 files changed, 80 insertions(+), 94 deletions(-) diff --git a/cmd/ef_tests/blockchain/test_runner.rs b/cmd/ef_tests/blockchain/test_runner.rs index 9e5d40f424..63333b36ff 100644 --- a/cmd/ef_tests/blockchain/test_runner.rs +++ b/cmd/ef_tests/blockchain/test_runner.rs @@ -29,7 +29,7 @@ pub fn run_ef_test(test_key: &str, test: &TestUnit) { } // Won't panic because test has been validated - let block: CoreBlock = block_fixture.block().unwrap().clone().into(); + let block: &CoreBlock = &block_fixture.block().unwrap().clone().into(); let hash = block.hash(); // Attempt to add the block as the head of the chain diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index a0ba66f9c4..f394e51d7e 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -2,7 +2,7 @@ use std::fs::{self, metadata}; use clap::ArgMatches; -use ethrex_blockchain::MAX_TRIES_IN_STORE; +use ethrex_blockchain::STATE_TRIES_TO_KEEP; use ethrex_vm::backends::EvmEngine; use tracing::info; @@ -49,14 +49,11 @@ pub fn import_blocks_from_path( utils::read_chain_file(path) }; - if blocks.len() <= MAX_TRIES_IN_STORE { - blockchain.import_blocks(blocks, true); + if blocks.len() <= STATE_TRIES_TO_KEEP { + blockchain.import_blocks(&blocks, true); } else { - let idx = blocks.len() - MAX_TRIES_IN_STORE; - let head = blocks[..idx].to_vec(); - let tail = blocks[idx..].to_vec(); - - blockchain.import_blocks(head, false); - blockchain.import_blocks(tail, true); + let idx = blocks.len() - STATE_TRIES_TO_KEEP; + blockchain.import_blocks(&blocks[..idx], false); + blockchain.import_blocks(&blocks[idx..], true); } } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index ddd50b437d..87810c9e38 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -28,7 +28,8 @@ use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; -pub const MAX_TRIES_IN_STORE: usize = 128; +/// The number of latest tries to store in the database (meaning that we would have the state for the last 128 blocks) +pub const STATE_TRIES_TO_KEEP: usize = 128; //TODO: Implement a struct Chain or BlockChain to encapsulate //functionality and canonical chain state and config @@ -131,9 +132,9 @@ impl Blockchain { Ok(()) } - pub fn add_block(&self, block: Block) -> Result<(), ChainError> { - self.execute_block(&block) - .and_then(|res| self.store_block(&block, res)) + pub fn add_block(&self, block: &Block) -> Result<(), ChainError> { + self.execute_block(block) + .and_then(|res| self.store_block(block, res)) } /// Adds multiple blocks in a batch. @@ -148,7 +149,7 @@ impl Blockchain { /// `as_canonical` determines whether the block number should be set as canonical for the block hash when committing to the db. pub fn add_blocks_in_batch( &self, - blocks: Vec, + blocks: &[Block], should_commit_intermediate_tries: bool, as_canonical: bool, ) -> Result<(), (ChainError, Option)> { @@ -192,7 +193,7 @@ impl Blockchain { fn add_blocks_in_batch_inner( &self, - blocks: Vec, + blocks: &[Block], should_commit_intermediate_tries: bool, as_canonical: bool, ) -> Result<(), (ChainError, Option)> { @@ -295,7 +296,7 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks(&self, blocks: Vec, should_commit_intermediate_tries: bool) { + pub fn import_blocks(&self, blocks: &[Block], should_commit_intermediate_tries: bool) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); if let Err((err, _)) = diff --git a/crates/blockchain/smoke_test.rs b/crates/blockchain/smoke_test.rs index 614589154c..0d85ab7ea2 100644 --- a/crates/blockchain/smoke_test.rs +++ b/crates/blockchain/smoke_test.rs @@ -28,21 +28,19 @@ mod blockchain_integration_test { // Add first block. We'll make it canonical. let block_1a = new_block(&store, &genesis_header); - let block_1a_header = block_1a.header.clone(); let hash_1a = block_1a.hash(); - blockchain.add_block(block_1a).unwrap(); + blockchain.add_block(&block_1a).unwrap(); store.set_canonical_block(1, hash_1a).unwrap(); let retrieved_1a = store.get_block_header(1).unwrap().unwrap(); - assert_eq!(retrieved_1a, block_1a_header); + assert_eq!(retrieved_1a, block_1a.header); assert!(is_canonical(&store, 1, hash_1a).unwrap()); // Add second block at height 1. Will not be canonical. let block_1b = new_block(&store, &genesis_header); - let block_1b_header = block_1b.header.clone(); let hash_1b = block_1b.hash(); blockchain - .add_block(block_1b) + .add_block(&block_1b) .expect("Could not add block 1b."); let retrieved_1b = store.get_block_header_by_hash(hash_1b).unwrap().unwrap(); @@ -50,10 +48,10 @@ mod blockchain_integration_test { assert!(!is_canonical(&store, 1, hash_1b).unwrap()); // Add a third block at height 2, child to the non canonical block. - let block_2 = new_block(&store, &block_1b_header); + let block_2 = new_block(&store, &block_1b.header); let hash_2 = block_2.hash(); blockchain - .add_block(block_2) + .add_block(&block_2) .expect("Could not add block 2."); let retrieved_2 = store.get_block_header_by_hash(hash_2).unwrap(); @@ -63,7 +61,7 @@ mod blockchain_integration_test { // Receive block 2 as new head. apply_fork_choice( &store, - hash_2, + block_2.hash(), genesis_header.compute_block_hash(), genesis_header.compute_block_hash(), ) @@ -86,16 +84,15 @@ mod blockchain_integration_test { // Build a single valid block. let block_1 = new_block(&store, &genesis_header); - let block_1_header = block_1.header.clone(); let hash_1 = block_1.header.compute_block_hash(); - blockchain.add_block(block_1).unwrap(); + blockchain.add_block(&block_1).unwrap(); apply_fork_choice(&store, hash_1, H256::zero(), H256::zero()).unwrap(); // Build a child, then change its parent, making it effectively a pending block. - let mut block_2 = new_block(&store, &block_1_header); + let mut block_2 = new_block(&store, &block_1.header); block_2.header.parent_hash = H256::random(); let hash_2 = block_2.header.compute_block_hash(); - let result = blockchain.add_block(block_2); + let result = blockchain.add_block(&block_2); assert!(matches!(result, Err(ChainError::ParentNotFound))); // block 2 should now be pending. @@ -121,31 +118,30 @@ mod blockchain_integration_test { // Add first block. Not canonical. let block_1a = new_block(&store, &genesis_header); let hash_1a = block_1a.hash(); - blockchain.add_block(block_1a).unwrap(); + blockchain.add_block(&block_1a).unwrap(); let retrieved_1a = store.get_block_header_by_hash(hash_1a).unwrap().unwrap(); assert!(!is_canonical(&store, 1, hash_1a).unwrap()); // Add second block at height 1. Canonical. let block_1b = new_block(&store, &genesis_header); - let block_1b_header = block_1b.header.clone(); let hash_1b = block_1b.hash(); blockchain - .add_block(block_1b) + .add_block(&block_1b) .expect("Could not add block 1b."); apply_fork_choice(&store, hash_1b, genesis_hash, genesis_hash).unwrap(); let retrieved_1b = store.get_block_header(1).unwrap().unwrap(); assert_ne!(retrieved_1a, retrieved_1b); - assert_eq!(retrieved_1b, block_1b_header); + assert_eq!(retrieved_1b, block_1b.header); assert!(is_canonical(&store, 1, hash_1b).unwrap()); assert_eq!(latest_canonical_block_hash(&store).unwrap(), hash_1b); // Add a third block at height 2, child to the canonical one. - let block_2 = new_block(&store, &block_1b_header); + let block_2 = new_block(&store, &block_1b.header); let hash_2 = block_2.hash(); blockchain - .add_block(block_2) + .add_block(&block_2) .expect("Could not add block 2."); apply_fork_choice(&store, hash_2, genesis_hash, genesis_hash).unwrap(); let retrieved_2 = store.get_block_header_by_hash(hash_2).unwrap(); @@ -158,7 +154,7 @@ mod blockchain_integration_test { // Receive block 1a as new head. apply_fork_choice( &store, - hash_1a, + block_1a.hash(), genesis_header.compute_block_hash(), genesis_header.compute_block_hash(), ) @@ -183,17 +179,16 @@ mod blockchain_integration_test { // Add block at height 1. let block_1 = new_block(&store, &genesis_header); - let block_1_header = block_1.header.clone(); let hash_1 = block_1.hash(); blockchain - .add_block(block_1) + .add_block(&block_1) .expect("Could not add block 1b."); // Add child at height 2. - let block_2 = new_block(&store, &block_1_header); + let block_2 = new_block(&store, &block_1.header); let hash_2 = block_2.hash(); blockchain - .add_block(block_2) + .add_block(&block_2) .expect("Could not add block 2."); assert!(!is_canonical(&store, 1, hash_1).unwrap()); @@ -233,16 +228,15 @@ mod blockchain_integration_test { // Add block at height 1. let block_1 = new_block(&store, &genesis_header); - let block_1_header = block_1.header.clone(); blockchain - .add_block(block_1) + .add_block(&block_1) .expect("Could not add block 1b."); // Add child at height 2. - let block_2 = new_block(&store, &block_1_header); + let block_2 = new_block(&store, &block_1.header); let hash_2 = block_2.hash(); blockchain - .add_block(block_2) + .add_block(&block_2) .expect("Could not add block 2."); assert_eq!(latest_canonical_block_hash(&store).unwrap(), genesis_hash); @@ -256,7 +250,7 @@ mod blockchain_integration_test { let block_1b = new_block(&store, &genesis_header); let hash_b = block_1b.hash(); blockchain - .add_block(block_1b) + .add_block(&block_1b) .expect("Could not add block b."); // The latest block should be the same. diff --git a/crates/l2/prover/tests/perf_zkvm.rs b/crates/l2/prover/tests/perf_zkvm.rs index 45d0189124..d8c5060ee1 100644 --- a/crates/l2/prover/tests/perf_zkvm.rs +++ b/crates/l2/prover/tests/perf_zkvm.rs @@ -81,7 +81,7 @@ async fn setup() -> (ProgramInput, Block) { block.body.transactions.len(), block.header.number ); - blockchain.add_block(block.clone()).unwrap(); + blockchain.add_block(block).unwrap(); } let block_to_prove = blocks.get(3).unwrap(); diff --git a/crates/l2/utils/prover/save_state.rs b/crates/l2/utils/prover/save_state.rs index 214b7f8bd9..8c1f97b68d 100644 --- a/crates/l2/utils/prover/save_state.rs +++ b/crates/l2/utils/prover/save_state.rs @@ -422,7 +422,7 @@ mod tests { // create blockchain let blockchain = Blockchain::default_with_store(store.clone()); for block in &blocks { - blockchain.add_block(block.clone()).unwrap(); + blockchain.add_block(block).unwrap(); } let mut account_updates_vec: Vec> = Vec::new(); diff --git a/crates/l2/utils/test_data_io.rs b/crates/l2/utils/test_data_io.rs index 27d2e344df..d4b5627288 100644 --- a/crates/l2/utils/test_data_io.rs +++ b/crates/l2/utils/test_data_io.rs @@ -74,7 +74,7 @@ pub fn generate_program_input( // create blockchain let blockchain = Blockchain::default_with_store(store.clone()); for block in chain { - blockchain.add_block(block)?; + blockchain.add_block(&block)?; } let parent_hash = block.header.parent_hash; diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 5965f20da7..edbd9c48c2 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -7,7 +7,7 @@ mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; use ethrex_blockchain::{ - error::ChainError, BatchBlockProcessingFailure, Blockchain, MAX_TRIES_IN_STORE, + error::ChainError, BatchBlockProcessingFailure, Blockchain, STATE_TRIES_TO_KEEP, }; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, @@ -297,7 +297,7 @@ impl SyncManager { .await?; } - // in full sync mode, headers are added if after added after the execution and validation + // in full sync mode, headers are added after the execution and validation if self.sync_mode == SyncMode::Snap { store.add_block_headers(block_hashes.clone(), block_headers)?; } @@ -342,7 +342,7 @@ impl SyncManager { .get_block_by_hash(*hash)? .ok_or(SyncError::CorruptDB)?; let block_number = block.header.number; - self.blockchain.add_block(block)?; + self.blockchain.add_block(&block)?; store.set_canonical_block(block_number, *hash)?; store.update_latest_block_number(block_number)?; } @@ -472,7 +472,7 @@ impl SyncManager { fn add_blocks( &self, - mut blocks: Vec, + blocks: Vec, store: Store, ) -> Result<(), (ChainError, Option)> { let last_block = blocks.last().unwrap().clone(); @@ -481,22 +481,20 @@ impl SyncManager { .get_latest_block_number() .map_err(|e| (e.into(), None))?; - if last_block.header.number.saturating_sub(latest_block_number) <= MAX_TRIES_IN_STORE as u64 + if last_block.header.number.saturating_sub(latest_block_number) + <= STATE_TRIES_TO_KEEP as u64 { - if blocks_len <= MAX_TRIES_IN_STORE { - self.blockchain.add_blocks_in_batch(blocks, true, true)?; + if blocks_len <= STATE_TRIES_TO_KEEP { + self.blockchain.add_blocks_in_batch(&blocks, true, true)?; } else { - let idx = blocks_len - MAX_TRIES_IN_STORE; - // Using `split_off` avoids cloning the slice, which would be necessary if we used `[..idx]` and `[idx..]` directly. - // This is avoids cloning all blocks which might get expensive if they are large. - let tail = blocks.split_off(idx); - let head = std::mem::take(&mut blocks); - - self.blockchain.add_blocks_in_batch(head, false, true)?; - self.blockchain.add_blocks_in_batch(tail, true, true)?; + let idx = blocks_len - STATE_TRIES_TO_KEEP; + self.blockchain + .add_blocks_in_batch(&blocks[..idx], false, true)?; + self.blockchain + .add_blocks_in_batch(&blocks[idx..], true, true)?; } } else { - self.blockchain.add_blocks_in_batch(blocks, false, true)?; + self.blockchain.add_blocks_in_batch(&blocks, false, true)?; } Ok(()) diff --git a/crates/networking/rpc/engine/payload.rs b/crates/networking/rpc/engine/payload.rs index 75a8334a4c..256c12ce99 100644 --- a/crates/networking/rpc/engine/payload.rs +++ b/crates/networking/rpc/engine/payload.rs @@ -582,7 +582,7 @@ fn handle_new_payload_v1_v2( } // All checks passed, execute payload - let payload_status = execute_payload(block, &context)?; + let payload_status = execute_payload(&block, &context)?; serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) } @@ -624,7 +624,7 @@ fn handle_new_payload_v3( } // All checks passed, execute payload - execute_payload(block, &context) + execute_payload(&block, &context) } // Elements of the list MUST be ordered by request_type in ascending order. @@ -670,7 +670,7 @@ fn validate_block_hash(payload: &ExecutionPayload, block: &Block) -> Result<(), Ok(()) } -fn execute_payload(block: Block, context: &RpcApiContext) -> Result { +fn execute_payload(block: &Block, context: &RpcApiContext) -> Result { let block_hash = block.hash(); let storage = &context.storage; // Return the valid message directly if we have it. diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 3427a00df6..63a9c4024d 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -16,8 +16,7 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// If `as_canonical` is true, each block is assumed to be part of the canonical chain, /// and the canonical hash is set to the block number. This optimizes writes when /// processing blocks in bulk. - fn add_batch_of_blocks(&self, blocks: Vec, as_canonical: bool) - -> Result<(), StoreError>; + fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError>; /// Add block header fn add_block_header( @@ -109,7 +108,7 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Adds a batch of receipts fn add_batch_of_receipts( &self, - blocks: Vec<(BlockHash, Vec)>, + blocks_receipts: Vec<(BlockHash, Vec)>, ) -> Result<(), StoreError>; /// Obtain receipt for a canonical block represented by the block number. diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 038c57d848..7f986fde4b 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -489,7 +489,7 @@ impl Store { pub fn add_batch_of_blocks( &self, - blocks: Vec, + blocks: &[Block], as_canonical: bool, ) -> Result<(), StoreError> { self.engine.add_batch_of_blocks(blocks, as_canonical) diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 2169195420..a9a4b26bcc 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,13 +143,9 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_blocks( - &self, - blocks: Vec, - as_canonical: bool, - ) -> Result<(), StoreError> { + fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { for block in blocks { - let header = block.header; + let header = block.header.clone(); let number = header.number; let hash = header.compute_block_hash(); let locations = block @@ -160,7 +156,7 @@ impl StoreEngine for Store { .map(|(i, tx)| (tx.compute_hash(), number, hash, i as u64)); self.add_transaction_locations(locations.collect())?; - self.add_block_body(hash, block.body)?; + self.add_block_body(hash, block.body.clone())?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; if as_canonical { @@ -231,10 +227,10 @@ impl StoreEngine for Store { receipts: Vec<(BlockHash, Vec)>, ) -> Result<(), StoreError> { let mut store = self.inner(); - for (index, (block_hash, receipts)) in receipts.iter().enumerate() { - let entry = store.receipts.entry(*block_hash).or_default(); + for (index, (block_hash, receipts)) in receipts.into_iter().enumerate() { + let entry = store.receipts.entry(block_hash).or_default(); for receipt in receipts { - entry.insert(index as u64, receipt.clone()); + entry.insert(index as u64, receipt); } } diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index a7d3406c0a..379b382f66 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -125,20 +125,15 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_batch_of_blocks( - &self, - blocks: Vec, - as_canonical: bool, - ) -> Result<(), StoreError> { + fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { let tx = self .db .begin_readwrite() .map_err(StoreError::LibmdbxError)?; for block in blocks { - let header = block.header; - let number = header.number; - let hash = header.compute_block_hash(); + let number = block.header.number; + let hash = block.hash(); for (index, transaction) in block.body.transactions.iter().enumerate() { tx.upsert::( @@ -148,13 +143,19 @@ impl StoreEngine for Store { .map_err(StoreError::LibmdbxError)?; } - tx.upsert::(hash.into(), block.body.into()) - .map_err(StoreError::LibmdbxError)?; + tx.upsert::( + hash.into(), + BlockBodyRLP::from_bytes(block.body.encode_to_vec()), + ) + .map_err(StoreError::LibmdbxError)?; - tx.upsert::(hash.into(), header.into()) - .map_err(StoreError::LibmdbxError)?; + tx.upsert::( + hash.into(), + BlockHeaderRLP::from_bytes(block.header.encode_to_vec()), + ) + .map_err(StoreError::LibmdbxError)?; - tx.upsert::(hash.into(), number.into()) + tx.upsert::(hash.into(), number) .map_err(StoreError::LibmdbxError)?; if as_canonical { From e439e6242fd4f6519e67d070586b8070fbb779e6 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 16:56:51 -0300 Subject: [PATCH 26/60] feat: implement redb new store batch function --- crates/storage/store_db/redb.rs | 68 +++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index d87dd6a74b..95866b6e05 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -253,6 +253,52 @@ impl StoreEngine for RedBStore { ) } + fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { + let write_txn = self.db.begin_write()?; + + for block in blocks { + let block_number = block.header.number; + let block_hash = block.hash(); + + let mut transaction_table = + write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; + for (index, transaction) in block.body.transactions.iter().enumerate() { + transaction_table.insert( + >::into(transaction.compute_hash()), + <(u64, H256, u64) as Into>>::into(( + block_number, + block_hash, + index as u64, + )), + )?; + } + + write_txn.open_table(HEADERS_TABLE)?.insert( + >::into(block_hash), + >::into(block.header.clone()), + )?; + + write_txn.open_table(BLOCK_BODIES_TABLE)?.insert( + >::into(block_hash), + >::into(block.body.clone()), + )?; + + write_txn + .open_table(BLOCK_NUMBERS_TABLE)? + .insert(>::into(block_hash), block_number)?; + + if as_canonical { + write_txn + .open_table(CANONICAL_BLOCK_HASHES_TABLE)? + .insert(block_number, >::into(block_hash))?; + } + } + + write_txn.commit()?; + + Ok(()) + } + fn get_block_body(&self, block_number: BlockNumber) -> Result, StoreError> { if let Some(hash) = self.get_block_hash_by_block_number(block_number)? { self.get_block_body_by_hash(hash) @@ -587,6 +633,28 @@ impl StoreEngine for RedBStore { self.write_batch(RECEIPTS_TABLE, key_values) } + fn add_batch_of_receipts( + &self, + blocks_receipts: Vec<(BlockHash, Vec)>, + ) -> Result<(), StoreError> { + let mut key_values = vec![]; + + for (block_hash, receipts) in blocks_receipts { + for (index, receipt) in receipts.iter().enumerate() { + let kv = ( + <(H256, u64) as Into>>::into(( + block_hash, + index as u64, + )), + >::into(receipt.clone()), + ); + key_values.push(kv) + } + } + + self.write_batch(RECEIPTS_TABLE, key_values) + } + fn add_transaction_locations( &self, locations: Vec<(H256, BlockNumber, BlockHash, Index)>, From 75b9b403692c3b6d9faa65eb876cedb060312741 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 18:52:54 -0300 Subject: [PATCH 27/60] revert: batch import --- cmd/ethrex/subcommands/import.rs | 9 +---- crates/blockchain/blockchain.rs | 67 +++++++++++++++++++------------- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index f394e51d7e..81d4ad8435 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -2,7 +2,6 @@ use std::fs::{self, metadata}; use clap::ArgMatches; -use ethrex_blockchain::STATE_TRIES_TO_KEEP; use ethrex_vm::backends::EvmEngine; use tracing::info; @@ -49,11 +48,5 @@ pub fn import_blocks_from_path( utils::read_chain_file(path) }; - if blocks.len() <= STATE_TRIES_TO_KEEP { - blockchain.import_blocks(&blocks, true); - } else { - let idx = blocks.len() - STATE_TRIES_TO_KEEP; - blockchain.import_blocks(&blocks[..idx], false); - blockchain.import_blocks(&blocks[idx..], true); - } + blockchain.import_blocks(&blocks); } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 87810c9e38..5218965098 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -296,37 +296,50 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks(&self, blocks: &[Block], should_commit_intermediate_tries: bool) { + pub fn import_blocks(&self, blocks: &[Block]) { let size = blocks.len(); - let last_block = blocks.last().unwrap().header.clone(); - if let Err((err, _)) = - self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) - { - warn!("Failed to add blocks: {:?}.", err); - }; - if let Err(err) = self.storage.update_latest_block_number(last_block.number) { - error!("Fatal: added block {} but could not update the block number, err {:?} -- aborting block import", last_block.number, err); - return; - }; - - let last_block_hash = last_block.compute_block_hash(); - match self.evm_engine { - EvmEngine::LEVM => { - // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. - let _ = apply_fork_choice( - &self.storage, - last_block_hash, - last_block_hash, - last_block_hash, + for block in blocks { + let hash = block.hash(); + info!( + "Adding block {} with hash {:#x}.", + block.header.number, hash + ); + if let Err(error) = self.add_block(block) { + warn!( + "Failed to add block {} with hash {:#x}: {}.", + block.header.number, hash, error ); } - EvmEngine::REVM => { - let _ = apply_fork_choice( - &self.storage, - last_block_hash, - last_block_hash, - last_block_hash, + if self + .storage + .update_latest_block_number(block.header.number) + .is_err() + { + error!("Fatal: added block {} but could not update the block number -- aborting block import", block.header.number); + break; + }; + if self + .storage + .set_canonical_block(block.header.number, hash) + .is_err() + { + error!( + "Fatal: added block {} but could not set it as canonical -- aborting block import", + block.header.number ); + break; + }; + } + if let Some(last_block) = blocks.last() { + let hash = last_block.hash(); + match self.evm_engine { + EvmEngine::LEVM => { + // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. + let _ = apply_fork_choice(&self.storage, hash, hash, hash); + } + EvmEngine::REVM => { + apply_fork_choice(&self.storage, hash, hash, hash).unwrap(); + } } } info!("Added {size} blocks to blockchain"); From 5561b2705f1f3b5730b5d3261645a6515d8f15c3 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 19:04:22 -0300 Subject: [PATCH 28/60] feat: show throughput for range of blocks --- crates/blockchain/blockchain.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 5218965098..faa90d8e4c 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -213,6 +213,7 @@ impl Blockchain { }; let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; + let mut total_gas_used = 0; let chain_config: ChainConfig = self .storage .get_chain_config() @@ -267,12 +268,15 @@ impl Blockchain { } all_receipts.push((block_hash, receipts)); + total_gas_used += block.header.gas_used; last_valid_hash = block_hash; Ok(()) }; + let interval = Instant::now(); + for (i, block) in blocks.iter().enumerate() { if let Err(err) = add_block(block, i) { return Err(( @@ -285,6 +289,7 @@ impl Blockchain { }; } + let blocks_len = blocks.len(); self.storage .add_batch_of_blocks(blocks, as_canonical) .map_err(|e| (e.into(), None))?; @@ -292,6 +297,13 @@ impl Blockchain { .add_batch_of_receipts(all_receipts) .map_err(|e| (e.into(), None))?; + let elapsed = interval.elapsed().as_millis(); + if elapsed != 0 && total_gas_used != 0 { + let as_gigas = (total_gas_used as f64).div(10_f64.powf(9_f64)); + let throughput = (as_gigas) / (elapsed as f64) * 1000_f64; + info!("[METRIC] BLOCK EXECUTION THROUGHPUT RANGE OF {blocks_len}: {throughput} Gigagas/s TIME SPENT: {elapsed} msecs"); + } + Ok(()) } From 7a7b1bf431f00109b3e3c1dc1dd47c6a716c27f4 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 12 Mar 2025 17:37:49 -0300 Subject: [PATCH 29/60] feat: import block commands in batch --- cmd/ethrex/cli.rs | 17 ++++++++ cmd/ethrex/ethrex.rs | 10 +++++ cmd/ethrex/subcommands/import.rs | 69 +++++++++++++++++++++++--------- crates/blockchain/blockchain.rs | 61 ++++++++++++++++++++++------ 4 files changed, 126 insertions(+), 31 deletions(-) diff --git a/cmd/ethrex/cli.rs b/cmd/ethrex/cli.rs index e6bb350ce2..89d9a192e0 100644 --- a/cmd/ethrex/cli.rs +++ b/cmd/ethrex/cli.rs @@ -151,6 +151,23 @@ pub fn cli() -> Command { .required(false) .action(clap::ArgAction::SetTrue) ) + ) + .subcommand( + Command::new("import-in-batch") + .about("Import blocks to the database") + .arg( + Arg::new("path") + .required(true) + .value_name("FILE_PATH/FOLDER") + .help("Path to a RLP chain file or a folder containing files with individual Blocks") + .action(ArgAction::Set), + ) + .arg( + Arg::new("removedb") + .long("removedb") + .required(false) + .action(clap::ArgAction::SetTrue) + ) ); cfg_if::cfg_if! { diff --git a/cmd/ethrex/ethrex.rs b/cmd/ethrex/ethrex.rs index 1d21aee1f1..05a36ca99b 100644 --- a/cmd/ethrex/ethrex.rs +++ b/cmd/ethrex/ethrex.rs @@ -52,6 +52,16 @@ async fn main() { return; } + if let Some(subcommand_matches) = matches.subcommand_matches("import-in-batch") { + import::import_blocks_from_path_in_batch( + subcommand_matches, + data_dir, + evm_engine, + &network, + ); + return; + } + let store = init_store(&data_dir, &network); let blockchain = init_blockchain(evm_engine, store.clone()); diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 81d4ad8435..530c4861e2 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -2,6 +2,8 @@ use std::fs::{self, metadata}; use clap::ArgMatches; +use ethrex_blockchain::STATE_TRIES_TO_KEEP; +use ethrex_common::types::Block; use ethrex_vm::backends::EvmEngine; use tracing::info; @@ -12,6 +14,26 @@ use crate::{ use super::removedb; +fn get_import_blocks(path: &str) -> Vec { + let path_metadata = metadata(path).expect("Failed to read path"); + if path_metadata.is_dir() { + let mut blocks = vec![]; + let dir_reader = fs::read_dir(path).expect("Failed to read blocks directory"); + for file_res in dir_reader { + let file = file_res.expect("Failed to open file in directory"); + let path = file.path(); + let s = path + .to_str() + .expect("Path could not be converted into string"); + blocks.push(utils::read_block_file(s)); + } + blocks + } else { + info!("Importing blocks from chain file: {}", path); + utils::read_chain_file(path) + } +} + pub fn import_blocks_from_path( matches: &ArgMatches, data_dir: String, @@ -27,26 +49,37 @@ pub fn import_blocks_from_path( } let store = init_store(&data_dir, network); - let blockchain = init_blockchain(evm, store); - let path_metadata = metadata(path).expect("Failed to read path"); - let blocks = if path_metadata.is_dir() { - let mut blocks = vec![]; - let dir_reader = fs::read_dir(path).expect("Failed to read blocks directory"); - for file_res in dir_reader { - let file = file_res.expect("Failed to open file in directory"); - let path = file.path(); - let s = path - .to_str() - .expect("Path could not be converted into string"); - blocks.push(utils::read_block_file(s)); - } - blocks - } else { - info!("Importing blocks from chain file: {}", path); - utils::read_chain_file(path) - }; + let blocks = get_import_blocks(path); blockchain.import_blocks(&blocks); } + +pub fn import_blocks_from_path_in_batch( + matches: &ArgMatches, + data_dir: String, + evm: EvmEngine, + network: &str, +) { + let remove_db = *matches.get_one::("removedb").unwrap_or(&false); + let path = matches + .get_one::("path") + .expect("No path provided to import blocks"); + if remove_db { + removedb::remove_db(&data_dir); + } + + let store = init_store(&data_dir, network); + let blockchain = init_blockchain(evm, store); + + let blocks = get_import_blocks(path); + + if blocks.len() <= STATE_TRIES_TO_KEEP { + blockchain.import_blocks_in_batch(&blocks, true); + } else { + let idx = blocks.len() - STATE_TRIES_TO_KEEP; + blockchain.import_blocks_in_batch(&blocks[..idx], false); + blockchain.import_blocks_in_batch(&blocks[idx..], true); + } +} diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index faa90d8e4c..886220f6a7 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -336,27 +336,62 @@ impl Blockchain { .is_err() { error!( - "Fatal: added block {} but could not set it as canonical -- aborting block import", - block.header.number - ); + "Fatal: added block {} but could not set it as canonical -- aborting block import", + block.header.number + ); break; }; } + if let Some(last_block) = blocks.last() { - let hash = last_block.hash(); - match self.evm_engine { - EvmEngine::LEVM => { - // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. - let _ = apply_fork_choice(&self.storage, hash, hash, hash); - } - EvmEngine::REVM => { - apply_fork_choice(&self.storage, hash, hash, hash).unwrap(); - } - } + self.apply_fork_choice_after_import(last_block.hash()); } + + info!("Added {size} blocks to blockchain"); + } + + //TODO: Forkchoice Update shouldn't be part of this function + pub fn import_blocks_in_batch(&self, blocks: &[Block], should_commit_intermediate_tries: bool) { + let size = blocks.len(); + let last_block = blocks.last().unwrap().header.clone(); + if let Err((err, _)) = + self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) + { + warn!("Failed to add blocks: {:?}.", err); + }; + if let Err(err) = self.storage.update_latest_block_number(last_block.number) { + error!("Fatal: added block {} but could not update the block number, err {:?} -- aborting block import", last_block.number, err); + return; + }; + + self.apply_fork_choice_after_import(last_block.compute_block_hash()); + info!("Added {size} blocks to blockchain"); } + fn apply_fork_choice_after_import(&self, last_block_hash: H256) { + match self.evm_engine { + EvmEngine::LEVM => { + // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. + let _ = apply_fork_choice( + &self.storage, + last_block_hash, + last_block_hash, + last_block_hash, + ); + } + EvmEngine::REVM => { + apply_fork_choice( + &self.storage, + last_block_hash, + last_block_hash, + last_block_hash, + ) + .unwrap(); + } + } + } + /// Add a blob transaction and its blobs bundle to the mempool checking that the transaction is valid #[cfg(feature = "c-kzg")] pub fn add_blob_transaction_to_pool( From 14a20da308217f249dd965f5b1842bf73a341fcd Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 14 Mar 2025 11:00:19 -0300 Subject: [PATCH 30/60] refactor: remove import-in-batch command and instead add a flag to existing import command --- cmd/ethrex/cli.rs | 16 +--------------- cmd/ethrex/ethrex.rs | 10 ---------- cmd/ethrex/subcommands/import.rs | 31 ++++--------------------------- 3 files changed, 5 insertions(+), 52 deletions(-) diff --git a/cmd/ethrex/cli.rs b/cmd/ethrex/cli.rs index 89d9a192e0..1e5aba12ec 100644 --- a/cmd/ethrex/cli.rs +++ b/cmd/ethrex/cli.rs @@ -151,22 +151,8 @@ pub fn cli() -> Command { .required(false) .action(clap::ArgAction::SetTrue) ) - ) - .subcommand( - Command::new("import-in-batch") - .about("Import blocks to the database") - .arg( - Arg::new("path") - .required(true) - .value_name("FILE_PATH/FOLDER") - .help("Path to a RLP chain file or a folder containing files with individual Blocks") - .action(ArgAction::Set), - ) .arg( - Arg::new("removedb") - .long("removedb") - .required(false) - .action(clap::ArgAction::SetTrue) + Arg::new("batch").long("batch").required(false).action(clap::ArgAction::SetTrue) ) ); diff --git a/cmd/ethrex/ethrex.rs b/cmd/ethrex/ethrex.rs index 7aa1e03bff..2294eb00cb 100644 --- a/cmd/ethrex/ethrex.rs +++ b/cmd/ethrex/ethrex.rs @@ -45,16 +45,6 @@ async fn main() { return; } - if let Some(subcommand_matches) = matches.subcommand_matches("import-in-batch") { - import::import_blocks_from_path_in_batch( - subcommand_matches, - data_dir, - evm_engine, - &network, - ); - return; - } - let store = init_store(&data_dir, &network); let blockchain = init_blockchain(evm_engine, store.clone()); diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 81236c80d1..74d4cd0aa0 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -2,7 +2,6 @@ use std::fs::{self, metadata}; use clap::ArgMatches; -use ethrex_blockchain::STATE_TRIES_TO_KEEP; use ethrex_common::types::Block; use ethrex_vm::backends::EvmEngine; use tracing::info; @@ -41,28 +40,8 @@ pub fn import_blocks_from_path( network: &str, ) { let remove_db = *matches.get_one::("removedb").unwrap_or(&false); - let path = matches - .get_one::("path") - .expect("No path provided to import blocks"); - if remove_db { - removedb::remove_db(&data_dir); - } - - let store = init_store(&data_dir, network); - let blockchain = init_blockchain(evm, store); - - let blocks = get_import_blocks(path); + let should_batch = *matches.get_one::("batch").unwrap_or(&false); - blockchain.import_blocks(&blocks); -} - -pub fn import_blocks_from_path_in_batch( - matches: &ArgMatches, - data_dir: String, - evm: EvmEngine, - network: &str, -) { - let remove_db = *matches.get_one::("removedb").unwrap_or(&false); let path = matches .get_one::("path") .expect("No path provided to import blocks"); @@ -75,12 +54,10 @@ pub fn import_blocks_from_path_in_batch( let blocks = get_import_blocks(path); - if blocks.len() <= STATE_TRIES_TO_KEEP { - blockchain.import_blocks_in_batch(&blocks, true); + if should_batch { + blockchain.import_blocks_in_batch(&blocks, false); } else { - let idx = blocks.len() - STATE_TRIES_TO_KEEP; - blockchain.import_blocks_in_batch(&blocks[..idx], false); - blockchain.import_blocks_in_batch(&blocks[idx..], true); + blockchain.import_blocks(&blocks); } } From b14780f0de9d61d26ac47e8479e5d2ae5ac68bd3 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 14 Mar 2025 11:08:25 -0300 Subject: [PATCH 31/60] refactor: apply if let fede suggestion --- crates/blockchain/blockchain.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index e431cd635e..35289b1cef 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -202,9 +202,8 @@ impl Blockchain { ) -> Result<(), (ChainError, Option)> { let mut last_valid_hash = H256::default(); - let first_block_header = match blocks.first() { - Some(block) => block.header.clone(), - None => return Err((ChainError::Custom("First block not found".into()), None)), + let Some(first_block_header) = blocks.first().map(|e| e.header.clone()) else { + return Err((ChainError::Custom("First block not found".into()), None)); }; let Some(mut state_trie) = self From 1aa6f62f8e5940a7c3c87ab1166a6c8b40805635 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 14 Mar 2025 11:08:52 -0300 Subject: [PATCH 32/60] temporarily import in batch to view diff in performance this will make ci fail --- cmd/ethrex/subcommands/import.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 74d4cd0aa0..38e6ef4f6f 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -57,7 +57,7 @@ pub fn import_blocks_from_path( if should_batch { blockchain.import_blocks_in_batch(&blocks, false); } else { - blockchain.import_blocks(&blocks); + blockchain.import_blocks_in_batch(&blocks, false); } } From 4c9ea145de52179f4115843197a82d4f5b8d53bd Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 14 Mar 2025 11:15:04 -0300 Subject: [PATCH 33/60] perf: redb open tables once before blocks loop --- crates/storage/store_db/redb.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index 264e29ca36..25ce1a945f 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -256,12 +256,20 @@ impl StoreEngine for RedBStore { fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { let write_txn = self.db.begin_write()?; + let mut transaction_table = write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; + let headers_table = write_txn.open_table(HEADERS_TABLE)?; + let block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; + let block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; + let canonical_block_hashes_table = if as_canonical { + Some(write_txn.open_table(CANONICAL_BLOCK_HASHES_TABLE)?) + } else { + None + }; + for block in blocks { let block_number = block.header.number; let block_hash = block.hash(); - let mut transaction_table = - write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; for (index, transaction) in block.body.transactions.iter().enumerate() { transaction_table.insert( >::into(transaction.compute_hash()), @@ -273,24 +281,21 @@ impl StoreEngine for RedBStore { )?; } - write_txn.open_table(HEADERS_TABLE)?.insert( + headers_table.insert( >::into(block_hash), >::into(block.header.clone()), )?; - write_txn.open_table(BLOCK_BODIES_TABLE)?.insert( + block_bodies_table.insert( >::into(block_hash), >::into(block.body.clone()), )?; - write_txn - .open_table(BLOCK_NUMBERS_TABLE)? + block_numbers_table .insert(>::into(block_hash), block_number)?; - if as_canonical { - write_txn - .open_table(CANONICAL_BLOCK_HASHES_TABLE)? - .insert(block_number, >::into(block_hash))?; + if let Some(table) = canonical_block_hashes_table.as_ref() { + table.insert(block_number, >::into(block_hash))?; } } From eda77bbfe703fe8515978a578cae49207b6b4f3c Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Sat, 15 Mar 2025 13:47:57 -0300 Subject: [PATCH 34/60] revert force blocks import in batch for normal imports --- cmd/ethrex/subcommands/import.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 38e6ef4f6f..74d4cd0aa0 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -57,7 +57,7 @@ pub fn import_blocks_from_path( if should_batch { blockchain.import_blocks_in_batch(&blocks, false); } else { - blockchain.import_blocks_in_batch(&blocks, false); + blockchain.import_blocks(&blocks); } } From 67c4316cb06f15c837d37116943815d65c892164 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Sat, 15 Mar 2025 14:00:33 -0300 Subject: [PATCH 35/60] fix: redb add batch of blocks --- crates/storage/store_db/redb.rs | 77 ++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index 25ce1a945f..a1e26e9455 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -256,46 +256,51 @@ impl StoreEngine for RedBStore { fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { let write_txn = self.db.begin_write()?; - let mut transaction_table = write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; - let headers_table = write_txn.open_table(HEADERS_TABLE)?; - let block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; - let block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; - let canonical_block_hashes_table = if as_canonical { - Some(write_txn.open_table(CANONICAL_BLOCK_HASHES_TABLE)?) - } else { - None - }; - - for block in blocks { - let block_number = block.header.number; - let block_hash = block.hash(); - - for (index, transaction) in block.body.transactions.iter().enumerate() { - transaction_table.insert( - >::into(transaction.compute_hash()), - <(u64, H256, u64) as Into>>::into(( - block_number, - block_hash, - index as u64, - )), + // Begin block so that tables are opened once and dropped at the end. + // This prevents ownership errors when to committing changes at the end. + { + let mut transaction_table = + write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; + let mut headers_table = write_txn.open_table(HEADERS_TABLE)?; + let mut block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; + let mut block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; + let mut canonical_block_hashes_table = if as_canonical { + Some(write_txn.open_table(CANONICAL_BLOCK_HASHES_TABLE)?) + } else { + None + }; + + for block in blocks { + let block_number = block.header.number; + let block_hash = block.hash(); + + for (index, transaction) in block.body.transactions.iter().enumerate() { + transaction_table.insert( + >::into(transaction.compute_hash()), + <(u64, H256, u64) as Into>>::into(( + block_number, + block_hash, + index as u64, + )), + )?; + } + + headers_table.insert( + >::into(block_hash), + >::into(block.header.clone()), )?; - } - headers_table.insert( - >::into(block_hash), - >::into(block.header.clone()), - )?; - - block_bodies_table.insert( - >::into(block_hash), - >::into(block.body.clone()), - )?; + block_bodies_table.insert( + >::into(block_hash), + >::into(block.body.clone()), + )?; - block_numbers_table - .insert(>::into(block_hash), block_number)?; + block_numbers_table + .insert(>::into(block_hash), block_number)?; - if let Some(table) = canonical_block_hashes_table.as_ref() { - table.insert(block_number, >::into(block_hash))?; + if let Some(ref mut table) = canonical_block_hashes_table { + table.insert(block_number, >::into(block_hash))?; + } } } From 5f114107738c0e37bb95bb3ca161074ddcbe8c19 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 12:16:35 -0300 Subject: [PATCH 36/60] refactor: panic on levm when trying to add in batch --- crates/blockchain/blockchain.rs | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 35289b1cef..7469d9f795 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -157,34 +157,9 @@ impl Blockchain { as_canonical: bool, ) -> Result<(), (ChainError, Option)> { match self.evm_engine { - // LEVM does not support batch block processing as it does not persist the state between block executions - // Therefore, we must commit to the db after each block EvmEngine::LEVM => { - let mut last_valid_hash = H256::default(); - for block in blocks { - let block_hash = block.hash(); - let block_number = block.header.number; - - if let Err(e) = self.add_block(block) { - return Err(( - e, - Some(BatchBlockProcessingFailure { - last_valid_hash, - failed_block_hash: block_hash, - }), - )); - }; - - if as_canonical { - self.storage - .set_canonical_block(block_number, block_hash) - .map_err(|e| (e.into(), None))?; - } - - last_valid_hash = block_hash; - } - - Ok(()) + // TODO(#2218): LEVM does not support batch block processing as it does not persist the state between block executions + todo!(); } EvmEngine::REVM => self.add_blocks_in_batch_inner( blocks, From e995b907025da578be654f2ee31c89b385804d1f Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 15:31:31 -0300 Subject: [PATCH 37/60] refactor: simplified execute blocks in batch --- cmd/ethrex/subcommands/import.rs | 7 +- crates/blockchain/blockchain.rs | 195 +++++++++++++-------------- crates/networking/p2p/sync.rs | 43 +----- crates/storage/api.rs | 6 +- crates/storage/store.rs | 12 +- crates/storage/store_db/in_memory.rs | 5 +- crates/storage/store_db/libmdbx.rs | 7 +- crates/storage/store_db/redb.rs | 11 +- crates/vm/backends/mod.rs | 5 +- 9 files changed, 115 insertions(+), 176 deletions(-) diff --git a/cmd/ethrex/subcommands/import.rs b/cmd/ethrex/subcommands/import.rs index 74d4cd0aa0..977eef4499 100644 --- a/cmd/ethrex/subcommands/import.rs +++ b/cmd/ethrex/subcommands/import.rs @@ -50,12 +50,15 @@ pub fn import_blocks_from_path( } let store = init_store(&data_dir, network); - let blockchain = init_blockchain(evm, store); + let blockchain = init_blockchain(evm, store.clone()); let blocks = get_import_blocks(path); if should_batch { - blockchain.import_blocks_in_batch(&blocks, false); + blockchain.import_blocks_in_batch(&blocks); + store + .mark_chain_as_canonical(&blocks) + .expect("Chain could not be marked as canonical in the db"); } else { blockchain.import_blocks(&blocks); } diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 7469d9f795..944f007ae3 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -23,7 +23,7 @@ use mempool::Mempool; use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; -use ethrex_storage::Store; +use ethrex_storage::{AccountUpdate, Store}; use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; @@ -63,7 +63,8 @@ impl Blockchain { } } - pub fn execute_block(&self, block: &Block) -> Result { + /// Executes a block withing a new vm instance and state + fn execute_block(&self, block: &Block) -> Result { // Validate if it can be the new head and find the parent let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { // If the parent is not present, we store it as pending. @@ -82,26 +83,43 @@ impl Blockchain { ); let execution_result = vm.execute_block(block)?; + // Validate execution went alright + validate_gas_used(&execution_result.receipts, &block.header)?; + validate_receipts_root(&block.header, &execution_result.receipts)?; + validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; + + Ok(execution_result) + } + + /// Executes a block from a given vm instance an does not clear its state + fn execute_block_from_state( + &self, + parent_header: &BlockHeader, + block: &Block, + chain_config: &ChainConfig, + vm: &mut Evm, + ) -> Result { + // Validate the block pre-execution + validate_block(block, parent_header, &chain_config)?; + + let execution_result = vm.execute_block_without_clearing_state(block)?; + + // Validate execution went alright + validate_gas_used(&execution_result.receipts, &block.header)?; + validate_receipts_root(&block.header, &execution_result.receipts)?; + validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; + Ok(execution_result) } pub fn store_block( &self, block: &Block, - execution_result: BlockExecutionResult, + receipts: Vec, + account_updates: Vec, ) -> Result<(), ChainError> { - // Assumes block is valid - let BlockExecutionResult { - receipts, - requests, - account_updates, - } = execution_result; - let chain_config = self.storage.get_chain_config()?; - let block_hash = block.header.compute_block_hash(); - validate_gas_used(&receipts, &block.header)?; - // Apply the account updates over the last block's state and compute the new state root let new_state_root = self .storage @@ -111,12 +129,6 @@ impl Blockchain { // Check state root matches the one in block header validate_state_root(&block.header, new_state_root)?; - // Check receipts root matches the one in block header - validate_receipts_root(&block.header, &receipts)?; - - // Processes requests from receipts, computes the requests_hash and compares it against the header - validate_requests_hash(&block.header, &chain_config, &requests)?; - self.storage.add_block(block.clone())?; self.storage.add_receipts(block_hash, receipts)?; @@ -128,7 +140,7 @@ impl Blockchain { let result = self .execute_block(block) - .and_then(|res| self.store_block(block, res)); + .and_then(|res| self.store_block(block, res.receipts, res.account_updates)); let interval = Instant::now().duration_since(since).as_millis(); if interval != 0 { @@ -146,34 +158,10 @@ impl Blockchain { /// - The error type ([`ChainError`]). /// - [`BatchProcessingFailure`] (if the error was caused by block processing). /// - /// `should_commit_intermediate_tries` determines whether the state tries for each block - /// should be stored in the database (the last one is always stored). - /// - /// `as_canonical` determines whether the block number should be set as canonical for the block hash when committing to the db. + /// Note: only the last block state trie is stored in the db pub fn add_blocks_in_batch( &self, blocks: &[Block], - should_commit_intermediate_tries: bool, - as_canonical: bool, - ) -> Result<(), (ChainError, Option)> { - match self.evm_engine { - EvmEngine::LEVM => { - // TODO(#2218): LEVM does not support batch block processing as it does not persist the state between block executions - todo!(); - } - EvmEngine::REVM => self.add_blocks_in_batch_inner( - blocks, - should_commit_intermediate_tries, - as_canonical, - ), - } - } - - fn add_blocks_in_batch_inner( - &self, - blocks: &[Block], - should_commit_intermediate_tries: bool, - as_canonical: bool, ) -> Result<(), (ChainError, Option)> { let mut last_valid_hash = H256::default(); @@ -189,31 +177,32 @@ impl Blockchain { return Err((ChainError::ParentNotFound, None)); }; - let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; - let mut total_gas_used = 0; - let chain_config: ChainConfig = self - .storage - .get_chain_config() - .map_err(|e| (e.into(), None))?; - let mut vm = Evm::new( self.evm_engine, self.storage.clone(), first_block_header.parent_hash, ); - let mut add_block = |block: &Block, i: usize| -> Result<(), ChainError> { - let is_first_block = i == 0; - let is_last_block = i == blocks.len() - 1; - - let block_hash = block.header.compute_block_hash(); + let blocks_len = blocks.len(); + let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; + let mut total_gas_used = 0; + let chain_config: ChainConfig = self + .storage + .get_chain_config() + .map_err(|e| (e.into(), None))?; - let parent_header = if is_first_block { - // for the first block, we need to query the store + let interval = Instant::now(); + for (i, block) in blocks.iter().enumerate() { + // for the first block, we need to query the store + let parent_header = if i == 0 { let Ok(parent_header) = find_parent_header(&block.header, &self.storage) else { - // If the parent is not present, we store it as pending. - self.storage.add_pending_block(block.clone())?; - return Err(ChainError::ParentNotFound); + return Err(( + ChainError::ParentNotFound, + Some(BatchBlockProcessingFailure { + failed_block_hash: block.hash(), + last_valid_hash, + }), + )); }; parent_header } else { @@ -221,54 +210,54 @@ impl Blockchain { blocks[i - 1].header.clone() }; - // Validate the block pre-execution - validate_block(block, &parent_header, &chain_config)?; - let BlockExecutionResult { - account_updates, receipts, - requests, - } = vm.execute_block_without_clearing_state(block)?; - - validate_gas_used(&receipts, &block.header)?; + account_updates, + .. + } = match self.execute_block_from_state(&parent_header, block, &chain_config, &mut vm) { + Ok(result) => result, + Err(err) => { + return Err(( + err, + Some(BatchBlockProcessingFailure { + failed_block_hash: block.hash(), + last_valid_hash, + }), + )) + } + }; self.storage - .apply_account_updates_to_trie(&account_updates, &mut state_trie)?; - - if should_commit_intermediate_tries || is_last_block { - let root_hash = state_trie.hash_no_commit(); - validate_state_root(&block.header, root_hash)?; - // commit to db after validating the root - state_trie.hash().map_err(StoreError::Trie)?; - validate_receipts_root(&block.header, &receipts)?; - validate_requests_hash(&block.header, &chain_config, &requests)?; - } - - all_receipts.push((block_hash, receipts)); + .apply_account_updates_to_trie(&account_updates, &mut state_trie) + .map_err(|err| { + ( + err.into(), + Some(BatchBlockProcessingFailure { + failed_block_hash: block.hash(), + last_valid_hash, + }), + ) + })?; + + last_valid_hash = block.hash(); total_gas_used += block.header.gas_used; + all_receipts.push((block.hash(), receipts)); + } - last_valid_hash = block_hash; - - Ok(()) + let Some(last_block) = blocks.last() else { + return Err((ChainError::Custom("Last block not found".into()), None)); }; - let interval = Instant::now(); + // Validate state root and store blocks + let root_hash = state_trie.hash_no_commit(); + validate_state_root(&last_block.header, root_hash).map_err(|err| (err, None))?; + // commit to db after validating the root + state_trie + .hash() + .map_err(|e| (ChainError::StoreError(e.into()), None))?; - for (i, block) in blocks.iter().enumerate() { - if let Err(err) = add_block(block, i) { - return Err(( - err, - Some(BatchBlockProcessingFailure { - failed_block_hash: block.hash(), - last_valid_hash, - }), - )); - }; - } - - let blocks_len = blocks.len(); self.storage - .add_batch_of_blocks(blocks, as_canonical) + .add_batch_of_blocks(blocks) .map_err(|e| (e.into(), None))?; self.storage .add_batch_of_receipts(all_receipts) @@ -328,12 +317,10 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks_in_batch(&self, blocks: &[Block], should_commit_intermediate_tries: bool) { + pub fn import_blocks_in_batch(&self, blocks: &[Block]) { let size = blocks.len(); let last_block = blocks.last().unwrap().header.clone(); - if let Err((err, _)) = - self.add_blocks_in_batch(blocks, should_commit_intermediate_tries, true) - { + if let Err((err, _)) = self.add_blocks_in_batch(blocks) { warn!("Failed to add blocks: {:?}.", err); }; if let Err(err) = self.storage.update_latest_block_number(last_block.number) { diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index edbd9c48c2..d4d785fd43 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -6,9 +6,7 @@ mod storage_healing; mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; -use ethrex_blockchain::{ - error::ChainError, BatchBlockProcessingFailure, Blockchain, STATE_TRIES_TO_KEEP, -}; +use ethrex_blockchain::{error::ChainError, BatchBlockProcessingFailure, Blockchain}; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, @@ -443,9 +441,7 @@ impl SyncManager { *current_head = last_block.hash(); *search_head = last_block.hash(); - let blocks_len = blocks.len(); - - if let Err((error, failure)) = self.add_blocks(blocks, store.clone()) { + if let Err((error, failure)) = self.blockchain.add_blocks_in_batch(&blocks) { warn!("Failed to add block during FullSync: {error}"); if let Some(BatchBlockProcessingFailure { failed_block_hash, @@ -464,38 +460,11 @@ impl SyncManager { return Err(error.into()); } + store + .mark_chain_as_canonical(&blocks) + .map_err(SyncError::Store)?; store.update_latest_block_number(last_block.header.number)?; - debug!("Executed & stored {} blocks", blocks_len); - - Ok(()) - } - - fn add_blocks( - &self, - blocks: Vec, - store: Store, - ) -> Result<(), (ChainError, Option)> { - let last_block = blocks.last().unwrap().clone(); - let blocks_len = blocks.len(); - let latest_block_number = store - .get_latest_block_number() - .map_err(|e| (e.into(), None))?; - - if last_block.header.number.saturating_sub(latest_block_number) - <= STATE_TRIES_TO_KEEP as u64 - { - if blocks_len <= STATE_TRIES_TO_KEEP { - self.blockchain.add_blocks_in_batch(&blocks, true, true)?; - } else { - let idx = blocks_len - STATE_TRIES_TO_KEEP; - self.blockchain - .add_blocks_in_batch(&blocks[..idx], false, true)?; - self.blockchain - .add_blocks_in_batch(&blocks[idx..], true, true)?; - } - } else { - self.blockchain.add_blocks_in_batch(&blocks, false, true)?; - } + debug!("Executed & stored {} blocks", blocks.len()); Ok(()) } diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 63a9c4024d..312156ce83 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -12,11 +12,7 @@ use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Add a batch of blocks in a single transaction. /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. - /// - /// If `as_canonical` is true, each block is assumed to be part of the canonical chain, - /// and the canonical hash is set to the block number. This optimizes writes when - /// processing blocks in bulk. - fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError>; + fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError>; /// Add block header fn add_block_header( diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 7f986fde4b..9bd74f6508 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -487,12 +487,12 @@ impl Store { self.add_block_number(hash, number) } - pub fn add_batch_of_blocks( - &self, - blocks: &[Block], - as_canonical: bool, - ) -> Result<(), StoreError> { - self.engine.add_batch_of_blocks(blocks, as_canonical) + pub fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + self.engine.add_batch_of_blocks(blocks) + } + + pub fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { + Ok(()) } pub fn add_initial_state(&self, genesis: Genesis) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index a9a4b26bcc..cb3cc95d7e 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,7 +143,7 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { + fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { for block in blocks { let header = block.header.clone(); let number = header.number; @@ -159,9 +159,6 @@ impl StoreEngine for Store { self.add_block_body(hash, block.body.clone())?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; - if as_canonical { - self.set_canonical_block(number, hash)?; - } } Ok(()) diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 379b382f66..29ab5058e5 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -125,7 +125,7 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { + fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { let tx = self .db .begin_readwrite() @@ -157,11 +157,6 @@ impl StoreEngine for Store { tx.upsert::(hash.into(), number) .map_err(StoreError::LibmdbxError)?; - - if as_canonical { - tx.upsert::(number, hash.into()) - .map_err(StoreError::LibmdbxError)?; - } } tx.commit().map_err(StoreError::LibmdbxError) diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index a1e26e9455..fbbb1ca5a1 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -253,7 +253,7 @@ impl StoreEngine for RedBStore { ) } - fn add_batch_of_blocks(&self, blocks: &[Block], as_canonical: bool) -> Result<(), StoreError> { + fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { let write_txn = self.db.begin_write()?; // Begin block so that tables are opened once and dropped at the end. @@ -264,11 +264,6 @@ impl StoreEngine for RedBStore { let mut headers_table = write_txn.open_table(HEADERS_TABLE)?; let mut block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; let mut block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; - let mut canonical_block_hashes_table = if as_canonical { - Some(write_txn.open_table(CANONICAL_BLOCK_HASHES_TABLE)?) - } else { - None - }; for block in blocks { let block_number = block.header.number; @@ -297,10 +292,6 @@ impl StoreEngine for RedBStore { block_numbers_table .insert(>::into(block_hash), block_number)?; - - if let Some(ref mut table) = canonical_block_hashes_table { - table.insert(block_number, >::into(block_hash))?; - } } } diff --git a/crates/vm/backends/mod.rs b/crates/vm/backends/mod.rs index 159c92d714..60f50f0df9 100644 --- a/crates/vm/backends/mod.rs +++ b/crates/vm/backends/mod.rs @@ -98,8 +98,9 @@ impl Evm { ) -> Result { match self { Evm::REVM { state } => REVM::execute_block(block, state), - Evm::LEVM { store_wrapper, .. } => { - LEVM::execute_block(block, store_wrapper.store.clone()) + Evm::LEVM { .. } => { + // TODO(#2218): LEVM does not support a way persist the state between block executions + todo!(); } } } From c39dc73b3dadcfa9ac8313106adbb4d1b6c667d5 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 15:44:15 -0300 Subject: [PATCH 38/60] feat: implement mark_chain_as_canonical for dbs --- crates/storage/api.rs | 3 +++ crates/storage/store.rs | 2 +- crates/storage/store_db/in_memory.rs | 10 ++++++++++ crates/storage/store_db/libmdbx.rs | 8 ++++++++ crates/storage/store_db/redb.rs | 8 ++++++++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 312156ce83..a654424235 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -14,6 +14,9 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError>; + /// Sets the blocks as part of the canonical chain + fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError>; + /// Add block header fn add_block_header( &self, diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 9bd74f6508..b179b94a8a 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -492,7 +492,7 @@ impl Store { } pub fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { - Ok(()) + self.engine.mark_chain_as_canonical(blocks) } pub fn add_initial_state(&self, genesis: Genesis) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index cb3cc95d7e..77c82f1f72 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -164,6 +164,16 @@ impl StoreEngine for Store { Ok(()) } + fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { + for block in blocks { + self.inner() + .canonical_hashes + .insert(block.header.number, block.hash()); + } + + Ok(()) + } + fn add_block_number( &self, block_hash: BlockHash, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 29ab5058e5..a81fde3c65 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -162,6 +162,14 @@ impl StoreEngine for Store { tx.commit().map_err(StoreError::LibmdbxError) } + fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { + let key_values = blocks + .iter() + .map(|e| (e.header.number.into(), e.hash().into())); + + self.write_batch::(key_values) + } + fn get_block_body(&self, block_number: BlockNumber) -> Result, StoreError> { if let Some(hash) = self.get_block_hash_by_block_number(block_number)? { self.get_block_body_by_hash(hash) diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index fbbb1ca5a1..04db8589ff 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -300,6 +300,14 @@ impl StoreEngine for RedBStore { Ok(()) } + fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { + let key_values = blocks + .iter() + .map(|e| (e.header.number, >::into(hash))); + + self.write_batch(CANONICAL_BLOCK_HASHES_TABLE, key_values) + } + fn get_block_body(&self, block_number: BlockNumber) -> Result, StoreError> { if let Some(hash) = self.get_block_hash_by_block_number(block_number)? { self.get_block_body_by_hash(hash) From 035fe2c65e3becd8af6af9c7036278dbc15b3347 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 16:16:54 -0300 Subject: [PATCH 39/60] fix: various full syncing bugs --- crates/networking/p2p/sync.rs | 82 +++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index d4d785fd43..224b832896 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -282,6 +282,7 @@ impl SyncManager { block_headers.remove(0); // Store headers and save hashes for full block retrieval all_block_hashes.extend_from_slice(&block_hashes[..]); + store.add_block_headers(block_hashes.clone(), block_headers.clone())?; if self.sync_mode == SyncMode::Full { self.download_and_run_blocks( @@ -295,11 +296,6 @@ impl SyncManager { .await?; } - // in full sync mode, headers are added after the execution and validation - if self.sync_mode == SyncMode::Snap { - store.add_block_headers(block_hashes.clone(), block_headers)?; - } - if sync_head_found { break; }; @@ -368,51 +364,53 @@ impl SyncManager { store: Store, ) -> Result<(), SyncError> { let mut current_chunk_idx = 0; - let chunks: Vec> = block_hashes + let block_hashes_chunks: Vec> = block_hashes .chunks(MAX_BLOCK_BODIES_TO_REQUEST) .map(|chunk| chunk.to_vec()) .collect(); - let mut chunk = match chunks.get(current_chunk_idx) { + let mut current_block_hashes_chunk = match block_hashes_chunks.get(current_chunk_idx) { Some(res) => res.clone(), None => return Ok(()), }; - let mut headers_idx = 0; + let mut headers_iter = block_headers.into_iter(); let mut blocks: Vec = vec![]; let max_tries = 10; let mut tries = 0; + let since = Instant::now(); loop { debug!("Requesting Block Bodies"); - if let Some(block_bodies) = self.peers.request_block_bodies(chunk.clone()).await { + if let Some(block_bodies) = self + .peers + .request_block_bodies(current_block_hashes_chunk.clone()) + .await + { let block_bodies_len = block_bodies.len(); - let first_block_hash = chunk.first().map_or(H256::default(), |a| *a); - let first_block_header_number = store - .get_block_header_by_hash(first_block_hash)? - .map_or(0, |h| h.number); + let first_block_hash = current_block_hashes_chunk + .first() + .map_or(H256::default(), |a| *a); debug!( - "Received {} Block Bodies, starting from block hash {:?} with number: {}", - block_bodies_len, first_block_hash, first_block_header_number + "Received {} Block Bodies, starting from block hash {:?}", + block_bodies_len, first_block_hash ); // Push blocks - for ((_, body), header) in chunk + for (_, body) in current_block_hashes_chunk .drain(..block_bodies_len) - .zip(block_bodies.into_iter()) - .zip(block_headers[headers_idx..block_bodies_len].iter()) + .zip(block_bodies) { + let header = headers_iter.next().ok_or(SyncError::BodiesNotFound)?; let block = Block::new(header.clone(), body); blocks.push(block); } - headers_idx += block_bodies_len; - - if chunk.is_empty() { + if current_block_hashes_chunk.is_empty() { current_chunk_idx += 1; - chunk = match chunks.get(current_chunk_idx) { + current_block_hashes_chunk = match block_hashes_chunks.get(current_chunk_idx) { Some(res) => res.clone(), None => break, }; @@ -434,13 +432,30 @@ impl SyncManager { } } - debug!("Starting to execute and validate blocks in batch"); + let blocks_len = blocks.len(); + debug!( + "Starting to execute and validate {} blocks in batch", + blocks_len + ); + let Some(first_block) = blocks.first().cloned() else { + return Err(SyncError::BodiesNotFound); + }; let Some(last_block) = blocks.last().cloned() else { return Err(SyncError::BodiesNotFound); }; *current_head = last_block.hash(); *search_head = last_block.hash(); + // To ensure proper execution, we set the chain as canonical before processing the blocks. + // Some opcodes rely on previous block hashes, and due to our current setup, we only support a single chain (no sidechains). + // As a result, we must set the chain upfront to writing to the database during execution. + // Each write operation introduces overhead no matter how small. + // + // For more details, refer to the `get_block_hash` function in [`LevmDatabase`] and the [`revm::Database`]. + store + .mark_chain_as_canonical(&blocks) + .map_err(SyncError::Store)?; + if let Err((error, failure)) = self.blockchain.add_blocks_in_batch(&blocks) { warn!("Failed to add block during FullSync: {error}"); if let Some(BatchBlockProcessingFailure { @@ -460,11 +475,24 @@ impl SyncManager { return Err(error.into()); } - store - .mark_chain_as_canonical(&blocks) - .map_err(SyncError::Store)?; store.update_latest_block_number(last_block.header.number)?; - debug!("Executed & stored {} blocks", blocks.len()); + let elapsed_secs: f64 = since.elapsed().as_millis() as f64 / 1000.0; + + let blocks_per_second = blocks_len as f64 / elapsed_secs; + + info!( + "[SYNCING] Requested, stored, and executed {} blocks in {:.3} seconds.\n\ + Started at block with hash {} (number {}).\n\ + Finished at block with hash {} (number {}).\n\ + Blocks per second: {:.3}", + blocks_len, + elapsed_secs, + first_block.hash(), + first_block.header.number, + last_block.hash(), + last_block.header.number, + blocks_per_second + ); Ok(()) } From 2e6ba2e975ee1f1d1ab47f5cf8f661cda282d6b2 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 16:21:35 -0300 Subject: [PATCH 40/60] refactor: increase block header limit to 1024 this is possible because now we collect the block bodies --- crates/networking/p2p/peer_handler.rs | 6 +++--- crates/networking/p2p/sync.rs | 7 +------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 753834b58d..e046f3c5b5 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -33,14 +33,14 @@ pub const REQUEST_RETRY_ATTEMPTS: usize = 5; pub const MAX_RESPONSE_BYTES: u64 = 512 * 1024; pub const HASH_MAX: H256 = H256([0xFF; 32]); -// Ask as much as 128 block bodies and 192 block headers per request -// these magic numbers are not part of the protocol and are taken from geth, see: +// Ask as much as 128 block bodies per request +// this magic number are not part of the protocol and is taken from geth, see: // https://github.com/ethereum/go-ethereum/blob/2585776aabbd4ae9b00050403b42afb0cee968ec/eth/downloader/downloader.go#L42-L43 // // Note: We noticed that while bigger values are supported // increasing them may be the cause of peers disconnection pub const MAX_BLOCK_BODIES_TO_REQUEST: usize = 128; -pub const MAX_BLOCK_HEADERS_TO_REQUEST: usize = 192; +pub const MAX_BLOCK_HEADERS_TO_REQUEST: usize = 1024; /// An abstraction over the [KademliaTable] containing logic to make requests to peers #[derive(Debug, Clone)] diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 224b832896..c27bbdc2c0 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -187,12 +187,7 @@ impl SyncManager { loop { debug!("Requesting Block Headers from {search_head}"); - let block_header_limit = match self.sync_mode { - SyncMode::Snap => MAX_BLOCK_HEADERS_TO_REQUEST, - // In Full sync mode, request the same number of block bodies as headers, - // since they are processed together at the same rate. - SyncMode::Full => MAX_BLOCK_BODIES_TO_REQUEST, - } as u64; + let block_header_limit = MAX_BLOCK_HEADERS_TO_REQUEST as u64; let Some(mut block_headers) = self .peers From 9fd003012d77db7a5a6478322376edf4fa3b001b Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 16:29:15 -0300 Subject: [PATCH 41/60] refactor: use existing block header limit --- crates/networking/p2p/peer_handler.rs | 1 - crates/networking/p2p/sync.rs | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index e046f3c5b5..c6e999c552 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -40,7 +40,6 @@ pub const HASH_MAX: H256 = H256([0xFF; 32]); // Note: We noticed that while bigger values are supported // increasing them may be the cause of peers disconnection pub const MAX_BLOCK_BODIES_TO_REQUEST: usize = 128; -pub const MAX_BLOCK_HEADERS_TO_REQUEST: usize = 1024; /// An abstraction over the [KademliaTable] containing logic to make requests to peers #[derive(Debug, Clone)] diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index c27bbdc2c0..42183c7292 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -31,10 +31,8 @@ use trie_rebuild::TrieRebuilder; use crate::{ kademlia::KademliaTable, - peer_handler::{ - BlockRequestOrder, PeerHandler, HASH_MAX, MAX_BLOCK_BODIES_TO_REQUEST, - MAX_BLOCK_HEADERS_TO_REQUEST, - }, + peer_handler::{BlockRequestOrder, PeerHandler, HASH_MAX, MAX_BLOCK_BODIES_TO_REQUEST}, + rlpx::eth::blocks::BLOCK_HEADER_LIMIT, }; /// The minimum amount of blocks from the head that we want to full sync during a snap sync @@ -187,7 +185,7 @@ impl SyncManager { loop { debug!("Requesting Block Headers from {search_head}"); - let block_header_limit = MAX_BLOCK_HEADERS_TO_REQUEST as u64; + let block_header_limit = BLOCK_HEADER_LIMIT; let Some(mut block_headers) = self .peers From fb61aae3fe6eced4c2e9f5f1bd67a349aae202f3 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 17:04:23 -0300 Subject: [PATCH 42/60] feat: trie don't commit and return data to commit instead --- crates/common/trie/state.rs | 26 +++++++++++++++--------- crates/common/trie/trie.rs | 17 +++++++++++++--- crates/storage/lib.rs | 4 ++-- crates/storage/store.rs | 40 ++++++++++++++++++++++++------------- 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/crates/common/trie/state.rs b/crates/common/trie/state.rs index b1d46f4e41..4d9086afee 100644 --- a/crates/common/trie/state.rs +++ b/crates/common/trie/state.rs @@ -46,6 +46,17 @@ impl TrieState { } } + /// Returns the cache changes that should be committed to the DB + pub fn get_nodes_to_commit_and_clear_cache( + &mut self, + root: &NodeHash, + ) -> Vec<(Vec, Vec)> { + let mut to_commit = vec![]; + self.to_commit_tail_recursive(root, &mut to_commit); + self.cache.clear(); + to_commit + } + /// Commits cache changes to DB and clears it /// Only writes nodes that follow the root's canonical trie pub fn commit(&mut self, root: &NodeHash) -> Result<(), TrieError> { @@ -57,39 +68,36 @@ impl TrieState { // Writes a node and its children into the DB fn commit_node(&mut self, node_hash: &NodeHash) -> Result<(), TrieError> { let mut to_commit = vec![]; - self.commit_node_tail_recursive(node_hash, &mut to_commit)?; + self.to_commit_tail_recursive(node_hash, &mut to_commit); self.db.put_batch(to_commit)?; Ok(()) } - // Writes a node and its children into the DB - fn commit_node_tail_recursive( + fn to_commit_tail_recursive( &mut self, node_hash: &NodeHash, acc: &mut Vec<(Vec, Vec)>, - ) -> Result<(), TrieError> { + ) { let Some(node) = self.cache.remove(node_hash) else { // If the node is not in the cache then it means it is already stored in the DB - return Ok(()); + return; }; // Commit children (if any) match &node { Node::Branch(n) => { for child in n.choices.iter() { if child.is_valid() { - self.commit_node_tail_recursive(child, acc)?; + self.to_commit_tail_recursive(child, acc); } } } - Node::Extension(n) => self.commit_node_tail_recursive(&n.child, acc)?, + Node::Extension(n) => self.to_commit_tail_recursive(&n.child, acc), Node::Leaf(_) => {} } // Commit self acc.push((node_hash.into(), node.encode_to_vec())); - - Ok(()) } /// Writes a node directly to the DB bypassing the cache diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index 3ee69ec322..6b003f9b90 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -68,6 +68,10 @@ impl Trie { } } + pub fn root(&self) -> Option<&NodeHash> { + self.root.as_ref() + } + /// Retrieve an RLP-encoded value from the trie given its RLP-encoded path. pub fn get(&self, path: &PathRLP) -> Result, TrieError> { if let Some(root) = &self.root { @@ -125,9 +129,7 @@ impl Trie { /// Returns keccak(RLP_NULL) if the trie is empty /// Also commits changes to the DB pub fn hash(&mut self) -> Result { - if let Some(ref root) = self.root { - self.state.commit(root)?; - } + self.commit()?; Ok(self .root .as_ref() @@ -144,6 +146,15 @@ impl Trie { .unwrap_or(*EMPTY_TRIE_HASH) } + pub fn commit(&mut self) -> Result<(), TrieError> { + if let Some(ref root) = self.root { + self.state.commit(root) + } else { + // nothing to commit + Ok(()) + } + } + /// Obtain a merkle proof for the given path. /// The proof will contain all the encoded nodes traversed until reaching the node where the path is stored (including this last node). /// The proof will still be constructed even if the path is not stored in the trie, proving its absence. diff --git a/crates/storage/lib.rs b/crates/storage/lib.rs index c843af80fa..8e867ecb86 100644 --- a/crates/storage/lib.rs +++ b/crates/storage/lib.rs @@ -8,6 +8,6 @@ mod utils; pub mod error; pub use store::{ - hash_address, hash_key, AccountUpdate, EngineType, Store, MAX_SNAPSHOT_READS, - STATE_TRIE_SEGMENTS, + hash_address, hash_key, AccountUpdate, DataToCommitAfterAccountUpdates, EngineType, Store, + MAX_SNAPSHOT_READS, STATE_TRIE_SEGMENTS, }; diff --git a/crates/storage/store.rs b/crates/storage/store.rs index be8dc72b97..065f1ced32 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -55,6 +55,11 @@ pub struct AccountUpdate { // removed_storage_keys: Vec, } +pub struct DataToCommitAfterAccountUpdates { + pub storage_tries: Vec<(H256, Trie)>, + pub accounts_code: Vec<(H256, Bytes)>, +} + impl AccountUpdate { /// Creates new empty update for the given account pub fn new(address: Address) -> AccountUpdate { @@ -364,7 +369,10 @@ impl Store { &self, account_updates: &[AccountUpdate], state_trie: &mut Trie, - ) -> Result<(), StoreError> { + ) -> Result { + let mut storage_tries = vec![]; + let mut accounts_code = vec![]; + for update in account_updates.iter() { let hashed_address = hash_address(&update.address); if update.removed { @@ -383,7 +391,7 @@ impl Store { account_state.code_hash = info.code_hash; // Store updated code in DB if let Some(code) = &update.code { - self.add_account_code(info.code_hash, code.clone())?; + accounts_code.push((info.code_hash, code.clone())); } } // Store the added storage in the account's storage trie and compute its new root @@ -400,14 +408,17 @@ impl Store { storage_trie.insert(hashed_key, storage_value.encode_to_vec())?; } } - // TODO: don't commit to db here to batch all the writes in a single tx - account_state.storage_root = storage_trie.hash()?; + account_state.storage_root = storage_trie.hash_no_commit(); + storage_tries.push((H256::from_slice(&hashed_address), storage_trie)); } state_trie.insert(hashed_address, account_state.encode_to_vec())?; } } - Ok(()) + Ok(DataToCommitAfterAccountUpdates { + storage_tries, + accounts_code, + }) } /// Adds all genesis accounts and returns the genesis block's state_root @@ -461,13 +472,6 @@ impl Store { self.engine.add_receipts(block_hash, receipts) } - pub fn add_batch_of_receipts( - &self, - batch: Vec<(BlockHash, Vec)>, - ) -> Result<(), StoreError> { - self.engine.add_batch_of_receipts(batch) - } - pub fn get_receipt( &self, block_number: BlockNumber, @@ -487,8 +491,16 @@ impl Store { self.add_block_number(hash, number) } - pub fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { - self.engine.add_batch_of_blocks(blocks) + pub fn add_batch_of_blocks( + &self, + blocks: &[Block], + receipts: HashMap>, + state_tries: Vec, + storage_tries: Vec<(H256, Trie)>, + accounts_code: Vec<(H256, Bytes)>, + ) -> Result<(), StoreError> { + self.engine + .add_batch_of_blocks(blocks, receipts, state_tries, storage_tries, accounts_code) } pub fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { From 9a66a2336077f7c7a8c1e31c8df52dcce25a5461 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 17:05:11 -0300 Subject: [PATCH 43/60] feat: store all block related info in a single tx --- crates/storage/api.rs | 11 ++++- crates/storage/store_db/in_memory.rs | 22 ++++++++- crates/storage/store_db/libmdbx.rs | 68 +++++++++++++++++++++++++++- crates/storage/store_db/redb.rs | 45 +----------------- 4 files changed, 98 insertions(+), 48 deletions(-) diff --git a/crates/storage/api.rs b/crates/storage/api.rs index a654424235..3543bec02b 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -4,7 +4,7 @@ use ethrex_common::types::{ payload::PayloadBundle, AccountState, Block, BlockBody, BlockHash, BlockHeader, BlockNumber, ChainConfig, Index, Receipt, Transaction, }; -use std::{fmt::Debug, panic::RefUnwindSafe}; +use std::{collections::HashMap, fmt::Debug, panic::RefUnwindSafe}; use crate::{error::StoreError, store::STATE_TRIE_SEGMENTS}; use ethrex_trie::{Nibbles, Trie}; @@ -12,7 +12,14 @@ use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Add a batch of blocks in a single transaction. /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. - fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError>; + fn add_batch_of_blocks( + &self, + blocks: &[Block], + receipts: HashMap>, + state_tries: Vec, + storage_tries: Vec<(H256, Trie)>, + accounts_code: Vec<(H256, Bytes)>, + ) -> Result<(), StoreError>; /// Sets the blocks as part of the canonical chain fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError>; diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 77c82f1f72..26234de20d 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,7 +143,14 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + fn add_batch_of_blocks( + &self, + blocks: &[Block], + receipts: HashMap>, + state_tries: Vec, + storage_tries: Vec<(H256, Trie)>, + accounts_code: Vec<(H256, Bytes)>, + ) -> Result<(), StoreError> { for block in blocks { let header = block.header.clone(); let number = header.number; @@ -159,6 +166,19 @@ impl StoreEngine for Store { self.add_block_body(hash, block.body.clone())?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; + self.add_receipts(hash, receipts.get(&hash).unwrap().to_vec())?; + } + + for (code_hash, code) in accounts_code { + self.add_account_code(code_hash, code)?; + } + + for mut trie in state_tries { + trie.commit()?; + } + + for (_, mut trie) in storage_tries { + trie.commit()?; } Ok(()) diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index a81fde3c65..0b3efc9ec8 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -27,6 +27,7 @@ use libmdbx::{ }; use libmdbx::{DatabaseOptions, Mode, PageSize, ReadWriteOptions, TransactionKind}; use serde_json; +use std::collections::HashMap; use std::fmt::{Debug, Formatter}; use std::path::Path; use std::sync::Arc; @@ -125,7 +126,14 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + fn add_batch_of_blocks( + &self, + blocks: &[Block], + receipts: HashMap>, + state_tries: Vec, + storage_tries: Vec<(H256, Trie)>, + accounts_code: Vec<(H256, Bytes)>, + ) -> Result<(), StoreError> { let tx = self .db .begin_readwrite() @@ -157,6 +165,48 @@ impl StoreEngine for Store { tx.upsert::(hash.into(), number) .map_err(StoreError::LibmdbxError)?; + + let receipts = receipts.get(&hash).unwrap(); + let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; + for (index, receipt) in receipts.clone().into_iter().enumerate() { + let key = (hash, index as u64).into(); + let receipt_rlp = receipt.encode_to_vec(); + let Some(entries) = IndexedChunk::from::(key, &receipt_rlp) else { + continue; + }; + for (k, v) in entries { + cursor.upsert(k, v).map_err(StoreError::LibmdbxError)?; + } + } + } + + for mut trie in state_tries { + let Some(root) = trie.root().cloned() else { + continue; + }; + let key_values = trie.state_mut().get_nodes_to_commit_and_clear_cache(&root); + + for (k, v) in key_values { + tx.upsert::(k, v) + .map_err(StoreError::LibmdbxError)?; + } + } + + for (account_hash, mut trie) in storage_tries { + let Some(root) = trie.root().cloned() else { + continue; + }; + let key_values = trie.state_mut().get_nodes_to_commit_and_clear_cache(&root); + + for (k, v) in key_values { + tx.upsert::((account_hash.0, node_hash_to_fixed_size(k)), v) + .map_err(StoreError::LibmdbxError)?; + } + } + + for (code_hash, code) in accounts_code { + tx.upsert::(code_hash.into(), code.into()) + .map_err(StoreError::LibmdbxError)?; } tx.commit().map_err(StoreError::LibmdbxError) @@ -870,6 +920,22 @@ impl IndexedChunk { } } +// In order to use NodeHash as key in a dupsort table we must encode it into a fixed size type +pub fn node_hash_to_fixed_size(node_hash: Vec) -> [u8; 33] { + // keep original len so we can re-construct it later + let original_len = node_hash.len(); + // original len will always be lower or equal to 32 bytes + debug_assert!(original_len <= 32); + // Pad the node_hash with zeros to make it fixed_size (in case of inline) + let mut node_hash = node_hash; + node_hash.resize(32, 0); + // Encode the node as [original_len, node_hash...] + std::array::from_fn(|i| match i { + 0 => original_len as u8, + n => node_hash[n - 1], + }) +} + table!( /// The canonical block hash for each block number. It represents the canonical chain. ( CanonicalBlockHashes ) BlockNumber => BlockHashRLP diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index 04db8589ff..b651c4eafe 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -254,50 +254,7 @@ impl StoreEngine for RedBStore { } fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { - let write_txn = self.db.begin_write()?; - - // Begin block so that tables are opened once and dropped at the end. - // This prevents ownership errors when to committing changes at the end. - { - let mut transaction_table = - write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; - let mut headers_table = write_txn.open_table(HEADERS_TABLE)?; - let mut block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; - let mut block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; - - for block in blocks { - let block_number = block.header.number; - let block_hash = block.hash(); - - for (index, transaction) in block.body.transactions.iter().enumerate() { - transaction_table.insert( - >::into(transaction.compute_hash()), - <(u64, H256, u64) as Into>>::into(( - block_number, - block_hash, - index as u64, - )), - )?; - } - - headers_table.insert( - >::into(block_hash), - >::into(block.header.clone()), - )?; - - block_bodies_table.insert( - >::into(block_hash), - >::into(block.body.clone()), - )?; - - block_numbers_table - .insert(>::into(block_hash), block_number)?; - } - } - - write_txn.commit()?; - - Ok(()) + todo!(); } fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { From e1e8e41307919810d1796f6f2f28483e4cc98b2d Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 17:38:41 -0300 Subject: [PATCH 44/60] feat: add blocks batcha account updates and write in a single tx --- crates/blockchain/blockchain.rs | 141 ++++++++++++++++++--------- crates/storage/api.rs | 2 +- crates/storage/store.rs | 40 +++++++- crates/storage/store_db/in_memory.rs | 2 +- crates/storage/store_db/libmdbx.rs | 2 +- crates/storage/store_db/redb.rs | 2 +- crates/vm/backends/revm/mod.rs | 2 +- 7 files changed, 137 insertions(+), 54 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 944f007ae3..a91c08cc52 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -18,12 +18,13 @@ use ethrex_common::types::{ BlockHeader, BlockNumber, ChainConfig, EIP4844Transaction, Receipt, Transaction, }; -use ethrex_common::{Address, H256}; +use ethrex_common::{Address, H160, H256}; use mempool::Mempool; +use std::collections::HashMap; use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; -use ethrex_storage::{AccountUpdate, Store}; +use ethrex_storage::{AccountUpdate, DataToCommitAfterAccountUpdates, Store}; use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; @@ -115,24 +116,41 @@ impl Blockchain { pub fn store_block( &self, block: &Block, - receipts: Vec, - account_updates: Vec, + execution_result: BlockExecutionResult, ) -> Result<(), ChainError> { - let block_hash = block.header.compute_block_hash(); - // Apply the account updates over the last block's state and compute the new state root - let new_state_root = self + let Some(mut state_trie) = self + .storage + .state_trie(block.header.parent_hash) + .map_err(ChainError::StoreError)? + else { + return Err(ChainError::ParentStateNotFound); + }; + + let DataToCommitAfterAccountUpdates { + storage_tries, + accounts_code, + } = self .storage - .apply_account_updates(block.header.parent_hash, &account_updates)? - .ok_or(ChainError::ParentStateNotFound)?; + .apply_account_updates_without_committing( + &execution_result.account_updates, + &mut state_trie, + ) + .map_err(ChainError::StoreError)?; + let new_state_root = state_trie.hash_no_commit(); // Check state root matches the one in block header validate_state_root(&block.header, new_state_root)?; - self.storage.add_block(block.clone())?; - self.storage.add_receipts(block_hash, receipts)?; - - Ok(()) + self.storage + .add_single_block_with_state_and_receipts( + block.clone(), + execution_result.receipts, + state_trie, + storage_tries, + accounts_code, + ) + .map_err(ChainError::StoreError) } pub fn add_block(&self, block: &Block) -> Result<(), ChainError> { @@ -140,7 +158,7 @@ impl Blockchain { let result = self .execute_block(block) - .and_then(|res| self.store_block(block, res.receipts, res.account_updates)); + .and_then(|res| self.store_block(block, res)); let interval = Instant::now().duration_since(since).as_millis(); if interval != 0 { @@ -177,6 +195,10 @@ impl Blockchain { return Err((ChainError::ParentNotFound, None)); }; + let chain_config: ChainConfig = self + .storage + .get_chain_config() + .map_err(|e| (e.into(), None))?; let mut vm = Evm::new( self.evm_engine, self.storage.clone(), @@ -184,12 +206,10 @@ impl Blockchain { ); let blocks_len = blocks.len(); - let mut all_receipts: Vec<(BlockHash, Vec)> = vec![]; + let mut all_receipts: HashMap> = HashMap::new(); + let mut all_account_updates: HashMap = HashMap::new(); let mut total_gas_used = 0; - let chain_config: ChainConfig = self - .storage - .get_chain_config() - .map_err(|e| (e.into(), None))?; + let mut transactions_count = 0; let interval = Instant::now(); for (i, block) in blocks.iter().enumerate() { @@ -227,21 +247,31 @@ impl Blockchain { } }; - self.storage - .apply_account_updates_to_trie(&account_updates, &mut state_trie) - .map_err(|err| { - ( - err.into(), - Some(BatchBlockProcessingFailure { - failed_block_hash: block.hash(), - last_valid_hash, - }), - ) - })?; + // Merge account updates + for account_update in account_updates { + let Some(cache) = all_account_updates.get_mut(&account_update.address) else { + all_account_updates.insert(account_update.address, account_update); + continue; + }; + + cache.removed = account_update.removed; + if let Some(code) = account_update.code { + cache.code = Some(code); + }; + + if let Some(info) = account_update.info { + cache.info = Some(info); + } + + for (k, v) in account_update.added_storage.into_iter() { + cache.added_storage.insert(k, v); + } + } last_valid_hash = block.hash(); total_gas_used += block.header.gas_used; - all_receipts.push((block.hash(), receipts)); + transactions_count += block.body.transactions.len(); + all_receipts.insert(block.hash(), receipts); } let Some(last_block) = blocks.last() else { @@ -250,26 +280,49 @@ impl Blockchain { // Validate state root and store blocks let root_hash = state_trie.hash_no_commit(); + let DataToCommitAfterAccountUpdates { + storage_tries, + accounts_code, + } = self + .storage + .apply_account_updates_without_committing( + &all_account_updates.into_values().collect::>(), + &mut state_trie, + ) + .map_err(|e| (e.into(), None))?; + validate_state_root(&last_block.header, root_hash).map_err(|err| (err, None))?; - // commit to db after validating the root - state_trie - .hash() - .map_err(|e| (ChainError::StoreError(e.into()), None))?; self.storage - .add_batch_of_blocks(blocks) - .map_err(|e| (e.into(), None))?; - self.storage - .add_batch_of_receipts(all_receipts) + .add_blocks_with_state_and_receipts( + blocks, + all_receipts, + vec![state_trie], + storage_tries, + accounts_code, + ) .map_err(|e| (e.into(), None))?; - let elapsed = interval.elapsed().as_millis(); - if elapsed != 0 && total_gas_used != 0 { + let elapsed_total = interval.elapsed().as_millis(); + let mut throughput = 0.0; + if elapsed_total != 0 && total_gas_used != 0 { let as_gigas = (total_gas_used as f64).div(10_f64.powf(9_f64)); - let throughput = (as_gigas) / (elapsed as f64) * 1000_f64; - info!("[METRIC] BLOCK EXECUTION THROUGHPUT RANGE OF {blocks_len}: {throughput} Gigagas/s TIME SPENT: {elapsed} msecs"); + throughput = (as_gigas) / (elapsed_total as f64) * 1000_f64; } + info!( + "[METRICS] {} blocks performance:\n + Total time: {} seconds\n + Blocks per second: {:.3}\n + Total transactions {}\n + Throughput: {} Gigagas/s\n", + blocks_len, + elapsed_total / 1000, + blocks_len as f64 / (elapsed_total as f64 / 1000_f64), + transactions_count, + throughput, + ); + Ok(()) } @@ -322,7 +375,7 @@ impl Blockchain { let last_block = blocks.last().unwrap().header.clone(); if let Err((err, _)) = self.add_blocks_in_batch(blocks) { warn!("Failed to add blocks: {:?}.", err); - }; + } if let Err(err) = self.storage.update_latest_block_number(last_block.number) { error!("Fatal: added block {} but could not update the block number, err {:?} -- aborting block import", last_block.number, err); return; diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 3543bec02b..71f0e8c70d 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -12,7 +12,7 @@ use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Add a batch of blocks in a single transaction. /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. - fn add_batch_of_blocks( + fn add_blocks_with_state_and_receipts( &self, blocks: &[Block], receipts: HashMap>, diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 065f1ced32..474e715684 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -365,7 +365,7 @@ impl Store { } /// Applies state transitions to the given trie and does not commit nor calculate root hash - pub fn apply_account_updates_to_trie( + pub fn apply_account_updates_without_committing( &self, account_updates: &[AccountUpdate], state_trie: &mut Trie, @@ -481,7 +481,6 @@ impl Store { } pub fn add_block(&self, block: Block) -> Result<(), StoreError> { - // TODO Maybe add both in a single tx? let header = block.header; let number = header.number; let hash = header.compute_block_hash(); @@ -491,7 +490,33 @@ impl Store { self.add_block_number(hash, number) } - pub fn add_batch_of_blocks( + /// Stores a block along with their associated receipts, state tries, + /// storage tries, and account code. This function is typically called after + /// executing a block to persist the data to the storage engine in a single transaction. + pub fn add_single_block_with_state_and_receipts( + &self, + block: Block, + receipts: Vec, + state_trie: Trie, + storage_tries: Vec<(H256, Trie)>, + accounts_code: Vec<(H256, Bytes)>, + ) -> Result<(), StoreError> { + let mut receipts_map = HashMap::new(); + receipts_map.insert(block.hash(), receipts); + + self.engine.add_blocks_with_state_and_receipts( + &[block], + receipts_map, + vec![state_trie], + storage_tries, + accounts_code, + ) + } + + /// Stores a batch of blocks along with their associated receipts, state tries, + /// storage tries, and account code. This function is typically called after + /// executing a block to persist the data to the storage engine in a single transaction. + pub fn add_blocks_with_state_and_receipts( &self, blocks: &[Block], receipts: HashMap>, @@ -499,8 +524,13 @@ impl Store { storage_tries: Vec<(H256, Trie)>, accounts_code: Vec<(H256, Bytes)>, ) -> Result<(), StoreError> { - self.engine - .add_batch_of_blocks(blocks, receipts, state_tries, storage_tries, accounts_code) + self.engine.add_blocks_with_state_and_receipts( + blocks, + receipts, + state_tries, + storage_tries, + accounts_code, + ) } pub fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 26234de20d..53889daea8 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,7 +143,7 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_blocks( + fn add_blocks_with_state_and_receipts( &self, blocks: &[Block], receipts: HashMap>, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 0b3efc9ec8..7d9924533b 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -126,7 +126,7 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_batch_of_blocks( + fn add_blocks_with_state_and_receipts( &self, blocks: &[Block], receipts: HashMap>, diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index b651c4eafe..584ea5e656 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -253,7 +253,7 @@ impl StoreEngine for RedBStore { ) } - fn add_batch_of_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + fn add_blocks_with_state_and_receipts(&self, blocks: &[Block]) -> Result<(), StoreError> { todo!(); } diff --git a/crates/vm/backends/revm/mod.rs b/crates/vm/backends/revm/mod.rs index 367ab81f7c..f2a17dde42 100644 --- a/crates/vm/backends/revm/mod.rs +++ b/crates/vm/backends/revm/mod.rs @@ -282,7 +282,7 @@ impl REVM { if account.is_contract_changed() { // Update code in db if let Some(code) = new_acc_info.code { - account_update.code = Some(code.original_bytes().clone().0); + account_update.code = Some(code.original_bytes().0); } } } From 937cdfb475efd4610238349585b6dcc6e6ceef39 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 17:39:34 -0300 Subject: [PATCH 45/60] chore: remove unused code --- crates/storage/api.rs | 6 ------ crates/storage/store_db/in_memory.rs | 15 --------------- crates/storage/store_db/libmdbx.rs | 21 --------------------- crates/storage/store_db/redb.rs | 22 ---------------------- 4 files changed, 64 deletions(-) diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 71f0e8c70d..4bb5f98579 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -111,12 +111,6 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { fn add_receipts(&self, block_hash: BlockHash, receipts: Vec) -> Result<(), StoreError>; - /// Adds a batch of receipts - fn add_batch_of_receipts( - &self, - blocks_receipts: Vec<(BlockHash, Vec)>, - ) -> Result<(), StoreError>; - /// Obtain receipt for a canonical block represented by the block number. fn get_receipt( &self, diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 53889daea8..67423737a5 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -249,21 +249,6 @@ impl StoreEngine for Store { Ok(()) } - fn add_batch_of_receipts( - &self, - receipts: Vec<(BlockHash, Vec)>, - ) -> Result<(), StoreError> { - let mut store = self.inner(); - for (index, (block_hash, receipts)) in receipts.into_iter().enumerate() { - let entry = store.receipts.entry(block_hash).or_default(); - for receipt in receipts { - entry.insert(index as u64, receipt); - } - } - - Ok(()) - } - fn get_receipt( &self, block_number: BlockNumber, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 7d9924533b..24207d9561 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -552,27 +552,6 @@ impl StoreEngine for Store { self.write_batch::(key_values.into_iter()) } - fn add_batch_of_receipts( - &self, - blocks_receipts: Vec<(BlockHash, Vec)>, - ) -> std::result::Result<(), StoreError> { - let mut key_values = vec![]; - - for (block_hash, receipts) in blocks_receipts { - for (index, receipt) in receipts.into_iter().enumerate() { - let key = (block_hash, index as u64).into(); - let receipt_rlp = receipt.encode_to_vec(); - let Some(mut entries) = IndexedChunk::from::(key, &receipt_rlp) else { - continue; - }; - - key_values.append(&mut entries); - } - } - - self.write_batch::(key_values.into_iter()) - } - fn get_receipts_for_block(&self, block_hash: &BlockHash) -> Result, StoreError> { let mut receipts = vec![]; let mut receipt_index = 0; diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index 584ea5e656..c85f940953 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -599,28 +599,6 @@ impl StoreEngine for RedBStore { self.write_batch(RECEIPTS_TABLE, key_values) } - fn add_batch_of_receipts( - &self, - blocks_receipts: Vec<(BlockHash, Vec)>, - ) -> Result<(), StoreError> { - let mut key_values = vec![]; - - for (block_hash, receipts) in blocks_receipts { - for (index, receipt) in receipts.iter().enumerate() { - let kv = ( - <(H256, u64) as Into>>::into(( - block_hash, - index as u64, - )), - >::into(receipt.clone()), - ); - key_values.push(kv) - } - } - - self.write_batch(RECEIPTS_TABLE, key_values) - } - fn add_transaction_locations( &self, locations: Vec<(H256, BlockNumber, BlockHash, Index)>, From c398a58725769f9b19bf3d06ca125e1cfce44712 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 18:10:55 -0300 Subject: [PATCH 46/60] chore: update changelog perf entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64fb0ce063..36edb334fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,4 +13,4 @@ #### 2025-03-11 -* Process blocks in batches when syncing and importing [#2174](https://github.com/lambdaclass/ethrex/pull/2174) \ No newline at end of file +* Process blocks in batches when syncing and importing and store blocks in a single tx [#2174](https://github.com/lambdaclass/ethrex/pull/2174) From c4c3a15e9e6a62b63d63877a8d884dc7b5302551 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 18:17:08 -0300 Subject: [PATCH 47/60] refactor: simplify metrics log --- crates/blockchain/blockchain.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index a91c08cc52..96a139237f 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -311,16 +311,8 @@ impl Blockchain { } info!( - "[METRICS] {} blocks performance:\n - Total time: {} seconds\n - Blocks per second: {:.3}\n - Total transactions {}\n - Throughput: {} Gigagas/s\n", - blocks_len, - elapsed_total / 1000, - blocks_len as f64 / (elapsed_total as f64 / 1000_f64), - transactions_count, - throughput, + "[METRICS] Executed and stored: Range: {}, Total transactions: {}, Throughput: {} Gigagas/s", + blocks_len, transactions_count, throughput ); Ok(()) From aff5cbfa3be4cad4db91b78001701ec86e89d548 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 18:39:30 -0300 Subject: [PATCH 48/60] feat: import blocks sequentially if we are within STATE_TRIES_TO_KEEP when syncing --- crates/networking/p2p/sync.rs | 40 +++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 42183c7292..56f6c9eca7 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -6,7 +6,9 @@ mod storage_healing; mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; -use ethrex_blockchain::{error::ChainError, BatchBlockProcessingFailure, Blockchain}; +use ethrex_blockchain::{ + error::ChainError, BatchBlockProcessingFailure, Blockchain, STATE_TRIES_TO_KEEP, +}; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, @@ -449,7 +451,7 @@ impl SyncManager { .mark_chain_as_canonical(&blocks) .map_err(SyncError::Store)?; - if let Err((error, failure)) = self.blockchain.add_blocks_in_batch(&blocks) { + if let Err((error, failure)) = self.add_blocks(&blocks, store.clone()) { warn!("Failed to add block during FullSync: {error}"); if let Some(BatchBlockProcessingFailure { failed_block_hash, @@ -489,6 +491,40 @@ impl SyncManager { Ok(()) } + + fn add_blocks( + &self, + blocks: &[Block], + store: Store, + ) -> Result<(), (ChainError, Option)> { + let last_block = blocks.last().unwrap().clone(); + let latest_block_number = store + .get_latest_block_number() + .map_err(|e| (e.into(), None))?; + + // If the difference between the last block's number and the latest block number is within + // STATE_TRIES_TO_KEEP, process the blocks sequentially so that the state of all blocks is stored. + if last_block.header.number.saturating_sub(latest_block_number) + <= STATE_TRIES_TO_KEEP as u64 + { + let mut last_valid_hash = H256::default(); + for block in blocks { + self.blockchain.add_block(block).map_err(|e| { + ( + e, + Some(BatchBlockProcessingFailure { + last_valid_hash, + failed_block_hash: block.hash(), + }), + ) + })?; + last_valid_hash = block.hash(); + } + Ok(()) + } else { + self.blockchain.add_blocks_in_batch(&blocks) + } + } } /// Fetches all block bodies for the given block hashes via p2p and stores them From 81ee241ba22d2c4ebd82a563a32204edc0673d69 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 18 Mar 2025 19:22:07 -0300 Subject: [PATCH 49/60] chore: address clippy warnings --- crates/blockchain/blockchain.rs | 6 +++--- crates/common/trie/state.rs | 10 +++++----- crates/networking/p2p/sync.rs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 96a139237f..c2f6379bb6 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -101,14 +101,14 @@ impl Blockchain { vm: &mut Evm, ) -> Result { // Validate the block pre-execution - validate_block(block, parent_header, &chain_config)?; + validate_block(block, parent_header, chain_config)?; let execution_result = vm.execute_block_without_clearing_state(block)?; // Validate execution went alright validate_gas_used(&execution_result.receipts, &block.header)?; validate_receipts_root(&block.header, &execution_result.receipts)?; - validate_requests_hash(&block.header, &chain_config, &execution_result.requests)?; + validate_requests_hash(&block.header, chain_config, &execution_result.requests)?; Ok(execution_result) } @@ -279,7 +279,6 @@ impl Blockchain { }; // Validate state root and store blocks - let root_hash = state_trie.hash_no_commit(); let DataToCommitAfterAccountUpdates { storage_tries, accounts_code, @@ -291,6 +290,7 @@ impl Blockchain { ) .map_err(|e| (e.into(), None))?; + let root_hash = state_trie.hash_no_commit(); validate_state_root(&last_block.header, root_hash).map_err(|err| (err, None))?; self.storage diff --git a/crates/common/trie/state.rs b/crates/common/trie/state.rs index 4d9086afee..ee0efe00f5 100644 --- a/crates/common/trie/state.rs +++ b/crates/common/trie/state.rs @@ -52,7 +52,7 @@ impl TrieState { root: &NodeHash, ) -> Vec<(Vec, Vec)> { let mut to_commit = vec![]; - self.to_commit_tail_recursive(root, &mut to_commit); + self.get_nodes_to_commit_tail_recursive(root, &mut to_commit); self.cache.clear(); to_commit } @@ -68,14 +68,14 @@ impl TrieState { // Writes a node and its children into the DB fn commit_node(&mut self, node_hash: &NodeHash) -> Result<(), TrieError> { let mut to_commit = vec![]; - self.to_commit_tail_recursive(node_hash, &mut to_commit); + self.get_nodes_to_commit_tail_recursive(node_hash, &mut to_commit); self.db.put_batch(to_commit)?; Ok(()) } - fn to_commit_tail_recursive( + fn get_nodes_to_commit_tail_recursive( &mut self, node_hash: &NodeHash, acc: &mut Vec<(Vec, Vec)>, @@ -89,11 +89,11 @@ impl TrieState { Node::Branch(n) => { for child in n.choices.iter() { if child.is_valid() { - self.to_commit_tail_recursive(child, acc); + self.get_nodes_to_commit_tail_recursive(child, acc); } } } - Node::Extension(n) => self.to_commit_tail_recursive(&n.child, acc), + Node::Extension(n) => self.get_nodes_to_commit_tail_recursive(&n.child, acc), Node::Leaf(_) => {} } // Commit self diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 56f6c9eca7..5618547a70 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -368,7 +368,7 @@ impl SyncManager { Some(res) => res.clone(), None => return Ok(()), }; - let mut headers_iter = block_headers.into_iter(); + let mut headers_iter = block_headers.iter(); let mut blocks: Vec = vec![]; let max_tries = 10; @@ -522,7 +522,7 @@ impl SyncManager { } Ok(()) } else { - self.blockchain.add_blocks_in_batch(&blocks) + self.blockchain.add_blocks_in_batch(blocks) } } } From c4cc9bbe93a8a45099e534a04916c374a9891794 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 19 Mar 2025 12:59:28 -0300 Subject: [PATCH 50/60] revert: add_blocks_with_state_and_receipts --- CHANGELOG.md | 2 +- crates/blockchain/blockchain.rs | 55 ++++++++---------- crates/common/trie/state.rs | 26 +++------ crates/common/trie/trie.rs | 4 -- crates/storage/api.rs | 19 +++--- crates/storage/lib.rs | 4 +- crates/storage/store.rs | 81 ++++++-------------------- crates/storage/store_db/in_memory.rs | 37 ++++++------ crates/storage/store_db/libmdbx.rs | 76 +++++++++--------------- crates/storage/store_db/redb.rs | 86 ++++++++++++++++++++++++++-- 10 files changed, 185 insertions(+), 205 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36edb334fd..6a45d80476 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,4 +13,4 @@ #### 2025-03-11 -* Process blocks in batches when syncing and importing and store blocks in a single tx [#2174](https://github.com/lambdaclass/ethrex/pull/2174) +* Process blocks in batches when syncing and importing [#2174](https://github.com/lambdaclass/ethrex/pull/2174) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index c2f6379bb6..f8f179485e 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -24,7 +24,7 @@ use std::collections::HashMap; use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; -use ethrex_storage::{AccountUpdate, DataToCommitAfterAccountUpdates, Store}; +use ethrex_storage::{AccountUpdate, Store}; use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; @@ -118,7 +118,7 @@ impl Blockchain { block: &Block, execution_result: BlockExecutionResult, ) -> Result<(), ChainError> { - // Apply the account updates over the last block's state and compute the new state root + // Apply the account updates over the block's parent state and compute the new state root let Some(mut state_trie) = self .storage .state_trie(block.header.parent_hash) @@ -127,29 +127,23 @@ impl Blockchain { return Err(ChainError::ParentStateNotFound); }; - let DataToCommitAfterAccountUpdates { - storage_tries, - accounts_code, - } = self - .storage - .apply_account_updates_without_committing( - &execution_result.account_updates, - &mut state_trie, - ) + self.storage + .apply_account_updates_to_trie(&execution_result.account_updates, &mut state_trie) .map_err(ChainError::StoreError)?; let new_state_root = state_trie.hash_no_commit(); // Check state root matches the one in block header validate_state_root(&block.header, new_state_root)?; + // Commit to db + state_trie + .commit() + .map_err(|e| ChainError::StoreError(e.into()))?; self.storage - .add_single_block_with_state_and_receipts( - block.clone(), - execution_result.receipts, - state_trie, - storage_tries, - accounts_code, - ) + .add_block(block.clone()) + .map_err(ChainError::StoreError)?; + self.storage + .add_receipts(block.hash(), execution_result.receipts) .map_err(ChainError::StoreError) } @@ -278,13 +272,9 @@ impl Blockchain { return Err((ChainError::Custom("Last block not found".into()), None)); }; - // Validate state root and store blocks - let DataToCommitAfterAccountUpdates { - storage_tries, - accounts_code, - } = self - .storage - .apply_account_updates_without_committing( + // Compute state trie + self.storage + .apply_account_updates_to_trie( &all_account_updates.into_values().collect::>(), &mut state_trie, ) @@ -293,14 +283,15 @@ impl Blockchain { let root_hash = state_trie.hash_no_commit(); validate_state_root(&last_block.header, root_hash).map_err(|err| (err, None))?; + // Commit to db + state_trie + .commit() + .map_err(|e| (ChainError::StoreError(e.into()), None))?; self.storage - .add_blocks_with_state_and_receipts( - blocks, - all_receipts, - vec![state_trie], - storage_tries, - accounts_code, - ) + .add_blocks(blocks) + .map_err(|e| (e.into(), None))?; + self.storage + .add_receipts_for_blocks(all_receipts) .map_err(|e| (e.into(), None))?; let elapsed_total = interval.elapsed().as_millis(); diff --git a/crates/common/trie/state.rs b/crates/common/trie/state.rs index ee0efe00f5..b1d46f4e41 100644 --- a/crates/common/trie/state.rs +++ b/crates/common/trie/state.rs @@ -46,17 +46,6 @@ impl TrieState { } } - /// Returns the cache changes that should be committed to the DB - pub fn get_nodes_to_commit_and_clear_cache( - &mut self, - root: &NodeHash, - ) -> Vec<(Vec, Vec)> { - let mut to_commit = vec![]; - self.get_nodes_to_commit_tail_recursive(root, &mut to_commit); - self.cache.clear(); - to_commit - } - /// Commits cache changes to DB and clears it /// Only writes nodes that follow the root's canonical trie pub fn commit(&mut self, root: &NodeHash) -> Result<(), TrieError> { @@ -68,36 +57,39 @@ impl TrieState { // Writes a node and its children into the DB fn commit_node(&mut self, node_hash: &NodeHash) -> Result<(), TrieError> { let mut to_commit = vec![]; - self.get_nodes_to_commit_tail_recursive(node_hash, &mut to_commit); + self.commit_node_tail_recursive(node_hash, &mut to_commit)?; self.db.put_batch(to_commit)?; Ok(()) } - fn get_nodes_to_commit_tail_recursive( + // Writes a node and its children into the DB + fn commit_node_tail_recursive( &mut self, node_hash: &NodeHash, acc: &mut Vec<(Vec, Vec)>, - ) { + ) -> Result<(), TrieError> { let Some(node) = self.cache.remove(node_hash) else { // If the node is not in the cache then it means it is already stored in the DB - return; + return Ok(()); }; // Commit children (if any) match &node { Node::Branch(n) => { for child in n.choices.iter() { if child.is_valid() { - self.get_nodes_to_commit_tail_recursive(child, acc); + self.commit_node_tail_recursive(child, acc)?; } } } - Node::Extension(n) => self.get_nodes_to_commit_tail_recursive(&n.child, acc), + Node::Extension(n) => self.commit_node_tail_recursive(&n.child, acc)?, Node::Leaf(_) => {} } // Commit self acc.push((node_hash.into(), node.encode_to_vec())); + + Ok(()) } /// Writes a node directly to the DB bypassing the cache diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index 6b003f9b90..047e525bc2 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -68,10 +68,6 @@ impl Trie { } } - pub fn root(&self) -> Option<&NodeHash> { - self.root.as_ref() - } - /// Retrieve an RLP-encoded value from the trie given its RLP-encoded path. pub fn get(&self, path: &PathRLP) -> Result, TrieError> { if let Some(root) = &self.root { diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 4bb5f98579..8ebfdfe7d3 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -12,14 +12,9 @@ use ethrex_trie::{Nibbles, Trie}; pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// Add a batch of blocks in a single transaction. /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. - fn add_blocks_with_state_and_receipts( - &self, - blocks: &[Block], - receipts: HashMap>, - state_tries: Vec, - storage_tries: Vec<(H256, Trie)>, - accounts_code: Vec<(H256, Bytes)>, - ) -> Result<(), StoreError>; + fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError>; + + fn add_block(&self, block: Block) -> Result<(), StoreError>; /// Sets the blocks as part of the canonical chain fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError>; @@ -107,10 +102,16 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { receipt: Receipt, ) -> Result<(), StoreError>; - /// Add receipt + /// Add receipts fn add_receipts(&self, block_hash: BlockHash, receipts: Vec) -> Result<(), StoreError>; + /// Adds receipts for a batch of blocks + fn add_receipts_for_blocks( + &self, + receipts: HashMap>, + ) -> Result<(), StoreError>; + /// Obtain receipt for a canonical block represented by the block number. fn get_receipt( &self, diff --git a/crates/storage/lib.rs b/crates/storage/lib.rs index 8e867ecb86..c843af80fa 100644 --- a/crates/storage/lib.rs +++ b/crates/storage/lib.rs @@ -8,6 +8,6 @@ mod utils; pub mod error; pub use store::{ - hash_address, hash_key, AccountUpdate, DataToCommitAfterAccountUpdates, EngineType, Store, - MAX_SNAPSHOT_READS, STATE_TRIE_SEGMENTS, + hash_address, hash_key, AccountUpdate, EngineType, Store, MAX_SNAPSHOT_READS, + STATE_TRIE_SEGMENTS, }; diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 474e715684..e899c1b46a 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -55,11 +55,6 @@ pub struct AccountUpdate { // removed_storage_keys: Vec, } -pub struct DataToCommitAfterAccountUpdates { - pub storage_tries: Vec<(H256, Trie)>, - pub accounts_code: Vec<(H256, Bytes)>, -} - impl AccountUpdate { /// Creates new empty update for the given account pub fn new(address: Address) -> AccountUpdate { @@ -365,14 +360,12 @@ impl Store { } /// Applies state transitions to the given trie and does not commit nor calculate root hash - pub fn apply_account_updates_without_committing( + /// Note: it does commit storage tries and new account codes + pub fn apply_account_updates_to_trie( &self, account_updates: &[AccountUpdate], state_trie: &mut Trie, - ) -> Result { - let mut storage_tries = vec![]; - let mut accounts_code = vec![]; - + ) -> Result<(), StoreError> { for update in account_updates.iter() { let hashed_address = hash_address(&update.address); if update.removed { @@ -391,7 +384,7 @@ impl Store { account_state.code_hash = info.code_hash; // Store updated code in DB if let Some(code) = &update.code { - accounts_code.push((info.code_hash, code.clone())); + self.add_account_code(account_state.code_hash, code.clone())?; } } // Store the added storage in the account's storage trie and compute its new root @@ -408,17 +401,13 @@ impl Store { storage_trie.insert(hashed_key, storage_value.encode_to_vec())?; } } - account_state.storage_root = storage_trie.hash_no_commit(); - storage_tries.push((H256::from_slice(&hashed_address), storage_trie)); + account_state.storage_root = storage_trie.hash()?; } state_trie.insert(hashed_address, account_state.encode_to_vec())?; } } - Ok(DataToCommitAfterAccountUpdates { - storage_tries, - accounts_code, - }) + Ok(()) } /// Adds all genesis accounts and returns the genesis block's state_root @@ -472,6 +461,13 @@ impl Store { self.engine.add_receipts(block_hash, receipts) } + pub fn add_receipts_for_blocks( + &self, + receipts: HashMap>, + ) -> Result<(), StoreError> { + self.engine.add_receipts_for_blocks(receipts) + } + pub fn get_receipt( &self, block_number: BlockNumber, @@ -481,56 +477,11 @@ impl Store { } pub fn add_block(&self, block: Block) -> Result<(), StoreError> { - let header = block.header; - let number = header.number; - let hash = header.compute_block_hash(); - self.add_transaction_locations(&block.body.transactions, number, hash)?; - self.add_block_body(hash, block.body)?; - self.add_block_header(hash, header)?; - self.add_block_number(hash, number) - } - - /// Stores a block along with their associated receipts, state tries, - /// storage tries, and account code. This function is typically called after - /// executing a block to persist the data to the storage engine in a single transaction. - pub fn add_single_block_with_state_and_receipts( - &self, - block: Block, - receipts: Vec, - state_trie: Trie, - storage_tries: Vec<(H256, Trie)>, - accounts_code: Vec<(H256, Bytes)>, - ) -> Result<(), StoreError> { - let mut receipts_map = HashMap::new(); - receipts_map.insert(block.hash(), receipts); - - self.engine.add_blocks_with_state_and_receipts( - &[block], - receipts_map, - vec![state_trie], - storage_tries, - accounts_code, - ) + self.engine.add_block(block) } - /// Stores a batch of blocks along with their associated receipts, state tries, - /// storage tries, and account code. This function is typically called after - /// executing a block to persist the data to the storage engine in a single transaction. - pub fn add_blocks_with_state_and_receipts( - &self, - blocks: &[Block], - receipts: HashMap>, - state_tries: Vec, - storage_tries: Vec<(H256, Trie)>, - accounts_code: Vec<(H256, Bytes)>, - ) -> Result<(), StoreError> { - self.engine.add_blocks_with_state_and_receipts( - blocks, - receipts, - state_tries, - storage_tries, - accounts_code, - ) + pub fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + self.engine.add_blocks(blocks) } pub fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 67423737a5..9b0478f4b5 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,14 +143,11 @@ impl StoreEngine for Store { Ok(()) } - fn add_blocks_with_state_and_receipts( - &self, - blocks: &[Block], - receipts: HashMap>, - state_tries: Vec, - storage_tries: Vec<(H256, Trie)>, - accounts_code: Vec<(H256, Bytes)>, - ) -> Result<(), StoreError> { + fn add_block(&self, block: Block) -> Result<(), StoreError> { + self.add_blocks(&[block]) + } + + fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { for block in blocks { let header = block.header.clone(); let number = header.number; @@ -166,19 +163,6 @@ impl StoreEngine for Store { self.add_block_body(hash, block.body.clone())?; self.add_block_header(hash, header)?; self.add_block_number(hash, number)?; - self.add_receipts(hash, receipts.get(&hash).unwrap().to_vec())?; - } - - for (code_hash, code) in accounts_code { - self.add_account_code(code_hash, code)?; - } - - for mut trie in state_tries { - trie.commit()?; - } - - for (_, mut trie) in storage_tries { - trie.commit()?; } Ok(()) @@ -429,6 +413,17 @@ impl StoreEngine for Store { Ok(()) } + fn add_receipts_for_blocks( + &self, + receipts: HashMap>, + ) -> Result<(), StoreError> { + for (block_hash, receipts) in receipts.into_iter() { + self.add_receipts(block_hash, receipts)?; + } + + Ok(()) + } + fn add_transaction_locations( &self, locations: Vec<(H256, BlockNumber, BlockHash, Index)>, diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index 24207d9561..f546a629d3 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -126,14 +126,11 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_blocks_with_state_and_receipts( - &self, - blocks: &[Block], - receipts: HashMap>, - state_tries: Vec, - storage_tries: Vec<(H256, Trie)>, - accounts_code: Vec<(H256, Bytes)>, - ) -> Result<(), StoreError> { + fn add_block(&self, block: Block) -> std::result::Result<(), StoreError> { + self.add_blocks(&[block]) + } + + fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { let tx = self .db .begin_readwrite() @@ -165,48 +162,6 @@ impl StoreEngine for Store { tx.upsert::(hash.into(), number) .map_err(StoreError::LibmdbxError)?; - - let receipts = receipts.get(&hash).unwrap(); - let mut cursor = tx.cursor::().map_err(StoreError::LibmdbxError)?; - for (index, receipt) in receipts.clone().into_iter().enumerate() { - let key = (hash, index as u64).into(); - let receipt_rlp = receipt.encode_to_vec(); - let Some(entries) = IndexedChunk::from::(key, &receipt_rlp) else { - continue; - }; - for (k, v) in entries { - cursor.upsert(k, v).map_err(StoreError::LibmdbxError)?; - } - } - } - - for mut trie in state_tries { - let Some(root) = trie.root().cloned() else { - continue; - }; - let key_values = trie.state_mut().get_nodes_to_commit_and_clear_cache(&root); - - for (k, v) in key_values { - tx.upsert::(k, v) - .map_err(StoreError::LibmdbxError)?; - } - } - - for (account_hash, mut trie) in storage_tries { - let Some(root) = trie.root().cloned() else { - continue; - }; - let key_values = trie.state_mut().get_nodes_to_commit_and_clear_cache(&root); - - for (k, v) in key_values { - tx.upsert::((account_hash.0, node_hash_to_fixed_size(k)), v) - .map_err(StoreError::LibmdbxError)?; - } - } - - for (code_hash, code) in accounts_code { - tx.upsert::(code_hash.into(), code.into()) - .map_err(StoreError::LibmdbxError)?; } tx.commit().map_err(StoreError::LibmdbxError) @@ -552,6 +507,27 @@ impl StoreEngine for Store { self.write_batch::(key_values.into_iter()) } + fn add_receipts_for_blocks( + &self, + receipts: HashMap>, + ) -> Result<(), StoreError> { + let mut key_values = vec![]; + + for (block_hash, receipts) in receipts.into_iter() { + for (index, receipt) in receipts.into_iter().enumerate() { + let key = (block_hash, index as u64).into(); + let receipt_rlp = receipt.encode_to_vec(); + let Some(mut entries) = IndexedChunk::from::(key, &receipt_rlp) else { + continue; + }; + + key_values.append(&mut entries); + } + } + + self.write_batch::(key_values.into_iter()) + } + fn get_receipts_for_block(&self, block_hash: &BlockHash) -> Result, StoreError> { let mut receipts = vec![]; let mut receipt_index = 0; diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index c85f940953..136492ef02 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -1,4 +1,4 @@ -use std::{borrow::Borrow, panic::RefUnwindSafe, sync::Arc}; +use std::{borrow::Borrow, collections::HashMap, panic::RefUnwindSafe, sync::Arc}; use crate::rlp::{AccountHashRLP, AccountStateRLP, BlockRLP, Rlp, TransactionHashRLP}; use crate::store::MAX_SNAPSHOT_READS; @@ -253,14 +253,65 @@ impl StoreEngine for RedBStore { ) } - fn add_blocks_with_state_and_receipts(&self, blocks: &[Block]) -> Result<(), StoreError> { - todo!(); + fn add_block(&self, block: Block) -> Result<(), StoreError> { + self.add_blocks(&[block]) + } + + fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { + let write_txn = self.db.begin_write()?; + + { + // Begin block so that tables are opened once and dropped at the end. + // This prevents ownership errors when to committing changes at the end. + { + let mut transaction_table = + write_txn.open_multimap_table(TRANSACTION_LOCATIONS_TABLE)?; + let mut headers_table = write_txn.open_table(HEADERS_TABLE)?; + let mut block_bodies_table = write_txn.open_table(BLOCK_BODIES_TABLE)?; + let mut block_numbers_table = write_txn.open_table(BLOCK_NUMBERS_TABLE)?; + + for block in blocks { + let block_number = block.header.number; + let block_hash = block.hash(); + + for (index, transaction) in block.body.transactions.iter().enumerate() { + transaction_table.insert( + >::into(transaction.compute_hash()), + <(u64, H256, u64) as Into>>::into( + (block_number, block_hash, index as u64), + ), + )?; + } + + headers_table.insert( + >::into(block_hash), + >::into(block.header.clone()), + )?; + block_bodies_table.insert( + >::into(block_hash), + >::into(block.body.clone()), + )?; + block_numbers_table + .insert(>::into(block_hash), block_number)?; + } + } + + write_txn.commit()?; + + Ok(()) + } } fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { let key_values = blocks .iter() - .map(|e| (e.header.number, >::into(hash))); + .map(|e| { + ( + e.header.number, + >::into(e.hash()), + ) + }) + .collect(); self.write_batch(CANONICAL_BLOCK_HASHES_TABLE, key_values) } @@ -382,6 +433,33 @@ impl StoreEngine for RedBStore { ) } + fn add_receipts_for_blocks( + &self, + receipts: HashMap>, + ) -> Result<(), StoreError> { + let mut key_values = vec![]; + + for (block_hash, receipts) in receipts.into_iter() { + let mut kv = receipts + .into_iter() + .enumerate() + .map(|(index, receipt)| { + ( + <(H256, u64) as Into>>::into(( + block_hash, + index as u64, + )), + >::into(receipt), + ) + }) + .collect(); + + key_values.append(&mut kv); + } + + self.write_batch(RECEIPTS_TABLE, key_values) + } + fn get_receipt( &self, block_number: BlockNumber, From a5107df8711c28cd9567f8188a5c44fe23b3f0be Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 19 Mar 2025 13:05:26 -0300 Subject: [PATCH 51/60] chore: address clippy warnings --- crates/storage/store_db/libmdbx.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index f546a629d3..964f712798 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -168,9 +168,7 @@ impl StoreEngine for Store { } fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError> { - let key_values = blocks - .iter() - .map(|e| (e.header.number.into(), e.hash().into())); + let key_values = blocks.iter().map(|e| (e.header.number, e.hash().into())); self.write_batch::(key_values) } @@ -875,22 +873,6 @@ impl IndexedChunk { } } -// In order to use NodeHash as key in a dupsort table we must encode it into a fixed size type -pub fn node_hash_to_fixed_size(node_hash: Vec) -> [u8; 33] { - // keep original len so we can re-construct it later - let original_len = node_hash.len(); - // original len will always be lower or equal to 32 bytes - debug_assert!(original_len <= 32); - // Pad the node_hash with zeros to make it fixed_size (in case of inline) - let mut node_hash = node_hash; - node_hash.resize(32, 0); - // Encode the node as [original_len, node_hash...] - std::array::from_fn(|i| match i { - 0 => original_len as u8, - n => node_hash[n - 1], - }) -} - table!( /// The canonical block hash for each block number. It represents the canonical chain. ( CanonicalBlockHashes ) BlockNumber => BlockHashRLP From 9e3f59b63e803757f921ee3f408c4a15057d514f Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Fri, 21 Mar 2025 19:58:49 +0100 Subject: [PATCH 52/60] Remove apply_account_updates_to_trie. --- crates/blockchain/blockchain.rs | 48 +++++++++---------------------- crates/storage/store.rs | 51 --------------------------------- 2 files changed, 13 insertions(+), 86 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index f8f179485e..f20b2a6129 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -118,27 +118,15 @@ impl Blockchain { block: &Block, execution_result: BlockExecutionResult, ) -> Result<(), ChainError> { - // Apply the account updates over the block's parent state and compute the new state root - let Some(mut state_trie) = self + // Apply the account updates over the last block's state and compute the new state root + let new_state_root = self .storage - .state_trie(block.header.parent_hash) - .map_err(ChainError::StoreError)? - else { - return Err(ChainError::ParentStateNotFound); - }; - - self.storage - .apply_account_updates_to_trie(&execution_result.account_updates, &mut state_trie) - .map_err(ChainError::StoreError)?; + .apply_account_updates(block.header.parent_hash, &execution_result.account_updates)? + .ok_or(ChainError::ParentStateNotFound)?; - let new_state_root = state_trie.hash_no_commit(); // Check state root matches the one in block header validate_state_root(&block.header, new_state_root)?; - // Commit to db - state_trie - .commit() - .map_err(|e| ChainError::StoreError(e.into()))?; self.storage .add_block(block.clone()) .map_err(ChainError::StoreError)?; @@ -181,14 +169,6 @@ impl Blockchain { return Err((ChainError::Custom("First block not found".into()), None)); }; - let Some(mut state_trie) = self - .storage - .state_trie(first_block_header.parent_hash) - .map_err(|e| (e.into(), None))? - else { - return Err((ChainError::ParentNotFound, None)); - }; - let chain_config: ChainConfig = self .storage .get_chain_config() @@ -272,21 +252,19 @@ impl Blockchain { return Err((ChainError::Custom("Last block not found".into()), None)); }; - // Compute state trie - self.storage - .apply_account_updates_to_trie( + // Apply the account updates over all blocks and compute the new state root + let new_state_root = self + .storage + .apply_account_updates( + first_block_header.parent_hash, &all_account_updates.into_values().collect::>(), - &mut state_trie, ) - .map_err(|e| (e.into(), None))?; + .map_err(|e| (e.into(), None))? + .ok_or((ChainError::ParentStateNotFound, None))?; - let root_hash = state_trie.hash_no_commit(); - validate_state_root(&last_block.header, root_hash).map_err(|err| (err, None))?; + // Check state root matches the one in block header + validate_state_root(&last_block.header, new_state_root).map_err(|e| (e, None))?; - // Commit to db - state_trie - .commit() - .map_err(|e| (ChainError::StoreError(e.into()), None))?; self.storage .add_blocks(blocks) .map_err(|e| (e.into(), None))?; diff --git a/crates/storage/store.rs b/crates/storage/store.rs index e9527625bf..f00b029a9a 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -359,57 +359,6 @@ impl Store { Ok(Some(state_trie.hash()?)) } - /// Applies state transitions to the given trie and does not commit nor calculate root hash - /// Note: it does commit storage tries and new account codes - pub fn apply_account_updates_to_trie( - &self, - account_updates: &[AccountUpdate], - state_trie: &mut Trie, - ) -> Result<(), StoreError> { - for update in account_updates.iter() { - let hashed_address = hash_address(&update.address); - if update.removed { - // Remove account from trie - state_trie.remove(hashed_address)?; - } else { - // Add or update AccountState in the trie - // Fetch current state or create a new state to be inserted - let mut account_state = match state_trie.get(&hashed_address)? { - Some(encoded_state) => AccountState::decode(&encoded_state)?, - None => AccountState::default(), - }; - if let Some(info) = &update.info { - account_state.nonce = info.nonce; - account_state.balance = info.balance; - account_state.code_hash = info.code_hash; - // Store updated code in DB - if let Some(code) = &update.code { - self.add_account_code(account_state.code_hash, code.clone())?; - } - } - // Store the added storage in the account's storage trie and compute its new root - if !update.added_storage.is_empty() { - let mut storage_trie = self.engine.open_storage_trie( - H256::from_slice(&hashed_address), - account_state.storage_root, - ); - for (storage_key, storage_value) in &update.added_storage { - let hashed_key = hash_key(storage_key); - if storage_value.is_zero() { - storage_trie.remove(hashed_key)?; - } else { - storage_trie.insert(hashed_key, storage_value.encode_to_vec())?; - } - } - account_state.storage_root = storage_trie.hash()?; - } - state_trie.insert(hashed_address, account_state.encode_to_vec())?; - } - } - - Ok(()) - } - /// Adds all genesis accounts and returns the genesis block's state_root pub fn setup_genesis_state_trie( &self, From f7a601883a298232d9cda86a3a7a22d40497e3cf Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Fri, 21 Mar 2025 20:02:23 +0100 Subject: [PATCH 53/60] Simplify add_block --- crates/storage/api.rs | 2 -- crates/storage/store.rs | 2 +- crates/storage/store_db/in_memory.rs | 4 ---- crates/storage/store_db/libmdbx.rs | 4 ---- crates/storage/store_db/redb.rs | 4 ---- 5 files changed, 1 insertion(+), 15 deletions(-) diff --git a/crates/storage/api.rs b/crates/storage/api.rs index 24f1ac73a3..1ebe823bc6 100644 --- a/crates/storage/api.rs +++ b/crates/storage/api.rs @@ -14,8 +14,6 @@ pub trait StoreEngine: Debug + Send + Sync + RefUnwindSafe { /// This will store -> BlockHeader, BlockBody, BlockTransactions, BlockNumber. fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError>; - fn add_block(&self, block: Block) -> Result<(), StoreError>; - /// Sets the blocks as part of the canonical chain fn mark_chain_as_canonical(&self, blocks: &[Block]) -> Result<(), StoreError>; diff --git a/crates/storage/store.rs b/crates/storage/store.rs index f00b029a9a..bc143e14e9 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -426,7 +426,7 @@ impl Store { } pub fn add_block(&self, block: Block) -> Result<(), StoreError> { - self.engine.add_block(block) + self.add_blocks(&[block]) } pub fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { diff --git a/crates/storage/store_db/in_memory.rs b/crates/storage/store_db/in_memory.rs index 22cf5dbfb4..a378ba5809 100644 --- a/crates/storage/store_db/in_memory.rs +++ b/crates/storage/store_db/in_memory.rs @@ -143,10 +143,6 @@ impl StoreEngine for Store { Ok(()) } - fn add_block(&self, block: Block) -> Result<(), StoreError> { - self.add_blocks(&[block]) - } - fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { for block in blocks { let header = block.header.clone(); diff --git a/crates/storage/store_db/libmdbx.rs b/crates/storage/store_db/libmdbx.rs index d430333058..7358442be9 100644 --- a/crates/storage/store_db/libmdbx.rs +++ b/crates/storage/store_db/libmdbx.rs @@ -126,10 +126,6 @@ impl StoreEngine for Store { self.write::(block_hash.into(), block_body.into()) } - fn add_block(&self, block: Block) -> std::result::Result<(), StoreError> { - self.add_blocks(&[block]) - } - fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { let tx = self .db diff --git a/crates/storage/store_db/redb.rs b/crates/storage/store_db/redb.rs index f3cca07d36..dfa7f4d999 100644 --- a/crates/storage/store_db/redb.rs +++ b/crates/storage/store_db/redb.rs @@ -253,10 +253,6 @@ impl StoreEngine for RedBStore { ) } - fn add_block(&self, block: Block) -> Result<(), StoreError> { - self.add_blocks(&[block]) - } - fn add_blocks(&self, blocks: &[Block]) -> Result<(), StoreError> { let write_txn = self.db.begin_write()?; From 2139269bc0a983b75df10370003d9bc0c75bc6f4 Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Fri, 21 Mar 2025 20:03:42 +0100 Subject: [PATCH 54/60] Update changelog date. --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a45d80476..f0c6ebff8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,14 +3,14 @@ ## Perf #### 2025-03-30 -* Faster block import, use a slice instead of copy -[#2097](https://github.com/lambdaclass/ethrex/pull/2097) +- Faster block import, use a slice instead of copy + [#2097](https://github.com/lambdaclass/ethrex/pull/2097) #### 2025-02-28 -* Don't recompute transaction senders when building blocks [#2097](https://github.com/lambdaclass/ethrex/pull/2097) +- Don't recompute transaction senders when building blocks [#2097](https://github.com/lambdaclass/ethrex/pull/2097) -#### 2025-03-11 +#### 2025-03-21 -* Process blocks in batches when syncing and importing [#2174](https://github.com/lambdaclass/ethrex/pull/2174) +- Process blocks in batches when syncing and importing [#2174](https://github.com/lambdaclass/ethrex/pull/2174) From 1b87f6cd77649390762fcc747cb6c4cb3111ea65 Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Fri, 21 Mar 2025 20:06:31 +0100 Subject: [PATCH 55/60] Remove block importing code. --- crates/blockchain/blockchain.rs | 61 ++++++++------------------------- 1 file changed, 14 insertions(+), 47 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index f20b2a6129..4521cafd94 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -288,7 +288,7 @@ impl Blockchain { } //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks(&self, blocks: &[Block]) { + pub fn import_blocks(&self, blocks: &Vec) { let size = blocks.len(); for block in blocks { let hash = block.hash(); @@ -316,58 +316,25 @@ impl Blockchain { .is_err() { error!( - "Fatal: added block {} but could not set it as canonical -- aborting block import", - block.header.number - ); + "Fatal: added block {} but could not set it as canonical -- aborting block import", + block.header.number + ); break; }; } - if let Some(last_block) = blocks.last() { - self.apply_fork_choice_after_import(last_block.hash()); - } - - info!("Added {size} blocks to blockchain"); - } - - //TODO: Forkchoice Update shouldn't be part of this function - pub fn import_blocks_in_batch(&self, blocks: &[Block]) { - let size = blocks.len(); - let last_block = blocks.last().unwrap().header.clone(); - if let Err((err, _)) = self.add_blocks_in_batch(blocks) { - warn!("Failed to add blocks: {:?}.", err); - } - if let Err(err) = self.storage.update_latest_block_number(last_block.number) { - error!("Fatal: added block {} but could not update the block number, err {:?} -- aborting block import", last_block.number, err); - return; - }; - - self.apply_fork_choice_after_import(last_block.compute_block_hash()); - - info!("Added {size} blocks to blockchain"); - } - - fn apply_fork_choice_after_import(&self, last_block_hash: H256) { - match self.evm_engine { - EvmEngine::LEVM => { - // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. - let _ = apply_fork_choice( - &self.storage, - last_block_hash, - last_block_hash, - last_block_hash, - ); - } - EvmEngine::REVM => { - apply_fork_choice( - &self.storage, - last_block_hash, - last_block_hash, - last_block_hash, - ) - .unwrap(); + let hash = last_block.hash(); + match self.evm_engine { + EvmEngine::LEVM => { + // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. + let _ = apply_fork_choice(&self.storage, hash, hash, hash); + } + EvmEngine::REVM => { + apply_fork_choice(&self.storage, hash, hash, hash).unwrap(); + } } } + info!("Added {size} blocks to blockchain"); } /// Add a blob transaction and its blobs bundle to the mempool checking that the transaction is valid From b7b06463ebf159fc3b88b2230e9a0f533bd0fe0d Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 25 Mar 2025 15:11:24 -0300 Subject: [PATCH 56/60] refactor: full sync move shared variables to SyncManager struct and simplify code --- crates/blockchain/blockchain.rs | 7 +- crates/common/trie/trie.rs | 6 +- crates/networking/p2p/peer_handler.rs | 7 +- crates/networking/p2p/sync.rs | 153 +++++++++++--------------- 4 files changed, 71 insertions(+), 102 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 4521cafd94..daf36d6a38 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -25,13 +25,10 @@ use std::{ops::Div, time::Instant}; use ethrex_storage::error::StoreError; use ethrex_storage::{AccountUpdate, Store}; -use ethrex_vm::backends::{BlockExecutionResult, Evm, EvmEngine}; +use ethrex_vm::{BlockExecutionResult, Evm, EvmEngine}; use fork_choice::apply_fork_choice; use tracing::{error, info, warn}; -/// The number of latest tries to store in the database (meaning that we would have the state for the last 128 blocks) -pub const STATE_TRIES_TO_KEEP: usize = 128; - //TODO: Implement a struct Chain or BlockChain to encapsulate //functionality and canonical chain state and config @@ -158,7 +155,7 @@ impl Blockchain { /// - The error type ([`ChainError`]). /// - [`BatchProcessingFailure`] (if the error was caused by block processing). /// - /// Note: only the last block state trie is stored in the db + /// Note: only the last block's state trie is stored in the db pub fn add_blocks_in_batch( &self, blocks: &[Block], diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index 047e525bc2..b4558754f5 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -144,11 +144,9 @@ impl Trie { pub fn commit(&mut self) -> Result<(), TrieError> { if let Some(ref root) = self.root { - self.state.commit(root) - } else { - // nothing to commit - Ok(()) + self.state.commit(root)?; } + Ok(()) } /// Obtain a merkle proof for the given path. diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index c6e999c552..742c9042be 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -14,7 +14,9 @@ use crate::{ kademlia::{KademliaTable, PeerChannels}, rlpx::{ eth::{ - blocks::{BlockBodies, BlockHeaders, GetBlockBodies, GetBlockHeaders}, + blocks::{ + BlockBodies, BlockHeaders, GetBlockBodies, GetBlockHeaders, BLOCK_HEADER_LIMIT, + }, receipts::{GetReceipts, Receipts}, }, message::Message as RLPxMessage, @@ -82,14 +84,13 @@ impl PeerHandler { &self, start: H256, order: BlockRequestOrder, - limit: u64, ) -> Option> { for _ in 0..REQUEST_RETRY_ATTEMPTS { let request_id = rand::random(); let request = RLPxMessage::GetBlockHeaders(GetBlockHeaders { id: request_id, startblock: start.into(), - limit, + limit: BLOCK_HEADER_LIMIT, skip: 0, reverse: matches!(order, BlockRequestOrder::NewToOld), }); diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 7ab7cb8e51..9da1e9b3b2 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -6,9 +6,7 @@ mod storage_healing; mod trie_rebuild; use bytecode_fetcher::bytecode_fetcher; -use ethrex_blockchain::{ - error::ChainError, BatchBlockProcessingFailure, Blockchain, STATE_TRIES_TO_KEEP, -}; +use ethrex_blockchain::{error::ChainError, BatchBlockProcessingFailure, Blockchain}; use ethrex_common::{ types::{Block, BlockHash, BlockHeader}, BigEndianHash, H256, U256, U512, @@ -34,7 +32,6 @@ use trie_rebuild::TrieRebuilder; use crate::{ kademlia::KademliaTable, peer_handler::{BlockRequestOrder, PeerHandler, HASH_MAX, MAX_BLOCK_BODIES_TO_REQUEST}, - rlpx::eth::blocks::BLOCK_HEADER_LIMIT, }; /// The minimum amount of blocks from the head that we want to full sync during a snap sync @@ -94,6 +91,9 @@ pub struct SyncManager { // Used for cancelling long-living tasks upon shutdown cancel_token: CancellationToken, blockchain: Arc, + search_head: H256, + current_head: H256, + sync_head_found: bool, } impl SyncManager { @@ -111,6 +111,9 @@ impl SyncManager { trie_rebuilder: None, cancel_token, blockchain, + current_head: H256::default(), + search_head: H256::default(), + sync_head_found: false, } } @@ -129,6 +132,9 @@ impl SyncManager { blockchain: Arc::new(Blockchain::default_with_store( Store::new("", EngineType::InMemory).unwrap(), )), + current_head: H256::default(), + search_head: H256::default(), + sync_head_found: false, } } @@ -143,7 +149,8 @@ impl SyncManager { pub async fn start_sync(&mut self, current_head: H256, sync_head: H256, store: Store) { info!("Syncing from current head {current_head} to sync_head {sync_head}"); let start_time = Instant::now(); - match self.sync_cycle(current_head, sync_head, store).await { + self.current_head = current_head; + match self.sync_cycle(sync_head, store).await { Ok(()) => { info!( "Sync cycle finished, time elapsed: {} secs", @@ -158,12 +165,7 @@ impl SyncManager { } /// Performs the sync cycle described in `start_sync`, returns an error if the sync fails at any given step and aborts all active processes - async fn sync_cycle( - &mut self, - mut current_head: H256, - sync_head: H256, - store: Store, - ) -> Result<(), SyncError> { + async fn sync_cycle(&mut self, sync_head: H256, store: Store) -> Result<(), SyncError> { // Request all block headers between the current head and the sync head // We will begin from the current head so that we download the earliest state first // This step is not parallelized @@ -174,7 +176,7 @@ impl SyncManager { if matches!(self.sync_mode, SyncMode::Snap) { if let Some(last_header) = store.get_header_download_checkpoint()? { // Set latest downloaded header as current head for header fetching - current_head = last_header; + self.current_head = last_header; } } @@ -184,15 +186,13 @@ impl SyncManager { }; // TODO(#2126): To avoid modifying the current_head while backtracking we use a separate search_head - let mut search_head = current_head; + self.search_head = self.current_head; loop { - debug!("Requesting Block Headers from {search_head}"); - let block_header_limit = BLOCK_HEADER_LIMIT; - + debug!("Requesting Block Headers from {}", self.search_head); let Some(mut block_headers) = self .peers - .request_block_headers(search_head, BlockRequestOrder::OldToNew, block_header_limit) + .request_block_headers(self.search_head, BlockRequestOrder::OldToNew) .await else { warn!("Sync failed to find target block header, aborting"); @@ -210,12 +210,12 @@ impl SyncManager { // TODO(#2126): This is just a temporary solution to avoid a bug where the sync would get stuck // on a loop when the target head is not found, i.e. on a reorg with a side-chain. if first_block_header == last_block_header - && first_block_header.compute_block_hash() == search_head - && search_head != sync_head + && first_block_header.compute_block_hash() == self.search_head + && self.search_head != sync_head { // There is no path to the sync head this goes back until it find a common ancerstor warn!("Sync failed to find target block header, going back to the previous parent"); - search_head = first_block_header.parent_hash; + self.search_head = first_block_header.parent_hash; continue; } @@ -241,23 +241,23 @@ impl SyncManager { } // Filter out everything after the sync_head - let mut sync_head_found = false; + self.sync_head_found = false; if let Some(index) = block_hashes.iter().position(|&hash| hash == sync_head) { - sync_head_found = true; + self.sync_head_found = true; block_hashes = block_hashes.iter().take(index + 1).cloned().collect(); } // Update current fetch head if needed let last_block_hash = last_block_header.compute_block_hash(); - if !sync_head_found { + if !self.sync_head_found { debug!( "Syncing head not found, updated current_head {:?}", last_block_hash ); - search_head = last_block_hash; - current_head = last_block_hash; + self.search_head = last_block_hash; + self.current_head = last_block_hash; if self.sync_mode == SyncMode::Snap { - store.set_header_download_checkpoint(current_head)?; + store.set_header_download_checkpoint(self.current_head)?; } } @@ -278,6 +278,7 @@ impl SyncManager { block_headers.remove(0); // Store headers and save hashes for full block retrieval all_block_hashes.extend_from_slice(&block_hashes[..]); + // This step is necessary for full sync because some opcodes depend on previous blocks during execution. store.add_block_headers(block_hashes.clone(), block_headers.clone())?; if self.sync_mode == SyncMode::Full { @@ -285,14 +286,12 @@ impl SyncManager { &block_hashes, &block_headers, sync_head, - &mut current_head, - &mut search_head, store.clone(), ) .await?; } - if sync_head_found { + if self.sync_head_found { break; }; } @@ -355,8 +354,6 @@ impl SyncManager { block_hashes: &[BlockHash], block_headers: &[BlockHeader], sync_head: BlockHash, - current_head: &mut H256, - search_head: &mut H256, store: Store, ) -> Result<(), SyncError> { let mut current_chunk_idx = 0; @@ -372,60 +369,45 @@ impl SyncManager { let mut headers_iter = block_headers.iter(); let mut blocks: Vec = vec![]; - let max_tries = 10; - let mut tries = 0; - let since = Instant::now(); loop { debug!("Requesting Block Bodies"); - if let Some(block_bodies) = self + let Some(block_bodies) = self .peers .request_block_bodies(current_block_hashes_chunk.clone()) .await - { - let block_bodies_len = block_bodies.len(); + else { + break; + }; - let first_block_hash = current_block_hashes_chunk - .first() - .map_or(H256::default(), |a| *a); + let block_bodies_len = block_bodies.len(); - debug!( - "Received {} Block Bodies, starting from block hash {:?}", - block_bodies_len, first_block_hash - ); + let first_block_hash = current_block_hashes_chunk + .first() + .map_or(H256::default(), |a| *a); - // Push blocks - for (_, body) in current_block_hashes_chunk - .drain(..block_bodies_len) - .zip(block_bodies) - { - let header = headers_iter.next().ok_or(SyncError::BodiesNotFound)?; - let block = Block::new(header.clone(), body); - blocks.push(block); - } + debug!( + "Received {} Block Bodies, starting from block hash {:?}", + block_bodies_len, first_block_hash + ); - if current_block_hashes_chunk.is_empty() { - current_chunk_idx += 1; - current_block_hashes_chunk = match block_hashes_chunks.get(current_chunk_idx) { - Some(res) => res.clone(), - None => break, - }; - }; - } else { - tries += 1; - // If we fail to retrieve block bodies, increment the retry counter. - // This failure could be due to missing bodies for some headers, possibly because: - // - Some headers belong to a sidechain, and not all peers have the corresponding bodies. - // - We are not verifying headers before requesting the bodies so they might be invalid - // - // To mitigate this, we update `current_head` and `search_head` to the last successfully processed block. - // This makes sure that the next header request starts from the last confirmed block (see below). - // - // TODO: validate headers before downloading the bodies - if tries >= max_tries { - break; - } + // Push blocks + for (_, body) in current_block_hashes_chunk + .drain(..block_bodies_len) + .zip(block_bodies) + { + let header = headers_iter.next().ok_or(SyncError::BodiesNotFound)?; + let block = Block::new(header.clone(), body); + blocks.push(block); } + + if current_block_hashes_chunk.is_empty() { + current_chunk_idx += 1; + current_block_hashes_chunk = match block_hashes_chunks.get(current_chunk_idx) { + Some(res) => res.clone(), + None => break, + }; + }; } let blocks_len = blocks.len(); @@ -439,12 +421,10 @@ impl SyncManager { let Some(last_block) = blocks.last().cloned() else { return Err(SyncError::BodiesNotFound); }; - *current_head = last_block.hash(); - *search_head = last_block.hash(); // To ensure proper execution, we set the chain as canonical before processing the blocks. // Some opcodes rely on previous block hashes, and due to our current setup, we only support a single chain (no sidechains). - // As a result, we must set the chain upfront to writing to the database during execution. + // As a result, we must store the headers and set the chain upfront to writing to the database during execution. // Each write operation introduces overhead no matter how small. // // For more details, refer to the `get_block_hash` function in [`LevmDatabase`] and the [`revm::Database`]. @@ -452,7 +432,7 @@ impl SyncManager { .mark_chain_as_canonical(&blocks) .map_err(SyncError::Store)?; - if let Err((error, failure)) = self.add_blocks(&blocks, store.clone()) { + if let Err((error, failure)) = self.add_blocks(&blocks) { warn!("Failed to add block during FullSync: {error}"); if let Some(BatchBlockProcessingFailure { failed_block_hash, @@ -471,9 +451,11 @@ impl SyncManager { return Err(error.into()); } + self.current_head = last_block.hash(); + self.search_head = last_block.hash(); store.update_latest_block_number(last_block.header.number)?; - let elapsed_secs: f64 = since.elapsed().as_millis() as f64 / 1000.0; + let elapsed_secs: f64 = since.elapsed().as_millis() as f64 / 1000.0; let blocks_per_second = blocks_len as f64 / elapsed_secs; info!( @@ -496,18 +478,9 @@ impl SyncManager { fn add_blocks( &self, blocks: &[Block], - store: Store, ) -> Result<(), (ChainError, Option)> { - let last_block = blocks.last().unwrap().clone(); - let latest_block_number = store - .get_latest_block_number() - .map_err(|e| (e.into(), None))?; - - // If the difference between the last block's number and the latest block number is within - // STATE_TRIES_TO_KEEP, process the blocks sequentially so that the state of all blocks is stored. - if last_block.header.number.saturating_sub(latest_block_number) - <= STATE_TRIES_TO_KEEP as u64 - { + // If we found the sync head, run the blocks sequentially to store all the blocks's state + if self.sync_head_found { let mut last_valid_hash = H256::default(); for block in blocks { self.blockchain.add_block(block).map_err(|e| { From 89413d880ce108b7e4fde8d34e82443d16e9c38a Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 25 Mar 2025 16:10:23 -0300 Subject: [PATCH 57/60] revert: moving heads into SyncManager struct --- crates/networking/p2p/sync.rs | 63 ++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 9da1e9b3b2..b373c5b9b1 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -91,9 +91,6 @@ pub struct SyncManager { // Used for cancelling long-living tasks upon shutdown cancel_token: CancellationToken, blockchain: Arc, - search_head: H256, - current_head: H256, - sync_head_found: bool, } impl SyncManager { @@ -111,9 +108,6 @@ impl SyncManager { trie_rebuilder: None, cancel_token, blockchain, - current_head: H256::default(), - search_head: H256::default(), - sync_head_found: false, } } @@ -132,9 +126,6 @@ impl SyncManager { blockchain: Arc::new(Blockchain::default_with_store( Store::new("", EngineType::InMemory).unwrap(), )), - current_head: H256::default(), - search_head: H256::default(), - sync_head_found: false, } } @@ -149,8 +140,7 @@ impl SyncManager { pub async fn start_sync(&mut self, current_head: H256, sync_head: H256, store: Store) { info!("Syncing from current head {current_head} to sync_head {sync_head}"); let start_time = Instant::now(); - self.current_head = current_head; - match self.sync_cycle(sync_head, store).await { + match self.sync_cycle(current_head, sync_head, store).await { Ok(()) => { info!( "Sync cycle finished, time elapsed: {} secs", @@ -165,7 +155,12 @@ impl SyncManager { } /// Performs the sync cycle described in `start_sync`, returns an error if the sync fails at any given step and aborts all active processes - async fn sync_cycle(&mut self, sync_head: H256, store: Store) -> Result<(), SyncError> { + async fn sync_cycle( + &mut self, + mut current_head: H256, + sync_head: H256, + store: Store, + ) -> Result<(), SyncError> { // Request all block headers between the current head and the sync head // We will begin from the current head so that we download the earliest state first // This step is not parallelized @@ -176,7 +171,7 @@ impl SyncManager { if matches!(self.sync_mode, SyncMode::Snap) { if let Some(last_header) = store.get_header_download_checkpoint()? { // Set latest downloaded header as current head for header fetching - self.current_head = last_header; + current_head = last_header; } } @@ -186,13 +181,14 @@ impl SyncManager { }; // TODO(#2126): To avoid modifying the current_head while backtracking we use a separate search_head - self.search_head = self.current_head; + let mut search_head = current_head; loop { - debug!("Requesting Block Headers from {}", self.search_head); + debug!("Requesting Block Headers from {search_head}"); + let Some(mut block_headers) = self .peers - .request_block_headers(self.search_head, BlockRequestOrder::OldToNew) + .request_block_headers(search_head, BlockRequestOrder::OldToNew) .await else { warn!("Sync failed to find target block header, aborting"); @@ -210,12 +206,12 @@ impl SyncManager { // TODO(#2126): This is just a temporary solution to avoid a bug where the sync would get stuck // on a loop when the target head is not found, i.e. on a reorg with a side-chain. if first_block_header == last_block_header - && first_block_header.compute_block_hash() == self.search_head - && self.search_head != sync_head + && first_block_header.compute_block_hash() == search_head + && search_head != sync_head { // There is no path to the sync head this goes back until it find a common ancerstor warn!("Sync failed to find target block header, going back to the previous parent"); - self.search_head = first_block_header.parent_hash; + search_head = first_block_header.parent_hash; continue; } @@ -241,23 +237,23 @@ impl SyncManager { } // Filter out everything after the sync_head - self.sync_head_found = false; + let mut sync_head_found = false; if let Some(index) = block_hashes.iter().position(|&hash| hash == sync_head) { - self.sync_head_found = true; + sync_head_found = true; block_hashes = block_hashes.iter().take(index + 1).cloned().collect(); } // Update current fetch head if needed let last_block_hash = last_block_header.compute_block_hash(); - if !self.sync_head_found { + if !sync_head_found { debug!( "Syncing head not found, updated current_head {:?}", last_block_hash ); - self.search_head = last_block_hash; - self.current_head = last_block_hash; + search_head = last_block_hash; + current_head = last_block_hash; if self.sync_mode == SyncMode::Snap { - store.set_header_download_checkpoint(self.current_head)?; + store.set_header_download_checkpoint(current_head)?; } } @@ -286,12 +282,15 @@ impl SyncManager { &block_hashes, &block_headers, sync_head, + &mut current_head, + &mut search_head, + sync_head_found, store.clone(), ) .await?; } - if self.sync_head_found { + if sync_head_found { break; }; } @@ -354,6 +353,9 @@ impl SyncManager { block_hashes: &[BlockHash], block_headers: &[BlockHeader], sync_head: BlockHash, + current_head: &mut BlockHash, + search_head: &mut BlockHash, + sync_head_found: bool, store: Store, ) -> Result<(), SyncError> { let mut current_chunk_idx = 0; @@ -432,7 +434,7 @@ impl SyncManager { .mark_chain_as_canonical(&blocks) .map_err(SyncError::Store)?; - if let Err((error, failure)) = self.add_blocks(&blocks) { + if let Err((error, failure)) = self.add_blocks(&blocks, sync_head_found) { warn!("Failed to add block during FullSync: {error}"); if let Some(BatchBlockProcessingFailure { failed_block_hash, @@ -451,8 +453,8 @@ impl SyncManager { return Err(error.into()); } - self.current_head = last_block.hash(); - self.search_head = last_block.hash(); + *current_head = last_block.hash(); + *search_head = last_block.hash(); store.update_latest_block_number(last_block.header.number)?; let elapsed_secs: f64 = since.elapsed().as_millis() as f64 / 1000.0; @@ -478,9 +480,10 @@ impl SyncManager { fn add_blocks( &self, blocks: &[Block], + sync_head_found: bool, ) -> Result<(), (ChainError, Option)> { // If we found the sync head, run the blocks sequentially to store all the blocks's state - if self.sync_head_found { + if sync_head_found { let mut last_valid_hash = H256::default(); for block in blocks { self.blockchain.add_block(block).map_err(|e| { From cd705a0d83494a8f8143a4780069a3a77bf79165 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau <76252340+MarcosNicolau@users.noreply.github.com> Date: Tue, 25 Mar 2025 16:13:24 -0300 Subject: [PATCH 58/60] Update crates/networking/p2p/peer_handler.rs Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com> --- crates/networking/p2p/peer_handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 742c9042be..237ec646be 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -36,7 +36,7 @@ pub const MAX_RESPONSE_BYTES: u64 = 512 * 1024; pub const HASH_MAX: H256 = H256([0xFF; 32]); // Ask as much as 128 block bodies per request -// this magic number are not part of the protocol and is taken from geth, see: +// this magic number is not part of the protocol and is taken from geth, see: // https://github.com/ethereum/go-ethereum/blob/2585776aabbd4ae9b00050403b42afb0cee968ec/eth/downloader/downloader.go#L42-L43 // // Note: We noticed that while bigger values are supported From c986f4eab4ec9595abad64019b50ae79ce859b6b Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 25 Mar 2025 16:16:35 -0300 Subject: [PATCH 59/60] chore: derive debug for BatchBlockProcessingFailure --- crates/blockchain/blockchain.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index daf36d6a38..9f77dce405 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -39,6 +39,7 @@ pub struct Blockchain { pub mempool: Mempool, } +#[derive(Debug, Clone)] pub struct BatchBlockProcessingFailure { pub last_valid_hash: H256, pub failed_block_hash: H256, From 6c037c737eba382230e01a359eb06eaf8ed29277 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 25 Mar 2025 17:14:46 -0300 Subject: [PATCH 60/60] fix: lint --- crates/networking/p2p/sync.rs | 40 ++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index b373c5b9b1..c331b26dda 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -278,16 +278,19 @@ impl SyncManager { store.add_block_headers(block_hashes.clone(), block_headers.clone())?; if self.sync_mode == SyncMode::Full { - self.download_and_run_blocks( - &block_hashes, - &block_headers, - sync_head, - &mut current_head, - &mut search_head, - sync_head_found, - store.clone(), - ) - .await?; + let last_block_hash = self + .download_and_run_blocks( + &block_hashes, + &block_headers, + sync_head, + sync_head_found, + store.clone(), + ) + .await?; + if let Some(last_block_hash) = last_block_hash { + current_head = last_block_hash; + search_head = current_head; + } } if sync_head_found { @@ -346,18 +349,19 @@ impl SyncManager { Ok(()) } - /// Requests block bodies from peers via p2p, executes and stores them - /// Returns an error if there was a problem while executing or validating the blocks + /// Attempts to fetch up to 1024 block bodies from peers via P2P, starting from the sync head. + /// Executes and stores the retrieved blocks. + /// + /// Returns an error if execution or validation fails. + /// On success, returns the hash of the last successfully executed block body. async fn download_and_run_blocks( &mut self, block_hashes: &[BlockHash], block_headers: &[BlockHeader], sync_head: BlockHash, - current_head: &mut BlockHash, - search_head: &mut BlockHash, sync_head_found: bool, store: Store, - ) -> Result<(), SyncError> { + ) -> Result, SyncError> { let mut current_chunk_idx = 0; let block_hashes_chunks: Vec> = block_hashes .chunks(MAX_BLOCK_BODIES_TO_REQUEST) @@ -366,7 +370,7 @@ impl SyncManager { let mut current_block_hashes_chunk = match block_hashes_chunks.get(current_chunk_idx) { Some(res) => res.clone(), - None => return Ok(()), + None => return Ok(None), }; let mut headers_iter = block_headers.iter(); let mut blocks: Vec = vec![]; @@ -453,8 +457,6 @@ impl SyncManager { return Err(error.into()); } - *current_head = last_block.hash(); - *search_head = last_block.hash(); store.update_latest_block_number(last_block.header.number)?; let elapsed_secs: f64 = since.elapsed().as_millis() as f64 / 1000.0; @@ -474,7 +476,7 @@ impl SyncManager { blocks_per_second ); - Ok(()) + Ok(Some(last_block.hash())) } fn add_blocks(