refactor: dedicated blockchain insert errors (#2712)

crates/blockchain-tree/src/blockchain_tree.rs

@@ -5,7 +5,10 @@ use crate::{
 };
 use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx};
 use reth_interfaces::{
-    blockchain_tree::BlockStatus,
+    blockchain_tree::{
+        error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind},
+        BlockStatus,
+    },
     consensus::{Consensus, ConsensusError},
     executor::BlockExecutionError,
     Error,
@@ -135,15 +138,22 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         })
     }
 
-    /// Check if block is known to blockchain tree or database and return its status.
+    /// Check if then block is known to blockchain tree or database and return its status.
     ///
     /// Function will check:
     /// * if block is inside database and return [BlockStatus::Valid] if it is.
     /// * if block is inside buffer and return [BlockStatus::Disconnected] if it is.
     /// * if block is part of the side chain and return [BlockStatus::Accepted] if it is.
-    /// * if block is part of the canonical chain that tree knowns, return [BlockStatus::Valid]. if
+    /// * if block is part of the canonical chain that tree knows, return [BlockStatus::Valid], if
     ///   it is.
-    pub(crate) fn is_block_known(&self, block: BlockNumHash) -> Result<Option<BlockStatus>, Error> {
+    ///
+    /// Returns an error if
+    ///    - an error occurred while reading from the database.
+    ///    - the block is already finalized
+    pub(crate) fn is_block_known(
+        &self,
+        block: BlockNumHash,
+    ) -> Result<Option<BlockStatus>, InsertBlockErrorKind> {
         let last_finalized_block = self.block_indices.last_finalized_block();
         // check db if block is finalized.
         if block.number <= last_finalized_block {
@@ -154,13 +164,11 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
                 }
             }
             // check if block is inside database
-            if self.externals.shareable_db().block_number(block.hash)?.is_some() {
+            if self.externals.database().block_number(block.hash)?.is_some() {
                 return Ok(Some(BlockStatus::Valid))
             }
 
-            return Err(BlockExecutionError::PendingBlockIsFinalized {
-                block_number: block.number,
-                block_hash: block.hash,
+            return Err(BlockchainTreeError::PendingBlockIsFinalized {
                 last_finalized: last_finalized_block,
             }
             .into())
@@ -265,7 +273,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     pub fn try_insert_block(
         &mut self,
         block: SealedBlockWithSenders,
-    ) -> Result<BlockStatus, Error> {
+    ) -> Result<BlockStatus, InsertBlockError> {
         let parent = block.parent_num_hash();
 
         // check if block parent can be found in Tree
@@ -275,7 +283,10 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         }
 
         // if not found, check if the parent can be found inside canonical chain.
-        if self.is_block_hash_canonical(&parent.hash)? {
+        if self
+            .is_block_hash_canonical(&parent.hash)
+            .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?
+        {
             return self.try_append_canonical_chain(block, parent)
         }
 
@@ -286,16 +297,22 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         {
             // we found the parent block in canonical chain
             if canonical_parent_number != parent.number {
-                return Err(Error::Consensus(ConsensusError::ParentBlockNumberMismatch {
-                    parent_block_number: canonical_parent_number,
-                    block_number: block.number,
-                }))
+                return Err(InsertBlockError::consensus_error(
+                    ConsensusError::ParentBlockNumberMismatch {
+                        parent_block_number: canonical_parent_number,
+                        block_number: block.number,
+                    },
+                    block.block,
+                ))
             }
         }
 
         // if there is a parent inside the buffer, validate against it.
         if let Some(buffered_parent) = self.buffered_blocks.block(parent) {
-            self.externals.consensus.validate_header_against_parent(&block, buffered_parent)?
+            self.externals
+                .consensus
+                .validate_header_against_parent(&block, buffered_parent)
+                .map_err(|err| InsertBlockError::consensus_error(err, block.block.clone()))?;
         }
 
         // insert block inside unconnected block buffer. Delaying it execution.
@@ -308,7 +325,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         &mut self,
         block: SealedBlockWithSenders,
         parent: BlockNumHash,
-    ) -> Result<BlockStatus, Error> {
+    ) -> Result<BlockStatus, InsertBlockError> {
         let block_num_hash = block.num_hash();
         debug!(target: "blockchain_tree", ?parent, "Appending block to canonical chain");
         // create new chain that points to that block
@@ -316,15 +333,25 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         // TODO save pending block to database
         // https://github.com/paradigmxyz/reth/issues/1713
 
-        let db = self.externals.shareable_db();
+        let db = self.externals.database();
 
         // Validate that the block is post merge
         let parent_td = db
-            .header_td(&block.parent_hash)?
-            .ok_or(BlockExecutionError::CanonicalChain { block_hash: block.parent_hash })?;
+            .header_td(&block.parent_hash)
+            .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?
+            .ok_or_else(|| {
+                InsertBlockError::tree_error(
+                    BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash },
+                    block.block.clone(),
+                )
+            })?;
+
         // Pass the parent total difficulty to short-circuit unnecessary calculations.
         if !self.externals.chain_spec.fork(Hardfork::Paris).active_at_ttd(parent_td, U256::ZERO) {
-            return Err(BlockExecutionError::BlockPreMerge { hash: block.hash }.into())
+            return Err(InsertBlockError::execution_error(
+                BlockExecutionError::BlockPreMerge { hash: block.hash },
+                block.block,
+            ))
         }
 
         let canonical_chain = self.canonical_chain();
@@ -335,11 +362,18 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         };
 
         let parent_header = db
-            .header(&block.parent_hash)?
-            .ok_or(BlockExecutionError::CanonicalChain { block_hash: block.parent_hash })?
+            .header(&block.parent_hash)
+            .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?
+            .ok_or_else(|| {
+                InsertBlockError::tree_error(
+                    BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash },
+                    block.block.clone(),
+                )
+            })?
             .seal(block.parent_hash);
+
         let chain = AppendableChain::new_canonical_fork(
-            &block,
+            block,
             &parent_header,
             canonical_chain.inner(),
             parent,
@@ -356,7 +390,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         &mut self,
         block: SealedBlockWithSenders,
         chain_id: BlockChainId,
-    ) -> Result<BlockStatus, Error> {
+    ) -> Result<BlockStatus, InsertBlockError> {
         debug!(target: "blockchain_tree", "Inserting block into side chain");
         let block_num_hash = block.num_hash();
         // Create a new sidechain by forking the given chain, or append the block if the parent
@@ -364,17 +398,28 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         let block_hashes = self.all_chain_hashes(chain_id);
 
         // get canonical fork.
-        let canonical_fork = self
-            .canonical_fork(chain_id)
-            .ok_or(BlockExecutionError::BlockSideChainIdConsistency { chain_id })?;
+        let canonical_fork = match self.canonical_fork(chain_id) {
+            None => {
+                return Err(InsertBlockError::tree_error(
+                    BlockchainTreeError::BlockSideChainIdConsistency { chain_id },
+                    block.block,
+                ))
+            }
+            Some(fork) => fork,
+        };
 
         // get chain that block needs to join to.
-        let parent_chain = self
-            .chains
-            .get_mut(&chain_id)
-            .ok_or(BlockExecutionError::BlockSideChainIdConsistency { chain_id })?;
-        let chain_tip = parent_chain.tip().hash();
+        let parent_chain = match self.chains.get_mut(&chain_id) {
+            Some(parent_chain) => parent_chain,
+            None => {
+                return Err(InsertBlockError::tree_error(
+                    BlockchainTreeError::BlockSideChainIdConsistency { chain_id },
+                    block.block,
+                ))
+            }
+        };
 
+        let chain_tip = parent_chain.tip().hash();
         let canonical_block_hashes = self.block_indices.canonical_chain();
 
         // append the block if it is continuing the side chain.
@@ -485,23 +530,29 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     pub fn insert_block_without_senders(
         &mut self,
         block: SealedBlock,
-    ) -> Result<BlockStatus, Error> {
-        let block = block.seal_with_senders().ok_or(BlockExecutionError::SenderRecoveryError)?;
-        self.insert_block(block)
+    ) -> Result<BlockStatus, InsertBlockError> {
+        match block.try_seal_with_senders() {
+            Ok(block) => self.insert_block(block),
+            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
+        }
     }
 
     /// Insert block for future execution.
     ///
     /// Returns an error if the block is invalid.
-    pub fn buffer_block(&mut self, block: SealedBlockWithSenders) -> Result<(), Error> {
-        self.validate_block(&block)?;
+    pub fn buffer_block(&mut self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
+        // validate block consensus rules
+        if let Err(err) = self.validate_block(&block) {
+            return Err(InsertBlockError::consensus_error(err, block.block))
+        }
+
         self.buffered_blocks.insert_block(block);
         Ok(())
     }
 
     /// Validate if block is correct and satisfies all the consensus rules that concern the header
     /// and block body itself.
-    fn validate_block(&self, block: &SealedBlockWithSenders) -> Result<(), Error> {
+    fn validate_block(&self, block: &SealedBlockWithSenders) -> Result<(), ConsensusError> {
         if let Err(e) =
             self.externals.consensus.validate_header_with_total_difficulty(block, U256::MAX)
         {
@@ -509,17 +560,17 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
                 "Failed to validate header for TD related check with error: {e:?}, block:{:?}",
                 block
             );
-            return Err(e.into())
+            return Err(e)
         }
 
         if let Err(e) = self.externals.consensus.validate_header(block) {
             info!("Failed to validate header with error: {e:?}, block:{:?}", block);
-            return Err(e.into())
+            return Err(e)
         }
 
         if let Err(e) = self.externals.consensus.validate_block(block) {
             info!("Failed to validate blocks with error: {e:?}, block:{:?}", block);
-            return Err(e.into())
+            return Err(e)
         }
 
         Ok(())
@@ -562,7 +613,10 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     ///
     /// If the senders have not already been recovered, call
     /// [`BlockchainTree::insert_block_without_senders`] instead.
-    pub fn insert_block(&mut self, block: SealedBlockWithSenders) -> Result<BlockStatus, Error> {
+    pub fn insert_block(
+        &mut self,
+        block: SealedBlockWithSenders,
+    ) -> Result<BlockStatus, InsertBlockError> {
         self.insert_block_inner(block, true)
     }
 
@@ -572,16 +626,20 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         &mut self,
         block: SealedBlockWithSenders,
         do_is_known_check: bool,
-    ) -> Result<BlockStatus, Error> {
-        // is block is known
+    ) -> Result<BlockStatus, InsertBlockError> {
+        // check if we already know this block
         if do_is_known_check {
-            if let Some(status) = self.is_block_known(block.num_hash())? {
-                return Ok(status)
+            match self.is_block_known(block.num_hash()) {
+                Ok(Some(status)) => return Ok(status),
+                Err(err) => return Err(InsertBlockError::new(block.block, err)),
+                _ => {}
             }
         }
 
-        // validate block hashes
-        self.validate_block(&block)?;
+        // validate block consensus rules
+        if let Err(err) = self.validate_block(&block) {
+            return Err(InsertBlockError::consensus_error(err, block.block))
+        }
 
         // try to insert block
         self.try_insert_block(block)
@@ -708,7 +766,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         // the db, then it is not canonical.
         if !self.block_indices.is_block_hash_canonical(hash) &&
             (self.block_by_hash(*hash).is_some() ||
-                self.externals.shareable_db().header(hash)?.is_none())
+                self.externals.database().header(hash)?.is_none())
         {
             return Ok(false)
         }
@@ -737,7 +795,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
             info!(target: "blockchain_tree", ?block_hash, "Block is already canonical");
             let td = self
                 .externals
-                .shareable_db()
+                .database()
                 .header_td(block_hash)?
                 .ok_or(BlockExecutionError::MissingTotalDifficulty { hash: *block_hash })?;
             if !self.externals.chain_spec.fork(Hardfork::Paris).active_at_ttd(td, U256::ZERO) {
@@ -748,6 +806,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
 
         let Some(chain_id) = self.block_indices.get_blocks_chain_id(block_hash) else {
             info!(target: "blockchain_tree", ?block_hash,  "Block hash not found in block indices");
+            // TODO: better error
             return Err(BlockExecutionError::BlockHashNotFoundInChain { block_hash: *block_hash }.into())
         };
         let chain = self.chains.remove(&chain_id).expect("To be present");
@@ -1060,7 +1119,7 @@ mod tests {
         tree.finalize_block(10);
 
         // block 2 parent is not known, block2 is buffered.
-        assert_eq!(tree.insert_block(block2.clone()), Ok(BlockStatus::Disconnected));
+        assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Disconnected);
 
         // Buffered block: [block2]
         // Trie state:
@@ -1075,19 +1134,19 @@ mod tests {
             )]))
             .assert(&tree);
 
-        assert_eq!(tree.is_block_known(block2.num_hash()), Ok(Some(BlockStatus::Disconnected)));
+        assert_eq!(
+            tree.is_block_known(block2.num_hash()).unwrap(),
+            Some(BlockStatus::Disconnected)
+        );
 
         // check if random block is known
         let old_block = BlockNumHash::new(1, H256([32; 32]));
-        let err = BlockExecutionError::PendingBlockIsFinalized {
-            block_number: old_block.number,
-            block_hash: old_block.hash,
-            last_finalized: 10,
-        };
-        assert_eq!(tree.is_block_known(old_block), Err(err.into()));
+        let err = BlockchainTreeError::PendingBlockIsFinalized { last_finalized: 10 };
+
+        assert_eq!(tree.is_block_known(old_block).unwrap_err().as_tree_error(), Some(err));
 
         // insert block1 and buffered block2 is inserted
-        assert_eq!(tree.insert_block(block1.clone()), Ok(BlockStatus::Valid));
+        assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid);
 
         // Buffered blocks: []
         // Trie state:
@@ -1107,10 +1166,10 @@ mod tests {
             .assert(&tree);
 
         // already inserted block will return true.
-        assert_eq!(tree.insert_block(block1.clone()), Ok(BlockStatus::Valid));
+        assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid);
 
         // block two is already inserted.
-        assert_eq!(tree.insert_block(block2.clone()), Ok(BlockStatus::Valid));
+        assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Valid);
 
         // make block1 canonical
         assert_eq!(tree.make_canonical(&block1.hash()), Ok(()));
@@ -1147,7 +1206,7 @@ mod tests {
         block2a.hash = block2a_hash;
 
         // reinsert two blocks that point to canonical chain
-        assert_eq!(tree.insert_block(block1a.clone()), Ok(BlockStatus::Accepted));
+        assert_eq!(tree.insert_block(block1a.clone()).unwrap(), BlockStatus::Accepted);
 
         TreeTester::default()
             .with_chain_num(1)
@@ -1159,7 +1218,7 @@ mod tests {
             .with_pending_blocks((block2.number + 1, HashSet::from([])))
             .assert(&tree);
 
-        assert_eq!(tree.insert_block(block2a.clone()), Ok(BlockStatus::Accepted));
+        assert_eq!(tree.insert_block(block2a.clone()).unwrap(), BlockStatus::Accepted);
         // Trie state:
         // b2   b2a (side chain)
         // |   /
@@ -1341,7 +1400,7 @@ mod tests {
         block2b.hash = H256([0x99; 32]);
         block2b.parent_hash = H256([0x88; 32]);
 
-        assert_eq!(tree.insert_block(block2b.clone()), Ok(BlockStatus::Disconnected));
+        assert_eq!(tree.insert_block(block2b.clone()).unwrap(), BlockStatus::Disconnected);
         TreeTester::default()
             .with_buffered_blocks(BTreeMap::from([(
                 block2b.number,
@@ -1352,7 +1411,8 @@ mod tests {
         // update canonical block to b2, this would make b2a be removed
         assert_eq!(tree.restore_canonical_hashes(12), Ok(()));
 
-        assert_eq!(tree.is_block_known(block2.num_hash()), Ok(Some(BlockStatus::Valid)));
+        assert_eq!(tree.is_block_known(block2.num_hash()).unwrap(), Some(BlockStatus::Valid));
+
         // Trie state:
         // b2 (finalized)
         // |

crates/blockchain-tree/src/chain.rs

@@ -4,7 +4,11 @@
 //! blocks, as well as a list of the blocks the chain is composed of.
 use crate::{post_state::PostState, PostStateDataRef};
 use reth_db::database::Database;
-use reth_interfaces::{consensus::Consensus, executor::BlockExecutionError, Error};
+use reth_interfaces::{
+    blockchain_tree::error::{BlockchainTreeError, InsertBlockError},
+    consensus::Consensus,
+    Error,
+};
 use reth_primitives::{
     BlockHash, BlockNumber, ForkBlock, SealedBlockWithSenders, SealedHeader, U256,
 };
@@ -55,12 +59,12 @@ impl AppendableChain {
 
     /// Create a new chain that forks off of the canonical chain.
     pub fn new_canonical_fork<DB, C, EF>(
-        block: &SealedBlockWithSenders,
+        block: SealedBlockWithSenders,
         parent_header: &SealedHeader,
         canonical_block_hashes: &BTreeMap<BlockNumber, BlockHash>,
         canonical_fork: ForkBlock,
         externals: &TreeExternals<DB, C, EF>,
-    ) -> Result<Self, Error>
+    ) -> Result<Self, InsertBlockError>
     where
         DB: Database,
         C: Consensus,
@@ -77,29 +81,33 @@ impl AppendableChain {
         };
 
         let changeset =
-            Self::validate_and_execute(block.clone(), parent_header, state_provider, externals)?;
+            Self::validate_and_execute(block.clone(), parent_header, state_provider, externals)
+                .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?;
 
-        Ok(Self { chain: Chain::new(vec![(block.clone(), changeset)]) })
+        Ok(Self { chain: Chain::new(vec![(block, changeset)]) })
     }
 
     /// Create a new chain that forks off of an existing sidechain.
-    pub fn new_chain_fork<DB, C, EF>(
+    pub(crate) fn new_chain_fork<DB, C, EF>(
         &self,
         block: SealedBlockWithSenders,
         side_chain_block_hashes: BTreeMap<BlockNumber, BlockHash>,
         canonical_block_hashes: &BTreeMap<BlockNumber, BlockHash>,
         canonical_fork: ForkBlock,
         externals: &TreeExternals<DB, C, EF>,
-    ) -> Result<Self, Error>
+    ) -> Result<Self, InsertBlockError>
     where
         DB: Database,
         C: Consensus,
         EF: ExecutorFactory,
     {
         let parent_number = block.number - 1;
-        let parent = self.blocks().get(&parent_number).ok_or(
-            BlockExecutionError::BlockNumberNotFoundInChain { block_number: parent_number },
-        )?;
+        let parent = self.blocks().get(&parent_number).ok_or_else(|| {
+            InsertBlockError::tree_error(
+                BlockchainTreeError::BlockNumberNotFoundInChain { block_number: parent_number },
+                block.block.clone(),
+            )
+        })?;
 
         let mut state = self.state.clone();
 
@@ -114,7 +122,8 @@ impl AppendableChain {
             canonical_fork,
         };
         let block_state =
-            Self::validate_and_execute(block.clone(), parent, post_state_data, externals)?;
+            Self::validate_and_execute(block.clone(), parent, post_state_data, externals)
+                .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?;
         state.extend(block_state);
 
         let chain =
@@ -144,7 +153,7 @@ impl AppendableChain {
         let unseal = unseal.unseal();
 
         //get state provider.
-        let db = externals.shareable_db();
+        let db = externals.database();
         // TODO, small perf can check if caonical fork is the latest state.
         let canonical_fork = post_state_data_provider.canonical_fork();
         let history_provider = db.history_by_block_number(canonical_fork.number)?;
@@ -164,7 +173,7 @@ impl AppendableChain {
         canonical_block_hashes: &BTreeMap<BlockNumber, BlockHash>,
         canonical_fork: ForkBlock,
         externals: &TreeExternals<DB, C, EF>,
-    ) -> Result<(), Error>
+    ) -> Result<(), InsertBlockError>
     where
         DB: Database,
         C: Consensus,
@@ -180,7 +189,8 @@ impl AppendableChain {
         };
 
         let block_state =
-            Self::validate_and_execute(block.clone(), parent_block, post_state_data, externals)?;
+            Self::validate_and_execute(block.clone(), parent_block, post_state_data, externals)
+                .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?;
         self.state.extend(block_state);
         self.blocks.insert(block.number, block);
         Ok(())

crates/blockchain-tree/src/externals.rs

@@ -35,7 +35,7 @@ impl<DB, C, EF> TreeExternals<DB, C, EF> {
 
 impl<DB: Database, C, EF> TreeExternals<DB, C, EF> {
     /// Return shareable database helper structure.
-    pub fn shareable_db(&self) -> ShareableDatabase<&DB> {
+    pub fn database(&self) -> ShareableDatabase<&DB> {
         ShareableDatabase::new(&self.db, self.chain_spec.clone())
     }
 }

crates/blockchain-tree/src/shareable.rs

@@ -3,7 +3,9 @@ use super::BlockchainTree;
 use parking_lot::RwLock;
 use reth_db::database::Database;
 use reth_interfaces::{
-    blockchain_tree::{BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer},
+    blockchain_tree::{
+        error::InsertBlockError, BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer,
+    },
     consensus::Consensus,
     Error,
 };
@@ -28,7 +30,7 @@ pub struct ShareableBlockchainTree<DB: Database, C: Consensus, EF: ExecutorFacto
 }
 
 impl<DB: Database, C: Consensus, EF: ExecutorFactory> ShareableBlockchainTree<DB, C, EF> {
-    /// Create New sharable database.
+    /// Create a new sharable database.
     pub fn new(tree: BlockchainTree<DB, C, EF>) -> Self {
         Self { tree: Arc::new(RwLock::new(tree)) }
     }
@@ -37,23 +39,21 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> ShareableBlockchainTree<DB
 impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
     for ShareableBlockchainTree<DB, C, EF>
 {
-    fn insert_block_without_senders(&self, block: SealedBlock) -> Result<BlockStatus, Error> {
-        let mut tree = self.tree.write();
-        // check if block is known before recovering all senders.
-        if let Some(status) = tree.is_block_known(block.num_hash())? {
-            return Ok(status)
+    fn insert_block_without_senders(
+        &self,
+        block: SealedBlock,
+    ) -> Result<BlockStatus, InsertBlockError> {
+        match block.try_seal_with_senders() {
+            Ok(block) => self.tree.write().insert_block_inner(block, true),
+            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
         }
-        let block = block
-            .seal_with_senders()
-            .ok_or(reth_interfaces::executor::BlockExecutionError::SenderRecoveryError)?;
-        tree.insert_block_inner(block, true)
     }
 
-    fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), Error> {
+    fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
         self.tree.write().buffer_block(block)
     }
 
-    fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, Error> {
+    fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, InsertBlockError> {
         trace!(target: "blockchain_tree", ?block, "Inserting block");
         self.tree.write().insert_block(block)
     }

crates/consensus/beacon/src/engine/mod.rs

@@ -51,6 +51,7 @@ mod event;
 pub(crate) mod sync;
 
 pub use event::BeaconConsensusEngineEvent;
+use reth_interfaces::blockchain_tree::error::InsertBlockError;
 
 /// The maximum number of invalid headers that can be tracked by the engine.
 const MAX_INVALID_HEADERS: u32 = 512u32;
@@ -269,10 +270,10 @@ where
     fn latest_valid_hash_for_invalid_payload(
         &self,
         parent_hash: H256,
-        tree_error: Option<&Error>,
+        tree_error: Option<&InsertBlockError>,
     ) -> Option<H256> {
         // check pre merge block error
-        if let Some(Error::Execution(BlockExecutionError::BlockPreMerge { .. })) = tree_error {
+        if tree_error.map(|err| err.kind().is_block_pre_merge()).unwrap_or_default() {
             return Some(H256::zero())
         }
 
@@ -672,7 +673,7 @@ where
             // received a new payload while we're still syncing to the target
             let latest_valid_hash =
                 self.latest_valid_hash_for_invalid_payload(parent_hash, Some(&error));
-            let status = PayloadStatusEnum::Invalid { validation_error: error.to_string() };
+            let status = PayloadStatusEnum::Invalid { validation_error: error.kind().to_string() };
             PayloadStatus::new(status, latest_valid_hash)
         } else {
             // successfully buffered the block
@@ -721,7 +722,8 @@ where
 
                 let latest_valid_hash =
                     self.latest_valid_hash_for_invalid_payload(parent_hash, Some(&error));
-                let status = PayloadStatusEnum::Invalid { validation_error: error.to_string() };
+                let status =
+                    PayloadStatusEnum::Invalid { validation_error: error.kind().to_string() };
                 PayloadStatus::new(status, latest_valid_hash)
             }
         }

crates/interfaces/src/blockchain_tree/error.rs

@@ -0,0 +1,186 @@
+//! Error handling for the blockchain tree
+
+use crate::{consensus::ConsensusError, executor::BlockExecutionError};
+use reth_primitives::{BlockHash, BlockNumber, SealedBlock};
+use std::fmt::Formatter;
+
+/// Various error cases that can occur when a block violates tree assumptions.
+#[derive(Debug, Clone, Copy, thiserror::Error, Eq, PartialEq)]
+#[allow(missing_docs)]
+pub enum BlockchainTreeError {
+    /// Thrown if the block number is lower than the last finalized block number.
+    #[error("Block number is lower than the last finalized block number #{last_finalized}")]
+    PendingBlockIsFinalized {
+        /// The block number of the last finalized block.
+        last_finalized: BlockNumber,
+    },
+    /// Thrown if no side chain could be found for the block.
+    #[error("BlockChainId can't be found in BlockchainTree with internal index {chain_id}")]
+    BlockSideChainIdConsistency {
+        /// The internal identifier for the side chain.
+        chain_id: u64,
+    },
+    #[error("Canonical chain header #{block_hash} can't be found ")]
+    CanonicalChain { block_hash: BlockHash },
+    #[error("Block number #{block_number} not found in blockchain tree chain")]
+    BlockNumberNotFoundInChain { block_number: BlockNumber },
+    #[error("Block hash {block_hash} not found in blockchain tree chain")]
+    BlockHashNotFoundInChain { block_hash: BlockHash },
+}
+
+/// Error thrown when inserting a block failed because the block is considered invalid.
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct InsertBlockError {
+    inner: Box<InsertBlockErrorData>,
+}
+
+// === impl InsertBlockError ===
+
+impl InsertBlockError {
+    /// Create a new InsertInvalidBlockError
+    pub fn new(block: SealedBlock, kind: InsertBlockErrorKind) -> Self {
+        Self { inner: InsertBlockErrorData::boxed(block, kind) }
+    }
+
+    /// Create a new InsertInvalidBlockError from a tree error
+    pub fn tree_error(error: BlockchainTreeError, block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKind::Tree(error))
+    }
+
+    /// Create a new InsertInvalidBlockError from a consensus error
+    pub fn consensus_error(error: ConsensusError, block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKind::Consensus(error))
+    }
+
+    /// Create a new InsertInvalidBlockError from a consensus error
+    pub fn sender_recovery_error(block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKind::SenderRecovery)
+    }
+
+    /// Create a new InsertInvalidBlockError from an execution error
+    pub fn execution_error(error: BlockExecutionError, block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKind::Execution(error))
+    }
+
+    /// Returns the error kind
+    #[inline]
+    pub fn kind(&self) -> &InsertBlockErrorKind {
+        &self.inner.kind
+    }
+
+    /// Returns the invalid block.
+    #[inline]
+    pub fn block(&self) -> &SealedBlock {
+        &self.inner.block
+    }
+}
+
+#[derive(Debug)]
+struct InsertBlockErrorData {
+    block: SealedBlock,
+    kind: InsertBlockErrorKind,
+}
+
+impl std::fmt::Display for InsertBlockErrorData {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Failed to insert block {:?}: {}", self.block.hash, self.kind)
+    }
+}
+
+impl std::error::Error for InsertBlockErrorData {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        Some(&self.kind)
+    }
+}
+
+impl InsertBlockErrorData {
+    fn new(block: SealedBlock, kind: InsertBlockErrorKind) -> Self {
+        Self { block, kind }
+    }
+
+    fn boxed(block: SealedBlock, kind: InsertBlockErrorKind) -> Box<Self> {
+        Box::new(Self::new(block, kind))
+    }
+}
+
+/// All error variants possible when inserting a block
+#[derive(Debug, thiserror::Error)]
+pub enum InsertBlockErrorKind {
+    /// Failed to recover senders for the block
+    #[error("Failed to recover senders for block")]
+    SenderRecovery,
+    /// Block violated consensus rules.
+    #[error(transparent)]
+    Consensus(ConsensusError),
+    /// Block execution failed.
+    #[error(transparent)]
+    Execution(BlockExecutionError),
+    /// Block violated tree invariants.
+    #[error(transparent)]
+    Tree(#[from] BlockchainTreeError),
+    /// An internal error occurred, like interacting with the database.
+    #[error("Internal error")]
+    Internal(Box<dyn std::error::Error + Send + Sync>),
+}
+
+impl InsertBlockErrorKind {
+    /// Returns true if the error is a tree error
+    pub fn is_tree_error(&self) -> bool {
+        matches!(self, InsertBlockErrorKind::Tree(_))
+    }
+
+    /// Returns true if the error is a consensus error
+    pub fn is_consensus_error(&self) -> bool {
+        matches!(self, InsertBlockErrorKind::Consensus(_))
+    }
+
+    /// Returns true if this is a block pre merge error.
+    pub fn is_block_pre_merge(&self) -> bool {
+        matches!(self, InsertBlockErrorKind::Execution(BlockExecutionError::BlockPreMerge { .. }))
+    }
+
+    /// Returns true if the error is an execution error
+    pub fn is_execution_error(&self) -> bool {
+        matches!(self, InsertBlockErrorKind::Execution(_))
+    }
+
+    /// Returns the error if it is a tree error
+    pub fn as_tree_error(&self) -> Option<BlockchainTreeError> {
+        match self {
+            InsertBlockErrorKind::Tree(err) => Some(*err),
+            _ => None,
+        }
+    }
+
+    /// Returns the error if it is a consensus error
+    pub fn as_consensus_error(&self) -> Option<&ConsensusError> {
+        match self {
+            InsertBlockErrorKind::Consensus(err) => Some(err),
+            _ => None,
+        }
+    }
+
+    /// Returns the error if it is an execution error
+    pub fn as_execution_error(&self) -> Option<&BlockExecutionError> {
+        match self {
+            InsertBlockErrorKind::Execution(err) => Some(err),
+            _ => None,
+        }
+    }
+}
+
+// This is a convenience impl to convert from crate::Error to InsertBlockErrorKind, most
+impl From<crate::Error> for InsertBlockErrorKind {
+    fn from(err: crate::Error) -> Self {
+        use crate::Error;
+
+        match err {
+            Error::Execution(err) => InsertBlockErrorKind::Execution(err),
+            Error::Consensus(err) => InsertBlockErrorKind::Consensus(err),
+            Error::Database(err) => InsertBlockErrorKind::Internal(Box::new(err)),
+            Error::Provider(err) => InsertBlockErrorKind::Internal(Box::new(err)),
+            Error::Network(err) => InsertBlockErrorKind::Internal(Box::new(err)),
+        }
+    }
+}

crates/interfaces/src/blockchain_tree/mod.rs

@@ -1,9 +1,11 @@
-use crate::{executor::BlockExecutionError, Error};
+use crate::{blockchain_tree::error::InsertBlockError, Error};
 use reth_primitives::{
     BlockHash, BlockNumHash, BlockNumber, SealedBlock, SealedBlockWithSenders, SealedHeader,
 };
 use std::collections::{BTreeMap, HashSet};
 
+pub mod error;
+
 /// * [BlockchainTreeEngine::insert_block]: Connect block to chain, execute it and if valid insert
 ///   block inside tree.
 /// * [BlockchainTreeEngine::finalize_block]: Remove chains that join to now finalized block, as
@@ -13,22 +15,29 @@ use std::collections::{BTreeMap, HashSet};
 ///   blocks from p2p. Do reorg in tables if canonical chain if needed.
 pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     /// Recover senders and call [`BlockchainTreeEngine::insert_block`].
-    fn insert_block_without_senders(&self, block: SealedBlock) -> Result<BlockStatus, Error> {
-        let block = block.seal_with_senders().ok_or(BlockExecutionError::SenderRecoveryError)?;
-        self.insert_block(block)
+    fn insert_block_without_senders(
+        &self,
+        block: SealedBlock,
+    ) -> Result<BlockStatus, InsertBlockError> {
+        match block.try_seal_with_senders() {
+            Ok(block) => self.insert_block(block),
+            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
+        }
     }
 
     /// Recover senders and call [`BlockchainTreeEngine::buffer_block`].
-    fn buffer_block_without_sender(&self, block: SealedBlock) -> Result<(), Error> {
-        let block = block.seal_with_senders().ok_or(BlockExecutionError::SenderRecoveryError)?;
-        self.buffer_block(block)
+    fn buffer_block_without_sender(&self, block: SealedBlock) -> Result<(), InsertBlockError> {
+        match block.try_seal_with_senders() {
+            Ok(block) => self.buffer_block(block),
+            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
+        }
     }
 
-    /// buffer block with senders
-    fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), Error>;
+    /// Buffer block with senders
+    fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError>;
 
     /// Insert block with senders
-    fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, Error>;
+    fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, InsertBlockError>;
 
     /// Finalize blocks up until and including `finalized_block`, and remove them from the tree.
     fn finalize_block(&self, finalized_block: BlockNumber);
@@ -102,6 +111,11 @@ pub trait BlockchainTreeViewer: Send + Sync {
     /// Caution: This will not return blocks from the canonical chain.
     fn block_by_hash(&self, hash: BlockHash) -> Option<SealedBlock>;
 
+    /// Returns true if the tree contains the block with matching hash.
+    fn contains(&self, hash: BlockHash) -> bool {
+        self.block_by_hash(hash).is_some()
+    }
+
     /// Canonical block number and hashes best known by the tree.
     fn canonical_blocks(&self) -> BTreeMap<BlockNumber, BlockHash>;
 

crates/interfaces/src/executor.rs

@@ -1,4 +1,4 @@
-use reth_primitives::{BlockHash, BlockNumHash, BlockNumber, Bloom, H256};
+use reth_primitives::{BlockHash, BlockNumHash, Bloom, H256};
 use thiserror::Error;
 
 /// BlockExecutor Errors
@@ -22,24 +22,13 @@ pub enum BlockExecutionError {
     BlockGasUsed { got: u64, expected: u64 },
     #[error("Provider error")]
     ProviderError,
-    #[error("BlockChainId can't be found in BlockchainTree with internal index {chain_id}")]
-    BlockSideChainIdConsistency { chain_id: u64 },
+    // TODO(mattsse): move this to tree error
+    #[error("Block hash {block_hash} not found in blockchain tree chain")]
+    BlockHashNotFoundInChain { block_hash: BlockHash },
     #[error(
         "Appending chain on fork (other_chain_fork:?) is not possible as the tip is {chain_tip:?}"
     )]
     AppendChainDoesntConnect { chain_tip: BlockNumHash, other_chain_fork: BlockNumHash },
-    #[error("Canonical chain header #{block_hash} can't be found ")]
-    CanonicalChain { block_hash: BlockHash },
-    #[error("Can't insert #{block_number} {block_hash} as last finalized block number is {last_finalized}")]
-    PendingBlockIsFinalized {
-        block_hash: BlockHash,
-        block_number: BlockNumber,
-        last_finalized: BlockNumber,
-    },
-    #[error("Block number #{block_number} not found in blockchain tree chain")]
-    BlockNumberNotFoundInChain { block_number: BlockNumber },
-    #[error("Block hash {block_hash} not found in blockchain tree chain")]
-    BlockHashNotFoundInChain { block_hash: BlockHash },
     #[error("Transaction error on revert: {inner:?}")]
     CanonicalRevert { inner: String },
     #[error("Transaction error on commit: {inner:?}")]

crates/primitives/src/block.rs

@@ -146,8 +146,15 @@ impl SealedBlock {
 
     /// Seal sealed block with recovered transaction senders.
     pub fn seal_with_senders(self) -> Option<SealedBlockWithSenders> {
-        let senders = self.senders()?;
-        Some(SealedBlockWithSenders { block: self, senders })
+        self.try_seal_with_senders().ok()
+    }
+
+    /// Seal sealed block with recovered transaction senders.
+    pub fn try_seal_with_senders(self) -> Result<SealedBlockWithSenders, Self> {
+        match self.senders() {
+            Some(senders) => Ok(SealedBlockWithSenders { block: self, senders }),
+            None => Err(self),
+        }
     }
 
     /// Unseal the block

crates/storage/provider/src/providers/mod.rs

@@ -35,6 +35,7 @@ mod state;
 use crate::{providers::chain_info::ChainInfoTracker, traits::BlockSource};
 pub use database::*;
 pub use post_state_provider::PostStateProvider;
+use reth_interfaces::blockchain_tree::error::InsertBlockError;
 
 /// The main type for interacting with the blockchain.
 ///
@@ -371,11 +372,17 @@ where
     DB: Send + Sync,
     Tree: BlockchainTreeEngine,
 {
-    fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<()> {
+    fn buffer_block(
+        &self,
+        block: SealedBlockWithSenders,
+    ) -> std::result::Result<(), InsertBlockError> {
         self.tree.buffer_block(block)
     }
 
-    fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus> {
+    fn insert_block(
+        &self,
+        block: SealedBlockWithSenders,
+    ) -> std::result::Result<BlockStatus, InsertBlockError> {
         self.tree.insert_block(block)
     }