From 8b8124d4a4d961efc89b1d804dab157380a4b495 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 1 May 2026 19:12:11 +1000 Subject: [PATCH 1/2] 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 --- .../test_utils/execution_block_generator.rs | 24 +++++++++++++++++++ consensus/fork_choice/src/fork_choice.rs | 22 ++++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 16d8c03062..4a46ce0f88 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -69,6 +69,13 @@ impl Block { } } + pub fn timestamp(&self) -> u64 { + match self { + Block::PoW(block) => block.timestamp, + Block::PoS(payload) => payload.timestamp(), + } + } + pub fn total_difficulty(&self) -> Option { match self { Block::PoW(block) => Some(block.total_difficulty), @@ -558,6 +565,23 @@ impl ExecutionBlockGenerator { 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() diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 477d1fa3b4..593aa27915 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/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, From 330348ea14bd58828564005795f99ffe874bc7c1 Mon Sep 17 00:00:00 2001 From: jking-aus <72330194+jking-aus@users.noreply.github.com> Date: Fri, 1 May 2026 14:44:25 +0200 Subject: [PATCH 2/2] fix: prevent duplicate column reconstruction dispatch (#9250) Fixes a flaky CI failure in `data_column_reconstruction_at_deadline` where 2 `column_reconstruction` events are emitted instead of the expected 1. - Change `queued_column_reconstructions` from `HashMap` to `HashMap>`, where `None` indicates reconstruction was already dispatched. - On dispatch (`ReadyColumnReconstruction`), set the entry to `None` instead of removing it. This prevents a subsequent gossip column from inserting a fresh reconstruction request into the now-vacant slot. - Prune stale `None` entries on each dispatch to keep the map bounded. Co-Authored-By: Josh King --- .../src/scheduler/work_reprocessing_queue.rs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_processor/src/scheduler/work_reprocessing_queue.rs b/beacon_node/beacon_processor/src/scheduler/work_reprocessing_queue.rs index 38306b3bb6..b1fa56af01 100644 --- a/beacon_node/beacon_processor/src/scheduler/work_reprocessing_queue.rs +++ b/beacon_node/beacon_processor/src/scheduler/work_reprocessing_queue.rs @@ -280,8 +280,8 @@ struct ReprocessQueue { queued_lc_updates: FnvHashMap, /// Light Client Updates per parent_root. awaiting_lc_updates_per_parent_root: HashMap>, - /// Column reconstruction per block root. - queued_column_reconstructions: HashMap, + /// Column reconstruction per block root. `None` means reconstruction was already dispatched. + queued_column_reconstructions: HashMap>, /// Queued backfill batches queued_backfill_batches: Vec, @@ -865,20 +865,20 @@ impl ReprocessQueue { && duration_from_current_slot >= reconstruction_deadline && current_slot == request.slot { - // If we are at least `reconstruction_deadline` seconds into the current slot, - // and the reconstruction request is for the current slot, process reconstruction immediately. reconstruction_delay = Duration::from_secs(0); } match self.queued_column_reconstructions.entry(request.block_root) { - Entry::Occupied(key) => { - self.column_reconstructions_delay_queue - .reset(key.get(), reconstruction_delay); + Entry::Occupied(entry) => { + if let Some(delay_key) = entry.get() { + self.column_reconstructions_delay_queue + .reset(delay_key, reconstruction_delay); + } } Entry::Vacant(vacant) => { let delay_key = self .column_reconstructions_delay_queue .insert(request, reconstruction_delay); - vacant.insert(delay_key); + vacant.insert(Some(delay_key)); } } } @@ -1039,7 +1039,9 @@ impl ReprocessQueue { } InboundEvent::ReadyColumnReconstruction(column_reconstruction) => { self.queued_column_reconstructions - .remove(&column_reconstruction.block_root); + .retain(|_, v| v.is_some()); + self.queued_column_reconstructions + .insert(column_reconstruction.block_root, None); if self .ready_work_tx .try_send(ReadyWork::ColumnReconstruction(column_reconstruction)) @@ -1398,7 +1400,10 @@ mod tests { queue.handle_message(InboundEvent::ReadyColumnReconstruction(reconstruction)); } - assert!(queue.queued_column_reconstructions.is_empty()); + assert_eq!( + queue.queued_column_reconstructions.get(&block_root), + Some(&None) + ); } /// Tests that column reconstruction queued after the deadline is triggered immediately