From af76565f9abcae91d257f9e55833ebb69b2a5c4e Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 17:58:02 -0300 Subject: [PATCH 01/20] Initial version --- crates/networking/p2p/peer_handler.rs | 72 ++++++++++++++++++--- crates/networking/p2p/sync.rs | 11 +++- crates/networking/p2p/sync/state_healing.rs | 13 +++- 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index a56e968cba..37bb94e80d 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1,5 +1,5 @@ use std::{ - collections::{HashSet, VecDeque}, + collections::{HashMap, HashSet, VecDeque}, io::ErrorKind, path::{Path, PathBuf}, sync::atomic::Ordering, @@ -1289,6 +1289,8 @@ impl PeerHandler { // list of tasks to be executed // Types are (start_index, end_index, starting_hash) // NOTE: end_index is NOT inclusive + + // TODO: here we need to push, for each big account, its current start/end_hash stuff. let mut tasks_queue_not_started = VecDeque::::new(); for i in 0..chunk_count { let chunk_start = chunk_size * i; @@ -1318,7 +1320,8 @@ impl PeerHandler { let mut completed_tasks = 0; // TODO: in a refactor, delete this replace with a structure that can handle removes - let mut accounts_done: Vec = Vec::new(); + // let mut accounts_done: Vec<(H256, Vec<(H256, H256)>)> = Vec::new(); + let mut accounts_done: HashMap> = HashMap::new(); let current_account_hashes = account_storage_roots .accounts_with_storage_root .iter() @@ -1384,8 +1387,21 @@ impl PeerHandler { self.peer_table.free_peer(peer_id).await; - for account in ¤t_account_hashes[start_index..remaining_start] { - accounts_done.push(*account); + for (index, account) in current_account_hashes[start_index..remaining_start] + .iter() + .enumerate() + { + // accounts_done.push((*account, vec![])); + // If this is not a big account, insert it as done. + // If it's a big account, its logic will be handled below + + if index + 1 < remaining_start { + accounts_done.insert(*account, vec![]); + } else { + if hash_start.is_zero() { + accounts_done.insert(*account, vec![]); + } + } } if remaining_start < remaining_end { @@ -1411,10 +1427,28 @@ impl PeerHandler { }; tasks_queue_not_started.push_back(task); task_count += 1; - accounts_done.push(current_account_hashes[remaining_start]); + // accounts_done.push(current_account_hashes[remaining_start]); + let old_intervals = accounts_done + .get_mut(¤t_account_hashes[remaining_start]) + .unwrap(); + for (old_start, end) in old_intervals { + if end == &hash_end { + *old_start = hash_start; + } + } account_storage_roots .healed_accounts .insert(current_account_hashes[start_index]); + } else { + let old_intervals = accounts_done + .get_mut(¤t_account_hashes[remaining_start]) + .unwrap(); + old_intervals.remove( + old_intervals + .iter() + .position(|(_old_start, end)| end == &hash_end) + .unwrap(), + ); } } else { if remaining_start + 1 < remaining_end { @@ -1445,6 +1479,13 @@ impl PeerHandler { let chunk_count = (missing_storage_range / chunk_size).as_usize().max(1); + // *intervals = Some(vec![]); + + accounts_done.insert(current_account_hashes[remaining_start], vec![]); + let intervals = accounts_done + .get_mut(¤t_account_hashes[remaining_start]) + .unwrap(); + for i in 0..chunk_count { let start_hash_u256 = start_hash_u256 + chunk_size * i; let start_hash = H256::from_uint(&start_hash_u256); @@ -1462,6 +1503,9 @@ impl PeerHandler { start_hash, end_hash: Some(end_hash), }; + + intervals.push((start_hash, end_hash)); + tasks_queue_not_started.push_back(task); task_count += 1; } @@ -1536,7 +1580,7 @@ impl PeerHandler { .iter() .skip(task.start_index) .take(task.end_index - task.start_index) - .map(|(hash, root)| (*hash, *root)) + .map(|(hash, (root, _))| (*hash, *root)) .unzip(); if task_count - completed_tasks < 30 { @@ -1594,12 +1638,20 @@ impl PeerHandler { .collect::, DumpError>>() .map_err(PeerHandlerError::DumpError)?; - for account_done in accounts_done { - account_storage_roots - .accounts_with_storage_root - .remove(&account_done); + for (account_done, intervals) in accounts_done { + if intervals.is_empty() { + account_storage_roots + .accounts_with_storage_root + .remove(&account_done); + } } + // accounts_with_storage_root tracks accounts to download storage ranges by + // setting account_hash -> storage_root (root for verify_range) + + // We need to track the following: (storage_root, Vec) with the vec having the start and + // end of each task to resume on a big account. The storage root needs to be modified each time the pivot jumps. + // Dropping the task sender so that the recv returns None drop(task_sender); diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 1e03858383..f29bf671fa 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -905,7 +905,7 @@ impl Syncer { .zip(account_states.iter()) .filter_map(|(hash, state)| { (state.storage_root != *EMPTY_TRIE_HASH) - .then_some((*hash, state.storage_root)) + .then_some((*hash, (state.storage_root, vec![]))) }), ); @@ -1342,12 +1342,19 @@ pub fn calculate_staleness_timestamp(timestamp: u64) -> u64 { pub struct AccountStorageRoots { /// The accounts that have not been healed are guaranteed to have the original storage root /// we can read this storage root - pub accounts_with_storage_root: BTreeMap, + // pub accounts_with_storage_root: BTreeMap, + pub accounts_with_storage_root: BTreeMap)>, /// If an account has been healed, it may return to a previous state, so we just store the account /// in a hashset pub healed_accounts: HashSet, } +// accounts_with_storage_root tracks accounts to download storage ranges by +// setting account_hash -> storage_root (root for verify_range) + +// We need to track the following: (storage_root, Vec) with the vec having the start and +// end of each task to resume on a big account. The storage root needs to be modified each time the pivot jumps. + #[derive(thiserror::Error, Debug)] pub enum SyncError { #[error(transparent)] diff --git a/crates/networking/p2p/sync/state_healing.rs b/crates/networking/p2p/sync/state_healing.rs index 9ea8c6caa1..ab6f2f78de 100644 --- a/crates/networking/p2p/sync/state_healing.rs +++ b/crates/networking/p2p/sync/state_healing.rs @@ -181,9 +181,18 @@ async fn heal_state_trie( } storage_accounts.healed_accounts.insert(account_hash); - storage_accounts + // Instead of removing, here we should set its new storage root value while + // keeping its vec of stuff intact. That way request storage ranges can resume + // with the new storage root. + let old_value = storage_accounts .accounts_with_storage_root - .remove(&account_hash); + .get_mut(&account_hash); + if let Some((old_root, _)) = old_value { + *old_root = account.storage_root; + } + // storage_accounts + // .accounts_with_storage_root + // .remove(&account_hash); } } leafs_healed += nodes From 24ada48e97c4fd776872b39b6c6926a1cd6e19d7 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 18:01:17 -0300 Subject: [PATCH 02/20] Lower snap limit for testing --- 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 37bb94e80d..948052324e 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -55,7 +55,7 @@ pub const MAX_HEADER_CHUNK: u64 = 500_000; // before we dump it into the file. This tunes how much memory ethrex uses during // the first steps of snap sync pub const RANGE_FILE_CHUNK_SIZE: usize = 1024 * 1024 * 512; // 512MB -pub const SNAP_LIMIT: usize = 128; +pub const SNAP_LIMIT: usize = 30; // Request as many as 128 block bodies per request // this magic number is not part of the protocol and is taken from geth, see: From 52180493c49343383db6758e124fff8bce4b4d12 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 18:05:18 -0300 Subject: [PATCH 03/20] Fix compilation --- crates/networking/p2p/sync/storage_healing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/networking/p2p/sync/storage_healing.rs b/crates/networking/p2p/sync/storage_healing.rs index 13474c4649..5386c03078 100644 --- a/crates/networking/p2p/sync/storage_healing.rs +++ b/crates/networking/p2p/sync/storage_healing.rs @@ -547,7 +547,7 @@ fn get_initial_downloads( account_paths .accounts_with_storage_root .par_iter() - .filter_map(|(acc_path, storage_root)| { + .filter_map(|(acc_path, (storage_root, _))| { if store .contains_storage_node(*acc_path, *storage_root) .expect("We should be able to open the store") From 2174f27169324573a38cf9d7d8962d5bcfe695b5 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 18:26:38 -0300 Subject: [PATCH 04/20] Debug stuff --- crates/networking/p2p/peer_handler.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 948052324e..009f30f34a 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1443,10 +1443,15 @@ impl PeerHandler { let old_intervals = accounts_done .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); + dbg!(¤t_account_hashes[remaining_start]); old_intervals.remove( old_intervals .iter() - .position(|(_old_start, end)| end == &hash_end) + .position(|(_old_start, end)| { + dbg!(&end); + dbg!(&hash_end); + end == &hash_end + }) .unwrap(), ); } From bf7717e0ff554be372b532045ea82df6ec4d57df Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 18:43:09 -0300 Subject: [PATCH 05/20] More debug stuff --- crates/networking/p2p/peer_handler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 009f30f34a..748b75dad3 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1443,13 +1443,14 @@ impl PeerHandler { let old_intervals = accounts_done .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); + dbg!(&old_intervals); dbg!(¤t_account_hashes[remaining_start]); + dbg!(&hash_end); old_intervals.remove( old_intervals .iter() .position(|(_old_start, end)| { dbg!(&end); - dbg!(&hash_end); end == &hash_end }) .unwrap(), From eff53c741d2d2cd5ddc4dfb5feaca374d30cbf10 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 19:00:23 -0300 Subject: [PATCH 06/20] Fix indexing bug --- 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 748b75dad3..cb68119dfc 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1395,7 +1395,7 @@ impl PeerHandler { // If this is not a big account, insert it as done. // If it's a big account, its logic will be handled below - if index + 1 < remaining_start { + if start_index + index + 1 < remaining_start { accounts_done.insert(*account, vec![]); } else { if hash_start.is_zero() { From 724ef1befdce53df3189141ae54353c43c4050b7 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 19:20:28 -0300 Subject: [PATCH 07/20] FIx --- crates/networking/p2p/peer_handler.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index cb68119dfc..079df59a38 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1387,21 +1387,10 @@ impl PeerHandler { self.peer_table.free_peer(peer_id).await; - for (index, account) in current_account_hashes[start_index..remaining_start] - .iter() - .enumerate() - { - // accounts_done.push((*account, vec![])); + for account in current_account_hashes[start_index..remaining_start].iter() { // If this is not a big account, insert it as done. // If it's a big account, its logic will be handled below - - if start_index + index + 1 < remaining_start { - accounts_done.insert(*account, vec![]); - } else { - if hash_start.is_zero() { - accounts_done.insert(*account, vec![]); - } - } + accounts_done.insert(*account, vec![]); } if remaining_start < remaining_end { From 46cc1b52d150841580fbe157bddaae608f4a61eb Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 19:32:36 -0300 Subject: [PATCH 08/20] Test --- crates/networking/p2p/peer_handler.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 079df59a38..f76a92e501 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1390,7 +1390,9 @@ impl PeerHandler { for account in current_account_hashes[start_index..remaining_start].iter() { // If this is not a big account, insert it as done. // If it's a big account, its logic will be handled below - accounts_done.insert(*account, vec![]); + if !accounts_done.contains_key(account) { + accounts_done.insert(*account, vec![]); + } } if remaining_start < remaining_end { From 9cd44e0c74cf83ef59af134fbe8b94f9253946c5 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 19:43:25 -0300 Subject: [PATCH 09/20] comment dbg! calls --- crates/networking/p2p/peer_handler.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index f76a92e501..9ef444ea41 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1434,14 +1434,14 @@ impl PeerHandler { let old_intervals = accounts_done .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); - dbg!(&old_intervals); - dbg!(¤t_account_hashes[remaining_start]); - dbg!(&hash_end); + // dbg!(&old_intervals); + // dbg!(¤t_account_hashes[remaining_start]); + // dbg!(&hash_end); old_intervals.remove( old_intervals .iter() .position(|(_old_start, end)| { - dbg!(&end); + // dbg!(&end); end == &hash_end }) .unwrap(), From e0b922ace85ded7e50b22844f31c80c993680304 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 23:17:12 -0300 Subject: [PATCH 10/20] Test --- crates/networking/p2p/sync/storage_healing.rs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/networking/p2p/sync/storage_healing.rs b/crates/networking/p2p/sync/storage_healing.rs index 5386c03078..b647146c40 100644 --- a/crates/networking/p2p/sync/storage_healing.rs +++ b/crates/networking/p2p/sync/storage_healing.rs @@ -543,26 +543,26 @@ fn get_initial_downloads( }) .collect::>(), ); - initial_requests.extend( - account_paths - .accounts_with_storage_root - .par_iter() - .filter_map(|(acc_path, (storage_root, _))| { - if store - .contains_storage_node(*acc_path, *storage_root) - .expect("We should be able to open the store") - { - return None; - } - Some(NodeRequest { - acc_path: Nibbles::from_bytes(&acc_path.0), - storage_path: Nibbles::default(), // We need to be careful, the root parent is a special case - parent: Nibbles::default(), - hash: *storage_root, - }) - }) - .collect::>(), - ); + // initial_requests.extend( + // account_paths + // .accounts_with_storage_root + // .par_iter() + // .filter_map(|(acc_path, (storage_root, _))| { + // if store + // .contains_storage_node(*acc_path, *storage_root) + // .expect("We should be able to open the store") + // { + // return None; + // } + // Some(NodeRequest { + // acc_path: Nibbles::from_bytes(&acc_path.0), + // storage_path: Nibbles::default(), // We need to be careful, the root parent is a special case + // parent: Nibbles::default(), + // hash: *storage_root, + // }) + // }) + // .collect::>(), + // ); initial_requests } From bce4a6e0d112c4956ec40963b0cde8f580fe382b Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Tue, 23 Sep 2025 23:57:18 -0300 Subject: [PATCH 11/20] Test --- crates/networking/p2p/peer_handler.rs | 74 ++++++++++++++++----------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 9ef444ea41..2a5c8755f4 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1476,37 +1476,53 @@ impl PeerHandler { let chunk_count = (missing_storage_range / chunk_size).as_usize().max(1); - // *intervals = Some(vec![]); - - accounts_done.insert(current_account_hashes[remaining_start], vec![]); - let intervals = accounts_done - .get_mut(¤t_account_hashes[remaining_start]) - .unwrap(); - - for i in 0..chunk_count { - let start_hash_u256 = start_hash_u256 + chunk_size * i; - let start_hash = H256::from_uint(&start_hash_u256); - let end_hash = if i == chunk_count - 1 { - H256::repeat_byte(0xff) - } else { - let end_hash_u256 = - start_hash_u256.checked_add(chunk_size).unwrap_or(U256::MAX); - H256::from_uint(&end_hash_u256) - }; - - let task = StorageTask { - start_index: remaining_start, - end_index: remaining_start + 1, - start_hash, - end_hash: Some(end_hash), - }; - - intervals.push((start_hash, end_hash)); + let maybe_old_intervals = + accounts_done.get(¤t_account_hashes[remaining_start]); + + if let Some(old_intervals) = maybe_old_intervals { + for (start_hash, end_hash) in old_intervals { + let task = StorageTask { + start_index: remaining_start, + end_index: remaining_start + 1, + start_hash: *start_hash, + end_hash: Some(*end_hash), + }; + + tasks_queue_not_started.push_back(task); + task_count += 1; + } + } else { + accounts_done.insert(current_account_hashes[remaining_start], vec![]); + let intervals = accounts_done + .get_mut(¤t_account_hashes[remaining_start]) + .unwrap(); - tasks_queue_not_started.push_back(task); - task_count += 1; + for i in 0..chunk_count { + let start_hash_u256 = start_hash_u256 + chunk_size * i; + let start_hash = H256::from_uint(&start_hash_u256); + let end_hash = if i == chunk_count - 1 { + H256::repeat_byte(0xff) + } else { + let end_hash_u256 = start_hash_u256 + .checked_add(chunk_size) + .unwrap_or(U256::MAX); + H256::from_uint(&end_hash_u256) + }; + + let task = StorageTask { + start_index: remaining_start, + end_index: remaining_start + 1, + start_hash, + end_hash: Some(end_hash), + }; + + intervals.push((start_hash, end_hash)); + + tasks_queue_not_started.push_back(task); + task_count += 1; + } + debug!("Split big storage account into {chunk_count} chunks."); } - debug!("Split big storage account into {chunk_count} chunks."); } } From 10ac1897e452458d131e42246b592508b50c64d5 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 01:18:52 -0300 Subject: [PATCH 12/20] Increase chunk_limit on request_storage_range --- 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 2a5c8755f4..fee49bb696 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1283,7 +1283,7 @@ impl PeerHandler { *METRICS.current_step.lock().await = "Requesting Storage Ranges".to_string(); debug!("Starting request_storage_ranges function"); // 1) split the range in chunks of same length - let chunk_size = 300; + let chunk_size = 1600; let chunk_count = (account_storage_roots.accounts_with_storage_root.len() / chunk_size) + 1; // list of tasks to be executed From 7ceb7233549c4255a6c53f06aed0d848d0f73e08 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 16:07:44 -0300 Subject: [PATCH 13/20] Handle accounts that go back to their previous state --- crates/networking/p2p/peer_handler.rs | 14 +++++++++++++- crates/networking/p2p/sync.rs | 5 +++-- crates/networking/p2p/sync/state_healing.rs | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index fee49bb696..a44ca7f6a4 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -12,6 +12,7 @@ use ethrex_common::{ types::{AccountState, BlockBody, BlockHeader, Receipt, validate_block_body}, }; use ethrex_rlp::encode::RLPEncode; +use ethrex_storage::Store; use ethrex_trie::Nibbles; use ethrex_trie::{Node, verify_range}; use rand::seq::SliceRandom; @@ -1279,6 +1280,7 @@ impl PeerHandler { account_storages_snapshots_dir: &Path, mut chunk_index: u64, pivot_header: &mut BlockHeader, + store: Store, ) -> Result { *METRICS.current_step.lock().await = "Requesting Storage Ranges".to_string(); debug!("Starting request_storage_ranges function"); @@ -1593,7 +1595,17 @@ impl PeerHandler { .iter() .skip(task.start_index) .take(task.end_index - task.start_index) - .map(|(hash, (root, _))| (*hash, *root)) + .map(|(hash, (root, _))| match root { + Some(root) => (*hash, *root), + None => ( + *hash, + store + .get_account_state_by_acc_hash(pivot_header.hash(), *hash) + .unwrap() + .unwrap() + .storage_root, + ), + }) .unzip(); if task_count - completed_tasks < 30 { diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index f29bf671fa..938638ab37 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -905,7 +905,7 @@ impl Syncer { .zip(account_states.iter()) .filter_map(|(hash, state)| { (state.storage_root != *EMPTY_TRIE_HASH) - .then_some((*hash, (state.storage_root, vec![]))) + .then_some((*hash, (Some(state.storage_root), vec![]))) }), ); @@ -999,6 +999,7 @@ impl Syncer { account_storages_snapshots_dir.as_ref(), chunk_index, &mut pivot_header, + store.clone(), ) .await .map_err(SyncError::PeerHandler)?; @@ -1343,7 +1344,7 @@ pub struct AccountStorageRoots { /// The accounts that have not been healed are guaranteed to have the original storage root /// we can read this storage root // pub accounts_with_storage_root: BTreeMap, - pub accounts_with_storage_root: BTreeMap)>, + pub accounts_with_storage_root: BTreeMap, Vec<(H256, H256)>)>, /// If an account has been healed, it may return to a previous state, so we just store the account /// in a hashset pub healed_accounts: HashSet, diff --git a/crates/networking/p2p/sync/state_healing.rs b/crates/networking/p2p/sync/state_healing.rs index ab6f2f78de..75249c7fae 100644 --- a/crates/networking/p2p/sync/state_healing.rs +++ b/crates/networking/p2p/sync/state_healing.rs @@ -188,7 +188,7 @@ async fn heal_state_trie( .accounts_with_storage_root .get_mut(&account_hash); if let Some((old_root, _)) = old_value { - *old_root = account.storage_root; + *old_root = None; } // storage_accounts // .accounts_with_storage_root From 0b39d50da9374d6c07543072b5a245d985387c69 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 16:18:15 -0300 Subject: [PATCH 14/20] Fix --- crates/networking/p2p/peer_handler.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index a44ca7f6a4..fd54efb043 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1436,16 +1436,10 @@ impl PeerHandler { let old_intervals = accounts_done .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); - // dbg!(&old_intervals); - // dbg!(¤t_account_hashes[remaining_start]); - // dbg!(&hash_end); old_intervals.remove( old_intervals .iter() - .position(|(_old_start, end)| { - // dbg!(&end); - end == &hash_end - }) + .position(|(_old_start, end)| end == &hash_end) .unwrap(), ); } @@ -1478,10 +1472,11 @@ impl PeerHandler { let chunk_count = (missing_storage_range / chunk_size).as_usize().max(1); - let maybe_old_intervals = - accounts_done.get(¤t_account_hashes[remaining_start]); + let maybe_old_intervals = account_storage_roots + .accounts_with_storage_root + .get(¤t_account_hashes[remaining_start]); - if let Some(old_intervals) = maybe_old_intervals { + if let Some((_, old_intervals)) = maybe_old_intervals { for (start_hash, end_hash) in old_intervals { let task = StorageTask { start_index: remaining_start, From be20007bf51f6e20db60deba80711be4390b9660 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 17:56:59 -0300 Subject: [PATCH 15/20] Properly use accounts_with_storage_root instead of accounts_done --- crates/networking/p2p/peer_handler.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index fd54efb043..954b49c04b 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1421,7 +1421,8 @@ impl PeerHandler { tasks_queue_not_started.push_back(task); task_count += 1; // accounts_done.push(current_account_hashes[remaining_start]); - let old_intervals = accounts_done + let (_, old_intervals) = account_storage_roots + .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); for (old_start, end) in old_intervals { @@ -1433,7 +1434,8 @@ impl PeerHandler { .healed_accounts .insert(current_account_hashes[start_index]); } else { - let old_intervals = accounts_done + let (_, old_intervals) = account_storage_roots + .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); old_intervals.remove( @@ -1442,6 +1444,10 @@ impl PeerHandler { .position(|(_old_start, end)| end == &hash_end) .unwrap(), ); + if old_intervals.is_empty() { + accounts_done + .insert(current_account_hashes[remaining_start], vec![]); + } } } else { if remaining_start + 1 < remaining_end { @@ -1489,8 +1495,11 @@ impl PeerHandler { task_count += 1; } } else { - accounts_done.insert(current_account_hashes[remaining_start], vec![]); - let intervals = accounts_done + account_storage_roots + .accounts_with_storage_root + .insert(current_account_hashes[remaining_start], (None, vec![])); + let (_, intervals) = account_storage_roots + .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) .unwrap(); From a0bec1a313f3f104374e9430558345439e8c47aa Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 18:50:01 -0300 Subject: [PATCH 16/20] Test --- crates/networking/p2p/peer_handler.rs | 57 ++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index 954b49c04b..b8f572f3ef 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1483,16 +1483,53 @@ impl PeerHandler { .get(¤t_account_hashes[remaining_start]); if let Some((_, old_intervals)) = maybe_old_intervals { - for (start_hash, end_hash) in old_intervals { - let task = StorageTask { - start_index: remaining_start, - end_index: remaining_start + 1, - start_hash: *start_hash, - end_hash: Some(*end_hash), - }; - - tasks_queue_not_started.push_back(task); - task_count += 1; + if !old_intervals.is_empty() { + for (start_hash, end_hash) in old_intervals { + let task = StorageTask { + start_index: remaining_start, + end_index: remaining_start + 1, + start_hash: *start_hash, + end_hash: Some(*end_hash), + }; + + tasks_queue_not_started.push_back(task); + task_count += 1; + } + } else { + account_storage_roots.accounts_with_storage_root.insert( + current_account_hashes[remaining_start], + (None, vec![]), + ); + let (_, intervals) = account_storage_roots + .accounts_with_storage_root + .get_mut(¤t_account_hashes[remaining_start]) + .unwrap(); + + for i in 0..chunk_count { + let start_hash_u256 = start_hash_u256 + chunk_size * i; + let start_hash = H256::from_uint(&start_hash_u256); + let end_hash = if i == chunk_count - 1 { + H256::repeat_byte(0xff) + } else { + let end_hash_u256 = start_hash_u256 + .checked_add(chunk_size) + .unwrap_or(U256::MAX); + H256::from_uint(&end_hash_u256) + }; + + let task = StorageTask { + start_index: remaining_start, + end_index: remaining_start + 1, + start_hash, + end_hash: Some(end_hash), + }; + + intervals.push((start_hash, end_hash)); + + tasks_queue_not_started.push_back(task); + task_count += 1; + } + debug!("Split big storage account into {chunk_count} chunks."); } } else { account_storage_roots From fb460c24c2ea96727ce1dc8b88b78f29fd504292 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 21:27:19 -0300 Subject: [PATCH 17/20] Remove commented code --- crates/networking/p2p/sync/storage_healing.rs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/crates/networking/p2p/sync/storage_healing.rs b/crates/networking/p2p/sync/storage_healing.rs index b647146c40..a331951d77 100644 --- a/crates/networking/p2p/sync/storage_healing.rs +++ b/crates/networking/p2p/sync/storage_healing.rs @@ -543,26 +543,6 @@ fn get_initial_downloads( }) .collect::>(), ); - // initial_requests.extend( - // account_paths - // .accounts_with_storage_root - // .par_iter() - // .filter_map(|(acc_path, (storage_root, _))| { - // if store - // .contains_storage_node(*acc_path, *storage_root) - // .expect("We should be able to open the store") - // { - // return None; - // } - // Some(NodeRequest { - // acc_path: Nibbles::from_bytes(&acc_path.0), - // storage_path: Nibbles::default(), // We need to be careful, the root parent is a special case - // parent: Nibbles::default(), - // hash: *storage_root, - // }) - // }) - // .collect::>(), - // ); initial_requests } From c55e74aa5af32bd027d38d429c750efafb9950f9 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 21:29:06 -0300 Subject: [PATCH 18/20] Remove commented stuff --- crates/networking/p2p/peer_handler.rs | 13 +------------ crates/networking/p2p/sync.rs | 7 ------- crates/networking/p2p/sync/state_healing.rs | 6 ------ 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index b8f572f3ef..c8be36d820 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -56,7 +56,7 @@ pub const MAX_HEADER_CHUNK: u64 = 500_000; // before we dump it into the file. This tunes how much memory ethrex uses during // the first steps of snap sync pub const RANGE_FILE_CHUNK_SIZE: usize = 1024 * 1024 * 512; // 512MB -pub const SNAP_LIMIT: usize = 30; +pub const SNAP_LIMIT: usize = 128; // Request as many as 128 block bodies per request // this magic number is not part of the protocol and is taken from geth, see: @@ -1292,7 +1292,6 @@ impl PeerHandler { // Types are (start_index, end_index, starting_hash) // NOTE: end_index is NOT inclusive - // TODO: here we need to push, for each big account, its current start/end_hash stuff. let mut tasks_queue_not_started = VecDeque::::new(); for i in 0..chunk_count { let chunk_start = chunk_size * i; @@ -1322,7 +1321,6 @@ impl PeerHandler { let mut completed_tasks = 0; // TODO: in a refactor, delete this replace with a structure that can handle removes - // let mut accounts_done: Vec<(H256, Vec<(H256, H256)>)> = Vec::new(); let mut accounts_done: HashMap> = HashMap::new(); let current_account_hashes = account_storage_roots .accounts_with_storage_root @@ -1390,8 +1388,6 @@ impl PeerHandler { self.peer_table.free_peer(peer_id).await; for account in current_account_hashes[start_index..remaining_start].iter() { - // If this is not a big account, insert it as done. - // If it's a big account, its logic will be handled below if !accounts_done.contains_key(account) { accounts_done.insert(*account, vec![]); } @@ -1420,7 +1416,6 @@ impl PeerHandler { }; tasks_queue_not_started.push_back(task); task_count += 1; - // accounts_done.push(current_account_hashes[remaining_start]); let (_, old_intervals) = account_storage_roots .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) @@ -1712,12 +1707,6 @@ impl PeerHandler { } } - // accounts_with_storage_root tracks accounts to download storage ranges by - // setting account_hash -> storage_root (root for verify_range) - - // We need to track the following: (storage_root, Vec) with the vec having the start and - // end of each task to resume on a big account. The storage root needs to be modified each time the pivot jumps. - // Dropping the task sender so that the recv returns None drop(task_sender); diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 938638ab37..35148a8916 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -1343,19 +1343,12 @@ pub fn calculate_staleness_timestamp(timestamp: u64) -> u64 { pub struct AccountStorageRoots { /// The accounts that have not been healed are guaranteed to have the original storage root /// we can read this storage root - // pub accounts_with_storage_root: BTreeMap, pub accounts_with_storage_root: BTreeMap, Vec<(H256, H256)>)>, /// If an account has been healed, it may return to a previous state, so we just store the account /// in a hashset pub healed_accounts: HashSet, } -// accounts_with_storage_root tracks accounts to download storage ranges by -// setting account_hash -> storage_root (root for verify_range) - -// We need to track the following: (storage_root, Vec) with the vec having the start and -// end of each task to resume on a big account. The storage root needs to be modified each time the pivot jumps. - #[derive(thiserror::Error, Debug)] pub enum SyncError { #[error(transparent)] diff --git a/crates/networking/p2p/sync/state_healing.rs b/crates/networking/p2p/sync/state_healing.rs index 75249c7fae..08214734b9 100644 --- a/crates/networking/p2p/sync/state_healing.rs +++ b/crates/networking/p2p/sync/state_healing.rs @@ -181,18 +181,12 @@ async fn heal_state_trie( } storage_accounts.healed_accounts.insert(account_hash); - // Instead of removing, here we should set its new storage root value while - // keeping its vec of stuff intact. That way request storage ranges can resume - // with the new storage root. let old_value = storage_accounts .accounts_with_storage_root .get_mut(&account_hash); if let Some((old_root, _)) = old_value { *old_root = None; } - // storage_accounts - // .accounts_with_storage_root - // .remove(&account_hash); } } leafs_healed += nodes From b1e71cae10aef0596725ff77536d0e6c4d56de7e Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Wed, 24 Sep 2025 21:30:58 -0300 Subject: [PATCH 19/20] Add TODO to address before merging --- crates/networking/p2p/peer_handler.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index c8be36d820..c9540b3dca 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1491,6 +1491,7 @@ impl PeerHandler { task_count += 1; } } else { + // TODO: DRY account_storage_roots.accounts_with_storage_root.insert( current_account_hashes[remaining_start], (None, vec![]), From 565fd13594745dc568ea8b145a0e76d19cf36e92 Mon Sep 17 00:00:00 2001 From: Javier Chatruc Date: Thu, 25 Sep 2025 16:54:08 -0300 Subject: [PATCH 20/20] Clippy --- crates/networking/p2p/peer_handler.rs | 20 ++++++++++++-------- crates/networking/p2p/sync.rs | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/networking/p2p/peer_handler.rs b/crates/networking/p2p/peer_handler.rs index c9540b3dca..bdd4c56507 100644 --- a/crates/networking/p2p/peer_handler.rs +++ b/crates/networking/p2p/peer_handler.rs @@ -1418,8 +1418,7 @@ impl PeerHandler { task_count += 1; let (_, old_intervals) = account_storage_roots .accounts_with_storage_root - .get_mut(¤t_account_hashes[remaining_start]) - .unwrap(); + .get_mut(¤t_account_hashes[remaining_start]).ok_or(PeerHandlerError::UnrecoverableError("Tried to get the old download intervals for an account but did not find them".to_owned()))?; for (old_start, end) in old_intervals { if end == &hash_end { *old_start = hash_start; @@ -1432,12 +1431,15 @@ impl PeerHandler { let (_, old_intervals) = account_storage_roots .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) - .unwrap(); + .ok_or(PeerHandlerError::UnrecoverableError("Tried to get the old download intervals for an account but did not find them".to_owned()))?; old_intervals.remove( old_intervals .iter() .position(|(_old_start, end)| end == &hash_end) - .unwrap(), + .ok_or(PeerHandlerError::UnrecoverableError( + "Could not find an old interval that we were tracking" + .to_owned(), + ))?, ); if old_intervals.is_empty() { accounts_done @@ -1499,7 +1501,7 @@ impl PeerHandler { let (_, intervals) = account_storage_roots .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) - .unwrap(); + .ok_or(PeerHandlerError::UnrecoverableError("Tried to get the old download intervals for an account but did not find them".to_owned()))?; for i in 0..chunk_count { let start_hash_u256 = start_hash_u256 + chunk_size * i; @@ -1534,7 +1536,7 @@ impl PeerHandler { let (_, intervals) = account_storage_roots .accounts_with_storage_root .get_mut(¤t_account_hashes[remaining_start]) - .unwrap(); + .ok_or(PeerHandlerError::UnrecoverableError("Trie to get the old download intervals for an account but did not find them".to_owned()))?; for i in 0..chunk_count { let start_hash_u256 = start_hash_u256 + chunk_size * i; @@ -1638,8 +1640,8 @@ impl PeerHandler { *hash, store .get_account_state_by_acc_hash(pivot_header.hash(), *hash) - .unwrap() - .unwrap() + .expect("Failed to get account in state trie") + .expect("Could not find account that should have been downloaded or healed") .storage_root, ), }) @@ -2098,6 +2100,8 @@ pub enum PeerHandlerError { NoResponseFromPeer, #[error("Dumping snapshots to disk failed {0:?}")] DumpError(DumpError), + #[error("Encountered an unexpected error. This is a bug {0}")] + UnrecoverableError(String), } #[derive(Debug, Clone, std::hash::Hash)] diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 35148a8916..100f45fed8 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -1339,6 +1339,7 @@ pub fn calculate_staleness_timestamp(timestamp: u64) -> u64 { timestamp + (SNAP_LIMIT as u64 * 12) } #[derive(Debug, Default)] +#[allow(clippy::type_complexity)] /// We store for optimization the accounts that need to heal storage pub struct AccountStorageRoots { /// The accounts that have not been healed are guaranteed to have the original storage root