From 1ed5ae14bfadeee3dd542dda1bd2dfd65e6ed5ee Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 7 Sep 2023 22:05:32 +0200 Subject: [PATCH] fix: ensure only canonical state is returned if requested by number/hash (#4517) --- crates/rpc/rpc/src/eth/api/mod.rs | 2 + crates/storage/provider/src/providers/mod.rs | 59 -------------------- crates/storage/provider/src/traits/state.rs | 15 +++-- 3 files changed, 13 insertions(+), 63 deletions(-) diff --git a/crates/rpc/rpc/src/eth/api/mod.rs b/crates/rpc/rpc/src/eth/api/mod.rs index e09c84969f62..35bd40637735 100644 --- a/crates/rpc/rpc/src/eth/api/mod.rs +++ b/crates/rpc/rpc/src/eth/api/mod.rs @@ -200,6 +200,8 @@ where BlockReaderIdExt + ChainSpecProvider + StateProviderFactory + EvmEnvProvider + 'static, { /// Returns the state at the given [BlockId] enum. + /// + /// Note: if not [BlockNumberOrTag::Pending] then this will only return canonical state. See also pub fn state_at_block_id(&self, at: BlockId) -> EthResult> { Ok(self.provider().state_by_block_id(at)?) } diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index 4a930e9c17bf..812606a5d397 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -469,65 +469,6 @@ where self.database.latest() } - /// Returns a [StateProviderBox] indexed by the given [BlockId]. - fn state_by_block_id(&self, block_id: BlockId) -> Result> { - match block_id { - BlockId::Number(block_number) => self.state_by_block_number_or_tag(block_number), - BlockId::Hash(rpc_block_hash) => { - let block_hash = rpc_block_hash.into(); - let mut state = self.history_by_block_hash(block_hash); - - // we failed to get the state by hash, from disk, hash block be the pending block - if state.is_err() && !rpc_block_hash.require_canonical.unwrap_or(false) { - if let Ok(Some(pending)) = self.pending_state_by_hash(block_hash) { - // we found pending block by hash - state = Ok(pending) - } - } - - state - } - } - } - - /// Returns a [StateProviderBox] indexed by the given block number or tag. - fn state_by_block_number_or_tag( - &self, - number_or_tag: BlockNumberOrTag, - ) -> Result> { - match number_or_tag { - BlockNumberOrTag::Latest => self.latest(), - BlockNumberOrTag::Finalized => { - // we can only get the finalized state by hash, not by num - let hash = match self.finalized_block_hash()? { - Some(hash) => hash, - None => return Err(ProviderError::FinalizedBlockNotFound.into()), - }; - - self.state_by_block_hash(hash) - } - BlockNumberOrTag::Safe => { - // we can only get the safe state by hash, not by num - let hash = match self.safe_block_hash()? { - Some(hash) => hash, - None => return Err(ProviderError::SafeBlockNotFound.into()), - }; - - self.state_by_block_hash(hash) - } - BlockNumberOrTag::Earliest => self.history_by_block_number(0), - BlockNumberOrTag::Pending => self.pending(), - BlockNumberOrTag::Number(num) => { - let mut state = self.history_by_block_number(num); - if state.is_err() && num == self.chain_info.get_canonical_block_number() + 1 { - // we don't have the block on disk yet but the number is the pending block - state = self.pending(); - } - state - } - } - } - fn history_by_block_number(&self, block_number: BlockNumber) -> Result> { trace!(target: "providers::blockchain", ?block_number, "Getting history by block number"); self.ensure_canonical_block(block_number)?; diff --git a/crates/storage/provider/src/traits/state.rs b/crates/storage/provider/src/traits/state.rs index 139f2b7261b0..66b66bb9eb72 100644 --- a/crates/storage/provider/src/traits/state.rs +++ b/crates/storage/provider/src/traits/state.rs @@ -99,6 +99,8 @@ pub trait StateProviderFactory: BlockIdReader + Send + Sync { fn latest(&self) -> Result>; /// Returns a [StateProvider] indexed by the given [BlockId]. + /// + /// Note: if a number or hash is provided this will only look at historical(canonical) state. fn state_by_block_id(&self, block_id: BlockId) -> Result> { match block_id { BlockId::Number(block_number) => self.state_by_block_number_or_tag(block_number), @@ -107,6 +109,8 @@ pub trait StateProviderFactory: BlockIdReader + Send + Sync { } /// Returns a [StateProvider] indexed by the given block number or tag. + /// + /// Note: if a number is provided this will only look at historical(canonical) state. fn state_by_block_number_or_tag( &self, number_or_tag: BlockNumberOrTag, @@ -119,8 +123,8 @@ pub trait StateProviderFactory: BlockIdReader + Send + Sync { Some(hash) => hash, None => return Err(ProviderError::FinalizedBlockNotFound.into()), }; - - self.state_by_block_hash(hash) + // only look at historical state + self.history_by_block_hash(hash) } BlockNumberOrTag::Safe => { // we can only get the safe state by hash, not by num @@ -129,11 +133,14 @@ pub trait StateProviderFactory: BlockIdReader + Send + Sync { None => return Err(ProviderError::SafeBlockNotFound.into()), }; - self.state_by_block_hash(hash) + self.history_by_block_hash(hash) } BlockNumberOrTag::Earliest => self.history_by_block_number(0), BlockNumberOrTag::Pending => self.pending(), - BlockNumberOrTag::Number(num) => self.history_by_block_number(num), + BlockNumberOrTag::Number(num) => { + // Note: The `BlockchainProvider` could also lookup the tree for the given block number, if for example the block number is `latest + 1`, however this should only support canonical state: + self.history_by_block_number(num) + } } }