convert ``OptimismBlockExecution`` error variant into a general purpose error variant (#8100)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

Cargo.lock

@@ -7007,6 +7007,7 @@ dependencies = [
  "reth-revm",
  "revm",
  "revm-primitives",
+ "thiserror",
  "tracing",
 ]
 

crates/blockchain-tree/src/blockchain_tree.rs

@@ -2162,7 +2162,7 @@ mod tests {
             .assert(&tree);
 
         // unwind canonical
-        assert_eq!(tree.unwind(block1.number), Ok(()));
+        assert!(tree.unwind(block1.number).is_ok());
         // Trie state:
         //    b2   b2a (pending block)
         //   /    /
@@ -2226,7 +2226,7 @@ mod tests {
             .assert(&tree);
 
         // update canonical block to b2, this would make b2a be removed
-        assert_eq!(tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(12), Ok(()));
+        assert!(tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(12).is_ok());
 
         assert_eq!(
             tree.is_block_known(block2.num_hash()).unwrap(),

crates/ethereum/evm/src/execute.rs

@@ -523,9 +523,10 @@ mod tests {
             .expect_err(
                 "Executing cancun block without parent beacon block root field should fail",
             );
+
         assert_eq!(
-            err,
-            BlockExecutionError::Validation(BlockValidationError::MissingParentBeaconBlockRoot)
+            err.as_validation().unwrap().clone(),
+            BlockValidationError::MissingParentBeaconBlockRoot
         );
 
         // fix header, set a gas limit

crates/interfaces/src/blockchain_tree/error.rs

@@ -293,8 +293,7 @@ impl InsertBlockErrorKind {
                     BlockExecutionError::CanonicalCommit { .. } |
                     BlockExecutionError::AppendChainDoesntConnect { .. } |
                     BlockExecutionError::UnavailableForTest => false,
-                    #[cfg(feature = "optimism")]
-                    BlockExecutionError::OptimismBlockExecution(_) => false,
+                    BlockExecutionError::Other(_) => false,
                 }
             }
             InsertBlockErrorKind::Tree(err) => {

crates/interfaces/src/error.rs

@@ -16,7 +16,7 @@ pub type RethResult<T> = Result<T, RethError>;
 /// This enum encapsulates various error types that can occur during blockchain interactions.
 ///
 /// It allows for structured error handling based on the nature of the encountered issue.
-#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)]
+#[derive(Debug, thiserror::Error)]
 pub enum RethError {
     /// Error encountered during block execution.
     #[error(transparent)]

crates/interfaces/src/executor.rs

@@ -80,7 +80,7 @@ pub enum BlockValidationError {
 }
 
 /// BlockExecutor Errors
-#[derive(Error, Debug, Clone, PartialEq, Eq)]
+#[derive(Error, Debug)]
 pub enum BlockExecutionError {
     /// Validation error, transparently wrapping `BlockValidationError`
     #[error(transparent)]
@@ -118,35 +118,28 @@ pub enum BlockExecutionError {
     /// Error when fetching latest block state.
     #[error(transparent)]
     LatestBlock(#[from] ProviderError),
-
     /// Optimism Block Executor Errors
-    #[cfg(feature = "optimism")]
     #[error(transparent)]
-    OptimismBlockExecution(#[from] OptimismBlockExecutionError),
-}
-
-/// Optimism Block Executor Errors
-#[cfg(feature = "optimism")]
-#[derive(Error, Debug, Clone, PartialEq, Eq)]
-pub enum OptimismBlockExecutionError {
-    /// Error when trying to parse L1 block info
-    #[error("could not get L1 block info from L2 block: {message:?}")]
-    L1BlockInfoError {
-        /// The inner error message
-        message: String,
-    },
-    /// Thrown when force deploy of create2deployer code fails.
-    #[error("failed to force create2deployer account code")]
-    ForceCreate2DeployerFail,
-    /// Thrown when a blob transaction is included in a sequencer's block.
-    #[error("blob transaction included in sequencer block")]
-    BlobTransactionRejected,
-    /// Thrown when a database account could not be loaded.
-    #[error("failed to load account {0}")]
-    AccountLoadFailed(reth_primitives::Address),
+    Other(Box<dyn std::error::Error + Send + Sync>),
 }
 
 impl BlockExecutionError {
+    /// Create a new `BlockExecutionError::Other` variant.
+    pub fn other<E>(error: E) -> Self
+    where
+        E: std::error::Error + Send + Sync + 'static,
+    {
+        Self::Other(Box::new(error))
+    }
+
+    /// Returns the inner `BlockValidationError` if the error is a validation error.
+    pub const fn as_validation(&self) -> Option<&BlockValidationError> {
+        match self {
+            Self::Validation(err) => Some(err),
+            _ => None,
+        }
+    }
+
     /// Returns `true` if the error is fatal.
     ///
     /// This represents an unrecoverable database related error.

crates/optimism/evm/Cargo.toml

@@ -23,6 +23,7 @@ revm.workspace = true
 revm-primitives.workspace = true
 
 # misc
+thiserror.workspace = true
 tracing.workspace = true
 
 [dev-dependencies]

crates/optimism/evm/src/error.rs

@@ -0,0 +1,29 @@
+//! Error types for the Optimism EVM module.
+
+use reth_interfaces::executor::BlockExecutionError;
+
+/// Optimism Block Executor Errors
+#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
+pub enum OptimismBlockExecutionError {
+    /// Error when trying to parse L1 block info
+    #[error("could not get L1 block info from L2 block: {message:?}")]
+    L1BlockInfoError {
+        /// The inner error message
+        message: String,
+    },
+    /// Thrown when force deploy of create2deployer code fails.
+    #[error("failed to force create2deployer account code")]
+    ForceCreate2DeployerFail,
+    /// Thrown when a blob transaction is included in a sequencer's block.
+    #[error("blob transaction included in sequencer block")]
+    BlobTransactionRejected,
+    /// Thrown when a database account could not be loaded.
+    #[error("failed to load account {0}")]
+    AccountLoadFailed(reth_primitives::Address),
+}
+
+impl From<OptimismBlockExecutionError> for BlockExecutionError {
+    fn from(err: OptimismBlockExecutionError) -> Self {
+        BlockExecutionError::other(err)
+    }
+}

crates/optimism/evm/src/execute.rs

@@ -1,6 +1,9 @@
 //! Optimism block executor.
 
-use crate::{l1::ensure_create2_deployer, verify::verify_receipts, OptimismEvmConfig};
+use crate::{
+    l1::ensure_create2_deployer, verify::verify_receipts, OptimismBlockExecutionError,
+    OptimismEvmConfig,
+};
 use reth_evm::{
     execute::{
         BatchBlockExecutionOutput, BatchExecutor, BlockExecutionInput, BlockExecutionOutput,
@@ -9,7 +12,7 @@ use reth_evm::{
     ConfigureEvm, ConfigureEvmEnv,
 };
 use reth_interfaces::{
-    executor::{BlockExecutionError, BlockValidationError, OptimismBlockExecutionError},
+    executor::{BlockExecutionError, BlockValidationError},
     provider::ProviderError,
 };
 use reth_primitives::{
@@ -141,13 +144,8 @@ where
         // blocks will always have at least a single transaction in them (the L1 info transaction),
         // so we can safely assume that this will always be triggered upon the transition and that
         // the above check for empty blocks will never be hit on OP chains.
-        ensure_create2_deployer(self.chain_spec.clone(), block.timestamp, evm.db_mut()).map_err(
-            |_| {
-                BlockExecutionError::OptimismBlockExecution(
-                    OptimismBlockExecutionError::ForceCreate2DeployerFail,
-                )
-            },
-        )?;
+        ensure_create2_deployer(self.chain_spec.clone(), block.timestamp, evm.db_mut())
+            .map_err(|_| OptimismBlockExecutionError::ForceCreate2DeployerFail)?;
 
         let mut cumulative_gas_used = 0;
         let mut receipts = Vec::with_capacity(block.body.len());
@@ -167,9 +165,7 @@ where
 
             // An optimism block should never contain blob transactions.
             if matches!(transaction.tx_type(), TxType::Eip4844) {
-                return Err(BlockExecutionError::OptimismBlockExecution(
-                    OptimismBlockExecutionError::BlobTransactionRejected,
-                ));
+                return Err(OptimismBlockExecutionError::BlobTransactionRejected.into());
             }
 
             // Cache the depositor account prior to the state transition for the deposit nonce.
@@ -184,11 +180,7 @@ where
                         .map(|acc| acc.account_info().unwrap_or_default())
                 })
                 .transpose()
-                .map_err(|_| {
-                    BlockExecutionError::OptimismBlockExecution(
-                        OptimismBlockExecutionError::AccountLoadFailed(*sender),
-                    )
-                })?;
+                .map_err(|_| OptimismBlockExecutionError::AccountLoadFailed(*sender))?;
 
             let mut buf = Vec::with_capacity(transaction.length_without_header());
             transaction.encode_enveloped(&mut buf);

crates/optimism/evm/src/l1.rs

@@ -1,9 +1,7 @@
 //! Optimism-specific implementation and utilities for the executor
 
-use reth_interfaces::{
-    executor::{self as reth_executor, BlockExecutionError},
-    RethError,
-};
+use crate::OptimismBlockExecutionError;
+use reth_interfaces::{executor::BlockExecutionError, RethError};
 use reth_primitives::{address, b256, hex, Address, Block, Bytes, ChainSpec, Hardfork, B256, U256};
 use revm::{
     primitives::{Bytecode, HashMap, SpecId},
@@ -29,20 +27,19 @@ const L1_BLOCK_ECOTONE_SELECTOR: [u8; 4] = hex!("440a5e20");
 /// transaction in the L2 block.
 ///
 /// Returns an error if the L1 info transaction is not found, if the block is empty.
-pub fn extract_l1_info(block: &Block) -> Result<L1BlockInfo, BlockExecutionError> {
+pub fn extract_l1_info(block: &Block) -> Result<L1BlockInfo, OptimismBlockExecutionError> {
     let l1_info_tx_data = block
         .body
         .first()
-        .ok_or(reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
+        .ok_or(OptimismBlockExecutionError::L1BlockInfoError {
             message: "could not find l1 block info tx in the L2 block".to_string(),
         })
         .map(|tx| tx.input())?;
 
     if l1_info_tx_data.len() < 4 {
-        return Err(reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
+        return Err(OptimismBlockExecutionError::L1BlockInfoError {
             message: "invalid l1 block info transaction calldata in the L2 block".to_string(),
-        }
-        .into())
+        })
     }
 
     // If the first 4 bytes of the calldata are the L1BlockInfoEcotone selector, then we parse the
@@ -56,7 +53,7 @@ pub fn extract_l1_info(block: &Block) -> Result<L1BlockInfo, BlockExecutionError
 }
 
 /// Parses the calldata of the [L1BlockInfo] transaction pre-Ecotone hardfork.
-pub fn parse_l1_info_tx_bedrock(data: &[u8]) -> Result<L1BlockInfo, BlockExecutionError> {
+pub fn parse_l1_info_tx_bedrock(data: &[u8]) -> Result<L1BlockInfo, OptimismBlockExecutionError> {
     // The setL1BlockValues tx calldata must be exactly 260 bytes long, considering that
     // we already removed the first 4 bytes (the function selector). Detailed breakdown:
     //   32 bytes for the block number
@@ -68,33 +65,25 @@ pub fn parse_l1_info_tx_bedrock(data: &[u8]) -> Result<L1BlockInfo, BlockExecuti
     // + 32 bytes for the fee overhead
     // + 32 bytes for the fee scalar
     if data.len() != 256 {
-        return Err(reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "unexpected l1 block info tx calldata length found".to_string(),
-            },
-        ))
+        return Err(OptimismBlockExecutionError::L1BlockInfoError {
+            message: "unexpected l1 block info tx calldata length found".to_string(),
+        })
     }
 
     let l1_base_fee = U256::try_from_be_slice(&data[64..96]).ok_or(
-        BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 base fee".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 base fee".to_string(),
+        },
     )?;
     let l1_fee_overhead = U256::try_from_be_slice(&data[192..224]).ok_or(
-        BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 fee overhead".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 fee overhead".to_string(),
+        },
     )?;
     let l1_fee_scalar = U256::try_from_be_slice(&data[224..256]).ok_or(
-        BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 fee scalar".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 fee scalar".to_string(),
+        },
     )?;
 
     let mut l1block = L1BlockInfo::default();
@@ -120,42 +109,32 @@ pub fn parse_l1_info_tx_bedrock(data: &[u8]) -> Result<L1BlockInfo, BlockExecuti
 /// + 32 bytes for the blob base fee
 /// + 32 bytes for the block hash
 /// + 32 bytes for the batcher hash
-pub fn parse_l1_info_tx_ecotone(data: &[u8]) -> Result<L1BlockInfo, BlockExecutionError> {
+pub fn parse_l1_info_tx_ecotone(data: &[u8]) -> Result<L1BlockInfo, OptimismBlockExecutionError> {
     if data.len() != 160 {
-        return Err(reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "unexpected l1 block info tx calldata length found".to_string(),
-            },
-        ))
+        return Err(OptimismBlockExecutionError::L1BlockInfoError {
+            message: "unexpected l1 block info tx calldata length found".to_string(),
+        })
     }
 
     let l1_blob_base_fee_scalar = U256::try_from_be_slice(&data[8..12]).ok_or(
-        reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 blob base fee scalar".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 blob base fee scalar".to_string(),
+        },
     )?;
     let l1_base_fee_scalar = U256::try_from_be_slice(&data[12..16]).ok_or(
-        reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 base fee scalar".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 base fee scalar".to_string(),
+        },
     )?;
     let l1_base_fee = U256::try_from_be_slice(&data[32..64]).ok_or(
-        reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 blob base fee".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 blob base fee".to_string(),
+        },
     )?;
     let l1_blob_base_fee = U256::try_from_be_slice(&data[64..96]).ok_or(
-        reth_executor::BlockExecutionError::OptimismBlockExecution(
-            reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                message: "could not convert l1 blob base fee".to_string(),
-            },
-        ),
+        OptimismBlockExecutionError::L1BlockInfoError {
+            message: "could not convert l1 blob base fee".to_string(),
+        },
     )?;
 
     let mut l1block = L1BlockInfo::default();
@@ -216,11 +195,10 @@ impl RethL1BlockInfo for L1BlockInfo {
         } else if chain_spec.is_fork_active_at_timestamp(Hardfork::Bedrock, timestamp) {
             SpecId::BEDROCK
         } else {
-            return Err(reth_executor::BlockExecutionError::OptimismBlockExecution(
-                reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                    message: "Optimism hardforks are not active".to_string(),
-                },
-            ))
+            return Err(OptimismBlockExecutionError::L1BlockInfoError {
+                message: "Optimism hardforks are not active".to_string(),
+            }
+            .into())
         };
         Ok(self.calculate_tx_l1_cost(input, spec_id))
     }
@@ -236,11 +214,10 @@ impl RethL1BlockInfo for L1BlockInfo {
         } else if chain_spec.is_fork_active_at_timestamp(Hardfork::Bedrock, timestamp) {
             SpecId::BEDROCK
         } else {
-            return Err(reth_executor::BlockExecutionError::OptimismBlockExecution(
-                reth_executor::OptimismBlockExecutionError::L1BlockInfoError {
-                    message: "Optimism hardforks are not active".to_string(),
-                },
-            ))
+            return Err(OptimismBlockExecutionError::L1BlockInfoError {
+                message: "Optimism hardforks are not active".to_string(),
+            }
+            .into())
         };
         Ok(self.data_gas(input, spec_id))
     }

crates/optimism/evm/src/lib.rs

@@ -22,7 +22,9 @@ pub use execute::*;
 pub mod l1;
 pub use l1::*;
 
+mod error;
 pub mod verify;
+pub use error::OptimismBlockExecutionError;
 
 /// Optimism-related EVM configuration.
 #[derive(Debug, Default, Clone, Copy)]

crates/storage/provider/src/chain.rs

@@ -498,7 +498,7 @@ mod tests {
         let chain2 =
             Chain { blocks: BTreeMap::from([(3, block3), (4, block4)]), ..Default::default() };
 
-        assert_eq!(chain1.append_chain(chain2.clone()), Ok(()));
+        assert!(chain1.append_chain(chain2.clone()).is_ok());
 
         // chain1 got changed so this will fail
         assert!(chain1.append_chain(chain2).is_err());