fix: ensure only canonical state is returned if requested by number/h…

…ash (paradigmxyz#4517)

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 <https://github.com/paradigmxyz/reth/issues/4515>
     pub fn state_at_block_id(&self, at: BlockId) -> EthResult<StateProviderBox<'_>> {
         Ok(self.provider().state_by_block_id(at)?)
     }

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<StateProviderBox<'_>> {
-        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<StateProviderBox<'_>> {
-        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<StateProviderBox<'_>> {
         trace!(target: "providers::blockchain", ?block_number, "Getting history by block number");
         self.ensure_canonical_block(block_number)?;

crates/storage/provider/src/traits/state.rs

@@ -99,6 +99,8 @@ pub trait StateProviderFactory: BlockIdReader + Send + Sync {
     fn latest(&self) -> Result<StateProviderBox<'_>>;
 
     /// 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<StateProviderBox<'_>> {
         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: <https://github.com/paradigmxyz/reth/issues/4515>
+                self.history_by_block_number(num)
+            }
         }
     }