From 2d8d7a019f9390836f7d417d89f24bdf90f9d18b Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:32:23 +0200 Subject: [PATCH] Fix FCR spec safety edges --- .../beacon_chain/src/canonical_head.rs | 2 +- consensus/fast_confirmation/src/lib.rs | 4 +- .../fast_confirmation/src/optimizations.rs | 25 ++++++++++ .../fast_confirmation/src/slot_assignments.rs | 49 +++++++++++++------ 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 9b8fd575224..1c1a0fcf3e3 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -1042,7 +1042,7 @@ impl BeaconChain { let confirmed_block_hash = confirmed_node .execution_status .block_hash() - .or(confirmed_node.execution_payload_block_hash) + .or(confirmed_node.execution_payload_parent_hash) .ok_or(FastConfirnmationError::NodeHasNoBlockHash( fcr.confirmed_root, ))?; diff --git a/consensus/fast_confirmation/src/lib.rs b/consensus/fast_confirmation/src/lib.rs index b4fed7304cd..e89be201faf 100644 --- a/consensus/fast_confirmation/src/lib.rs +++ b/consensus/fast_confirmation/src/lib.rs @@ -1016,7 +1016,9 @@ impl FastConfirmationRule { for ((vote_root, vote_epoch), balance) in balance_by_vote_checkpoint.iter() { // Spec: get_checkpoint_for_block(store, latest_messages[i].root, // get_latest_message_epoch(latest_messages[i])). - if get_checkpoint_for_block::(vote_root, vote_epoch, proto_array) == Some(target) { + if !optimizations::is_invalid_or_descendant(proto_array, vote_root) + && get_checkpoint_for_block::(vote_root, vote_epoch, proto_array) == Some(target) + { score = score.safe_add(balance)?; } } diff --git a/consensus/fast_confirmation/src/optimizations.rs b/consensus/fast_confirmation/src/optimizations.rs index e92a18f279c..e6d2db7b815 100644 --- a/consensus/fast_confirmation/src/optimizations.rs +++ b/consensus/fast_confirmation/src/optimizations.rs @@ -223,6 +223,9 @@ impl<'a> ChainProjector<'a> { /// The deepest canonical position `vote_root` descends from, or `None` if it covers no block /// on the segment. fn project(&self, vote_root: Hash256) -> Option { + if is_invalid_or_descendant(self.proto_array, vote_root) { + return None; + } let &start_idx = self.proto_array.indices.get(&vote_root)?; let mut idx = start_idx; loop { @@ -281,6 +284,28 @@ pub fn precompute_chain_attestation_scores( Ok(scores) } +pub(crate) fn is_invalid_or_descendant(proto_array: &ProtoArray, root: Hash256) -> bool { + let Some(&start_idx) = proto_array.indices.get(&root) else { + return false; + }; + let mut idx = start_idx; + loop { + let Some(node) = proto_array.nodes.get(idx) else { + return false; + }; + if node + .execution_status() + .is_ok_and(|status| status.is_invalid()) + { + return true; + } + let Some(parent_idx) = node.parent() else { + return false; + }; + idx = parent_idx; + } +} + /// Sum unslashed balances by LMD vote root (skipping zero and equivocating votes), so the ancestor /// projection runs once per distinct root rather than once per validator. pub(crate) fn aggregate_vote_balances( diff --git a/consensus/fast_confirmation/src/slot_assignments.rs b/consensus/fast_confirmation/src/slot_assignments.rs index faae1b91f9b..53ea3c48f6b 100644 --- a/consensus/fast_confirmation/src/slot_assignments.rs +++ b/consensus/fast_confirmation/src/slot_assignments.rs @@ -2,7 +2,7 @@ use crate::Error; use safe_arith::SafeArith; -use types::{BeaconState, EthSpec, Hash256, RelativeEpoch, Slot}; +use types::{BeaconState, EthSpec, Hash256, RelativeEpoch, Slot, compute_committee_range_in_epoch}; /// Per-validator committee slot assignments across a 3-epoch window. /// @@ -74,25 +74,27 @@ impl SlotAssignments { .committee_cache(relative_epoch) .map_err(|e| Error::CommitteeCacheUninitialized(format!("{e:?}")))?; - let shuffling = committee_cache.shuffling(); let committees_per_slot = committee_cache.committees_per_slot() as usize; let epoch_start = epoch.start_slot(E::slots_per_epoch()); + let epoch_committee_count = committee_cache.epoch_committee_count()?; - // Each position in the shuffled list maps to a committee, which maps to a slot. - // committee_index_in_epoch = position * total_committees / shuffling_len - // slot_offset = committee_index_in_epoch / committees_per_slot - let total_committees = committees_per_slot.safe_mul(E::slots_per_epoch() as usize)?; - let shuffling_len = shuffling.len(); - - for (position, &val_idx) in shuffling.iter().enumerate() { - let committee_index = position - .safe_mul(total_committees)? - .safe_div(shuffling_len)?; + for committee_index in 0..epoch_committee_count { + let Some(range) = compute_committee_range_in_epoch( + epoch_committee_count, + committee_index, + committee_cache.shuffling().len(), + )? + else { + continue; + }; let slot_offset = committee_index.safe_div(committees_per_slot)?; let slot = epoch_start.safe_add(slot_offset as u64)?; - if val_idx < validator_count { - let idx = val_idx.safe_mul(NUM_EPOCH_COLUMNS)?.safe_add(col)?; - *new_slots.get_mut(idx).ok_or(Error::IndexOutOfBounds(idx))? = slot; + + for &val_idx in &committee_cache.shuffling()[range] { + if val_idx < validator_count { + let idx = val_idx.safe_mul(NUM_EPOCH_COLUMNS)?.safe_add(col)?; + *new_slots.get_mut(idx).ok_or(Error::IndexOutOfBounds(idx))? = slot; + } } } } @@ -264,4 +266,21 @@ mod tests { ); } } + + #[test] + fn assignments_match_committee_cache_boundaries() { + let (state, _spec) = genesis_state(65); + let sa = SlotAssignments::new::(&state).expect("should build"); + let committee_cache = state + .committee_cache(RelativeEpoch::Current) + .expect("committee cache"); + + for val_idx in 0..state.validators().len() { + let duty = committee_cache + .get_attestation_duties(val_idx) + .expect("duty lookup") + .expect("active validator duty"); + assert_eq!(sa.get(val_idx, 1), Some(duty.slot)); + } + } }