From 813fe72b8c1859602902f5918d35efc0b651c0c6 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 14:05:41 +0000 Subject: [PATCH 01/37] feat(evm, root): pass state change source to state hook --- crates/engine/tree/benches/state_root_task.rs | 6 +- crates/engine/tree/src/tree/root.rs | 18 +++-- crates/ethereum/evm/src/execute.rs | 14 ++-- crates/evm/src/metrics.rs | 14 ++-- crates/evm/src/system_calls/mod.rs | 71 ++++++++++++++++--- crates/optimism/evm/src/execute.rs | 12 ++-- 6 files changed, 100 insertions(+), 35 deletions(-) diff --git a/crates/engine/tree/benches/state_root_task.rs b/crates/engine/tree/benches/state_root_task.rs index 71a090fc9de4..d5b0f7980d7b 100644 --- a/crates/engine/tree/benches/state_root_task.rs +++ b/crates/engine/tree/benches/state_root_task.rs @@ -7,7 +7,7 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; use proptest::test_runner::TestRunner; use rand::Rng; use reth_engine_tree::tree::root::{StateRootConfig, StateRootTask}; -use reth_evm::system_calls::OnStateHook; +use reth_evm::system_calls::{OnStateHook, StateChangeSource}; use reth_primitives_traits::{Account as RethAccount, StorageEntry}; use reth_provider::{ providers::ConsistentDbView, @@ -225,8 +225,8 @@ fn bench_state_root(c: &mut Criterion) { let mut hook = task.state_hook(); let handle = task.spawn(); - for update in state_updates { - hook.on_state(&update) + for (i, update) in state_updates.into_iter().enumerate() { + hook.on_state(StateChangeSource::Transaction(i), &update) } drop(hook); diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 60fe5fe3c60c..0fbde431d981 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -5,7 +5,7 @@ use derive_more::derive::Deref; use metrics::Histogram; use rayon::iter::{ParallelBridge, ParallelIterator}; use reth_errors::{ProviderError, ProviderResult}; -use reth_evm::system_calls::OnStateHook; +use reth_evm::system_calls::{OnStateHook, StateChangeSource}; use reth_metrics::Metrics; use reth_provider::{ providers::ConsistentDbView, BlockReader, DBProvider, DatabaseProviderFactory, @@ -147,7 +147,7 @@ pub enum StateRootMessage { /// Prefetch proof targets PrefetchProofs(MultiProofTargets), /// New state update from transaction execution - StateUpdate(EvmState), + StateUpdate(StateChangeSource, EvmState), /// Empty proof for a specific state update EmptyProof { /// The index of this proof in the sequence of state updates @@ -537,8 +537,10 @@ where pub fn state_hook(&self) -> impl OnStateHook { let state_hook = self.state_hook_sender(); - move |state: &EvmState| { - if let Err(error) = state_hook.send(StateRootMessage::StateUpdate(state.clone())) { + move |source: StateChangeSource, state: &EvmState| { + if let Err(error) = + state_hook.send(StateRootMessage::StateUpdate(source, state.clone())) + { error!(target: "engine::root", ?error, "Failed to send state update"); } } @@ -760,7 +762,7 @@ where ); self.on_prefetch_proof(targets); } - StateRootMessage::StateUpdate(update) => { + StateRootMessage::StateUpdate(source, update) => { trace!(target: "engine::root", "processing StateRootMessage::StateUpdate"); if updates_received == 0 { first_update_time = Some(Instant::now()); @@ -771,6 +773,7 @@ where updates_received += 1; debug!( target: "engine::root", + ?source, len = update.len(), total_updates = updates_received, "Received new state update" @@ -1169,6 +1172,7 @@ fn extend_multi_proof_targets_ref(targets: &mut MultiProofTargets, other: &Multi mod tests { use super::*; use alloy_primitives::map::B256Set; + use reth_evm::system_calls::StateChangeSource; use reth_primitives_traits::{Account as RethAccount, StorageEntry}; use reth_provider::{ providers::ConsistentDbView, test_utils::create_test_provider_factory, HashingWriter, @@ -1345,8 +1349,8 @@ mod tests { let mut state_hook = task.state_hook(); let handle = task.spawn(); - for update in state_updates { - state_hook.on_state(&update); + for (i, update) in state_updates.into_iter().enumerate() { + state_hook.on_state(StateChangeSource::Transaction(i), &update); } drop(state_hook); diff --git a/crates/ethereum/evm/src/execute.rs b/crates/ethereum/evm/src/execute.rs index 29f51231fe9c..5313f55c52dc 100644 --- a/crates/ethereum/evm/src/execute.rs +++ b/crates/ethereum/evm/src/execute.rs @@ -16,7 +16,7 @@ use reth_evm::{ BlockExecutionStrategy, BlockExecutionStrategyFactory, BlockValidationError, ExecuteOutput, }, state_change::post_block_balance_increments, - system_calls::{OnStateHook, SystemCaller}, + system_calls::{OnStateHook, StateChangePostBlockSource, StateChangeSource, SystemCaller}, ConfigureEvm, Database, Evm, }; use reth_primitives::{EthPrimitives, Receipt, RecoveredBlock}; @@ -140,7 +140,7 @@ where let mut cumulative_gas_used = 0; let mut receipts = Vec::with_capacity(block.body().transaction_count()); - for (sender, transaction) in block.transactions_with_sender() { + for (tx_index, (sender, transaction)) in block.transactions_with_sender().enumerate() { // The sum of the transaction's gas limit, Tg, and the gas utilized in this block prior, // must be no greater than the block's gasLimit. let block_available_gas = block.gas_limit() - cumulative_gas_used; @@ -162,7 +162,8 @@ where error: Box::new(err), } })?; - self.system_caller.on_state(&result_and_state.state); + self.system_caller + .on_state(StateChangeSource::Transaction(tx_index), &result_and_state.state); let ResultAndState { result, state } = result_and_state; evm.db_mut().commit(state); @@ -228,7 +229,10 @@ where .map_err(|_| BlockValidationError::IncrementBalanceFailed)?; // call state hook with changes due to balance increments. let balance_state = balance_increment_state(&balance_increments, &mut self.state)?; - self.system_caller.on_state(&balance_state); + self.system_caller.on_state( + StateChangeSource::PostBlock(StateChangePostBlockSource::BalanceIncrements), + &balance_state, + ); Ok(requests) } @@ -1131,7 +1135,7 @@ mod tests { let tx_clone = tx.clone(); let _output = executor - .execute_with_state_hook(block, move |state: &EvmState| { + .execute_with_state_hook(block, move |_, state: &EvmState| { if let Some(account) = state.get(&withdrawal_recipient) { let _ = tx_clone.send(account.info.balance); } diff --git a/crates/evm/src/metrics.rs b/crates/evm/src/metrics.rs index 2bd68102a07d..e92a3e9eb76e 100644 --- a/crates/evm/src/metrics.rs +++ b/crates/evm/src/metrics.rs @@ -2,7 +2,11 @@ //! //! Block processing related to syncing should take care to update the metrics by using either //! [`ExecutorMetrics::execute_metered`] or [`ExecutorMetrics::metered_one`]. -use crate::{execute::Executor, system_calls::OnStateHook, Database}; +use crate::{ + execute::Executor, + system_calls::{OnStateHook, StateChangeSource}, + Database, +}; use alloy_consensus::BlockHeader; use metrics::{Counter, Gauge, Histogram}; use reth_execution_types::BlockExecutionOutput; @@ -18,7 +22,7 @@ struct MeteredStateHook { } impl OnStateHook for MeteredStateHook { - fn on_state(&mut self, state: &EvmState) { + fn on_state(&mut self, source: StateChangeSource, state: &EvmState) { // Update the metrics for the number of accounts, storage slots and bytecodes loaded let accounts = state.keys().len(); let storage_slots = state.values().map(|account| account.storage.len()).sum::(); @@ -33,7 +37,7 @@ impl OnStateHook for MeteredStateHook { self.metrics.bytecodes_loaded_histogram.record(bytecodes as f64); // Call the original state hook - self.inner_hook.on_state(state); + self.inner_hook.on_state(source, state); } } @@ -178,7 +182,7 @@ mod tests { F: OnStateHook + 'static, { // Call hook with our mock state - hook.on_state(&self.state); + hook.on_state(StateChangeSource::Transaction(0), &self.state); Ok(BlockExecutionResult { receipts: vec![], @@ -202,7 +206,7 @@ mod tests { } impl OnStateHook for ChannelStateHook { - fn on_state(&mut self, _state: &EvmState) { + fn on_state(&mut self, _source: StateChangeSource, _state: &EvmState) { let _ = self.sender.send(self.output); } } diff --git a/crates/evm/src/system_calls/mod.rs b/crates/evm/src/system_calls/mod.rs index 535bda938322..d1400378c8b3 100644 --- a/crates/evm/src/system_calls/mod.rs +++ b/crates/evm/src/system_calls/mod.rs @@ -21,15 +21,48 @@ mod eip7251; /// A hook that is called after each state change. pub trait OnStateHook { /// Invoked with the state after each system call. - fn on_state(&mut self, state: &EvmState); + fn on_state(&mut self, source: StateChangeSource, state: &EvmState); +} + +/// Source of the state change +#[derive(Debug, Clone, Copy)] +pub enum StateChangeSource { + /// Transaction with its index + Transaction(usize), + /// Pre-block state transition + PreBlock(StateChangePreBlockSource), + /// Post-block state transition + PostBlock(StateChangePostBlockSource), +} + +/// Source of the pre-block state change +#[derive(Debug, Clone, Copy)] +pub enum StateChangePreBlockSource { + /// EIP-2935 blockhashes contract + BlockHashesContract, + /// EIP-4788 beacon root contract + BeaconRootContract, + /// EIP-7002 withdrawal requests contract + WithdrawalRequestsContract, +} + +/// Source of the post-block state change +#[derive(Debug, Clone, Copy)] +pub enum StateChangePostBlockSource { + /// Balance increments from block rewards and withdrawals + BalanceIncrements, + /// EIP-7002 withdrawal requests contract + WithdrawalRequestsContract, + /// EIP-7251 consolidation requests contract + ConsolidationRequestsContract, } impl OnStateHook for F where - F: FnMut(&EvmState), + F: FnMut(StateChangeSource, &EvmState), { - fn on_state(&mut self, state: &EvmState) { - self(state) + fn on_state(&mut self, source: StateChangeSource, state: &EvmState) { + self(source, state) } } @@ -39,7 +72,7 @@ where pub struct NoopHook; impl OnStateHook for NoopHook { - fn on_state(&mut self, _state: &EvmState) {} + fn on_state(&mut self, _source: StateChangeSource, _state: &EvmState) {} } /// An ephemeral helper type for executing system calls. @@ -161,7 +194,10 @@ where if let Some(res) = result_and_state { if let Some(ref mut hook) = self.hook { - hook.on_state(&res.state); + hook.on_state( + StateChangeSource::PreBlock(StateChangePreBlockSource::BlockHashesContract), + &res.state, + ); } evm.db_mut().commit(res.state); } @@ -211,7 +247,10 @@ where if let Some(res) = result_and_state { if let Some(ref mut hook) = self.hook { - hook.on_state(&res.state); + hook.on_state( + StateChangeSource::PreBlock(StateChangePreBlockSource::BeaconRootContract), + &res.state, + ); } evm.db_mut().commit(res.state); } @@ -245,7 +284,12 @@ where let result_and_state = eip7002::transact_withdrawal_requests_contract_call(evm)?; if let Some(ref mut hook) = self.hook { - hook.on_state(&result_and_state.state); + hook.on_state( + StateChangeSource::PostBlock( + StateChangePostBlockSource::WithdrawalRequestsContract, + ), + &result_and_state.state, + ); } evm.db_mut().commit(result_and_state.state); @@ -277,7 +321,12 @@ where let result_and_state = eip7251::transact_consolidation_requests_contract_call(evm)?; if let Some(ref mut hook) = self.hook { - hook.on_state(&result_and_state.state); + hook.on_state( + StateChangeSource::PostBlock( + StateChangePostBlockSource::ConsolidationRequestsContract, + ), + &result_and_state.state, + ); } evm.db_mut().commit(result_and_state.state); @@ -285,9 +334,9 @@ where } /// Delegate to stored `OnStateHook`, noop if hook is `None`. - pub fn on_state(&mut self, state: &EvmState) { + pub fn on_state(&mut self, source: StateChangeSource, state: &EvmState) { if let Some(ref mut hook) = &mut self.hook { - hook.on_state(state); + hook.on_state(source, state); } } } diff --git a/crates/optimism/evm/src/execute.rs b/crates/optimism/evm/src/execute.rs index 3ae3c43c42bc..dd04f9d22f0b 100644 --- a/crates/optimism/evm/src/execute.rs +++ b/crates/optimism/evm/src/execute.rs @@ -15,7 +15,7 @@ use reth_evm::{ BlockExecutionStrategy, BlockExecutionStrategyFactory, BlockValidationError, ExecuteOutput, }, state_change::post_block_balance_increments, - system_calls::{OnStateHook, SystemCaller}, + system_calls::{OnStateHook, StateChangePostBlockSource, StateChangeSource, SystemCaller}, ConfigureEvmFor, Database, Evm, }; use reth_optimism_chainspec::OpChainSpec; @@ -175,7 +175,7 @@ where let mut cumulative_gas_used = 0; let mut receipts = Vec::with_capacity(block.body().transaction_count()); - for (sender, transaction) in block.transactions_with_sender() { + for (tx_index, (sender, transaction)) in block.transactions_with_sender().enumerate() { // The sum of the transaction’s gas limit, Tg, and the gas utilized in this block prior, // must be no greater than the block’s gasLimit. let block_available_gas = block.gas_limit() - cumulative_gas_used; @@ -219,7 +219,8 @@ where ?transaction, "Executed transaction" ); - self.system_caller.on_state(&result_and_state.state); + self.system_caller + .on_state(StateChangeSource::Transaction(tx_index), &result_and_state.state); let ResultAndState { result, state } = result_and_state; evm.db_mut().commit(state); @@ -275,7 +276,10 @@ where .map_err(|_| BlockValidationError::IncrementBalanceFailed)?; // call state hook with changes due to balance increments. let balance_state = balance_increment_state(&balance_increments, &mut self.state)?; - self.system_caller.on_state(&balance_state); + self.system_caller.on_state( + StateChangeSource::PostBlock(StateChangePostBlockSource::BalanceIncrements), + &balance_state, + ); Ok(Requests::default()) } From 2d859c7d8311a9279ccc5179893aa679aec732cd Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 14:06:37 +0000 Subject: [PATCH 02/37] some comments --- crates/engine/tree/src/tree/root.rs | 2 +- crates/evm/src/system_calls/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 0fbde431d981..4981d277aeef 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -146,7 +146,7 @@ impl StateRootConfig { pub enum StateRootMessage { /// Prefetch proof targets PrefetchProofs(MultiProofTargets), - /// New state update from transaction execution + /// New state update from transaction execution with its source StateUpdate(StateChangeSource, EvmState), /// Empty proof for a specific state update EmptyProof { diff --git a/crates/evm/src/system_calls/mod.rs b/crates/evm/src/system_calls/mod.rs index d1400378c8b3..25310410580a 100644 --- a/crates/evm/src/system_calls/mod.rs +++ b/crates/evm/src/system_calls/mod.rs @@ -20,7 +20,7 @@ mod eip7251; /// A hook that is called after each state change. pub trait OnStateHook { - /// Invoked with the state after each system call. + /// Invoked with the source of the change and the state after each system call. fn on_state(&mut self, source: StateChangeSource, state: &EvmState); } From 31f5b48eae9b0beb46ce47d26b46aac8f1e8b477 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:36:04 +0000 Subject: [PATCH 03/37] log on multiproof --- crates/engine/tree/src/tree/root.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 4981d277aeef..3c9780f24d34 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -317,6 +317,7 @@ fn evm_state_to_hashed_post_state(update: EvmState) -> HashedPostState { #[derive(Debug)] struct MultiproofInput { config: StateRootConfig, + source: Option, hashed_state_update: HashedPostState, proof_targets: MultiProofTargets, proof_sequence_number: u64, @@ -392,14 +393,17 @@ where } /// Spawns a multiproof calculation. - fn spawn_multiproof(&mut self, input: MultiproofInput) { - let MultiproofInput { + fn spawn_multiproof( + &mut self, + MultiproofInput { config, + source, hashed_state_update, proof_targets, proof_sequence_number, state_root_message_sender, - } = input; + }: MultiproofInput, + ) { let thread_pool = self.thread_pool.clone(); self.thread_pool.spawn(move || { @@ -421,6 +425,7 @@ where target: "engine::root", proof_sequence_number, ?elapsed, + ?source, ?account_targets, ?storage_targets, "Multiproof calculated", @@ -598,6 +603,7 @@ where self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), + source: None, hashed_state_update: Default::default(), proof_targets, proof_sequence_number: self.proof_sequencer.next_sequence(), @@ -658,13 +664,19 @@ where /// Handles state updates. /// /// Returns proof targets derived from the state update. - fn on_state_update(&mut self, update: EvmState, proof_sequence_number: u64) { + fn on_state_update( + &mut self, + source: StateChangeSource, + update: EvmState, + proof_sequence_number: u64, + ) { let hashed_state_update = evm_state_to_hashed_post_state(update); let proof_targets = get_proof_targets(&hashed_state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), + source: Some(source), hashed_state_update, proof_targets, proof_sequence_number, @@ -779,7 +791,7 @@ where "Received new state update" ); let next_sequence = self.proof_sequencer.next_sequence(); - self.on_state_update(update, next_sequence); + self.on_state_update(source, update, next_sequence); } StateRootMessage::FinishedStateUpdates => { trace!(target: "engine::root", "processing StateRootMessage::FinishedStateUpdates"); From 440e870359fe2e06d0882804281f4e284fa2393e Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:56:07 +0000 Subject: [PATCH 04/37] perf(root): chunk state updates in state root task --- Cargo.lock | 1 + crates/engine/tree/Cargo.toml | 5 ++- crates/engine/tree/src/tree/root.rs | 59 ++++++++++++++++++++++------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c8d4f25b99d..f9e7b8d8395b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7347,6 +7347,7 @@ dependencies = [ "crossbeam-channel", "derive_more", "futures", + "itertools 0.13.0", "metrics", "mini-moka", "proptest", diff --git a/crates/engine/tree/Cargo.toml b/crates/engine/tree/Cargo.toml index eb5ac3db1c50..b3141d062d0b 100644 --- a/crates/engine/tree/Cargo.toml +++ b/crates/engine/tree/Cargo.toml @@ -54,10 +54,11 @@ metrics.workspace = true reth-metrics = { workspace = true, features = ["common"] } # misc -schnellru.workspace = true +derive_more.workspace = true +itertools.workspace = true rayon.workspace = true +schnellru.workspace = true tracing.workspace = true -derive_more.workspace = true # optional deps for test-utils reth-prune-types = { workspace = true, optional = true } diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 70153c6017ee..2b88acb516b1 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -2,6 +2,7 @@ use alloy_primitives::map::HashSet; use derive_more::derive::Deref; +use itertools::Itertools; use metrics::Histogram; use rayon::iter::{ParallelBridge, ParallelIterator}; use reth_errors::{ProviderError, ProviderResult}; @@ -656,18 +657,51 @@ where /// Handles state updates. /// /// Returns proof targets derived from the state update. - fn on_state_update(&mut self, update: EvmState, proof_sequence_number: u64) { - let hashed_state_update = evm_state_to_hashed_post_state(update); - let proof_targets = get_proof_targets(&hashed_state_update, &self.fetched_proof_targets); - extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + fn on_state_update(&mut self, update: EvmState) { + let mut hashed_state_update = evm_state_to_hashed_post_state(update); + + for chunk in &hashed_state_update.accounts.into_iter().chunks(5) { + let (accounts, storages) = chunk + .map(|(address, account)| { + ( + (address, account), + ( + address, + hashed_state_update.storages.remove(&address).unwrap_or_default(), + ), + ) + }) + .unzip(); + let account_hashed_state_update = HashedPostState { accounts, storages }; + + let proof_targets = + get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); + extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + + self.multiproof_manager.spawn_or_queue(MultiproofInput { + config: self.config.clone(), + hashed_state_update: account_hashed_state_update, + proof_targets, + proof_sequence_number: self.proof_sequencer.next_sequence(), + state_root_message_sender: self.tx.clone(), + }); + } - self.multiproof_manager.spawn_or_queue(MultiproofInput { - config: self.config.clone(), - hashed_state_update, - proof_targets, - proof_sequence_number, - state_root_message_sender: self.tx.clone(), - }); + if !hashed_state_update.storages.is_empty() { + let storages_state_update = + HashedPostState { accounts: Default::default(), ..hashed_state_update }; + let proof_targets = + get_proof_targets(&storages_state_update, &self.fetched_proof_targets); + extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + + self.multiproof_manager.spawn_or_queue(MultiproofInput { + config: self.config.clone(), + hashed_state_update: storages_state_update, + proof_targets, + proof_sequence_number: self.proof_sequencer.next_sequence(), + state_root_message_sender: self.tx.clone(), + }); + } } /// Handler for new proof calculated, aggregates all the existing sequential proofs. @@ -775,8 +809,7 @@ where total_updates = updates_received, "Received new state update" ); - let next_sequence = self.proof_sequencer.next_sequence(); - self.on_state_update(update, next_sequence); + self.on_state_update(update); } StateRootMessage::FinishedStateUpdates => { trace!(target: "engine::root", "processing StateRootMessage::FinishedStateUpdates"); From e93d46594c489b6a518d3908bbdb40a18cc8461d Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:58:21 +0000 Subject: [PATCH 05/37] add todo for prefetch --- crates/engine/tree/src/tree/root.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 2b88acb516b1..15803caa5d23 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -595,6 +595,8 @@ where let proof_targets = self.get_prefetch_proof_targets(targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + // TODO: chunk proof targets and spawn separate multiproof tasks + self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), hashed_state_update: Default::default(), From 1ceeb1e001578ddf81078348dd075f6401a2b07d Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 16:42:14 +0000 Subject: [PATCH 06/37] chunk storage slots by 5 as well --- crates/engine/tree/src/tree/root.rs | 73 ++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 15803caa5d23..7eba8c9654fe 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -662,31 +662,58 @@ where fn on_state_update(&mut self, update: EvmState) { let mut hashed_state_update = evm_state_to_hashed_post_state(update); - for chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let (accounts, storages) = chunk - .map(|(address, account)| { - ( - (address, account), - ( - address, - hashed_state_update.storages.remove(&address).unwrap_or_default(), - ), - ) - }) - .unzip(); - let account_hashed_state_update = HashedPostState { accounts, storages }; + for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { + let mut chunks = Vec::>::new(); + + for (address, account) in accounts_chunk { + let storages = + hashed_state_update.storages.remove(&address).unwrap_or_default().storage; + let chunks_num = storages.len().div_ceil(5); + if chunks.len() < chunks_num { + chunks.resize(chunks_num, Vec::new()); + } - let proof_targets = - get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); - extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + let storages = storages.into_iter().chunks(5); + let mut storages_iter = storages.into_iter(); - self.multiproof_manager.spawn_or_queue(MultiproofInput { - config: self.config.clone(), - hashed_state_update: account_hashed_state_update, - proof_targets, - proof_sequence_number: self.proof_sequencer.next_sequence(), - state_root_message_sender: self.tx.clone(), - }); + chunks[0].push(( + address, + account, + if let Some(storages) = storages_iter.next() { + HashedStorage::from_iter(false, storages) + } else { + HashedStorage::new(false) + }, + )); + + for (i, storages_chunk) in storages_iter.enumerate() { + chunks[i + 1].push(( + address, + account, + HashedStorage::from_iter(false, storages_chunk), + )); + } + } + + for chunk in chunks { + let (accounts, storages) = chunk + .into_iter() + .map(|(address, account, storages)| ((address, account), (address, storages))) + .unzip(); + let account_hashed_state_update = HashedPostState { accounts, storages }; + + let proof_targets = + get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); + extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + + self.multiproof_manager.spawn_or_queue(MultiproofInput { + config: self.config.clone(), + hashed_state_update: account_hashed_state_update, + proof_targets, + proof_sequence_number: self.proof_sequencer.next_sequence(), + state_root_message_sender: self.tx.clone(), + }); + } } if !hashed_state_update.storages.is_empty() { From 5b12022d846799e9d2e2ba62de8dea2d3d60f425 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 16:49:36 +0000 Subject: [PATCH 07/37] at least one element --- 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 7eba8c9654fe..9e8e50ca4da5 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -663,7 +663,7 @@ where let mut hashed_state_update = evm_state_to_hashed_post_state(update); for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks = Vec::>::new(); + let mut chunks = vec![vec![]; 1]; for (address, account) in accounts_chunk { let storages = From f41000ef744d2a1b348b132a2d055bf3d87c6451 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 16:54:47 +0000 Subject: [PATCH 08/37] updates accounting --- crates/engine/tree/src/tree/root.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 9e8e50ca4da5..69e84ff96942 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -659,9 +659,11 @@ where /// Handles state updates. /// /// Returns proof targets derived from the state update. - fn on_state_update(&mut self, update: EvmState) { + fn on_state_update(&mut self, update: EvmState) -> u64 { let mut hashed_state_update = evm_state_to_hashed_post_state(update); + let mut total_updates = 0; + for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { let mut chunks = vec![vec![]; 1]; @@ -713,6 +715,7 @@ where proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); + total_updates += 1; } } @@ -730,7 +733,10 @@ where proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); + total_updates += 1; } + + total_updates } /// Handler for new proof calculated, aggregates all the existing sequential proofs. @@ -831,14 +837,16 @@ where } last_update_time = Some(Instant::now()); - updates_received += 1; + let update_size = update.len(); + let new_updates = self.on_state_update(update); + updates_received += new_updates; debug!( target: "engine::root", - len = update.len(), + update_size, + new_updates, total_updates = updates_received, "Received new state update" ); - self.on_state_update(update); } StateRootMessage::FinishedStateUpdates => { trace!(target: "engine::root", "processing StateRootMessage::FinishedStateUpdates"); From f40ce0c151be8dbc3ae551a12a23dfcf91330d11 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:04:24 +0000 Subject: [PATCH 09/37] fix comment --- crates/engine/tree/src/tree/root.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 69e84ff96942..e4e0b332984e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -656,9 +656,9 @@ where targets } - /// Handles state updates. + /// Handles state update. /// - /// Returns proof targets derived from the state update. + /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, update: EvmState) -> u64 { let mut hashed_state_update = evm_state_to_hashed_post_state(update); From 976d5a2cc3b1418c7d714a9436e005387ef98a00 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:08:21 +0000 Subject: [PATCH 10/37] revertme: print max --- crates/engine/tree/src/tree/root.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index e4e0b332984e..58b3a689bd43 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -37,7 +37,7 @@ use std::{ }, time::{Duration, Instant}, }; -use tracing::{debug, error, trace, trace_span}; +use tracing::{debug, error, info, trace, trace_span}; /// The level below which the sparse trie hashes are calculated in [`update_sparse_trie`]. const SPARSE_TRIE_INCREMENTAL_LEVEL: usize = 2; @@ -660,6 +660,13 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, update: EvmState) -> u64 { + let max = update.iter().max_by_key(|(_, account)| account.storage.len()); + if let Some((address, account)) = max { + if account.storage.len() > 100 { + info!(target: "engine::root", ?address, ?account, "Max storage slot len"); + } + } + let mut hashed_state_update = evm_state_to_hashed_post_state(update); let mut total_updates = 0; From ca1e115549f3535fdda1c7007780a9d6d8a5f59e Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:31:00 +0000 Subject: [PATCH 11/37] revertme: more than 500 --- 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 1fbaf84ef7c2..89c99b8ab59d 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -669,7 +669,7 @@ where fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { let max = update.iter().max_by_key(|(_, account)| account.storage.len()); if let Some((address, account)) = max { - if account.storage.len() > 100 { + if account.storage.len() > 500 { info!(target: "engine::root", ?source, ?address, ?account, "Max storage slot len"); } } From c20669ffa85bd62be3c7103738c3cd6f8eb7f113 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:45:42 +0000 Subject: [PATCH 12/37] revert print --- crates/engine/tree/src/tree/root.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 89c99b8ab59d..b5f919d22a2e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -37,7 +37,7 @@ use std::{ }, time::{Duration, Instant}, }; -use tracing::{debug, error, info, trace, trace_span}; +use tracing::{debug, error, trace, trace_span}; /// The level below which the sparse trie hashes are calculated in [`update_sparse_trie`]. const SPARSE_TRIE_INCREMENTAL_LEVEL: usize = 2; @@ -667,12 +667,6 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let max = update.iter().max_by_key(|(_, account)| account.storage.len()); - if let Some((address, account)) = max { - if account.storage.len() > 500 { - info!(target: "engine::root", ?source, ?address, ?account, "Max storage slot len"); - } - } let mut hashed_state_update = evm_state_to_hashed_post_state(update); let mut total_updates = 0; From 89b61deab123553ff9f1ed8588af1756570a0379 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:27:46 +0000 Subject: [PATCH 13/37] use btreemap instead --- crates/engine/tree/src/tree/root.rs | 72 ++++++++++------------------- 1 file changed, 24 insertions(+), 48 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index b5f919d22a2e..fb26b38e8f6b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -669,83 +669,59 @@ where fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { let mut hashed_state_update = evm_state_to_hashed_post_state(update); - let mut total_updates = 0; + let mut state_updates = Vec::new(); for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks = vec![vec![]; 1]; + let mut chunks = BTreeMap::new(); for (address, account) in accounts_chunk { - let storages = - hashed_state_update.storages.remove(&address).unwrap_or_default().storage; - let chunks_num = storages.len().div_ceil(5); - if chunks.len() < chunks_num { - chunks.resize(chunks_num, Vec::new()); - } - - let storages = storages.into_iter().chunks(5); - let mut storages_iter = storages.into_iter(); - - chunks[0].push(( - address, - account, - if let Some(storages) = storages_iter.next() { - HashedStorage::from_iter(false, storages) - } else { - HashedStorage::new(false) - }, - )); + let storages = hashed_state_update + .storages + .remove(&address) + .unwrap_or_default() + .storage + .into_iter() + .chunks(5); - for (i, storages_chunk) in storages_iter.enumerate() { - chunks[i + 1].push(( + for (index, storage_chunk) in storages.into_iter().enumerate() { + chunks.entry(index).or_insert_with(Vec::new).push(( address, account, - HashedStorage::from_iter(false, storages_chunk), + HashedStorage::from_iter(false, storage_chunk), )); } } - for chunk in chunks { + for chunk in chunks.into_values() { let (accounts, storages) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) .unzip(); - let account_hashed_state_update = HashedPostState { accounts, storages }; - - let proof_targets = - get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); - extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - - self.multiproof_manager.spawn_or_queue(MultiproofInput { - config: self.config.clone(), - source: Some(source), - hashed_state_update: account_hashed_state_update, - proof_targets, - proof_sequence_number: self.proof_sequencer.next_sequence(), - state_root_message_sender: self.tx.clone(), - }); - total_updates += 1; + + state_updates.push(HashedPostState { accounts, storages }); } } - if !hashed_state_update.storages.is_empty() { - let storages_state_update = - HashedPostState { accounts: Default::default(), ..hashed_state_update }; - let proof_targets = - get_proof_targets(&storages_state_update, &self.fetched_proof_targets); + state_updates + .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); + } + + let total_updates = state_updates.len(); + for state_update in state_updates { + let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: Some(source), - hashed_state_update: storages_state_update, + hashed_state_update: state_update, proof_targets, proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); - total_updates += 1; } - total_updates + total_updates as u64 } /// Handler for new proof calculated, aggregates all the existing sequential proofs. From cf1bc7b0f30b942e092f902f2a87a8d1cf4b370e Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:31:24 +0000 Subject: [PATCH 14/37] use vec again but nicer indexing in it --- crates/engine/tree/src/tree/root.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index fb26b38e8f6b..e306b075a7f1 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -672,7 +672,7 @@ where let mut state_updates = Vec::new(); for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks = BTreeMap::new(); + let mut chunks: Vec> = Vec::new(); for (address, account) in accounts_chunk { let storages = hashed_state_update @@ -684,7 +684,11 @@ where .chunks(5); for (index, storage_chunk) in storages.into_iter().enumerate() { - chunks.entry(index).or_insert_with(Vec::new).push(( + if chunks.len() <= index { + chunks.push(Vec::new()); + } + + chunks[index].push(( address, account, HashedStorage::from_iter(false, storage_chunk), @@ -692,7 +696,7 @@ where } } - for chunk in chunks.into_values() { + for chunk in chunks { let (accounts, storages) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) @@ -701,6 +705,7 @@ where state_updates.push(HashedPostState { accounts, storages }); } } + if !hashed_state_update.storages.is_empty() { state_updates .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); From dbef92114cc4f940e2c828e23ec099350cfd76a4 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:42:12 +0000 Subject: [PATCH 15/37] Revert "use vec again but nicer indexing in it" This reverts commit cf1bc7b0f30b942e092f902f2a87a8d1cf4b370e. --- crates/engine/tree/src/tree/root.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index e306b075a7f1..fb26b38e8f6b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -672,7 +672,7 @@ where let mut state_updates = Vec::new(); for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks: Vec> = Vec::new(); + let mut chunks = BTreeMap::new(); for (address, account) in accounts_chunk { let storages = hashed_state_update @@ -684,11 +684,7 @@ where .chunks(5); for (index, storage_chunk) in storages.into_iter().enumerate() { - if chunks.len() <= index { - chunks.push(Vec::new()); - } - - chunks[index].push(( + chunks.entry(index).or_insert_with(Vec::new).push(( address, account, HashedStorage::from_iter(false, storage_chunk), @@ -696,7 +692,7 @@ where } } - for chunk in chunks { + for chunk in chunks.into_values() { let (accounts, storages) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) @@ -705,7 +701,6 @@ where state_updates.push(HashedPostState { accounts, storages }); } } - if !hashed_state_update.storages.is_empty() { state_updates .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); From 8af60fbcaf47197e005f5ff629ce380d92ff8ad5 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:42:14 +0000 Subject: [PATCH 16/37] Revert "use btreemap instead" This reverts commit 89b61deab123553ff9f1ed8588af1756570a0379. --- crates/engine/tree/src/tree/root.rs | 72 +++++++++++++++++++---------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index fb26b38e8f6b..b5f919d22a2e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -669,59 +669,83 @@ where fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { let mut hashed_state_update = evm_state_to_hashed_post_state(update); - let mut state_updates = Vec::new(); + let mut total_updates = 0; for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks = BTreeMap::new(); + let mut chunks = vec![vec![]; 1]; for (address, account) in accounts_chunk { - let storages = hashed_state_update - .storages - .remove(&address) - .unwrap_or_default() - .storage - .into_iter() - .chunks(5); + let storages = + hashed_state_update.storages.remove(&address).unwrap_or_default().storage; + let chunks_num = storages.len().div_ceil(5); + if chunks.len() < chunks_num { + chunks.resize(chunks_num, Vec::new()); + } + + let storages = storages.into_iter().chunks(5); + let mut storages_iter = storages.into_iter(); + + chunks[0].push(( + address, + account, + if let Some(storages) = storages_iter.next() { + HashedStorage::from_iter(false, storages) + } else { + HashedStorage::new(false) + }, + )); - for (index, storage_chunk) in storages.into_iter().enumerate() { - chunks.entry(index).or_insert_with(Vec::new).push(( + for (i, storages_chunk) in storages_iter.enumerate() { + chunks[i + 1].push(( address, account, - HashedStorage::from_iter(false, storage_chunk), + HashedStorage::from_iter(false, storages_chunk), )); } } - for chunk in chunks.into_values() { + for chunk in chunks { let (accounts, storages) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) .unzip(); - - state_updates.push(HashedPostState { accounts, storages }); + let account_hashed_state_update = HashedPostState { accounts, storages }; + + let proof_targets = + get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); + extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + + self.multiproof_manager.spawn_or_queue(MultiproofInput { + config: self.config.clone(), + source: Some(source), + hashed_state_update: account_hashed_state_update, + proof_targets, + proof_sequence_number: self.proof_sequencer.next_sequence(), + state_root_message_sender: self.tx.clone(), + }); + total_updates += 1; } } - if !hashed_state_update.storages.is_empty() { - state_updates - .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); - } - let total_updates = state_updates.len(); - for state_update in state_updates { - let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); + if !hashed_state_update.storages.is_empty() { + let storages_state_update = + HashedPostState { accounts: Default::default(), ..hashed_state_update }; + let proof_targets = + get_proof_targets(&storages_state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: Some(source), - hashed_state_update: state_update, + hashed_state_update: storages_state_update, proof_targets, proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); + total_updates += 1; } - total_updates as u64 + total_updates } /// Handler for new proof calculated, aggregates all the existing sequential proofs. From e630c80918d32092fd76073881ba4985e2a43d3f Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:43:42 +0000 Subject: [PATCH 17/37] collect into one vec first --- crates/engine/tree/src/tree/root.rs | 37 ++++++++++------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index b5f919d22a2e..802f45ea78f0 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -667,10 +667,9 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let mut hashed_state_update = evm_state_to_hashed_post_state(update); - - let mut total_updates = 0; + let mut state_updates = Vec::new(); + let mut hashed_state_update = evm_state_to_hashed_post_state(update); for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { let mut chunks = vec![vec![]; 1]; @@ -709,43 +708,31 @@ where .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) .unzip(); - let account_hashed_state_update = HashedPostState { accounts, storages }; - - let proof_targets = - get_proof_targets(&account_hashed_state_update, &self.fetched_proof_targets); - extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - - self.multiproof_manager.spawn_or_queue(MultiproofInput { - config: self.config.clone(), - source: Some(source), - hashed_state_update: account_hashed_state_update, - proof_targets, - proof_sequence_number: self.proof_sequencer.next_sequence(), - state_root_message_sender: self.tx.clone(), - }); - total_updates += 1; + state_updates.push(HashedPostState { accounts, storages }); } } if !hashed_state_update.storages.is_empty() { - let storages_state_update = - HashedPostState { accounts: Default::default(), ..hashed_state_update }; - let proof_targets = - get_proof_targets(&storages_state_update, &self.fetched_proof_targets); + state_updates + .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); + } + + let total_updates = state_updates.len(); + for state_update in state_updates { + let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: Some(source), - hashed_state_update: storages_state_update, + hashed_state_update: state_update, proof_targets, proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); - total_updates += 1; } - total_updates + total_updates as u64 } /// Handler for new proof calculated, aggregates all the existing sequential proofs. From c6f1fcba46e0c8588e9be1b098b039d43b11e65e Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:39:29 +0000 Subject: [PATCH 18/37] comments --- crates/engine/tree/src/tree/root.rs | 62 +++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 2ef8f08e7731..c85aff208278 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -8,6 +8,7 @@ use rayon::iter::{ParallelBridge, ParallelIterator}; use reth_errors::{ProviderError, ProviderResult}; use reth_evm::system_calls::{OnStateHook, StateChangeSource}; use reth_metrics::Metrics; +use reth_primitives_traits::Account; use reth_provider::{ providers::ConsistentDbView, BlockReader, DBProvider, DatabaseProviderFactory, StateCommitmentProvider, @@ -43,6 +44,11 @@ use tracing::{debug, error, trace, trace_span}; /// The level below which the sparse trie hashes are calculated in [`update_sparse_trie`]. const SPARSE_TRIE_INCREMENTAL_LEVEL: usize = 2; +/// Maximum number of account targets in a multiproof. +const MULTIPROOF_ACCOUNTS_CHUNK_SIZE: usize = 5; +/// Maximum number of storage slots targets per account in a multiproof. +const MULTIPROOF_STORAGES_CHUNK_SIZE: usize = 5; + /// Determines the size of the rayon thread pool to be used in [`StateRootTask`]. /// /// The value is determined as `max(NUM_THREADS - 2, 3)`: @@ -685,26 +691,49 @@ where /// Handles state update. /// + /// Chunks into multiple state updates, so that each has at most + /// `MULTIPROOF_ACCOUNTS_CHUNK_SIZE` accounts and `MULTIPROOF_STORAGES_CHUNK_SIZE` storage + /// slots per account. + /// + /// After chunking, [`MultiproofManager::spawn_or_queue`] is called for each state update. + /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { + let mut hashed_state_update = evm_state_to_hashed_post_state(update); + + // Chunked state updates. Each `HashedPostState` will have at most + // `MULTIPROOF_ACCOUNTS_CHUNK_SIZE` accounts and `MULTIPROOF_STORAGES_CHUNK_SIZE` + // storage slots per account. let mut state_updates = Vec::new(); - let mut hashed_state_update = evm_state_to_hashed_post_state(update); - for accounts_chunk in &hashed_state_update.accounts.into_iter().chunks(5) { - let mut chunks = vec![vec![]; 1]; + // Iterate over updated accounts in chunks + for accounts_chunk in + &hashed_state_update.accounts.into_iter().chunks(MULTIPROOF_ACCOUNTS_CHUNK_SIZE) + { + // Outer vector is a list of chunks. Inner vectors represent accounts with storage slots + // that we need to fetch multiproofs for. + // + // We will have at least one chunk per account. If it has no storage slots, it will be + // the only chunk. + let mut state_update_chunks: Vec, HashedStorage)>> = + vec![vec![]; 1]; for (address, account) in accounts_chunk { let storages = hashed_state_update.storages.remove(&address).unwrap_or_default().storage; - let chunks_num = storages.len().div_ceil(5); - if chunks.len() < chunks_num { - chunks.resize(chunks_num, Vec::new()); + + // Increase the length of the chunks vector if needed. + let chunks_num = storages.len().div_ceil(MULTIPROOF_STORAGES_CHUNK_SIZE); + if state_update_chunks.len() < chunks_num { + state_update_chunks.resize(chunks_num, Vec::new()); } - let storages = storages.into_iter().chunks(5); + // Iterate over updated storage slots in chunks + let storages = storages.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); let mut storages_iter = storages.into_iter(); - chunks[0].push(( + // Each account will have at least one chunk. + state_update_chunks[0].push(( address, account, if let Some(storages) = storages_iter.next() { @@ -714,8 +743,9 @@ where }, )); + // Process the rest of the storage slots and add them to the list of chunks. for (i, storages_chunk) in storages_iter.enumerate() { - chunks[i + 1].push(( + state_update_chunks[i + 1].push(( address, account, HashedStorage::from_iter(false, storages_chunk), @@ -723,7 +753,9 @@ where } } - for chunk in chunks { + // Transform chunks into separate lists of account updates and storage updates, and add + // them to the list of state updates. + for chunk in state_update_chunks { let (accounts, storages) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) @@ -732,11 +764,17 @@ where } } + // If there are any accounts with storages that were not present in + // `hashed_state_update.accounts`, add them all as a single state update. + // TODO: chunk them as well if !hashed_state_update.storages.is_empty() { - state_updates - .push(HashedPostState { accounts: Default::default(), ..hashed_state_update }); + state_updates.push(HashedPostState { + accounts: Default::default(), + storages: hashed_state_update.storages, + }); } + // Spawn multiproofs for each of the chunked state updates. let total_updates = state_updates.len(); for state_update in state_updates { let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); From 898fe1db614843d78468dde713d2a2782a266840 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 21 Feb 2025 15:16:11 +0000 Subject: [PATCH 19/37] iterate all together --- crates/engine/tree/src/tree/root.rs | 63 ++++++++++++++++++----------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index c85aff208278..fef877ef503c 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1,6 +1,10 @@ //! State root task related functionality. -use alloy_primitives::map::HashSet; +use alloy_primitives::{ + keccak256, + map::{B256Map, HashSet}, + B256, +}; use derive_more::derive::Deref; use itertools::Itertools; use metrics::Histogram; @@ -30,7 +34,6 @@ use reth_trie_sparse::{ errors::{SparseStateTrieResult, SparseTrieErrorKind}, SparseStateTrie, }; -use revm_primitives::{keccak256, B256}; use std::{ collections::{BTreeMap, VecDeque}, sync::{ @@ -706,30 +709,46 @@ where // storage slots per account. let mut state_updates = Vec::new(); - // Iterate over updated accounts in chunks - for accounts_chunk in - &hashed_state_update.accounts.into_iter().chunks(MULTIPROOF_ACCOUNTS_CHUNK_SIZE) + // Iterate over updated accounts with associated storages in chunks + let accounts_with_storages = hashed_state_update + .accounts + .into_iter() + .map(|(address, account)| { + ( + address, + Some(account), + hashed_state_update.storages.remove(&address).unwrap_or_default(), + ) + }) + .collect::>(); + for accounts_chunk in &accounts_with_storages + .into_iter() + .chain( + hashed_state_update + .storages + .into_iter() + .map(|(address, storage)| (address, None, storage)), + ) + .map(|(address, account, storage)| (address, account, storage.storage)) + .chunks(MULTIPROOF_ACCOUNTS_CHUNK_SIZE) { // Outer vector is a list of chunks. Inner vectors represent accounts with storage slots // that we need to fetch multiproofs for. // // We will have at least one chunk per account. If it has no storage slots, it will be // the only chunk. - let mut state_update_chunks: Vec, HashedStorage)>> = + let mut state_update_chunks: Vec>, HashedStorage)>> = vec![vec![]; 1]; - for (address, account) in accounts_chunk { - let storages = - hashed_state_update.storages.remove(&address).unwrap_or_default().storage; - + for (address, account, storage) in accounts_chunk { // Increase the length of the chunks vector if needed. - let chunks_num = storages.len().div_ceil(MULTIPROOF_STORAGES_CHUNK_SIZE); + let chunks_num = storage.len().div_ceil(MULTIPROOF_STORAGES_CHUNK_SIZE); if state_update_chunks.len() < chunks_num { state_update_chunks.resize(chunks_num, Vec::new()); } // Iterate over updated storage slots in chunks - let storages = storages.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); + let storages = storage.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); let mut storages_iter = storages.into_iter(); // Each account will have at least one chunk. @@ -756,24 +775,20 @@ where // Transform chunks into separate lists of account updates and storage updates, and add // them to the list of state updates. for chunk in state_update_chunks { - let (accounts, storages) = chunk + let (accounts, storages): (Vec<_>, B256Map<_>) = chunk .into_iter() .map(|(address, account, storages)| ((address, account), (address, storages))) .unzip(); - state_updates.push(HashedPostState { accounts, storages }); + state_updates.push(HashedPostState { + accounts: accounts + .into_iter() + .filter_map(|(address, account)| account.map(|account| (address, account))) + .collect(), + storages, + }); } } - // If there are any accounts with storages that were not present in - // `hashed_state_update.accounts`, add them all as a single state update. - // TODO: chunk them as well - if !hashed_state_update.storages.is_empty() { - state_updates.push(HashedPostState { - accounts: Default::default(), - storages: hashed_state_update.storages, - }); - } - // Spawn multiproofs for each of the chunked state updates. let total_updates = state_updates.len(); for state_update in state_updates { From ed5ae5d895eb47cfff89b3208003f649cc7f27ad Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Fri, 21 Feb 2025 18:23:38 +0000 Subject: [PATCH 20/37] use separate iterator struct --- crates/engine/tree/src/tree/root.rs | 195 ++++++++++++++-------------- 1 file changed, 94 insertions(+), 101 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index fef877ef503c..fc9977d7906b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -2,7 +2,7 @@ use alloy_primitives::{ keccak256, - map::{B256Map, HashSet}, + map::{B256Map, B256Set, HashSet}, B256, }; use derive_more::derive::Deref; @@ -12,7 +12,6 @@ use rayon::iter::{ParallelBridge, ParallelIterator}; use reth_errors::{ProviderError, ProviderResult}; use reth_evm::system_calls::{OnStateHook, StateChangeSource}; use reth_metrics::Metrics; -use reth_primitives_traits::Account; use reth_provider::{ providers::ConsistentDbView, BlockReader, DBProvider, DatabaseProviderFactory, StateCommitmentProvider, @@ -35,7 +34,7 @@ use reth_trie_sparse::{ SparseStateTrie, }; use std::{ - collections::{BTreeMap, VecDeque}, + collections::{hash_map::Entry, BTreeMap, VecDeque}, sync::{ mpsc::{self, channel, Receiver, Sender}, Arc, @@ -630,16 +629,16 @@ where let proof_targets = self.get_prefetch_proof_targets(targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - // TODO: chunk proof targets and spawn separate multiproof tasks - - self.multiproof_manager.spawn_or_queue(MultiproofInput { - config: self.config.clone(), - source: None, - hashed_state_update: Default::default(), - proof_targets, - proof_sequence_number: self.proof_sequencer.next_sequence(), - state_root_message_sender: self.tx.clone(), - }); + for chunk in ChunkedProofTargets::new(proof_targets).flatten() { + self.multiproof_manager.spawn_or_queue(MultiproofInput { + config: self.config.clone(), + source: None, + hashed_state_update: HashedPostState::default(), + proof_targets: chunk, + proof_sequence_number: self.proof_sequencer.next_sequence(), + state_root_message_sender: self.tx.clone(), + }); + } } /// Calls `get_proof_targets` with existing proof targets for prefetching. @@ -702,104 +701,52 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let mut hashed_state_update = evm_state_to_hashed_post_state(update); + let mut state_update = evm_state_to_hashed_post_state(update); + let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); + extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - // Chunked state updates. Each `HashedPostState` will have at most - // `MULTIPROOF_ACCOUNTS_CHUNK_SIZE` accounts and `MULTIPROOF_STORAGES_CHUNK_SIZE` - // storage slots per account. - let mut state_updates = Vec::new(); + let mut total_updates = 0; - // Iterate over updated accounts with associated storages in chunks - let accounts_with_storages = hashed_state_update - .accounts - .into_iter() - .map(|(address, account)| { - ( - address, - Some(account), - hashed_state_update.storages.remove(&address).unwrap_or_default(), - ) - }) - .collect::>(); - for accounts_chunk in &accounts_with_storages - .into_iter() - .chain( - hashed_state_update - .storages - .into_iter() - .map(|(address, storage)| (address, None, storage)), - ) - .map(|(address, account, storage)| (address, account, storage.storage)) - .chunks(MULTIPROOF_ACCOUNTS_CHUNK_SIZE) - { - // Outer vector is a list of chunks. Inner vectors represent accounts with storage slots - // that we need to fetch multiproofs for. - // - // We will have at least one chunk per account. If it has no storage slots, it will be - // the only chunk. - let mut state_update_chunks: Vec>, HashedStorage)>> = - vec![vec![]; 1]; - - for (address, account, storage) in accounts_chunk { - // Increase the length of the chunks vector if needed. - let chunks_num = storage.len().div_ceil(MULTIPROOF_STORAGES_CHUNK_SIZE); - if state_update_chunks.len() < chunks_num { - state_update_chunks.resize(chunks_num, Vec::new()); - } + for chunk in ChunkedProofTargets::new(proof_targets).flatten() { + total_updates += 1; - // Iterate over updated storage slots in chunks - let storages = storage.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); - let mut storages_iter = storages.into_iter(); - - // Each account will have at least one chunk. - state_update_chunks[0].push(( - address, - account, - if let Some(storages) = storages_iter.next() { - HashedStorage::from_iter(false, storages) - } else { - HashedStorage::new(false) - }, - )); - - // Process the rest of the storage slots and add them to the list of chunks. - for (i, storages_chunk) in storages_iter.enumerate() { - state_update_chunks[i + 1].push(( - address, - account, - HashedStorage::from_iter(false, storages_chunk), - )); + let mut accounts = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); + let mut storages = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); + + for (&address, storage_slots) in &chunk { + if let Some(account) = state_update.accounts.remove(&address) { + accounts.insert(address, account); } - } - // Transform chunks into separate lists of account updates and storage updates, and add - // them to the list of state updates. - for chunk in state_update_chunks { - let (accounts, storages): (Vec<_>, B256Map<_>) = chunk - .into_iter() - .map(|(address, account, storages)| ((address, account), (address, storages))) - .unzip(); - state_updates.push(HashedPostState { - accounts: accounts - .into_iter() - .filter_map(|(address, account)| account.map(|account| (address, account))) - .collect(), - storages, - }); + if !storage_slots.is_empty() { + let state_storage = state_update.storages.entry(address); + let mut hashed_storage = HashedStorage::default(); + match state_storage { + Entry::Occupied(mut entry) => { + for storage_slot in storage_slots { + let value = entry + .get_mut() + .storage + .remove(storage_slot) + .expect("storage slot should be present"); + hashed_storage.storage.insert(*storage_slot, value); + } + + if entry.get_mut().storage.is_empty() { + entry.remove(); + } + } + Entry::Vacant(_) => unreachable!(), + } + storages.insert(address, hashed_storage); + } } - } - - // Spawn multiproofs for each of the chunked state updates. - let total_updates = state_updates.len(); - for state_update in state_updates { - let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); - extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: Some(source), - hashed_state_update: state_update, - proof_targets, + hashed_state_update: HashedPostState { accounts, storages }, + proof_targets: chunk, proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), }); @@ -1208,6 +1155,52 @@ fn get_proof_targets( targets } +/// Iterator over proof targets chunks. +/// +/// Each chunk will have at most [`MULTIPROOF_ACCOUNTS_CHUNK_SIZE`] accounts and +/// [`MULTIPROOF_STORAGES_CHUNK_SIZE`] storage slots per account. +/// +/// This iterator will yield items of type [`Vec>`], with each mapping having a +/// maximum length of [`MULTIPROOF_ACCOUNTS_CHUNK_SIZE`], and each mapping value having a maximum +/// length of [`MULTIPROOF_STORAGES_CHUNK_SIZE`]. +struct ChunkedProofTargets { + proof_targets: MultiProofTargets, +} + +impl ChunkedProofTargets { + fn new(proof_targets: MultiProofTargets) -> Self { + Self { proof_targets } + } +} + +impl Iterator for ChunkedProofTargets { + type Item = Vec>; + + fn next(&mut self) -> Option { + if self.proof_targets.is_empty() { + return None; + } + + let mut chunks = vec![B256Map::::default(); 1]; + + let accounts_chunk: B256Map = + self.proof_targets.drain().take(MULTIPROOF_ACCOUNTS_CHUNK_SIZE).collect(); + + for (address, storage_slots) in accounts_chunk { + let storage_chunks = storage_slots.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); + + for (i, chunk) in storage_chunks.into_iter().enumerate() { + if i >= chunks.len() { + chunks.push(B256Map::default()); + } + chunks[i].entry(address).or_default().extend(chunk); + } + } + + Some(chunks) + } +} + /// Calculate multiproof for the targets. #[inline] fn calculate_multiproof( From b4f736c8bc5f673d3867243e9a7a08b93af22221 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:12:06 +0000 Subject: [PATCH 21/37] wip --- crates/engine/tree/src/tree/root.rs | 63 ++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index fc9977d7906b..3e6b91004be5 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -2,7 +2,7 @@ use alloy_primitives::{ keccak256, - map::{B256Map, B256Set, HashSet}, + map::{B256Map, HashSet}, B256, }; use derive_more::derive::Deref; @@ -629,7 +629,13 @@ where let proof_targets = self.get_prefetch_proof_targets(targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - for chunk in ChunkedProofTargets::new(proof_targets).flatten() { + for chunk in ChunkedProofTargets::new( + proof_targets, + MULTIPROOF_ACCOUNTS_CHUNK_SIZE, + MULTIPROOF_STORAGES_CHUNK_SIZE, + ) + .flatten() + { self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: None, @@ -707,7 +713,13 @@ where let mut total_updates = 0; - for chunk in ChunkedProofTargets::new(proof_targets).flatten() { + for chunk in ChunkedProofTargets::new( + proof_targets, + MULTIPROOF_ACCOUNTS_CHUNK_SIZE, + MULTIPROOF_STORAGES_CHUNK_SIZE, + ) + .flatten() + { total_updates += 1; let mut accounts = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); @@ -1165,29 +1177,35 @@ fn get_proof_targets( /// length of [`MULTIPROOF_STORAGES_CHUNK_SIZE`]. struct ChunkedProofTargets { proof_targets: MultiProofTargets, + accounts_chunk_size: usize, + storages_chunk_size: usize, } impl ChunkedProofTargets { - fn new(proof_targets: MultiProofTargets) -> Self { - Self { proof_targets } + fn new( + proof_targets: MultiProofTargets, + accounts_chunk_size: usize, + storages_chunk_size: usize, + ) -> Self { + Self { proof_targets, accounts_chunk_size, storages_chunk_size } } } impl Iterator for ChunkedProofTargets { - type Item = Vec>; + type Item = Vec; fn next(&mut self) -> Option { if self.proof_targets.is_empty() { return None; } - let mut chunks = vec![B256Map::::default(); 1]; + let mut chunks = vec![MultiProofTargets::default(); 1]; - let accounts_chunk: B256Map = - self.proof_targets.drain().take(MULTIPROOF_ACCOUNTS_CHUNK_SIZE).collect(); + let accounts_chunk: MultiProofTargets = + self.proof_targets.drain().take(self.accounts_chunk_size).collect(); for (address, storage_slots) in accounts_chunk { - let storage_chunks = storage_slots.into_iter().chunks(MULTIPROOF_STORAGES_CHUNK_SIZE); + let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); for (i, chunk) in storage_chunks.into_iter().enumerate() { if i >= chunks.len() { @@ -1816,4 +1834,29 @@ mod tests { vec![slot2].into_iter().collect::() ); } + + #[test] + fn test_chunked_proof_targets() { + let address1 = B256::from([1; 32]); + let address2 = B256::from([2; 32]); + + let slot1 = B256::from([1; 32]); + let slot2 = B256::from([2; 32]); + let slot3 = B256::from([3; 32]); + + let targets = MultiProofTargets::from_iter([ + (address1, vec![slot1, slot2, slot3].into_iter().collect::()), + (address2, vec![slot2, slot3].into_iter().collect::()), + ]); + let chunks = ChunkedProofTargets::new(targets, 1, 2).flatten().collect::>(); + + assert_eq!( + chunks, + vec![ + MultiProofTargets::from_iter([(address1, B256Set::from_iter([slot1, slot2]))]), + MultiProofTargets::from_iter([(address1, B256Set::from_iter([slot3]))]), + MultiProofTargets::from_iter([(address2, B256Set::from_iter([slot2, slot3]))]) + ] + ); + } } From cbbb764e603791c8e4bc1e7efe94c3815ab06d4b Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:25:21 +0000 Subject: [PATCH 22/37] fix draining --- crates/engine/tree/src/tree/root.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 3e6b91004be5..77529cd74606 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1201,10 +1201,10 @@ impl Iterator for ChunkedProofTargets { let mut chunks = vec![MultiProofTargets::default(); 1]; - let accounts_chunk: MultiProofTargets = - self.proof_targets.drain().take(self.accounts_chunk_size).collect(); - - for (address, storage_slots) in accounts_chunk { + let keys: Vec = + self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect(); + for address in keys { + let storage_slots = self.proof_targets.remove(&address).unwrap(); let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); for (i, chunk) in storage_chunks.into_iter().enumerate() { From 489c2e75a39d7b5bad54a290abdde06da264f2cb Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:35:19 +0000 Subject: [PATCH 23/37] extend storages set --- crates/engine/tree/src/tree/root.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 77529cd74606..ab2a3ec1fb82 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -723,7 +723,8 @@ where total_updates += 1; let mut accounts = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); - let mut storages = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); + let mut storages = + B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { if let Some(account) = state_update.accounts.remove(&address) { @@ -732,7 +733,7 @@ where if !storage_slots.is_empty() { let state_storage = state_update.storages.entry(address); - let mut hashed_storage = HashedStorage::default(); + let mut hashed_storage = B256Map::default(); match state_storage { Entry::Occupied(mut entry) => { for storage_slot in storage_slots { @@ -741,7 +742,7 @@ where .storage .remove(storage_slot) .expect("storage slot should be present"); - hashed_storage.storage.insert(*storage_slot, value); + hashed_storage.insert(*storage_slot, value); } if entry.get_mut().storage.is_empty() { @@ -750,7 +751,7 @@ where } Entry::Vacant(_) => unreachable!(), } - storages.insert(address, hashed_storage); + storages.entry(address).or_default().storage.extend(hashed_storage); } } From cb4ff209d9eb4ca0e08eb322f2578fdee6387e2c Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:43:30 +0000 Subject: [PATCH 24/37] do not modify state update --- crates/engine/tree/src/tree/root.rs | 38 ++++++++++++----------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index ab2a3ec1fb82..d9502090307b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -34,7 +34,7 @@ use reth_trie_sparse::{ SparseStateTrie, }; use std::{ - collections::{hash_map::Entry, BTreeMap, VecDeque}, + collections::{BTreeMap, VecDeque}, sync::{ mpsc::{self, channel, Receiver, Sender}, Arc, @@ -707,7 +707,7 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let mut state_update = evm_state_to_hashed_post_state(update); + let state_update = evm_state_to_hashed_post_state(update); let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); @@ -727,31 +727,23 @@ where B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { - if let Some(account) = state_update.accounts.remove(&address) { - accounts.insert(address, account); + if let Some(account) = state_update.accounts.get(&address) { + accounts.insert(address, *account); } if !storage_slots.is_empty() { - let state_storage = state_update.storages.entry(address); - let mut hashed_storage = B256Map::default(); - match state_storage { - Entry::Occupied(mut entry) => { - for storage_slot in storage_slots { - let value = entry - .get_mut() - .storage - .remove(storage_slot) - .expect("storage slot should be present"); - hashed_storage.insert(*storage_slot, value); - } - - if entry.get_mut().storage.is_empty() { - entry.remove(); - } - } - Entry::Vacant(_) => unreachable!(), + let mut storage = B256Map::default(); + for storage_slot in storage_slots { + let value = state_update + .storages + .get(&address) + .expect("storage should be present") + .storage + .get(storage_slot) + .expect("storage slot should be present"); + storage.insert(*storage_slot, *value); } - storages.entry(address).or_default().storage.extend(hashed_storage); + storages.entry(address).or_default().storage.extend(storage); } } From c40876013906f53f722b65c2e80e4cf1c193e427 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:49:05 +0000 Subject: [PATCH 25/37] back to modifying state update, add assert --- crates/engine/tree/src/tree/root.rs | 36 ++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index d9502090307b..4400eb79622f 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -34,7 +34,7 @@ use reth_trie_sparse::{ SparseStateTrie, }; use std::{ - collections::{BTreeMap, VecDeque}, + collections::{hash_map::Entry, BTreeMap, VecDeque}, sync::{ mpsc::{self, channel, Receiver, Sender}, Arc, @@ -707,7 +707,7 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let state_update = evm_state_to_hashed_post_state(update); + let mut state_update = evm_state_to_hashed_post_state(update); let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); @@ -727,21 +727,29 @@ where B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { - if let Some(account) = state_update.accounts.get(&address) { - accounts.insert(address, *account); + if let Some(account) = state_update.accounts.remove(&address) { + accounts.insert(address, account); } if !storage_slots.is_empty() { + let state_storage = state_update.storages.entry(address); let mut storage = B256Map::default(); - for storage_slot in storage_slots { - let value = state_update - .storages - .get(&address) - .expect("storage should be present") - .storage - .get(storage_slot) - .expect("storage slot should be present"); - storage.insert(*storage_slot, *value); + match state_storage { + Entry::Occupied(mut entry) => { + let entry_mut = entry.get_mut(); + + for storage_slot in storage_slots { + let value = entry_mut + .storage + .remove(storage_slot) + .expect("storage slot should be present"); + storage.insert(*storage_slot, value); + } + if entry_mut.storage.is_empty() { + entry.remove(); + } + } + Entry::Vacant(_) => unreachable!(), } storages.entry(address).or_default().storage.extend(storage); } @@ -757,6 +765,8 @@ where }); } + debug_assert!(state_update.is_empty()); + total_updates as u64 } From ebd773d83a5376a0beb9bd7dfa2dc52270950c45 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:57:51 +0000 Subject: [PATCH 26/37] do not consume storage slots fully --- crates/engine/tree/src/tree/root.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 4400eb79622f..b9b18803ec9e 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1198,24 +1198,36 @@ impl Iterator for ChunkedProofTargets { type Item = Vec; fn next(&mut self) -> Option { - if self.proof_targets.is_empty() { + let mut chunks = vec![MultiProofTargets::default(); 1]; + + let address_keys = + self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect::>(); + if address_keys.is_empty() { return None; } - let mut chunks = vec![MultiProofTargets::default(); 1]; + for address in address_keys { + let Entry::Occupied(mut storage) = self.proof_targets.entry(address) else { + unreachable!() + }; + let storage_mut = storage.get_mut(); + let storage_keys = + storage_mut.iter().take(self.storages_chunk_size).copied().collect::>(); + let storage_slots = storage_keys + .into_iter() + .map(|slot| storage_mut.remove(&slot).then_some(slot).unwrap()); - let keys: Vec = - self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect(); - for address in keys { - let storage_slots = self.proof_targets.remove(&address).unwrap(); let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); - for (i, chunk) in storage_chunks.into_iter().enumerate() { if i >= chunks.len() { chunks.push(B256Map::default()); } chunks[i].entry(address).or_default().extend(chunk); } + + if storage_mut.is_empty() { + storage.remove(); + } } Some(chunks) From 4ec8324991ba484411a4c428d62b154dafcc8b67 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:04:21 +0000 Subject: [PATCH 27/37] Revert "do not consume storage slots fully" This reverts commit ebd773d83a5376a0beb9bd7dfa2dc52270950c45. --- crates/engine/tree/src/tree/root.rs | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index b9b18803ec9e..4400eb79622f 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1198,36 +1198,24 @@ impl Iterator for ChunkedProofTargets { type Item = Vec; fn next(&mut self) -> Option { - let mut chunks = vec![MultiProofTargets::default(); 1]; - - let address_keys = - self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect::>(); - if address_keys.is_empty() { + if self.proof_targets.is_empty() { return None; } - for address in address_keys { - let Entry::Occupied(mut storage) = self.proof_targets.entry(address) else { - unreachable!() - }; - let storage_mut = storage.get_mut(); - let storage_keys = - storage_mut.iter().take(self.storages_chunk_size).copied().collect::>(); - let storage_slots = storage_keys - .into_iter() - .map(|slot| storage_mut.remove(&slot).then_some(slot).unwrap()); + let mut chunks = vec![MultiProofTargets::default(); 1]; + let keys: Vec = + self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect(); + for address in keys { + let storage_slots = self.proof_targets.remove(&address).unwrap(); let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); + for (i, chunk) in storage_chunks.into_iter().enumerate() { if i >= chunks.len() { chunks.push(B256Map::default()); } chunks[i].entry(address).or_default().extend(chunk); } - - if storage_mut.is_empty() { - storage.remove(); - } } Some(chunks) From 66b1464fa4cc606da8f6c55d55d1f981792b5492 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:04:23 +0000 Subject: [PATCH 28/37] Revert "back to modifying state update, add assert" This reverts commit c40876013906f53f722b65c2e80e4cf1c193e427. --- crates/engine/tree/src/tree/root.rs | 36 +++++++++++------------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 4400eb79622f..d9502090307b 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -34,7 +34,7 @@ use reth_trie_sparse::{ SparseStateTrie, }; use std::{ - collections::{hash_map::Entry, BTreeMap, VecDeque}, + collections::{BTreeMap, VecDeque}, sync::{ mpsc::{self, channel, Receiver, Sender}, Arc, @@ -707,7 +707,7 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let mut state_update = evm_state_to_hashed_post_state(update); + let state_update = evm_state_to_hashed_post_state(update); let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); @@ -727,29 +727,21 @@ where B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { - if let Some(account) = state_update.accounts.remove(&address) { - accounts.insert(address, account); + if let Some(account) = state_update.accounts.get(&address) { + accounts.insert(address, *account); } if !storage_slots.is_empty() { - let state_storage = state_update.storages.entry(address); let mut storage = B256Map::default(); - match state_storage { - Entry::Occupied(mut entry) => { - let entry_mut = entry.get_mut(); - - for storage_slot in storage_slots { - let value = entry_mut - .storage - .remove(storage_slot) - .expect("storage slot should be present"); - storage.insert(*storage_slot, value); - } - if entry_mut.storage.is_empty() { - entry.remove(); - } - } - Entry::Vacant(_) => unreachable!(), + for storage_slot in storage_slots { + let value = state_update + .storages + .get(&address) + .expect("storage should be present") + .storage + .get(storage_slot) + .expect("storage slot should be present"); + storage.insert(*storage_slot, *value); } storages.entry(address).or_default().storage.extend(storage); } @@ -765,8 +757,6 @@ where }); } - debug_assert!(state_update.is_empty()); - total_updates as u64 } From c3c7ee86dc273cf5f16ac9e7871975c746f2ae00 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:04:24 +0000 Subject: [PATCH 29/37] Revert "do not modify state update" This reverts commit cb4ff209d9eb4ca0e08eb322f2578fdee6387e2c. --- crates/engine/tree/src/tree/root.rs | 38 +++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index d9502090307b..ab2a3ec1fb82 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -34,7 +34,7 @@ use reth_trie_sparse::{ SparseStateTrie, }; use std::{ - collections::{BTreeMap, VecDeque}, + collections::{hash_map::Entry, BTreeMap, VecDeque}, sync::{ mpsc::{self, channel, Receiver, Sender}, Arc, @@ -707,7 +707,7 @@ where /// /// Returns number of new updates generated by one state update. fn on_state_update(&mut self, source: StateChangeSource, update: EvmState) -> u64 { - let state_update = evm_state_to_hashed_post_state(update); + let mut state_update = evm_state_to_hashed_post_state(update); let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); @@ -727,23 +727,31 @@ where B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { - if let Some(account) = state_update.accounts.get(&address) { - accounts.insert(address, *account); + if let Some(account) = state_update.accounts.remove(&address) { + accounts.insert(address, account); } if !storage_slots.is_empty() { - let mut storage = B256Map::default(); - for storage_slot in storage_slots { - let value = state_update - .storages - .get(&address) - .expect("storage should be present") - .storage - .get(storage_slot) - .expect("storage slot should be present"); - storage.insert(*storage_slot, *value); + let state_storage = state_update.storages.entry(address); + let mut hashed_storage = B256Map::default(); + match state_storage { + Entry::Occupied(mut entry) => { + for storage_slot in storage_slots { + let value = entry + .get_mut() + .storage + .remove(storage_slot) + .expect("storage slot should be present"); + hashed_storage.insert(*storage_slot, value); + } + + if entry.get_mut().storage.is_empty() { + entry.remove(); + } + } + Entry::Vacant(_) => unreachable!(), } - storages.entry(address).or_default().storage.extend(storage); + storages.entry(address).or_default().storage.extend(hashed_storage); } } From 48a4720e29860ef8e8a35065b6da76699ce13951 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:04:25 +0000 Subject: [PATCH 30/37] Revert "extend storages set" This reverts commit 489c2e75a39d7b5bad54a290abdde06da264f2cb. --- crates/engine/tree/src/tree/root.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index ab2a3ec1fb82..77529cd74606 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -723,8 +723,7 @@ where total_updates += 1; let mut accounts = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); - let mut storages = - B256Map::::with_capacity_and_hasher(chunk.len(), Default::default()); + let mut storages = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); for (&address, storage_slots) in &chunk { if let Some(account) = state_update.accounts.remove(&address) { @@ -733,7 +732,7 @@ where if !storage_slots.is_empty() { let state_storage = state_update.storages.entry(address); - let mut hashed_storage = B256Map::default(); + let mut hashed_storage = HashedStorage::default(); match state_storage { Entry::Occupied(mut entry) => { for storage_slot in storage_slots { @@ -742,7 +741,7 @@ where .storage .remove(storage_slot) .expect("storage slot should be present"); - hashed_storage.insert(*storage_slot, value); + hashed_storage.storage.insert(*storage_slot, value); } if entry.get_mut().storage.is_empty() { @@ -751,7 +750,7 @@ where } Entry::Vacant(_) => unreachable!(), } - storages.entry(address).or_default().storage.extend(hashed_storage); + storages.insert(address, hashed_storage); } } From 28d9c636c4e9c94d8b7175a0bedc7ccef718746a Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:05:52 +0000 Subject: [PATCH 31/37] revertme: add logs --- crates/engine/tree/src/tree/root.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 77529cd74606..a747942488b9 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -711,6 +711,9 @@ where let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); + debug!(target: "engine::root", ?state_update, "State update"); + debug!(target: "engine::root", ?proof_targets, "Proof targets"); + let mut total_updates = 0; for chunk in ChunkedProofTargets::new( @@ -720,6 +723,8 @@ where ) .flatten() { + debug!(target: "engine::root", ?chunk, "Chunk"); + total_updates += 1; let mut accounts = B256Map::with_capacity_and_hasher(chunk.len(), Default::default()); From 9850f9c5eb02ff62ff81a25ad5b2f476177d5db1 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:39:43 +0000 Subject: [PATCH 32/37] fix test --- Cargo.lock | 1 + Cargo.toml | 3 +- crates/engine/invalid-block-hooks/Cargo.toml | 2 +- crates/engine/tree/Cargo.toml | 1 + crates/engine/tree/src/tree/root.rs | 34 ++++++++++++++------ crates/trie/sparse/Cargo.toml | 2 +- 6 files changed, 30 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 38657b1a6adf..ba7203947047 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7434,6 +7434,7 @@ dependencies = [ "itertools 0.14.0", "metrics", "mini-moka", + "pretty_assertions", "proptest", "rand 0.8.5", "rayon", diff --git a/Cargo.toml b/Cargo.toml index b0ab50f6626d..db2e7a860f11 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -602,13 +602,14 @@ arbitrary = "1.3" assert_matches = "1.5.0" criterion = { package = "codspeed-criterion-compat", version = "2.7" } pprof = "0.14" +pretty_assertions = "1.4" proptest = "1.4" proptest-derive = "0.5" +rstest = "0.24.0" serial_test = { default-features = false, version = "3" } similar-asserts = { version = "1.5.0", features = ["serde"] } tempfile = "3.8" test-fuzz = "7" -rstest = "0.24.0" # allocators tikv-jemalloc-ctl = "0.6" diff --git a/crates/engine/invalid-block-hooks/Cargo.toml b/crates/engine/invalid-block-hooks/Cargo.toml index a7b0153d0d4b..045cccdba354 100644 --- a/crates/engine/invalid-block-hooks/Cargo.toml +++ b/crates/engine/invalid-block-hooks/Cargo.toml @@ -35,6 +35,6 @@ futures.workspace = true # misc eyre.workspace = true jsonrpsee.workspace = true -pretty_assertions = "1.4" +pretty_assertions.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/engine/tree/Cargo.toml b/crates/engine/tree/Cargo.toml index c44a59dc8405..9fb7516112b1 100644 --- a/crates/engine/tree/Cargo.toml +++ b/crates/engine/tree/Cargo.toml @@ -92,6 +92,7 @@ revm-state.workspace = true assert_matches.workspace = true criterion.workspace = true crossbeam-channel = "0.5.13" +pretty_assertions.workspace = true proptest.workspace = true rand.workspace = true diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 8090d96054aa..09a3351a5fbb 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1278,6 +1278,8 @@ impl Iterator for ChunkedProofTargets { let storage_slots = self.proof_targets.remove(&address).unwrap(); let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); + chunks[0].entry(address).or_default(); + for (i, chunk) in storage_chunks.into_iter().enumerate() { if i >= chunks.len() { chunks.push(B256Map::default()); @@ -1910,24 +1912,36 @@ mod tests { fn test_chunked_proof_targets() { let address1 = B256::from([1; 32]); let address2 = B256::from([2; 32]); + let address3 = B256::from([3; 32]); let slot1 = B256::from([1; 32]); let slot2 = B256::from([2; 32]); let slot3 = B256::from([3; 32]); - let targets = MultiProofTargets::from_iter([ + let mut targets = MultiProofTargets::from_iter([ (address1, vec![slot1, slot2, slot3].into_iter().collect::()), (address2, vec![slot2, slot3].into_iter().collect::()), + (address3, B256Set::default()), ]); - let chunks = ChunkedProofTargets::new(targets, 1, 2).flatten().collect::>(); + let chunks = ChunkedProofTargets::new(targets.clone(), 1, 2); - assert_eq!( - chunks, - vec![ - MultiProofTargets::from_iter([(address1, B256Set::from_iter([slot1, slot2]))]), - MultiProofTargets::from_iter([(address1, B256Set::from_iter([slot3]))]), - MultiProofTargets::from_iter([(address2, B256Set::from_iter([slot2, slot3]))]) - ] - ); + for chunk in chunks.flatten() { + assert_eq!(chunk.len(), 1); + for (address, slots) in chunk { + assert!(slots.len() <= 2); + + let Entry::Occupied(mut entry) = targets.entry(address) else { + panic!("address not found"); + }; + let entry_mut = entry.get_mut(); + for slot in slots { + entry_mut.remove(&slot); + } + if entry_mut.is_empty() { + entry.remove(); + } + } + } + assert!(targets.is_empty()); } } diff --git a/crates/trie/sparse/Cargo.toml b/crates/trie/sparse/Cargo.toml index 908d03950e9f..a8784a086b34 100644 --- a/crates/trie/sparse/Cargo.toml +++ b/crates/trie/sparse/Cargo.toml @@ -39,7 +39,7 @@ arbitrary.workspace = true assert_matches.workspace = true criterion.workspace = true itertools.workspace = true -pretty_assertions = "1.4" +pretty_assertions.workspace = true proptest-arbitrary-interop.workspace = true proptest.workspace = true rand.workspace = true From fdae0492c5522ac12b66209ad54033f8e08e1f9a Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:42:39 +0000 Subject: [PATCH 33/37] comments --- crates/engine/tree/src/tree/root.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 09a3351a5fbb..d32099339892 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1926,8 +1926,10 @@ mod tests { let chunks = ChunkedProofTargets::new(targets.clone(), 1, 2); for chunk in chunks.flatten() { + // Every chunk should have at most one address assert_eq!(chunk.len(), 1); for (address, slots) in chunk { + // Every chunk should have at most two slots per address assert!(slots.len() <= 2); let Entry::Occupied(mut entry) = targets.entry(address) else { @@ -1942,6 +1944,7 @@ mod tests { } } } + // Verify that chunked iterator had all targets from the original list assert!(targets.is_empty()); } } From 28c26347f97d334c5ea2f0a72d6b1bd33b76aa5b Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:44:17 +0000 Subject: [PATCH 34/37] comments in iterator --- crates/engine/tree/src/tree/root.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index d32099339892..ac42b85caa04 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -1270,14 +1270,18 @@ impl Iterator for ChunkedProofTargets { return None; } + // Every address will be added to at least first chunk let mut chunks = vec![MultiProofTargets::default(); 1]; let keys: Vec = self.proof_targets.keys().take(self.accounts_chunk_size).copied().collect(); for address in keys { - let storage_slots = self.proof_targets.remove(&address).unwrap(); + let storage_slots = self.proof_targets.remove(&address).expect("address not found"); let storage_chunks = storage_slots.into_iter().chunks(self.storages_chunk_size); + // Initialize the chunk with an address with no storage slots. We need to do this, + // because the account may have no storage slots in the targets, but we still need to + // add it to the chunk. chunks[0].entry(address).or_default(); for (i, chunk) in storage_chunks.into_iter().enumerate() { From 4af3fc4872b0cae2060d897fae76d3a09cb810e8 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 12:48:49 +0000 Subject: [PATCH 35/37] revertme: more logs --- crates/engine/tree/src/tree/root.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index ac42b85caa04..35a2a92b978c 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -825,10 +825,12 @@ where } } + let hashed_state_update = HashedPostState { accounts, storages }; + debug!(target: "engine::tree", ?hashed_state_update, ?chunk, "Spawning multiproof"); self.multiproof_manager.spawn_or_queue(MultiproofInput { config: self.config.clone(), source: Some(source), - hashed_state_update: HashedPostState { accounts, storages }, + hashed_state_update, proof_targets: chunk, proof_sequence_number: self.proof_sequencer.next_sequence(), state_root_message_sender: self.tx.clone(), From 254794a2b4f5292d579f1695af0ca2766588180f Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 13:07:47 +0000 Subject: [PATCH 36/37] assert state update is empty --- crates/engine/tree/src/tree/root.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 35a2a92b978c..223d6f52c046 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -837,6 +837,8 @@ where }); } + debug_assert!(state_update.is_empty()); + total_updates as u64 } From 2fae52e87905ef01104aa5c231596e949a8859c4 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Sat, 22 Feb 2025 13:13:02 +0000 Subject: [PATCH 37/37] better logs --- crates/engine/tree/src/tree/root.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/tree/src/tree/root.rs b/crates/engine/tree/src/tree/root.rs index 223d6f52c046..d58eff233952 100644 --- a/crates/engine/tree/src/tree/root.rs +++ b/crates/engine/tree/src/tree/root.rs @@ -777,7 +777,7 @@ where let proof_targets = get_proof_targets(&state_update, &self.fetched_proof_targets); extend_multi_proof_targets_ref(&mut self.fetched_proof_targets, &proof_targets); - debug!(target: "engine::root", ?state_update, "State update"); + debug!(target: "engine::root", ?state_update, "State update before"); debug!(target: "engine::root", ?proof_targets, "Proof targets"); let mut total_updates = 0; @@ -837,7 +837,7 @@ where }); } - debug_assert!(state_update.is_empty()); + debug!(target: "engine::tree", ?state_update, "State update after"); total_updates as u64 }