From 980197e2f94c026a1e525ff2762f315b670d7c1b Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Thu, 3 Oct 2024 17:44:56 -0400 Subject: [PATCH] revert some API changes --- crates/blockchain-tree/src/chain.rs | 3 +- .../src/providers/blockchain_provider.rs | 46 +++++++------------ .../provider/src/providers/database/mod.rs | 6 +-- .../src/providers/database/provider.rs | 33 ++++--------- crates/storage/storage-api/src/state.rs | 2 +- 5 files changed, 28 insertions(+), 62 deletions(-) diff --git a/crates/blockchain-tree/src/chain.rs b/crates/blockchain-tree/src/chain.rs index f72ba1ee2d934..393e525d5ae20 100644 --- a/crates/blockchain-tree/src/chain.rs +++ b/crates/blockchain-tree/src/chain.rs @@ -199,8 +199,7 @@ impl AppendableChain { // State root calculation can take a while, and we're sure no write transaction // will be open in parallel. See https://github.com/paradigmxyz/reth/issues/7509. .disable_long_read_transaction_safety() - .try_into_history_at_block(canonical_fork.number) - .map_err(|(err, _)| err)?; + .try_into_history_at_block(canonical_fork.number)?; let provider = BundleStateProvider::new(state_provider, bundle_state_data_provider); diff --git a/crates/storage/provider/src/providers/blockchain_provider.rs b/crates/storage/provider/src/providers/blockchain_provider.rs index 3cebc604c3791..b769b82d3984c 100644 --- a/crates/storage/provider/src/providers/blockchain_provider.rs +++ b/crates/storage/provider/src/providers/blockchain_provider.rs @@ -128,26 +128,6 @@ impl BlockchainProvider2 { (start, end) } - fn state_by_block_hash_with_provider( - &self, - hash: BlockHash, - provider: DatabaseProviderRO, - ) -> ProviderResult { - trace!(target: "providers::blockchain", ?hash, "Getting state by block hash"); - let provider = match self.history_by_block_hash_with_provider(hash, provider) { - Ok(state) => return Ok(state), - Err((_, provider)) => provider, - }; - - if let Ok(Some(pending)) = self.pending_state_by_hash_with_provider(hash, provider) { - // .. or this could be the pending state - Ok(pending) - } else { - // if we couldn't find it anywhere, then we should return an error - Err(ProviderError::StateForHashNotFound(hash)) - } - } - /// Fetches storage block changesets for the given block using an existing database provider fn storage_block_changeset_with_provider( &self, @@ -282,8 +262,7 @@ impl BlockchainProvider2 { if let Some(pending) = self.canonical_in_memory_state.pending_state() { if pending.hash() == block_hash { return Ok(Some(Box::new( - self.block_state_provider_with_provider(&pending, provider) - .map_err(|(err, _)| err)?, + self.block_state_provider_with_provider(&pending, provider)?, ))); } } @@ -406,7 +385,7 @@ impl BlockchainProvider2 { let hash = provider .block_hash(block_range_end)? .ok_or_else(|| ProviderError::HeaderNotFound(block_range_end.into()))?; - let state_provider = self.state_by_block_hash_with_provider(hash, provider)?; + let state_provider = self.history_by_block_hash_with_provider(hash, provider)?; // add account changeset changes for (block_number, account_before) in account_changeset.into_iter().rev() { @@ -595,7 +574,7 @@ impl BlockchainProvider2 { &self, block_hash: BlockHash, provider: DatabaseProviderRO, - ) -> Result)> { + ) -> Result { trace!(target: "providers::blockchain", ?block_hash, "Getting history by block hash"); if let Some(block_state) = self.canonical_in_memory_state.state_by_hash(block_hash) { @@ -615,8 +594,7 @@ impl BlockchainProvider2 { &self, state: &BlockState, provider: DatabaseProviderRO, - ) -> Result)> - { + ) -> Result { let anchor_hash = state.anchor().hash; let latest_historical = provider.history_by_block_hash(anchor_hash)?; Ok(self.canonical_in_memory_state.state_provider_from_state(state, latest_historical)) @@ -628,7 +606,7 @@ impl BlockchainProvider2 { state: &BlockState, ) -> ProviderResult { let provider = self.database_provider_ro()?; - self.block_state_provider_with_provider(state, provider).map_err(|(err, _)| err) + self.block_state_provider_with_provider(state, provider) } /// Fetches data from either in-memory state or persistent storage for a range of transactions. @@ -1588,7 +1566,7 @@ impl StateProviderFactory for BlockchainProvider2 { self.get_in_memory_or_storage_by_block( block_hash.into(), - |provider| provider.history_by_block_hash(block_hash).map_err(|(err, _)| err), + |provider| provider.history_by_block_hash(block_hash), |block_state| { let state_provider = self.block_state_provider(&block_state)?; Ok(Box::new(state_provider)) @@ -1598,8 +1576,16 @@ impl StateProviderFactory for BlockchainProvider2 { fn state_by_block_hash(&self, hash: BlockHash) -> ProviderResult { trace!(target: "providers::blockchain", ?hash, "Getting state by block hash"); - let provider = self.database_provider_ro()?; - self.state_by_block_hash_with_provider(hash, provider) + if let Ok(state) = self.history_by_block_hash(hash) { + // This could be tracked by a historical block + Ok(state) + } else if let Ok(Some(pending)) = self.pending_state_by_hash(hash) { + // .. or this could be the pending state + Ok(pending) + } else { + // if we couldn't find it anywhere, then we should return an error + Err(ProviderError::StateForHashNotFound(hash)) + } } /// Returns the state provider for pending state. diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index 97c77c404edda..520b514527b22 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -165,8 +165,7 @@ impl ProviderFactory { &self, block_number: BlockNumber, ) -> ProviderResult { - let state_provider = - self.provider()?.try_into_history_at_block(block_number).map_err(|(err, _)| err)?; + let state_provider = self.provider()?.try_into_history_at_block(block_number)?; trace!(target: "providers::db", ?block_number, "Returning historical state provider for block number"); Ok(state_provider) } @@ -179,8 +178,7 @@ impl ProviderFactory { .block_number(block_hash)? .ok_or(ProviderError::BlockHashNotFound(block_hash))?; - let state_provider = - self.provider()?.try_into_history_at_block(block_number).map_err(|(err, _)| err)?; + let state_provider = self.provider()?.try_into_history_at_block(block_number)?; trace!(target: "providers::db", ?block_number, %block_hash, "Returning historical state provider for block hash"); Ok(state_provider) } diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index d839322cb4be0..c80b258ab5119 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -185,7 +185,7 @@ impl TryIntoHistoricalStateProvider fn try_into_history_at_block( self, mut block_number: BlockNumber, - ) -> Result { + ) -> ProviderResult { if block_number == self.best_block_number().unwrap_or_default() && block_number == self.last_block_number().unwrap_or_default() { @@ -196,15 +196,9 @@ impl TryIntoHistoricalStateProvider block_number += 1; let account_history_prune_checkpoint = - match self.get_prune_checkpoint(PruneSegment::AccountHistory) { - Ok(checkpoint) => checkpoint, - Err(err) => return Err((err, self)), - }; + self.get_prune_checkpoint(PruneSegment::AccountHistory)?; let storage_history_prune_checkpoint = - match self.get_prune_checkpoint(PruneSegment::StorageHistory) { - Ok(checkpoint) => checkpoint, - Err(err) => return Err((err, self)), - }; + self.get_prune_checkpoint(PruneSegment::StorageHistory)?; let mut state_provider = HistoricalStateProvider::new(self.tx, block_number, self.static_file_provider); @@ -315,25 +309,14 @@ where impl DatabaseProvider { /// Storage provider for state at that given block hash - pub fn history_by_block_hash( - self, - block_hash: BlockHash, - ) -> Result { - let block_number_res = self.block_number(block_hash); - - let block_number_res = match block_number_res { - Ok(val) => val, - Err(err) => return Err((err, self)), - }; - - let block_number = match block_number_res { - Some(block_number) => block_number, - None => return Err((ProviderError::BlockHashNotFound(block_hash), self)), - }; + pub fn history_by_block_hash(self, block_hash: BlockHash) -> ProviderResult { + let block_number = self + .block_number(block_hash)? + .ok_or_else(|| ProviderError::BlockHashNotFound(block_hash))?; let state_provider = self.try_into_history_at_block(block_number)?; - trace!(target: "providers::db", ?block_number_res, %block_hash, "Returning historical state provider for block hash with database provider"); + trace!(target: "providers::db", ?block_number, %block_hash, "Returning historical state provider for block hash with database provider"); Ok(state_provider) } } diff --git a/crates/storage/storage-api/src/state.rs b/crates/storage/storage-api/src/state.rs index b6c5605f1670c..c85593bd6748b 100644 --- a/crates/storage/storage-api/src/state.rs +++ b/crates/storage/storage-api/src/state.rs @@ -89,7 +89,7 @@ pub trait TryIntoHistoricalStateProvider { fn try_into_history_at_block( self, block_number: BlockNumber, - ) -> Result + ) -> Result where Self: Sized; }