Avoid 0x00 block hashes in fcU (#9233)

- Avoid sending 0x00 block hashes for the safe and finalized block hashes post-Gloas.
- Add code to check this inside the mock EL, which will be reached in all Gloas beacon chain tests


Co-Authored-By: Michael Sproul <michael@sigmaprime.io>

beacon_node/execution_layer/src/test_utils/execution_block_generator.rs

@@ -69,6 +69,13 @@ impl<E: EthSpec> Block<E> {
         }
     }
 
+    pub fn timestamp(&self) -> u64 {
+        match self {
+            Block::PoW(block) => block.timestamp,
+            Block::PoS(payload) => payload.timestamp(),
+        }
+    }
+
     pub fn total_difficulty(&self) -> Option<Uint256> {
         match self {
             Block::PoW(block) => Some(block.total_difficulty),
@@ -558,6 +565,23 @@ impl<E: EthSpec> ExecutionBlockGenerator<E> {
             self.insert_block(Block::PoS(payload))?;
         }
 
+        // Post-Gloas, the justified and finalized block hashes must be non-zero, since the
+        // CL always has a known parent_block_hash to reference.
+        if let Some(head_block) = self.blocks.get(&head_block_hash)
+            && self
+                .get_fork_at_timestamp(head_block.timestamp())
+                .gloas_enabled()
+        {
+            assert!(
+                forkchoice_state.safe_block_hash != ExecutionBlockHash::zero(),
+                "post-Gloas safe_block_hash must not be zero"
+            );
+            assert!(
+                forkchoice_state.finalized_block_hash != ExecutionBlockHash::zero(),
+                "post-Gloas finalized_block_hash must not be zero"
+            );
+        }
+
         let unknown_head_block_hash = !self.blocks.contains_key(&head_block_hash);
         let unknown_safe_block_hash = forkchoice_state.safe_block_hash
             != ExecutionBlockHash::zero()

consensus/fork_choice/src/fork_choice.rs

@@ -564,9 +564,9 @@ where
         // For Gloas blocks, `execution_status` is Irrelevant (no embedded payload).
         // If the payload envelope was received (Full), use the bid's block_hash as the
         // execution chain head. Otherwise fall back to the parent hash (Pending) or None.
-        // TODO(gloas): this is a bit messy, and we probably need a similar treatment for
-        // justified/finalized
-        // Can fix as part of: https://github.com/sigp/lighthouse/issues/8957
+        // For justified/finalized hashes we always use the bid's parent_block_hash, since the
+        // payload from the justified/finalized block is not itself justified/finalized due to
+        // being applied immediately prior to the next block.
         let head_hash = self.get_block(&head_root).and_then(|b| {
             b.execution_status
                 .block_hash()
@@ -579,12 +579,16 @@ where
         });
         let justified_root = self.justified_checkpoint().root;
         let finalized_root = self.finalized_checkpoint().root;
-        let justified_hash = self
-            .get_block(&justified_root)
-            .and_then(|b| b.execution_status.block_hash());
-        let finalized_hash = self
-            .get_block(&finalized_root)
-            .and_then(|b| b.execution_status.block_hash());
+        let justified_hash = self.get_block(&justified_root).and_then(|b| {
+            b.execution_status
+                .block_hash()
+                .or(b.execution_payload_parent_hash)
+        });
+        let finalized_hash = self.get_block(&finalized_root).and_then(|b| {
+            b.execution_status
+                .block_hash()
+                .or(b.execution_payload_parent_hash)
+        });
         self.forkchoice_update_parameters = ForkchoiceUpdateParameters {
             head_root,
             head_hash,