From 38cf6c900ddd2a2266acf01c97abc159db314a07 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Tue, 26 Nov 2024 22:26:22 +0400 Subject: [PATCH] refactor: improve state writing functions for db provider (#12885) --- crates/stages/stages/src/stages/execution.rs | 2 +- .../src/providers/database/provider.rs | 112 ++---------------- crates/storage/provider/src/traits/state.rs | 11 +- 3 files changed, 18 insertions(+), 107 deletions(-) diff --git a/crates/stages/stages/src/stages/execution.rs b/crates/stages/stages/src/stages/execution.rs index f7ee52fbfe..3c31dea91f 100644 --- a/crates/stages/stages/src/stages/execution.rs +++ b/crates/stages/stages/src/stages/execution.rs @@ -411,7 +411,7 @@ where // Unwind account and storage changesets, as well as receipts. // // This also updates `PlainStorageState` and `PlainAccountState`. - let bundle_state_with_receipts = provider.take_state(range.clone())?; + let bundle_state_with_receipts = provider.take_state_above(unwind_to)?; // Prepare the input for post unwind commit hook, where an `ExExNotification` will be sent. if self.exex_manager_handle.has_exexs() { diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 750a186504..8e4d22067d 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -15,7 +15,7 @@ use crate::{ HeaderSyncGapProvider, HistoricalStateProvider, HistoricalStateProviderRef, HistoryWriter, LatestStateProvider, LatestStateProviderRef, OriginalValuesKnown, ProviderError, PruneCheckpointReader, PruneCheckpointWriter, RevertsInit, StageCheckpointReader, - StateChangeWriter, StateCommitmentProvider, StateProviderBox, StateReader, StateWriter, + StateChangeWriter, StateCommitmentProvider, StateProviderBox, StateWriter, StaticFileProviderFactory, StatsReader, StorageLocation, StorageReader, StorageTrieWriter, TransactionVariant, TransactionsProvider, TransactionsProviderExt, TrieWriter, WithdrawalsProvider, @@ -679,94 +679,6 @@ impl DatabaseProvider { }) } - /// Return the last N blocks of state, recreating the [`ExecutionOutcome`]. - /// - /// 1. Iterate over the [`BlockBodyIndices`][tables::BlockBodyIndices] table to get all the - /// transaction ids. - /// 2. Iterate over the [`StorageChangeSets`][tables::StorageChangeSets] table and the - /// [`AccountChangeSets`][tables::AccountChangeSets] tables in reverse order to reconstruct - /// the changesets. - /// - In order to have both the old and new values in the changesets, we also access the - /// plain state tables. - /// 3. While iterating over the changeset tables, if we encounter a new account or storage slot, - /// we: - /// 1. Take the old value from the changeset - /// 2. Take the new value from the plain state - /// 3. Save the old value to the local state - /// 4. While iterating over the changeset tables, if we encounter an account/storage slot we - /// have seen before we: - /// 1. Take the old value from the changeset - /// 2. Take the new value from the local state - /// 3. Set the local state to the value in the changeset - /// - /// If the range is empty, or there are no blocks for the given range, then this returns `None`. - pub fn get_state( - &self, - range: RangeInclusive, - ) -> ProviderResult> { - if range.is_empty() { - return Ok(None) - } - let start_block_number = *range.start(); - - // We are not removing block meta as it is used to get block changesets. - let block_bodies = self.get::(range.clone())?; - - // get transaction receipts - let Some(from_transaction_num) = block_bodies.first().map(|bodies| bodies.1.first_tx_num()) - else { - return Ok(None) - }; - let Some(to_transaction_num) = block_bodies.last().map(|bodies| bodies.1.last_tx_num()) - else { - return Ok(None) - }; - - let storage_range = BlockNumberAddress::range(range.clone()); - - let storage_changeset = self.get::(storage_range)?; - let account_changeset = self.get::(range)?; - - // This is not working for blocks that are not at tip. as plain state is not the last - // state of end range. We should rename the functions or add support to access - // History state. Accessing history state can be tricky but we are not gaining - // anything. - let mut plain_accounts_cursor = self.tx.cursor_read::()?; - let mut plain_storage_cursor = self.tx.cursor_dup_read::()?; - - let (state, reverts) = self.populate_bundle_state( - account_changeset, - storage_changeset, - &mut plain_accounts_cursor, - &mut plain_storage_cursor, - )?; - - // iterate over block body and create ExecutionResult - let mut receipt_iter = - self.get::(from_transaction_num..=to_transaction_num)?.into_iter(); - - let mut receipts = Vec::with_capacity(block_bodies.len()); - // loop break if we are at the end of the blocks. - for (_, block_body) in block_bodies { - let mut block_receipts = Vec::with_capacity(block_body.tx_count as usize); - for _ in block_body.tx_num_range() { - if let Some((_, receipt)) = receipt_iter.next() { - block_receipts.push(Some(receipt)); - } - } - receipts.push(block_receipts); - } - - Ok(Some(ExecutionOutcome::new_init( - state, - reverts, - Vec::new(), - receipts.into(), - start_block_number, - Vec::new(), - ))) - } - /// Populate a [`BundleStateInit`] and [`RevertsInit`] using cursors over the /// [`PlainAccountState`] and [`PlainStorageState`] tables, based on the given storage and /// account changesets. @@ -2039,9 +1951,11 @@ impl StateChangeWriter /// 1. Take the old value from the changeset /// 2. Take the new value from the local state /// 3. Set the local state to the value in the changeset - fn remove_state(&self, range: RangeInclusive) -> ProviderResult<()> { + fn remove_state_above(&self, block: BlockNumber) -> ProviderResult<()> { + let range = block + 1..=self.last_block_number()?; + if range.is_empty() { - return Ok(()) + return Ok(()); } // We are not removing block meta as it is used to get block changesets. @@ -2131,7 +2045,9 @@ impl StateChangeWriter /// 1. Take the old value from the changeset /// 2. Take the new value from the local state /// 3. Set the local state to the value in the changeset - fn take_state(&self, range: RangeInclusive) -> ProviderResult { + fn take_state_above(&self, block: BlockNumber) -> ProviderResult { + let range = block + 1..=self.last_block_number()?; + if range.is_empty() { return Ok(ExecutionOutcome::default()) } @@ -2672,12 +2588,6 @@ impl HistoryWriter for DatabaseProvi } } -impl StateReader for DatabaseProvider { - fn get_state(&self, block: BlockNumber) -> ProviderResult> { - self.get_state(block..=block) - } -} - impl BlockExecutionWriter for DatabaseProvider { @@ -2691,7 +2601,7 @@ impl BlockExecu self.unwind_trie_state_range(range.clone())?; // get execution res - let execution_state = self.take_state(range.clone())?; + let execution_state = self.take_state_above(block)?; let blocks = self.sealed_block_with_senders_range(range)?; @@ -2712,10 +2622,10 @@ impl BlockExecu ) -> ProviderResult<()> { let range = block + 1..=self.last_block_number()?; - self.unwind_trie_state_range(range.clone())?; + self.unwind_trie_state_range(range)?; // remove execution res - self.remove_state(range)?; + self.remove_state_above(block)?; // remove block bodies it is needed for both get block range and get block execution results // that is why it is deleted afterwards. diff --git a/crates/storage/provider/src/traits/state.rs b/crates/storage/provider/src/traits/state.rs index ec189a95e3..057d3a19a7 100644 --- a/crates/storage/provider/src/traits/state.rs +++ b/crates/storage/provider/src/traits/state.rs @@ -6,7 +6,6 @@ use revm::db::{ states::{PlainStateReverts, StateChangeset}, OriginalValuesKnown, }; -use std::ops::RangeInclusive; use super::StorageLocation; @@ -39,9 +38,11 @@ pub trait StateChangeWriter { /// Writes the hashed state changes to the database fn write_hashed_state(&self, hashed_state: &HashedPostStateSorted) -> ProviderResult<()>; - /// Remove the block range of state. - fn remove_state(&self, range: RangeInclusive) -> ProviderResult<()>; + /// Remove the block range of state above the given block. The state of the passed block is not + /// removed. + fn remove_state_above(&self, block: BlockNumber) -> ProviderResult<()>; - /// Take the block range of state, recreating the [`ExecutionOutcome`]. - fn take_state(&self, range: RangeInclusive) -> ProviderResult; + /// Take the block range of state, recreating the [`ExecutionOutcome`]. The state of the passed + /// block is not removed. + fn take_state_above(&self, block: BlockNumber) -> ProviderResult; }