feat: add better engine insert block error type (paradigmxyz#9884)

megaeth-labs · Jul 30, 2024 · a90fc90 · a90fc90
1 parent 334a2d4
commit a90fc90
Show file tree

Hide file tree

Showing 2 changed files with 182 additions and 14 deletions.
diff --git a/crates/blockchain-tree-api/src/error.rs b/crates/blockchain-tree-api/src/error.rs
@@ -1,7 +1,9 @@
 //! Error handling for the blockchain tree
 
 use reth_consensus::ConsensusError;
-use reth_execution_errors::{BlockExecutionError, BlockValidationError};
+use reth_execution_errors::{
+    BlockExecutionError, BlockValidationError, InternalBlockExecutionError,
+};
 use reth_primitives::{BlockHash, BlockNumber, SealedBlock};
 pub use reth_storage_errors::provider::ProviderError;
 
@@ -207,6 +209,168 @@ impl InsertBlockErrorData {
     }
 }
 
+/// Error thrown when inserting a block failed because the block is considered invalid.
+#[derive(thiserror::Error)]
+#[error(transparent)]
+pub struct InsertBlockErrorTwo {
+    inner: Box<InsertBlockErrorDataTwo>,
+}
+
+// === impl InsertBlockErrorTwo ===
+
+impl InsertBlockErrorTwo {
+    /// Create a new `InsertInvalidBlockErrorTwo`
+    pub fn new(block: SealedBlock, kind: InsertBlockErrorKindTwo) -> Self {
+        Self { inner: InsertBlockErrorDataTwo::boxed(block, kind) }
+    }
+
+    /// Create a new `InsertInvalidBlockError` from a consensus error
+    pub fn consensus_error(error: ConsensusError, block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKindTwo::Consensus(error))
+    }
+
+    /// Create a new `InsertInvalidBlockError` from a consensus error
+    pub fn sender_recovery_error(block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKindTwo::SenderRecovery)
+    }
+
+    /// Create a new `InsertInvalidBlockError` from an execution error
+    pub fn execution_error(error: BlockExecutionError, block: SealedBlock) -> Self {
+        Self::new(block, InsertBlockErrorKindTwo::Execution(error))
+    }
+
+    /// Consumes the error and returns the block that resulted in the error
+    #[inline]
+    pub fn into_block(self) -> SealedBlock {
+        self.inner.block
+    }
+
+    /// Returns the error kind
+    #[inline]
+    pub const fn kind(&self) -> &InsertBlockErrorKindTwo {
+        &self.inner.kind
+    }
+
+    /// Returns the block that resulted in the error
+    #[inline]
+    pub const fn block(&self) -> &SealedBlock {
+        &self.inner.block
+    }
+
+    /// Consumes the type and returns the block and error kind.
+    #[inline]
+    pub fn split(self) -> (SealedBlock, InsertBlockErrorKindTwo) {
+        let inner = *self.inner;
+        (inner.block, inner.kind)
+    }
+}
+
+impl std::fmt::Debug for InsertBlockErrorTwo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Debug::fmt(&self.inner, f)
+    }
+}
+
+struct InsertBlockErrorDataTwo {
+    block: SealedBlock,
+    kind: InsertBlockErrorKindTwo,
+}
+
+impl std::fmt::Display for InsertBlockErrorDataTwo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "Failed to insert block (hash={}, number={}, parent_hash={}): {}",
+            self.block.hash(),
+            self.block.number,
+            self.block.parent_hash,
+            self.kind
+        )
+    }
+}
+
+impl std::fmt::Debug for InsertBlockErrorDataTwo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("InsertBlockError")
+            .field("error", &self.kind)
+            .field("hash", &self.block.hash())
+            .field("number", &self.block.number)
+            .field("parent_hash", &self.block.parent_hash)
+            .field("num_txs", &self.block.body.len())
+            .finish_non_exhaustive()
+    }
+}
+
+impl std::error::Error for InsertBlockErrorDataTwo {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        Some(&self.kind)
+    }
+}
+
+impl InsertBlockErrorDataTwo {
+    const fn new(block: SealedBlock, kind: InsertBlockErrorKindTwo) -> Self {
+        Self { block, kind }
+    }
+
+    fn boxed(block: SealedBlock, kind: InsertBlockErrorKindTwo) -> Box<Self> {
+        Box::new(Self::new(block, kind))
+    }
+}
+
+/// All error variants possible when inserting a block
+#[derive(Debug, thiserror::Error)]
+pub enum InsertBlockErrorKindTwo {
+    /// Failed to recover senders for the block
+    #[error("failed to recover senders for block")]
+    SenderRecovery,
+    /// Block violated consensus rules.
+    #[error(transparent)]
+    Consensus(#[from] ConsensusError),
+    /// Block execution failed.
+    #[error(transparent)]
+    Execution(#[from] BlockExecutionError),
+    /// Provider error.
+    #[error(transparent)]
+    Provider(#[from] ProviderError),
+}
+
+impl InsertBlockErrorKindTwo {
+    /// Returns true if the error is caused by an invalid block
+    ///
+    /// This is intended to be used to determine if the block should be marked as invalid.
+    #[allow(clippy::match_same_arms)]
+    pub fn is_invalid_block(self) -> Result<(), InsertBlockFatalError> {
+        match self {
+            Self::SenderRecovery | Self::Consensus(_) => Ok(()),
+            // other execution errors that are considered internal errors
+            Self::Execution(err) => {
+                match err {
+                    BlockExecutionError::Validation(_) | BlockExecutionError::Consensus(_) => {
+                        // this is caused by an invalid block
+                        Ok(())
+                    }
+                    // these are internal errors, not caused by an invalid block
+                    BlockExecutionError::Internal(error) => {
+                        Err(InsertBlockFatalError::BlockExecutionError(error))
+                    }
+                }
+            }
+            Self::Provider(err) => Err(InsertBlockFatalError::Provider(err)),
+        }
+    }
+}
+
+/// Error variants that are not caused by invalid blocks
+#[derive(Debug, thiserror::Error)]
+pub enum InsertBlockFatalError {
+    /// A provider error
+    #[error(transparent)]
+    Provider(#[from] ProviderError),
+    /// An internal / fatal block execution error
+    #[error(transparent)]
+    BlockExecutionError(#[from] InternalBlockExecutionError),
+}
+
 /// All error variants possible when inserting a block
 #[derive(Debug, thiserror::Error)]
 pub enum InsertBlockErrorKind {

diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs
@@ -9,9 +9,10 @@ use reth_beacon_consensus::{
     OnForkChoiceUpdated, MIN_BLOCKS_FOR_PIPELINE_RUN,
 };
 use reth_blockchain_tree::{
-    error::InsertBlockErrorKind, BlockAttachment, BlockBuffer, BlockStatus,
+    error::{InsertBlockErrorKindTwo, InsertBlockErrorTwo},
+    BlockAttachment, BlockBuffer, BlockStatus,
 };
-use reth_blockchain_tree_api::{error::InsertBlockError, InsertPayloadOk};
+use reth_blockchain_tree_api::InsertPayloadOk;
 use reth_chain_state::{
     CanonicalInMemoryState, ExecutedBlock, MemoryOverlayStateProvider, NewCanonicalChain,
 };
@@ -1016,16 +1017,19 @@ where
         debug!(target: "engine", elapsed = ?now.elapsed(), %block_count ,"connected buffered blocks");
     }
 
-    fn buffer_block_without_senders(&mut self, block: SealedBlock) -> Result<(), InsertBlockError> {
+    fn buffer_block_without_senders(
+        &mut self,
+        block: SealedBlock,
+    ) -> Result<(), InsertBlockErrorTwo> {
         match block.try_seal_with_senders() {
             Ok(block) => self.buffer_block(block),
-            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
+            Err(block) => Err(InsertBlockErrorTwo::sender_recovery_error(block)),
         }
     }
 
-    fn buffer_block(&mut self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
+    fn buffer_block(&mut self, block: SealedBlockWithSenders) -> Result<(), InsertBlockErrorTwo> {
         if let Err(err) = self.validate_block(&block) {
-            return Err(InsertBlockError::consensus_error(err, block.block))
+            return Err(InsertBlockErrorTwo::consensus_error(err, block.block))
         }
         self.state.buffer.insert_block(block);
         Ok(())
@@ -1233,25 +1237,25 @@ where
     fn insert_block_without_senders(
         &mut self,
         block: SealedBlock,
-    ) -> Result<InsertPayloadOk, InsertBlockError> {
+    ) -> Result<InsertPayloadOk, InsertBlockErrorTwo> {
         match block.try_seal_with_senders() {
             Ok(block) => self.insert_block(block),
-            Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
+            Err(block) => Err(InsertBlockErrorTwo::sender_recovery_error(block)),
         }
     }
 
     fn insert_block(
         &mut self,
         block: SealedBlockWithSenders,
-    ) -> Result<InsertPayloadOk, InsertBlockError> {
+    ) -> Result<InsertPayloadOk, InsertBlockErrorTwo> {
         self.insert_block_inner(block.clone())
-            .map_err(|kind| InsertBlockError::new(block.block, kind))
+            .map_err(|kind| InsertBlockErrorTwo::new(block.block, kind))
     }
 
     fn insert_block_inner(
         &mut self,
         block: SealedBlockWithSenders,
-    ) -> Result<InsertPayloadOk, InsertBlockErrorKind> {
+    ) -> Result<InsertPayloadOk, InsertBlockErrorKindTwo> {
         if self.block_by_hash(block.hash())?.is_some() {
             let attachment = BlockAttachment::Canonical; // TODO: remove or revise attachment
             return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid(attachment)))
@@ -1260,13 +1264,13 @@ where
         // validate block consensus rules
         self.validate_block(&block)?;
 
-        let state_provider = self.state_provider(block.parent_hash).unwrap();
+        let state_provider = self.state_provider(block.parent_hash)?;
         let executor = self.executor_provider.executor(StateProviderDatabase::new(&state_provider));
 
         let block_number = block.number;
         let block_hash = block.hash();
         let block = block.unseal();
-        let output = executor.execute((&block, U256::MAX).into()).unwrap();
+        let output = executor.execute((&block, U256::MAX).into())?;
         self.consensus.validate_block_post_execution(
             &block,
             PostExecutionInput::new(&output.receipts, &output.requests),