Remove pre execution block from DA checker if execution failed to allow re-processing.

jimmygchen · jimmygchen · commit 5f2e85c5625b · 2025-09-17T18:53:36.000+10:00
diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -3377,7 +3377,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     .set_time_consensus_verified(block_root, block_slot, timestamp)
             }
 
-            let executed_block = chain.into_executed_block(execution_pending).await?;
+            let executed_block = chain
+                .into_executed_block(execution_pending)
+                .await
+                .inspect_err(|_| {
+                    // If the block fails execution for whatever reason (e.g. engine offline),
+                    // and we keep it in the cache, then the node will NOT perform lookup and
+                    // reprocess this block until the block is evicted from DA checker, causing the
+                    // chain to get stuck temporarily if the block is canonical. Therefore we remove
+                    // it from the cache if execution fails.
+                    self.data_availability_checker
+                        .remove_block_on_execution_error(&block_root);
+                })?;
 
             // Record the *additional* time it took to wait for execution layer verification.
             if let Some(timestamp) = self.slot_clock.now_duration() {
diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs
@@ -356,6 +356,13 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
             .put_pre_execution_block(block_root, block)
     }
 
+    /// Removes a pre-execution block from the cache.
+    /// This does NOT remove an existing executed block.
+    pub fn remove_block_on_execution_error(&self, block_root: &Hash256) {
+        self.availability_cache
+            .remove_pre_execution_block(block_root);
+    }
+
     /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may
     /// include the fully available block.
     ///
diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs
@@ -704,6 +704,15 @@ impl<T: BeaconChainTypes> DataAvailabilityCheckerInner<T> {
         Ok(())
     }
 
+    /// Removes a pre-execution block from the cache.
+    /// This does NOT remove an existing executed block.
+    pub fn remove_pre_execution_block(&self, block_root: &Hash256) {
+        // The read lock is immediately dropped so we can safely remove the block from the cache.
+        if let Some(BlockProcessStatus::NotValidated(_)) = self.get_cached_block(block_root) {
+            self.critical.write().pop(block_root);
+        }
+    }
+
     /// Check if we have all the blobs for a block. If we do, return the Availability variant that
     /// triggers import of the block.
     pub fn put_executed_block(
@@ -1429,4 +1438,34 @@ mod pending_components_tests {
 
         assert_cache_consistent(cache, max_len);
     }
+
+    #[test]
+    fn should_not_insert_pre_execution_block_if_executed_block_exists() {
+        let (pre_execution_block, blobs, random_blobs, max_len) = pre_setup();
+        let (executed_block, blobs, random_blobs) =
+            setup_pending_components(pre_execution_block.clone(), blobs, random_blobs);
+
+        let block_root = pre_execution_block.canonical_root();
+        let max_blobs_per_block = 6; // doesn't matter here
+        let mut pending_component = <PendingComponents<E>>::empty(block_root, max_blobs_per_block);
+
+        let pre_execution_block = Arc::new(pre_execution_block);
+        pending_component.insert_pre_execution_block(pre_execution_block.clone());
+        assert!(
+            matches!(pending_component.block, Some(CachedBlock::PreExecution(_))),
+            "pre execution block inserted"
+        );
+
+        pending_component.insert_executed_block(executed_block);
+        assert!(
+            matches!(pending_component.block, Some(CachedBlock::Executed(_))),
+            "executed block inserted"
+        );
+
+        pending_component.insert_pre_execution_block(pre_execution_block);
+        assert!(
+            matches!(pending_component.block, Some(CachedBlock::Executed(_))),
+            "executed block should remain"
+        );
+    }
 }
diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs
@@ -1123,6 +1123,11 @@ impl TestRig {
     }
 
     fn simulate_block_gossip_processing_becomes_invalid(&mut self, block_root: Hash256) {
+        self.harness
+            .chain
+            .data_availability_checker
+            .remove_block_on_execution_error(&block_root);
+
         self.send_sync_message(SyncMessage::GossipBlockProcessResult {
             block_root,
             imported: false,
