fix: add InsertBlockErrorKind::is_invalid_block block function (#3423)

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

@@ -956,26 +956,20 @@ where
         err: InsertBlockError,
     ) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
         let (block, error) = err.split();
-        match error {
-            InsertBlockErrorKind::Internal(err) => {
-                // this is an internal error that is unrelated to the payload
-                Err(BeaconOnNewPayloadError::Internal(err))
-            }
-            InsertBlockErrorKind::SenderRecovery |
-            InsertBlockErrorKind::Consensus(_) |
-            InsertBlockErrorKind::Execution(_) |
-            InsertBlockErrorKind::Tree(_) => {
-                // all of these occurred if the payload is invalid
-                let parent_hash = block.parent_hash;
 
-                // keep track of the invalid header
-                self.invalid_headers.insert(block.header);
+        if error.is_invalid_block() {
+            // all of these occurred if the payload is invalid
+            let parent_hash = block.parent_hash;
 
-                let latest_valid_hash =
-                    self.latest_valid_hash_for_invalid_payload(parent_hash, Some(&error));
-                let status = PayloadStatusEnum::Invalid { validation_error: error.to_string() };
-                Ok(PayloadStatus::new(status, latest_valid_hash))
-            }
+            // keep track of the invalid header
+            self.invalid_headers.insert(block.header);
+
+            let latest_valid_hash =
+                self.latest_valid_hash_for_invalid_payload(parent_hash, Some(&error));
+            let status = PayloadStatusEnum::Invalid { validation_error: error.to_string() };
+            Ok(PayloadStatus::new(status, latest_valid_hash))
+        } else {
+            Err(BeaconOnNewPayloadError::Internal(Box::new(error)))
         }
     }
 
@@ -1089,8 +1083,7 @@ where
             }
             Err(err) => {
                 warn!(target: "consensus::engine", ?err, "Failed to insert downloaded block");
-                if !matches!(err.kind(), InsertBlockErrorKind::Internal(_)) {
-                    // non-internal error kinds occur if the payload is invalid
+                if err.kind().is_invalid_block() {
                     self.invalid_headers.insert(err.into_block().header);
                 }
             }

crates/interfaces/src/blockchain_tree/error.rs

@@ -174,6 +174,48 @@ impl InsertBlockErrorKind {
         matches!(self, InsertBlockErrorKind::Consensus(_))
     }
 
+    /// 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.
+    pub fn is_invalid_block(&self) -> bool {
+        match self {
+            InsertBlockErrorKind::SenderRecovery | InsertBlockErrorKind::Consensus(_) => true,
+            // other execution errors that are considered internal errors
+            InsertBlockErrorKind::Execution(err) => {
+                match err {
+                    BlockExecutionError::Validation(_) => {
+                        // this is caused by an invalid block
+                        true
+                    }
+                    // these are internal errors, not caused by an invalid block
+                    BlockExecutionError::ProviderError |
+                    BlockExecutionError::CanonicalRevert { .. } |
+                    BlockExecutionError::CanonicalCommit { .. } |
+                    BlockExecutionError::BlockHashNotFoundInChain { .. } |
+                    BlockExecutionError::AppendChainDoesntConnect { .. } |
+                    BlockExecutionError::UnavailableForTest => false,
+                }
+            }
+            InsertBlockErrorKind::Tree(err) => {
+                match err {
+                    BlockchainTreeError::PendingBlockIsFinalized { .. } => {
+                        // the block's number is lower than the finalized block's number
+                        true
+                    }
+                    BlockchainTreeError::BlockSideChainIdConsistency { .. } |
+                    BlockchainTreeError::CanonicalChain { .. } |
+                    BlockchainTreeError::BlockNumberNotFoundInChain { .. } |
+                    BlockchainTreeError::BlockHashNotFoundInChain { .. } |
+                    BlockchainTreeError::BlockBufferingFailed { .. } => false,
+                }
+            }
+            InsertBlockErrorKind::Internal(_) => {
+                // any other error, such as database errors, are considered internal errors
+                false
+            }
+        }
+    }
+
     /// Returns true if this is a block pre merge error.
     pub fn is_block_pre_merge(&self) -> bool {
         matches!(