diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index ff1b807785..0e37a7ffb3 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -956,26 +956,20 @@ where err: InsertBlockError, ) -> Result { 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); } } diff --git a/crates/interfaces/src/blockchain_tree/error.rs b/crates/interfaces/src/blockchain_tree/error.rs index e9bee5105e..c8913911cb 100644 --- a/crates/interfaces/src/blockchain_tree/error.rs +++ b/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!(