From ecb8abfe4d43bebab8589122ec320ba1d27993f4 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Fri, 29 Nov 2024 15:14:05 +0100 Subject: [PATCH 01/12] test(engine): add get-proof_targets unit tests --- crates/engine/tree/src/tree/root.rs | 132 ++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 31e79ca04b57..aeab46e57ed0 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -793,4 +793,136 @@ mod tests { assert_eq!(ready.len(), 5); assert!(!sequencer.has_pending()); } + + fn create_get_proof_targets_state() -> HashedPostState { + let mut state = HashedPostState::default(); + + let addr1 = B256::random(); + let addr2 = B256::random(); + state.accounts.insert(addr1, Some(Default::default())); + state.accounts.insert(addr2, Some(Default::default())); + + let mut storage = HashedStorage::default(); + let slot1 = B256::random(); + let slot2 = B256::random(); + storage.storage.insert(slot1, U256::ZERO); + storage.storage.insert(slot2, U256::from(1)); + state.storages.insert(addr1, storage); + + state + } + + #[test] + fn test_get_proof_targets_new_account_targets() { + let state = create_get_proof_targets_state(); + let fetched = HashMap::default(); + + let targets = get_proof_targets(&state, &fetched); + + // should return all accounts as targets since nothing was fetched before + assert_eq!(targets.len(), state.accounts.len()); + for addr in state.accounts.keys() { + assert!(targets.contains_key(addr)); + } + } + + #[test] + fn test_get_proof_targets_new_storage_targets() { + let state = create_get_proof_targets_state(); + let fetched = HashMap::default(); + + let targets = get_proof_targets(&state, &fetched); + + // verify storage slots are included for accounts with storage + for (addr, storage) in &state.storages { + assert!(targets.contains_key(addr)); + let target_slots = &targets[addr]; + assert_eq!(target_slots.len(), storage.storage.len()); + for slot in storage.storage.keys() { + assert!(target_slots.contains(slot)); + } + } + } + + #[test] + fn test_get_proof_targets_filter_already_fetched_accounts() { + let mut state = create_get_proof_targets_state(); + let mut fetched = HashMap::default(); + + // create a new account without storage + let new_addr = B256::random(); + state.accounts.insert(new_addr, Some(Default::default())); + + // mark one account without storage as already fetched + fetched.insert(new_addr, HashSet::default()); + + let targets = get_proof_targets(&state, &fetched); + + // should not include the already fetched account that has no storage + assert!(!targets.contains_key(&new_addr)); + + // should include accounts with storage even if fetched + for (addr, _) in state.storages.iter() { + assert!(targets.contains_key(addr)); + } + } + + #[test] + fn test_get_proof_targets_filter_already_fetched_storage() { + let state = create_get_proof_targets_state(); + let mut fetched = HashMap::default(); + + // mark one storage slot as already fetched + let (addr, storage) = state.storages.iter().next().unwrap(); + let mut fetched_slots = HashSet::default(); + let fetched_slot = *storage.storage.keys().next().unwrap(); + fetched_slots.insert(fetched_slot); + fetched.insert(*addr, fetched_slots); + + let targets = get_proof_targets(&state, &fetched); + + // should not include the already fetched storage slot + let target_slots = &targets[addr]; + assert!(!target_slots.contains(&fetched_slot)); + assert_eq!(target_slots.len(), storage.storage.len() - 1); + } + + #[test] + fn test_get_proof_targets_empty_state() { + let state = HashedPostState::default(); + let fetched = HashMap::default(); + + let targets = get_proof_targets(&state, &fetched); + + assert!(targets.is_empty()); + } + + #[test] + fn test_get_proof_targets_mixed_fetched_state() { + let mut state = HashedPostState::default(); + let mut fetched = HashMap::default(); + + let addr1 = B256::random(); + let addr2 = B256::random(); + let slot1 = B256::random(); + let slot2 = B256::random(); + + state.accounts.insert(addr1, Some(Default::default())); + state.accounts.insert(addr2, Some(Default::default())); + + let mut storage = HashedStorage::default(); + storage.storage.insert(slot1, U256::ZERO); + storage.storage.insert(slot2, U256::from(1)); + state.storages.insert(addr1, storage); + + let mut fetched_slots = HashSet::default(); + fetched_slots.insert(slot1); + fetched.insert(addr1, fetched_slots); + + let targets = get_proof_targets(&state, &fetched); + + assert!(targets.contains_key(&addr2)); + assert!(!targets[&addr1].contains(&slot1)); + assert!(targets[&addr1].contains(&slot2)); + } } From feec0f704e90da7aa3bd6a3355fe7f540e5c683f Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Fri, 29 Nov 2024 16:32:48 +0100 Subject: [PATCH 02/12] clippy --- crates/engine/tree/src/tree/root.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index aeab46e57ed0..a0a8cf77b08e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -862,7 +862,7 @@ mod tests { assert!(!targets.contains_key(&new_addr)); // should include accounts with storage even if fetched - for (addr, _) in state.storages.iter() { + for addr in state.storages.keys() { assert!(targets.contains_key(addr)); } } From 72625467fbf6946c3b13caead60ae5d2884bb91c Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Fri, 29 Nov 2024 17:31:26 +0100 Subject: [PATCH 03/12] fix test_get_proof_targets_filter_already_fetched_accounts to reflect correct behavior and adapt code to make it pass --- crates/engine/tree/src/tree/root.rs | 87 ++++++++++++++++++----------- 1 file changed, 53 insertions(+), 34 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index a0a8cf77b08e..8ece6811a9a0 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -464,31 +464,50 @@ where } } +/// Returns accounts only with those storages that were not already fetched, and +/// if there are no such storages and the account itself was already fetched, the +/// account shouldn't be included. fn get_proof_targets( state_update: &HashedPostState, fetched_proof_targets: &HashMap>, ) -> HashMap> { - state_update - .accounts - .keys() - .filter(|hashed_address| !fetched_proof_targets.contains_key(*hashed_address)) - .map(|hashed_address| (*hashed_address, HashSet::default())) - .chain(state_update.storages.iter().map(|(hashed_address, storage)| { - let fetched_storage_proof_targets = fetched_proof_targets.get(hashed_address); - ( - *hashed_address, - storage - .storage - .keys() - .filter(|slot| { - !fetched_storage_proof_targets - .is_some_and(|targets| targets.contains(*slot)) - }) - .copied() - .collect(), - ) - })) - .collect() + let mut targets = HashMap::default(); + + // first collect all new accounts (not previously fetched) + for &hashed_address in state_update.accounts.keys() { + if !fetched_proof_targets.contains_key(&hashed_address) { + targets.insert(hashed_address, HashSet::default()); + } + } + + // then process storage slots for all accounts in the state update + for (hashed_address, storage) in state_update.storages.iter() { + // only process storage if we either: + // 1. haven't fetched this account at all (it's in our targets), or + // 2. have fetched the account but need new storage slots + if targets.contains_key(hashed_address) || + fetched_proof_targets.contains_key(hashed_address) + { + let target_slots = targets.entry(*hashed_address).or_default(); + let fetched_storage_slots = fetched_proof_targets.get(hashed_address); + + // add only storage slots that haven't been fetched yet + for slot in storage.storage.keys() { + if !fetched_storage_slots.is_some_and(|fetched_slots| fetched_slots.contains(slot)) + { + target_slots.insert(*slot); + } + } + + // if we didn't find any new storage slots and this account was previously fetched, + // remove it from targets + if target_slots.is_empty() && fetched_proof_targets.contains_key(hashed_address) { + targets.remove(hashed_address); + } + } + } + + targets } /// Updates the sparse trie with the given proofs and state, and returns the updated trie and the @@ -846,25 +865,25 @@ mod tests { #[test] fn test_get_proof_targets_filter_already_fetched_accounts() { - let mut state = create_get_proof_targets_state(); + let state = create_get_proof_targets_state(); let mut fetched = HashMap::default(); - // create a new account without storage - let new_addr = B256::random(); - state.accounts.insert(new_addr, Some(Default::default())); + // select an account that has no storage updates + let fetched_addr = state + .accounts + .keys() + .find(|&&addr| !state.storages.contains_key(&addr)) + .expect("Should have an account without storage"); - // mark one account without storage as already fetched - fetched.insert(new_addr, HashSet::default()); + // mark the account as already fetched + fetched.insert(*fetched_addr, HashSet::default()); let targets = get_proof_targets(&state, &fetched); - // should not include the already fetched account that has no storage - assert!(!targets.contains_key(&new_addr)); - - // should include accounts with storage even if fetched - for addr in state.storages.keys() { - assert!(targets.contains_key(addr)); - } + // should not include the already fetched account since it has no storage updates + assert!(!targets.contains_key(fetched_addr)); + // other accounts should still be included + assert_eq!(targets.len(), state.accounts.len() - 1); } #[test] From c5b0bb2ea396a780c819cc07001f0cbed420f976 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Sun, 1 Dec 2024 20:22:46 +0100 Subject: [PATCH 04/12] clippy --- crates/engine/tree/src/tree/root.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 8ece6811a9a0..344dfe1cb9c8 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -481,7 +481,7 @@ fn get_proof_targets( } // then process storage slots for all accounts in the state update - for (hashed_address, storage) in state_update.storages.iter() { + for (hashed_address, storage) in &state_update.storages { // only process storage if we either: // 1. haven't fetched this account at all (it's in our targets), or // 2. have fetched the account but need new storage slots From d8fbd4f09c3a23067e4bc1c9a47801f8645d1f85 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 13:26:58 +0100 Subject: [PATCH 05/12] if let for checking new storage slots --- crates/engine/tree/src/tree/root.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 344dfe1cb9c8..e181c060fa18 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -489,14 +489,18 @@ fn get_proof_targets( fetched_proof_targets.contains_key(hashed_address) { let target_slots = targets.entry(*hashed_address).or_default(); - let fetched_storage_slots = fetched_proof_targets.get(hashed_address); - - // add only storage slots that haven't been fetched yet - for slot in storage.storage.keys() { - if !fetched_storage_slots.is_some_and(|fetched_slots| fetched_slots.contains(slot)) - { - target_slots.insert(*slot); + if let Some(fetched_storage_slots) = fetched_proof_targets.get(hashed_address) { + // We have previously fetched slots for this address + // Only add slots that haven't been fetched yet + for slot in storage.storage.keys() { + if !fetched_storage_slots.contains(slot) { + target_slots.insert(*slot); + } } + } else { + // No slots have been fetched for this address yet + // Add all slots from the storage + target_slots.extend(storage.storage.keys().copied()); } // if we didn't find any new storage slots and this account was previously fetched, From 0257c5fde0c0e2f24878d43da30335f3496ce103 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 13:45:55 +0100 Subject: [PATCH 06/12] use entry api to check fetched_proof_targets --- crates/engine/tree/src/tree/root.rs | 30 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index e181c060fa18..0531b8ad44f8 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -13,7 +13,7 @@ use reth_trie_parallel::root::ParallelStateRootError; use reth_trie_sparse::{SparseStateTrie, SparseStateTrieResult, SparseTrieError}; use revm_primitives::{keccak256, EvmState, B256}; use std::{ - collections::BTreeMap, + collections::{hash_map::Entry, BTreeMap}, sync::{ mpsc::{self, Receiver, Sender}, Arc, @@ -216,7 +216,7 @@ where view: ConsistentDbView, input: Arc, update: EvmState, - fetched_proof_targets: &HashMap>, + fetched_proof_targets: &mut HashMap>, proof_sequence_number: u64, state_root_message_sender: Sender, ) -> HashMap> { @@ -318,7 +318,7 @@ where ); // TODO(alexey): store proof targets in `ProofSequecner` to avoid recomputing them - let targets = get_proof_targets(&state, &HashMap::default()); + let targets = get_proof_targets(&state, &mut HashMap::default()); let tx = self.tx.clone(); rayon::spawn(move || { @@ -361,7 +361,7 @@ where self.config.consistent_view.clone(), self.config.input.clone(), update, - &self.fetched_proof_targets, + &mut self.fetched_proof_targets, self.proof_sequencer.next_sequence(), self.tx.clone(), ); @@ -469,13 +469,13 @@ where /// account shouldn't be included. fn get_proof_targets( state_update: &HashedPostState, - fetched_proof_targets: &HashMap>, + fetched_proof_targets: &mut HashMap>, ) -> HashMap> { let mut targets = HashMap::default(); // first collect all new accounts (not previously fetched) for &hashed_address in state_update.accounts.keys() { - if !fetched_proof_targets.contains_key(&hashed_address) { + if let Entry::Vacant(_) = fetched_proof_targets.entry(hashed_address) { targets.insert(hashed_address, HashSet::default()); } } @@ -838,9 +838,9 @@ mod tests { #[test] fn test_get_proof_targets_new_account_targets() { let state = create_get_proof_targets_state(); - let fetched = HashMap::default(); + let mut fetched = HashMap::default(); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); // should return all accounts as targets since nothing was fetched before assert_eq!(targets.len(), state.accounts.len()); @@ -852,9 +852,9 @@ mod tests { #[test] fn test_get_proof_targets_new_storage_targets() { let state = create_get_proof_targets_state(); - let fetched = HashMap::default(); + let mut fetched = HashMap::default(); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); // verify storage slots are included for accounts with storage for (addr, storage) in &state.storages { @@ -882,7 +882,7 @@ mod tests { // mark the account as already fetched fetched.insert(*fetched_addr, HashSet::default()); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); // should not include the already fetched account since it has no storage updates assert!(!targets.contains_key(fetched_addr)); @@ -902,7 +902,7 @@ mod tests { fetched_slots.insert(fetched_slot); fetched.insert(*addr, fetched_slots); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); // should not include the already fetched storage slot let target_slots = &targets[addr]; @@ -913,9 +913,9 @@ mod tests { #[test] fn test_get_proof_targets_empty_state() { let state = HashedPostState::default(); - let fetched = HashMap::default(); + let mut fetched = HashMap::default(); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); assert!(targets.is_empty()); } @@ -942,7 +942,7 @@ mod tests { fetched_slots.insert(slot1); fetched.insert(addr1, fetched_slots); - let targets = get_proof_targets(&state, &fetched); + let targets = get_proof_targets(&state, &mut fetched); assert!(targets.contains_key(&addr2)); assert!(!targets[&addr1].contains(&slot1)); From 5d987dfaf4fa69baf66d390844abb7a81409eb55 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 13:59:14 +0100 Subject: [PATCH 07/12] test and fix for unmodified account with storage updates --- crates/engine/tree/src/tree/root.rs | 40 ++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 0531b8ad44f8..a6f75bad7e1b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -482,11 +482,13 @@ fn get_proof_targets( // then process storage slots for all accounts in the state update for (hashed_address, storage) in &state_update.storages { - // only process storage if we either: - // 1. haven't fetched this account at all (it's in our targets), or - // 2. have fetched the account but need new storage slots + // only process storage if account: + // 1. is new (in targets), or + // 2. was previously fetched (in fetched_proof_targets), or + // 3. has storage updates even if not modified if targets.contains_key(hashed_address) || - fetched_proof_targets.contains_key(hashed_address) + fetched_proof_targets.contains_key(hashed_address) || + state_update.storages.contains_key(hashed_address) { let target_slots = targets.entry(*hashed_address).or_default(); if let Some(fetched_storage_slots) = fetched_proof_targets.get(hashed_address) { @@ -948,4 +950,34 @@ mod tests { assert!(!targets[&addr1].contains(&slot1)); assert!(targets[&addr1].contains(&slot2)); } + + #[test] + fn test_get_proof_targets_unmodified_account_with_storage() { + let mut state = HashedPostState::default(); + let mut fetched = HashMap::default(); + + let addr = B256::random(); + let slot1 = B256::random(); + let slot2 = B256::random(); + + // don't add the account to state.accounts (simulating unmodified account) + // but add storage updates for this account + let mut storage = HashedStorage::default(); + storage.storage.insert(slot1, U256::from(1)); + storage.storage.insert(slot2, U256::from(2)); + state.storages.insert(addr, storage); + + assert!(!state.accounts.contains_key(&addr)); + assert!(!fetched.contains_key(&addr)); + + let targets = get_proof_targets(&state, &mut fetched); + + // verify that we still get the storage slots for the unmodified account + assert!(targets.contains_key(&addr)); + + let target_slots = &targets[&addr]; + assert_eq!(target_slots.len(), 2); + assert!(target_slots.contains(&slot1)); + assert!(target_slots.contains(&slot2)); + } } From 4a8457ef3bdaa9cdfa311c9f6fe685a25fad5623 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 15:12:30 +0100 Subject: [PATCH 08/12] remove unnecesary check --- crates/engine/tree/src/tree/root.rs | 41 +++++++++++------------------ 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index a6f75bad7e1b..7dce2904ae53 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -482,34 +482,25 @@ fn get_proof_targets( // then process storage slots for all accounts in the state update for (hashed_address, storage) in &state_update.storages { - // only process storage if account: - // 1. is new (in targets), or - // 2. was previously fetched (in fetched_proof_targets), or - // 3. has storage updates even if not modified - if targets.contains_key(hashed_address) || - fetched_proof_targets.contains_key(hashed_address) || - state_update.storages.contains_key(hashed_address) - { - let target_slots = targets.entry(*hashed_address).or_default(); - if let Some(fetched_storage_slots) = fetched_proof_targets.get(hashed_address) { - // We have previously fetched slots for this address - // Only add slots that haven't been fetched yet - for slot in storage.storage.keys() { - if !fetched_storage_slots.contains(slot) { - target_slots.insert(*slot); - } + let target_slots = targets.entry(*hashed_address).or_default(); + if let Some(fetched_storage_slots) = fetched_proof_targets.get(hashed_address) { + // We have previously fetched slots for this address + // Only add slots that haven't been fetched yet + for slot in storage.storage.keys() { + if !fetched_storage_slots.contains(slot) { + target_slots.insert(*slot); } - } else { - // No slots have been fetched for this address yet - // Add all slots from the storage - target_slots.extend(storage.storage.keys().copied()); } + } else { + // No slots have been fetched for this address yet + // Add all slots from the storage + target_slots.extend(storage.storage.keys().copied()); + } - // if we didn't find any new storage slots and this account was previously fetched, - // remove it from targets - if target_slots.is_empty() && fetched_proof_targets.contains_key(hashed_address) { - targets.remove(hashed_address); - } + // if we didn't find any new storage slots and this account was previously fetched, + // remove it from targets + if target_slots.is_empty() && fetched_proof_targets.contains_key(hashed_address) { + targets.remove(hashed_address); } } From b37a0b9f51b8935bf6e4cf1040a9cf57a6f879a8 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 15:25:08 +0100 Subject: [PATCH 09/12] Update crates/engine/tree/src/tree/root.rs Co-authored-by: Roman Krasiuk --- crates/engine/tree/src/tree/root.rs | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 7dce2904ae53..39d3bb30623c 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -482,25 +482,15 @@ fn get_proof_targets( // then process storage slots for all accounts in the state update for (hashed_address, storage) in &state_update.storages { - let target_slots = targets.entry(*hashed_address).or_default(); - if let Some(fetched_storage_slots) = fetched_proof_targets.get(hashed_address) { - // We have previously fetched slots for this address - // Only add slots that haven't been fetched yet - for slot in storage.storage.keys() { - if !fetched_storage_slots.contains(slot) { - target_slots.insert(*slot); - } - } - } else { - // No slots have been fetched for this address yet - // Add all slots from the storage - target_slots.extend(storage.storage.keys().copied()); - } + let fetched = fetched_proof_targets.get(hashed_address); + let mut changed_slots = storage + .storage + .keys() + .filter(|slot| !fetched.is_some_and(|f| f.contains(*slot))) + .peekable(); - // if we didn't find any new storage slots and this account was previously fetched, - // remove it from targets - if target_slots.is_empty() && fetched_proof_targets.contains_key(hashed_address) { - targets.remove(hashed_address); + if changed_slots.peek().is_some() { + targets.entry(*hashed_address).or_default().extend(changed_slots); } } From 57a1feb7218dbbe11647b6aab02f505ba2187019 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 15:26:13 +0100 Subject: [PATCH 10/12] Update crates/engine/tree/src/tree/root.rs Co-authored-by: Roman Krasiuk --- crates/engine/tree/src/tree/root.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 39d3bb30623c..087cd1a4727e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -475,7 +475,7 @@ fn get_proof_targets( // first collect all new accounts (not previously fetched) for &hashed_address in state_update.accounts.keys() { - if let Entry::Vacant(_) = fetched_proof_targets.entry(hashed_address) { + if fetched_proof_targets.contains_key(&hashed_address) { targets.insert(hashed_address, HashSet::default()); } } From 3f5e88dc1d9f54c2e3353766208ca661195f8ac7 Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 15:31:46 +0100 Subject: [PATCH 11/12] fetched_proof_targets not mut --- crates/engine/tree/src/tree/root.rs | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 087cd1a4727e..a8d06e68b7d0 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -13,7 +13,7 @@ use reth_trie_parallel::root::ParallelStateRootError; use reth_trie_sparse::{SparseStateTrie, SparseStateTrieResult, SparseTrieError}; use revm_primitives::{keccak256, EvmState, B256}; use std::{ - collections::{hash_map::Entry, BTreeMap}, + collections::BTreeMap, sync::{ mpsc::{self, Receiver, Sender}, Arc, @@ -216,7 +216,7 @@ where view: ConsistentDbView, input: Arc, update: EvmState, - fetched_proof_targets: &mut HashMap>, + fetched_proof_targets: &HashMap>, proof_sequence_number: u64, state_root_message_sender: Sender, ) -> HashMap> { @@ -318,7 +318,7 @@ where ); // TODO(alexey): store proof targets in `ProofSequecner` to avoid recomputing them - let targets = get_proof_targets(&state, &mut HashMap::default()); + let targets = get_proof_targets(&state, &HashMap::default()); let tx = self.tx.clone(); rayon::spawn(move || { @@ -361,7 +361,7 @@ where self.config.consistent_view.clone(), self.config.input.clone(), update, - &mut self.fetched_proof_targets, + &self.fetched_proof_targets, self.proof_sequencer.next_sequence(), self.tx.clone(), ); @@ -469,7 +469,7 @@ where /// account shouldn't be included. fn get_proof_targets( state_update: &HashedPostState, - fetched_proof_targets: &mut HashMap>, + fetched_proof_targets: &HashMap>, ) -> HashMap> { let mut targets = HashMap::default(); @@ -821,9 +821,9 @@ mod tests { #[test] fn test_get_proof_targets_new_account_targets() { let state = create_get_proof_targets_state(); - let mut fetched = HashMap::default(); + let fetched = HashMap::default(); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); // should return all accounts as targets since nothing was fetched before assert_eq!(targets.len(), state.accounts.len()); @@ -835,9 +835,9 @@ mod tests { #[test] fn test_get_proof_targets_new_storage_targets() { let state = create_get_proof_targets_state(); - let mut fetched = HashMap::default(); + let fetched = HashMap::default(); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); // verify storage slots are included for accounts with storage for (addr, storage) in &state.storages { @@ -865,7 +865,7 @@ mod tests { // mark the account as already fetched fetched.insert(*fetched_addr, HashSet::default()); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); // should not include the already fetched account since it has no storage updates assert!(!targets.contains_key(fetched_addr)); @@ -885,7 +885,7 @@ mod tests { fetched_slots.insert(fetched_slot); fetched.insert(*addr, fetched_slots); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); // should not include the already fetched storage slot let target_slots = &targets[addr]; @@ -896,9 +896,9 @@ mod tests { #[test] fn test_get_proof_targets_empty_state() { let state = HashedPostState::default(); - let mut fetched = HashMap::default(); + let fetched = HashMap::default(); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); assert!(targets.is_empty()); } @@ -925,7 +925,7 @@ mod tests { fetched_slots.insert(slot1); fetched.insert(addr1, fetched_slots); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); assert!(targets.contains_key(&addr2)); assert!(!targets[&addr1].contains(&slot1)); @@ -935,7 +935,7 @@ mod tests { #[test] fn test_get_proof_targets_unmodified_account_with_storage() { let mut state = HashedPostState::default(); - let mut fetched = HashMap::default(); + let fetched = HashMap::default(); let addr = B256::random(); let slot1 = B256::random(); @@ -951,7 +951,7 @@ mod tests { assert!(!state.accounts.contains_key(&addr)); assert!(!fetched.contains_key(&addr)); - let targets = get_proof_targets(&state, &mut fetched); + let targets = get_proof_targets(&state, &fetched); // verify that we still get the storage slots for the unmodified account assert!(targets.contains_key(&addr)); From bc16ba30aeede2e0bab9d3f18d5e4ee3b98150da Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Mon, 2 Dec 2024 15:33:50 +0100 Subject: [PATCH 12/12] fix check to collect new accounts --- crates/engine/tree/src/tree/root.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index a8d06e68b7d0..70f54ce4d488 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -475,7 +475,7 @@ fn get_proof_targets( // first collect all new accounts (not previously fetched) for &hashed_address in state_update.accounts.keys() { - if fetched_proof_targets.contains_key(&hashed_address) { + if !fetched_proof_targets.contains_key(&hashed_address) { targets.insert(hashed_address, HashSet::default()); } }