feat(error): revamp make_canonical error

crates/blockchain-tree/src/blockchain_tree.rs

@@ -8,7 +8,7 @@ use crate::{
 use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx};
 use reth_interfaces::{
     blockchain_tree::{
-        error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind},
+        error::{BlockchainTreeError, CanonicalError, InsertBlockError, InsertBlockErrorKind},
         BlockStatus, CanonicalOutcome, InsertPayloadOk,
     },
     consensus::{Consensus, ConsensusError},
@@ -906,12 +906,12 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         if let Some(header) = self.find_canonical_header(block_hash)? {
             info!(target: "blockchain_tree", ?block_hash, "Block is already canonical, ignoring.");
             let td = self.externals.database().provider()?.header_td(block_hash)?.ok_or(
-                BlockExecutionError::from(BlockValidationError::MissingTotalDifficulty {
+                CanonicalError::from(BlockValidationError::MissingTotalDifficulty {
                     hash: *block_hash,
                 }),
             )?;
             if !self.externals.chain_spec.fork(Hardfork::Paris).active_at_ttd(td, U256::ZERO) {
-                return Err(BlockExecutionError::from(BlockValidationError::BlockPreMerge {
+                return Err(CanonicalError::from(BlockValidationError::BlockPreMerge {
                     hash: *block_hash,
                 })
                 .into())
@@ -921,10 +921,10 @@ 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 {
             warn!(target: "blockchain_tree", ?block_hash,  "Block hash not found in block indices");
-            // TODO: better error
-            return Err(
-                BlockExecutionError::BlockHashNotFoundInChain { block_hash: *block_hash }.into()
-            )
+            return Err(CanonicalError::from(BlockchainTreeError::BlockHashNotFoundInChain {
+                block_hash: *block_hash,
+            })
+            .into())
         };
         let chain = self.chains.remove(&chain_id).expect("To be present");
 

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

@@ -11,7 +11,7 @@ use futures::{Future, StreamExt};
 use reth_db::database::Database;
 use reth_interfaces::{
     blockchain_tree::{
-        error::{InsertBlockError, InsertBlockErrorKind},
+        error::{BlockchainTreeError, CanonicalError, InsertBlockError, InsertBlockErrorKind},
         BlockStatus, BlockchainTreeEngine, CanonicalOutcome, InsertPayloadOk,
     },
     consensus::ForkchoiceState,
@@ -668,7 +668,7 @@ where
                 PayloadStatus::new(PayloadStatusEnum::Valid, Some(state.head_block_hash))
             }
             Err(error) => {
-                if let Error::Execution(ref err) = error {
+                if let Error::Canonical(ref err) = error {
                     if err.is_fatal() {
                         tracing::error!(target: "consensus::engine", ?err, "Encountered fatal error");
                         return Err(error)
@@ -1398,7 +1398,9 @@ where
                     // it's part of the canonical chain: if it's the safe or the finalized block
                     if matches!(
                         err,
-                        Error::Execution(BlockExecutionError::BlockHashNotFoundInChain { .. })
+                        Error::Canonical(CanonicalError::BlockchainTree(
+                            BlockchainTreeError::BlockHashNotFoundInChain { .. }
+                        ))
                     ) {
                         // if the inserted block is the currently targeted `finalized` or `safe`
                         // block, we will attempt to make them canonical,

crates/interfaces/src/blockchain_tree/error.rs

@@ -33,6 +33,32 @@ pub enum BlockchainTreeError {
     BlockBufferingFailed { block_hash: BlockHash },
 }
 
+/// Result alias for `CanonicalError`
+pub type CanonicalResult<T> = std::result::Result<T, CanonicalError>;
+
+/// Canonical Errors
+#[allow(missing_docs)]
+#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
+pub enum CanonicalError {
+    #[error(transparent)]
+    Validation(#[from] BlockValidationError),
+    #[error(transparent)]
+    BlockchainTree(#[from] BlockchainTreeError),
+
+    // === transaction errors ===
+    #[error("Transaction error on revert: {inner:?}")]
+    CanonicalRevert { inner: String },
+    #[error("Transaction error on commit: {inner:?}")]
+    CanonicalCommit { inner: String },
+}
+
+impl CanonicalError {
+    /// Returns `true` if the error is fatal.
+    pub fn is_fatal(&self) -> bool {
+        matches!(self, Self::CanonicalCommit { .. } | Self::CanonicalRevert { .. })
+    }
+}
+
 /// Error thrown when inserting a block failed because the block is considered invalid.
 #[derive(thiserror::Error)]
 #[error(transparent)]
@@ -161,6 +187,9 @@ pub enum InsertBlockErrorKind {
     /// An internal error occurred, like interacting with the database.
     #[error("Internal error")]
     Internal(Box<dyn std::error::Error + Send + Sync>),
+    /// Canonical error.
+    #[error(transparent)]
+    Canonical(CanonicalError),
 }
 
 impl InsertBlockErrorKind {
@@ -213,6 +242,12 @@ impl InsertBlockErrorKind {
                 // any other error, such as database errors, are considered internal errors
                 false
             }
+            InsertBlockErrorKind::Canonical(err) => match err {
+                CanonicalError::BlockchainTree(_) |
+                CanonicalError::CanonicalCommit { .. } |
+                CanonicalError::CanonicalRevert { .. } => false,
+                CanonicalError::Validation(_) => true,
+            },
         }
     }
 
@@ -273,6 +308,7 @@ impl From<crate::Error> for InsertBlockErrorKind {
             Error::Provider(err) => InsertBlockErrorKind::Internal(Box::new(err)),
             Error::Network(err) => InsertBlockErrorKind::Internal(Box::new(err)),
             Error::Custom(err) => InsertBlockErrorKind::Internal(err.into()),
+            Error::Canonical(err) => InsertBlockErrorKind::Canonical(err),
         }
     }
 }

crates/interfaces/src/error.rs

@@ -20,6 +20,9 @@ pub enum Error {
     #[error(transparent)]
     Network(#[from] reth_network_api::NetworkError),
 
+    #[error(transparent)]
+    Canonical(#[from] crate::blockchain_tree::error::CanonicalError),
+
     #[error("{0}")]
     Custom(std::string::String),
 }