From 5aae563d84f3d7ae67bc63de5f230024a78cdaa4 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 1 Apr 2026 17:25:50 +1100 Subject: [PATCH 01/20] Remove proposer boost weight during upgrade --- .../src/schema_change/migration_schema_v29.rs | 47 ++++++++++++++++++- .../tests/payload_invalidation.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 6 +-- consensus/proto_array/src/proto_array.rs | 5 -- .../src/proto_array_fork_choice.rs | 31 ++---------- consensus/proto_array/src/ssz_container.rs | 7 ++- 6 files changed, 54 insertions(+), 44 deletions(-) diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs index 3069200fce..77d4be3443 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs @@ -1,7 +1,9 @@ use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY}; use crate::persisted_fork_choice::{PersistedForkChoiceV28, PersistedForkChoiceV29}; +use std::collections::HashMap; use store::hot_cold_store::HotColdDB; use store::{DBColumn, Error as StoreError, KeyValueStore, KeyValueStoreOp}; +use tracing::warn; use types::EthSpec; /// Upgrade from schema v28 to v29. @@ -49,8 +51,49 @@ pub fn upgrade_to_v29( } } - // Convert to v29 and encode. - let persisted_v29 = PersistedForkChoiceV29::from(persisted_v28); + // Read the previous proposer boost before converting to V29 (V29 no longer stores it). + let previous_proposer_boost = persisted_v28 + .fork_choice_v28 + .proto_array_v28 + .previous_proposer_boost; + + // Convert to v29. + let mut persisted_v29 = PersistedForkChoiceV29::from(persisted_v28); + + // Subtract the proposer boost from the boosted node and all its ancestors. + // + // In the V28 schema, `apply_score_changes` baked the proposer boost directly into node + // weights and back-propagated it up the parent chain. In V29, the boost is computed + // on-the-fly during the virtual tree walk. If we don't subtract the baked-in boost here, + // it will be double-counted after the upgrade. + if !previous_proposer_boost.root.is_zero() && previous_proposer_boost.score > 0 { + let score = previous_proposer_boost.score; + let indices: HashMap<_, _> = persisted_v29 + .fork_choice + .proto_array + .indices + .iter() + .cloned() + .collect(); + + if let Some(node_index) = indices.get(&previous_proposer_boost.root).copied() { + let nodes = &mut persisted_v29.fork_choice.proto_array.nodes; + let mut current = Some(node_index); + while let Some(idx) = current { + if let Some(node) = nodes.get_mut(idx) { + *node.weight_mut() = node.weight().saturating_sub(score); + current = node.parent(); + } else { + break; + } + } + } else { + warn!( + root = ?previous_proposer_boost.root, + "Proposer boost node missing from fork choice" + ); + } + } Ok(vec![ persisted_v29.as_kv_store_op(FORK_CHOICE_DB_KEY, db.get_config())?, diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 13672bbb63..947024e8c2 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1438,7 +1438,7 @@ async fn weights_after_resetting_optimistic_status() { .canonical_head .fork_choice_write_lock() .proto_array_mut() - .set_all_blocks_to_optimistic::(&rig.harness.chain.spec) + .set_all_blocks_to_optimistic::() .unwrap(); let new_weights = rig diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index cedd42cf01..c08e76020b 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1614,7 +1614,6 @@ where persisted_proto_array: proto_array::core::SszContainer, justified_balances: JustifiedBalances, reset_payload_statuses: ResetPayloadStatuses, - spec: &ChainSpec, ) -> Result> { let mut proto_array = ProtoArrayForkChoice::from_container( persisted_proto_array.clone(), @@ -1639,7 +1638,7 @@ where // Reset all blocks back to being "optimistic". This helps recover from an EL consensus // fault where an invalid payload becomes valid. - if let Err(e) = proto_array.set_all_blocks_to_optimistic::(spec) { + if let Err(e) = proto_array.set_all_blocks_to_optimistic::() { // If there is an error resetting the optimistic status then log loudly and revert // back to a proto-array which does not have the reset applied. This indicates a // significant error in Lighthouse and warrants detailed investigation. @@ -1669,7 +1668,6 @@ where persisted.proto_array, justified_balances, reset_payload_statuses, - spec, )?; let current_slot = fc_store.get_current_slot(); @@ -1703,7 +1701,7 @@ where // get a different result. fork_choice .proto_array - .set_all_blocks_to_optimistic::(spec)?; + .set_all_blocks_to_optimistic::()?; // If the second attempt at finding a head fails, return an error since we do not // expect this scenario. let _ = fork_choice.get_head(current_slot, spec)?; diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 361e4a86e2..f2a6f6d0dc 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -369,7 +369,6 @@ pub struct ProtoArray { pub prune_threshold: usize, pub nodes: Vec, pub indices: HashMap, - pub previous_proposer_boost: ProposerBoost, } impl ProtoArray { @@ -502,10 +501,6 @@ impl ProtoArray { } } - // Proposer boost is now applied on-the-fly in `get_weight` during the - // walk, so clear any stale boost from a prior call. - self.previous_proposer_boost = ProposerBoost::default(); - Ok(()) } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 72440b83b8..634a78823d 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -2,8 +2,7 @@ use crate::{ JustifiedBalances, error::Error, proto_array::{ - InvalidationOperation, Iter, NodeDelta, ProposerBoost, ProtoArray, ProtoNode, - calculate_committee_fraction, + InvalidationOperation, Iter, NodeDelta, ProtoArray, ProtoNode, calculate_committee_fraction, }, ssz_container::SszContainer, }; @@ -527,7 +526,6 @@ impl ProtoArrayForkChoice { prune_threshold: DEFAULT_PRUNE_THRESHOLD, nodes: Vec::with_capacity(1), indices: HashMap::with_capacity(1), - previous_proposer_boost: ProposerBoost::default(), }; let block = Block { @@ -880,10 +878,7 @@ impl ProtoArrayForkChoice { /// status to be optimistic. /// /// In practice this means forgetting any `VALID` or `INVALID` statuses. - pub fn set_all_blocks_to_optimistic( - &mut self, - spec: &ChainSpec, - ) -> Result<(), String> { + pub fn set_all_blocks_to_optimistic(&mut self) -> Result<(), String> { // Iterate backwards through all nodes in the `proto_array`. Whilst it's not strictly // required to do this process in reverse, it seems natural when we consider how LMD votes // are counted. @@ -906,7 +901,7 @@ impl ProtoArrayForkChoice { // Restore the weight of the node, it would have been set to `0` in // `apply_score_changes` when it was invalidated. - let mut restored_weight: u64 = self + let restored_weight: u64 = self .votes .0 .iter() @@ -922,26 +917,6 @@ impl ProtoArrayForkChoice { }) .sum(); - // If the invalid root was boosted, apply the weight to it and - // ancestors. - if let Some(proposer_score_boost) = spec.proposer_score_boost - && self.proto_array.previous_proposer_boost.root == node.root() - { - // Compute the score based upon the current balances. We can't rely on - // the `previous_proposr_boost.score` since it is set to zero with an - // invalid node. - let proposer_score = - calculate_committee_fraction::(&self.balances, proposer_score_boost) - .ok_or("Failed to compute proposer boost")?; - // Store the score we've applied here so it can be removed in - // a later call to `apply_score_changes`. - self.proto_array.previous_proposer_boost.score = proposer_score; - // Apply this boost to this node. - restored_weight = restored_weight - .checked_add(proposer_score) - .ok_or("Overflow when adding boost to weight")?; - } - // Add the restored weight to the node and all ancestors. if restored_weight > 0 { let mut node_or_ancestor = node; diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 80a6702210..69efb35027 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -38,6 +38,7 @@ pub struct SszContainer { #[superstruct(only(V29))] pub nodes: Vec, pub indices: Vec<(Hash256, usize)>, + #[superstruct(only(V28))] pub previous_proposer_boost: ProposerBoost, } @@ -50,7 +51,6 @@ impl SszContainerV29 { prune_threshold: proto_array.prune_threshold, nodes: proto_array.nodes.clone(), indices: proto_array.indices.iter().map(|(k, v)| (*k, *v)).collect(), - previous_proposer_boost: proto_array.previous_proposer_boost, } } } @@ -63,7 +63,6 @@ impl TryFrom<(SszContainerV29, JustifiedBalances)> for ProtoArrayForkChoice { prune_threshold: from.prune_threshold, nodes: from.nodes, indices: from.indices.into_iter().collect::>(), - previous_proposer_boost: from.previous_proposer_boost, }; Ok(Self { @@ -92,7 +91,6 @@ impl From for SszContainerV29 { }) .collect(), indices: v28.indices, - previous_proposer_boost: v28.previous_proposer_boost, } } } @@ -116,7 +114,8 @@ impl From for SszContainerV28 { }) .collect(), indices: v29.indices, - previous_proposer_boost: v29.previous_proposer_boost, + // Proposer boost is not tracked in V29 (computed on-the-fly), so reset it. + previous_proposer_boost: ProposerBoost::default(), } } } From 5f8605f67e9341112db44ebe27e39b099e1e8543 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 10:40:23 +1100 Subject: [PATCH 02/20] Disable optimistic sync for Gloas --- .../src/payload_envelope_verification/import.rs | 10 ++++++++++ .../src/payload_envelope_verification/mod.rs | 2 ++ 2 files changed, 12 insertions(+) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index ed121ccb94..6efabcdfa8 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -167,6 +167,16 @@ impl BeaconChain { .map_err(BeaconChainError::TokioJoin)? .ok_or(BeaconChainError::RuntimeShutdown)??; + // TODO(gloas): optimistic sync is not supported for Gloas, maybe we could re-add it + if payload_verification_outcome + .payload_verification_status + .is_optimistic() + { + return Err(EnvelopeError::OptimisticSyncNotSupported { + block_root: import_data.block_root, + }); + } + Ok(ExecutedEnvelope::new( signed_envelope, import_data, diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs index c707d62dc7..225d5a9892 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -182,6 +182,8 @@ pub enum EnvelopeError { payload_slot: Slot, latest_finalized_slot: Slot, }, + /// Optimistic sync is not supported for Gloas payload envelopes. + OptimisticSyncNotSupported { block_root: Hash256 }, /// Some Beacon Chain Error BeaconChainError(Arc), /// Some Beacon State error From 1c5a7bed7402481baddafe39c9a9c928e54ce7f1 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 10:56:34 +1100 Subject: [PATCH 03/20] Clarify name of `on_valid_payload_envelope_received` --- .../payload_envelope_verification/import.rs | 5 ++-- consensus/fork_choice/src/fork_choice.rs | 22 ++++++++++----- .../src/fork_choice_test_definition.rs | 2 +- consensus/proto_array/src/proto_array.rs | 27 +++++++------------ .../src/proto_array_fork_choice.rs | 11 ++++++-- testing/ef_tests/src/cases/fork_choice.rs | 2 +- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 6efabcdfa8..4a0a188e5e 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -253,9 +253,10 @@ impl BeaconChain { // avoiding taking other locks whilst holding this lock. let mut fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); - // Update the node's payload_status from PENDING to FULL in fork choice. + // Update the block's payload to received in fork choice, which creates the `Full` virtual + // node which can be eligible for head. fork_choice - .on_execution_payload(block_root) + .on_valid_payload_envelope_received(block_root) .map_err(|e| EnvelopeError::InternalError(format!("{e:?}")))?; // TODO(gloas) emit SSE event if the payload became the new head payload diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index c08e76020b..7f5ef51217 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -654,6 +654,20 @@ where } } + /// Mark a Gloas payload envelope as valid and received. + /// + /// This must only be called for valid Gloas payloads. + pub fn on_valid_payload_envelope_received( + &mut self, + block_root: Hash256, + ) -> Result<(), Error> { + self.proto_array + .on_valid_payload_envelope_received(block_root) + .map_err(Error::FailedToProcessValidExecutionPayload) + } + + /// Pre-Gloas only. + /// /// See `ProtoArrayForkChoice::process_execution_payload_validation` for documentation. pub fn on_valid_execution_payload( &mut self, @@ -664,6 +678,8 @@ where .map_err(Error::FailedToProcessValidExecutionPayload) } + /// Pre-Gloas only. + /// /// See `ProtoArrayForkChoice::process_execution_payload_invalidation` for documentation. pub fn on_invalid_execution_payload( &mut self, @@ -977,12 +993,6 @@ where Ok(()) } - pub fn on_execution_payload(&mut self, block_root: Hash256) -> Result<(), Error> { - self.proto_array - .on_execution_payload(block_root) - .map_err(Error::FailedToProcessValidExecutionPayload) - } - /// Update checkpoints in store if necessary fn update_checkpoints( &mut self, diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index ff9d70bad5..1901091dd6 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -502,7 +502,7 @@ impl ForkChoiceTestDefinition { } Operation::ProcessExecutionPayload { block_root } => { fork_choice - .on_execution_payload(block_root) + .on_valid_payload_envelope_received(block_root) .unwrap_or_else(|e| { panic!( "on_execution_payload op at index {} returned error: {}", diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index f2a6f6d0dc..6695079cd2 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -768,12 +768,10 @@ impl ProtoArray { Ok(!has_equivocation) } - /// Process an execution payload for a Gloas block. + /// Process a valid execution payload envelope for a Gloas block. /// - /// Sets `payload_received` to true, which makes `is_payload_timely` and - /// `is_payload_data_available` return true regardless of PTC votes. - /// This maps to `store.payload_states[root] = state` in the spec. - pub fn on_valid_execution_payload(&mut self, block_root: Hash256) -> Result<(), Error> { + /// Sets `payload_received` to true. + pub fn on_valid_payload_envelope_received(&mut self, block_root: Hash256) -> Result<(), Error> { let index = *self .indices .get(&block_root) @@ -809,6 +807,8 @@ impl ProtoArray { /// Updates the `verified_node_index` and all ancestors to have validated execution payloads. /// + /// This function is a no-op if called for a Gloas block. + /// /// Returns an error if: /// /// - The `verified_node_index` is unknown. @@ -852,18 +852,10 @@ impl ProtoArray { }); } }, - // Gloas nodes don't carry `ExecutionStatus`. Mark the validated - // block as payload-received so that `is_payload_timely` / - // `is_payload_data_available` and `index == 1` attestations work. - ProtoNode::V29(node) => { - if index == verified_node_index { - node.payload_received = true; - } - if let Some(parent_index) = node.parent { - parent_index - } else { - return Ok(()); - } + // Gloas nodes should not be marked valid by this function, which exists only + // for pre-Gloas fork choice. + ProtoNode::V29(_) => { + return Ok(()); } }; @@ -874,6 +866,7 @@ impl ProtoArray { /// Invalidate zero or more blocks, as specified by the `InvalidationOperation`. /// /// See the documentation of `InvalidationOperation` for usage. + // TODO(gloas): this needs some tests for the mixed Gloas/pre-Gloas case. pub fn propagate_execution_payload_invalidation( &mut self, op: &InvalidationOperation, diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 634a78823d..842cdfaa33 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -567,11 +567,18 @@ impl ProtoArrayForkChoice { }) } - pub fn on_execution_payload(&mut self, block_root: Hash256) -> Result<(), String> { + /// Mark a Gloas payload envelope as valid and received. + /// + /// This must only be called for valid Gloas payloads. + pub fn on_valid_payload_envelope_received( + &mut self, + block_root: Hash256, + ) -> Result<(), String> { self.proto_array - .on_valid_execution_payload(block_root) + .on_valid_payload_envelope_received(block_root) .map_err(|e| format!("Failed to process execution payload: {:?}", e)) } + /// See `ProtoArray::propagate_execution_payload_validation` for documentation. pub fn process_execution_payload_validation( &mut self, diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 22e8453e14..06f204ab01 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -1018,7 +1018,7 @@ impl Tester { .chain .canonical_head .fork_choice_write_lock() - .on_execution_payload(block_root); + .on_valid_payload_envelope_received(block_root); if valid { result.map_err(|e| { From 958c8cad3939f02bda2c6667bdb3f5835ad61db6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 11:05:33 +1100 Subject: [PATCH 04/20] Rename fork choice test def for clarity --- .../proto_array/src/fork_choice_test_definition.rs | 4 ++-- .../fork_choice_test_definition/gloas_payload.rs | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 1901091dd6..c9764d3e44 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -98,7 +98,7 @@ pub enum Operation { }, /// Simulate receiving and validating an execution payload for `block_root`. /// Sets `payload_received = true` on the V29 node via the live validation path. - ProcessExecutionPayload { + ProcessExecutionPayloadEnvelope { block_root: Hash256, }, AssertPayloadReceived { @@ -500,7 +500,7 @@ impl ForkChoiceTestDefinition { // the payload to be in payload_states (payload_received). node_v29.payload_received = is_timely || is_data_available; } - Operation::ProcessExecutionPayload { block_root } => { + Operation::ProcessExecutionPayloadEnvelope { block_root } => { fork_choice .on_valid_payload_envelope_received(block_root) .unwrap_or_else(|e| { diff --git a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs index 18d7a40b82..ea37780795 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs @@ -53,7 +53,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { // Mark root_1 as having received its execution payload so that // its FULL virtual node exists in the Gloas fork choice tree. - ops.push(Operation::ProcessExecutionPayload { + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(1), }); @@ -263,7 +263,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe // Mark root_1 as having received its execution payload so that // its FULL virtual node exists in the Gloas fork choice tree. - ops.push(Operation::ProcessExecutionPayload { + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(1), }); @@ -368,7 +368,7 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() // Mark root_1 as having received its execution payload so that // its FULL virtual node exists in the Gloas fork choice tree. - ops.push(Operation::ProcessExecutionPayload { + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(1), }); @@ -538,7 +538,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef // Mark root_1 as having received its execution payload so that // its FULL virtual node exists in the Gloas fork choice tree. - ops.push(Operation::ProcessExecutionPayload { + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(1), }); @@ -674,8 +674,8 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe expected_payload_status: None, }); - // ProcessExecutionPayload on genesis is a no-op (already received at init). - ops.push(Operation::ProcessExecutionPayload { + // ProcessExecutionPayloadEnvelope on genesis is a no-op (already received at init). + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(0), }); @@ -778,7 +778,7 @@ mod tests { // Mark root 2's execution payload as received so the Full virtual child exists. if first_gloas_block_full { - ops.push(Operation::ProcessExecutionPayload { + ops.push(Operation::ProcessExecutionPayloadEnvelope { block_root: get_root(2), }); } From 69d725097183a740d1a2cc81b42bb1da7d1b1cab Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 11:27:55 +1100 Subject: [PATCH 05/20] Tidy up payload attestation verification --- consensus/fork_choice/src/fork_choice.rs | 27 ++++++++++++++++--- .../indexed_payload_attestation.rs | 7 ----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 7f5ef51217..dd68497e23 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -185,6 +185,11 @@ pub enum InvalidAttestation { attestation_slot: Slot, current_slot: Slot, }, + /// One or more payload attesters are not part of the PTC. + PayloadAttestationAttestersNotInPtc { + attesting_indices_len: usize, + attesting_indices_in_ptc: usize, + }, } impl From for Error { @@ -1169,10 +1174,14 @@ where indexed_payload_attestation: &IndexedPayloadAttestation, is_from_block: AttestationFromBlock, ) -> Result<(), InvalidAttestation> { + // This check is from `is_valid_indexed_payload_attestation`, but we do it immediately to + // avoid wasting time on junk attestations. if indexed_payload_attestation.attesting_indices.is_empty() { return Err(InvalidAttestation::EmptyAggregationBitfield); } + // PTC attestation must be for a known block. If block is unknown, delay consideration until + // the block is found (responsibility of caller). let block = self .proto_array .get_block(&indexed_payload_attestation.data.beacon_block_root) @@ -1180,6 +1189,8 @@ where beacon_block_root: indexed_payload_attestation.data.beacon_block_root, })?; + // Not strictly part of the spec, but payload attestations to future slots are MORE INVALID + // than payload attestations to blocks at previous slots. if block.slot > indexed_payload_attestation.data.slot { return Err(InvalidAttestation::AttestsToFutureBlock { block: block.slot, @@ -1187,13 +1198,13 @@ where }); } - // Spec: `if data.slot != state.slot: return` — PTC votes can only - // change the vote for their assigned beacon block. + // PTC votes can only change the vote for their assigned beacon block, return early otherwise if block.slot != indexed_payload_attestation.data.slot { return Ok(()); } // Gossip payload attestations must be for the current slot. + // NOTE: signature is assumed to have been verified by caller. // https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md if matches!(is_from_block, AttestationFromBlock::False) && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot() @@ -1318,10 +1329,20 @@ where // Resolve validator indices to PTC committee positions. let ptc_indices: Vec = attestation - .attesting_indices_iter() + .attesting_indices + .iter() .filter_map(|vi| ptc.iter().position(|&p| p == *vi as usize)) .collect(); + // Check that all the attesters are in the PTC + if ptc_indices.len() != attestation.attesting_indices.len() { + return Err(InvalidAttestation::PayloadAttestationAttestersNotInPtc { + attesting_indices_len: attestation.attesting_indices.len(), + attesting_indices_in_ptc: ptc_indices.len(), + } + .into()); + } + for &ptc_index in &ptc_indices { self.proto_array.process_payload_attestation( attestation.data.beacon_block_root, diff --git a/consensus/types/src/attestation/indexed_payload_attestation.rs b/consensus/types/src/attestation/indexed_payload_attestation.rs index 4de805570c..bb2087e330 100644 --- a/consensus/types/src/attestation/indexed_payload_attestation.rs +++ b/consensus/types/src/attestation/indexed_payload_attestation.rs @@ -2,7 +2,6 @@ use crate::test_utils::TestRandom; use crate::{EthSpec, ForkName, PayloadAttestationData}; use bls::AggregateSignature; use context_deserialize::context_deserialize; -use core::slice::Iter; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use ssz_types::VariableList; @@ -21,12 +20,6 @@ pub struct IndexedPayloadAttestation { pub signature: AggregateSignature, } -impl IndexedPayloadAttestation { - pub fn attesting_indices_iter(&self) -> Iter<'_, u64> { - self.attesting_indices.iter() - } -} - #[cfg(test)] mod tests { use super::*; From bc7864b076ebe87c52a9ad33b669a188ec889a58 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 11:44:02 +1100 Subject: [PATCH 06/20] Split out InvalidPayloadAttestation error --- .../beacon_chain/src/block_verification.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 57 +++++++++++++------ consensus/fork_choice/src/lib.rs | 5 +- consensus/fork_choice/tests/tests.rs | 6 +- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 9bb519373a..1ce1137f1e 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1698,7 +1698,7 @@ impl ExecutionPendingBlock { indexed_payload_attestation, AttestationFromBlock::True, &ptc.0, - ) && !matches!(e, ForkChoiceError::InvalidAttestation(_)) + ) && !matches!(e, ForkChoiceError::InvalidPayloadAttestation(_)) { return Err(BlockError::BeaconChainError(Box::new(e.into()))); } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index dd68497e23..2dbefc763f 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -26,6 +26,7 @@ use types::{ #[derive(Debug)] pub enum Error { InvalidAttestation(InvalidAttestation), + InvalidPayloadAttestation(InvalidPayloadAttestation), InvalidAttesterSlashing(AttesterSlashingValidationError), InvalidBlock(InvalidBlock), ProtoArrayStringError(String), @@ -85,6 +86,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: InvalidPayloadAttestation) -> Self { + Error::InvalidPayloadAttestation(e) + } +} + impl From for Error { fn from(e: AttesterSlashingValidationError) -> Self { Error::InvalidAttesterSlashing(e) @@ -177,14 +184,24 @@ pub enum InvalidAttestation { /// Post-Gloas: attestation with index == 1 (payload_present) requires the block's /// payload to have been received (`root in store.payload_states`). PayloadNotReceived { beacon_block_root: Hash256 }, - /// A payload attestation votes payload_present for a block in the current slot, which is - /// invalid because the payload cannot be known yet. - PayloadPresentDuringSameSlot { slot: Slot }, +} + +#[derive(Debug, Clone, PartialEq)] +pub enum InvalidPayloadAttestation { + /// The payload attestation's attesting indices were empty. + EmptyAggregationBitfield, + /// The `payload_attestation.data.beacon_block_root` block is unknown. + UnknownHeadBlock { beacon_block_root: Hash256 }, + /// The payload attestation is attesting to a block that is later than itself. + AttestsToFutureBlock { block: Slot, attestation: Slot }, /// A gossip payload attestation must be for the current slot. PayloadAttestationNotCurrentSlot { attestation_slot: Slot, current_slot: Slot, }, + /// A payload attestation votes payload_present for a block in the current slot, which is + /// invalid because the payload cannot be known yet. + PayloadPresentDuringSameSlot { slot: Slot }, /// One or more payload attesters are not part of the PTC. PayloadAttestationAttestersNotInPtc { attesting_indices_len: usize, @@ -1173,11 +1190,11 @@ where &self, indexed_payload_attestation: &IndexedPayloadAttestation, is_from_block: AttestationFromBlock, - ) -> Result<(), InvalidAttestation> { + ) -> Result<(), InvalidPayloadAttestation> { // This check is from `is_valid_indexed_payload_attestation`, but we do it immediately to // avoid wasting time on junk attestations. if indexed_payload_attestation.attesting_indices.is_empty() { - return Err(InvalidAttestation::EmptyAggregationBitfield); + return Err(InvalidPayloadAttestation::EmptyAggregationBitfield); } // PTC attestation must be for a known block. If block is unknown, delay consideration until @@ -1185,14 +1202,14 @@ where let block = self .proto_array .get_block(&indexed_payload_attestation.data.beacon_block_root) - .ok_or(InvalidAttestation::UnknownHeadBlock { + .ok_or(InvalidPayloadAttestation::UnknownHeadBlock { beacon_block_root: indexed_payload_attestation.data.beacon_block_root, })?; // Not strictly part of the spec, but payload attestations to future slots are MORE INVALID // than payload attestations to blocks at previous slots. if block.slot > indexed_payload_attestation.data.slot { - return Err(InvalidAttestation::AttestsToFutureBlock { + return Err(InvalidPayloadAttestation::AttestsToFutureBlock { block: block.slot, attestation: indexed_payload_attestation.data.slot, }); @@ -1209,10 +1226,12 @@ where if matches!(is_from_block, AttestationFromBlock::False) && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot() { - return Err(InvalidAttestation::PayloadAttestationNotCurrentSlot { - attestation_slot: indexed_payload_attestation.data.slot, - current_slot: self.fc_store.get_current_slot(), - }); + return Err( + InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot { + attestation_slot: indexed_payload_attestation.data.slot, + current_slot: self.fc_store.get_current_slot(), + }, + ); } // A payload attestation voting payload_present for a block in the current slot is @@ -1222,7 +1241,9 @@ where && self.fc_store.get_current_slot() == block.slot && indexed_payload_attestation.data.payload_present { - return Err(InvalidAttestation::PayloadPresentDuringSameSlot { slot: block.slot }); + return Err(InvalidPayloadAttestation::PayloadPresentDuringSameSlot { + slot: block.slot, + }); } Ok(()) @@ -1336,11 +1357,13 @@ where // Check that all the attesters are in the PTC if ptc_indices.len() != attestation.attesting_indices.len() { - return Err(InvalidAttestation::PayloadAttestationAttestersNotInPtc { - attesting_indices_len: attestation.attesting_indices.len(), - attesting_indices_in_ptc: ptc_indices.len(), - } - .into()); + return Err( + InvalidPayloadAttestation::PayloadAttestationAttestersNotInPtc { + attesting_indices_len: attestation.attesting_indices.len(), + attesting_indices_in_ptc: ptc_indices.len(), + } + .into(), + ); } for &ptc_index in &ptc_indices { diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index 8f479125b7..159eab0ec0 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -4,8 +4,9 @@ mod metrics; pub use crate::fork_choice::{ AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters, - InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice, - PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation, ResetPayloadStatuses, + InvalidAttestation, InvalidBlock, InvalidPayloadAttestation, PayloadVerificationStatus, + PersistedForkChoice, PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation, + ResetPayloadStatuses, }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::{ diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 241e25d3e2..d6f937c0ca 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -11,7 +11,7 @@ use bls::AggregateSignature; use fixed_bytes::FixedBytesExtended; use fork_choice::{ AttestationFromBlock, ForkChoiceStore, InvalidAttestation, InvalidBlock, - PayloadVerificationStatus, QueuedAttestation, + InvalidPayloadAttestation, PayloadVerificationStatus, QueuedAttestation, }; use state_processing::state_advance::complete_state_advance; use std::fmt; @@ -969,8 +969,8 @@ async fn non_block_payload_attestation_for_previous_slot_is_rejected() { assert!( matches!( result, - Err(ForkChoiceError::InvalidAttestation( - InvalidAttestation::PayloadAttestationNotCurrentSlot { .. } + Err(ForkChoiceError::InvalidPayloadAttestation( + InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot { .. } )) ), "gossip payload attestation for previous slot should be rejected, got: {:?}", From cc7e727f90bf28e1ba33413d31c5b34f4147d2ad Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 11:46:34 +1100 Subject: [PATCH 07/20] Tidy latest_message --- consensus/proto_array/src/proto_array_fork_choice.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 842cdfaa33..0ecaea3971 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -73,6 +73,7 @@ impl From for VoteTrackerV28 { } } +/// Spec's `LatestMessage` type. Only used in tests. pub struct LatestMessage { pub slot: Slot, pub root: Hash256, @@ -1064,10 +1065,9 @@ impl ProtoArrayForkChoice { .is_finalized_checkpoint_or_descendant::(descendant_root, best_finalized_checkpoint) } + /// NOTE: only used in tests. pub fn latest_message(&self, validator_index: usize) -> Option { - if validator_index < self.votes.0.len() { - let vote = &self.votes.0[validator_index]; - + if let Some(vote) = self.votes.0.get(validator_index) { if *vote == VoteTracker::default() { None } else { From 727535bcc9768c99589dcc60e8ee0f5f2807814b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 11:50:16 +1100 Subject: [PATCH 08/20] Remove spurious payload attestation condition --- consensus/fork_choice/src/fork_choice.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 2dbefc763f..0993ae95a3 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -199,9 +199,6 @@ pub enum InvalidPayloadAttestation { attestation_slot: Slot, current_slot: Slot, }, - /// A payload attestation votes payload_present for a block in the current slot, which is - /// invalid because the payload cannot be known yet. - PayloadPresentDuringSameSlot { slot: Slot }, /// One or more payload attesters are not part of the PTC. PayloadAttestationAttestersNotInPtc { attesting_indices_len: usize, @@ -1234,18 +1231,6 @@ where ); } - // A payload attestation voting payload_present for a block in the current slot is - // invalid: the payload cannot be known yet. This only applies to gossip attestations; - // payload attestations from blocks have already been validated by the block producer. - if matches!(is_from_block, AttestationFromBlock::False) - && self.fc_store.get_current_slot() == block.slot - && indexed_payload_attestation.data.payload_present - { - return Err(InvalidPayloadAttestation::PayloadPresentDuringSameSlot { - slot: block.slot, - }); - } - Ok(()) } From d7f67dae21203a3691f96b4d9d2c63995ffaba53 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 13:27:43 +1100 Subject: [PATCH 09/20] Remove incorrect comment --- consensus/proto_array/src/proto_array.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 6695079cd2..f259ed7e06 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -165,8 +165,6 @@ pub struct ProtoNode { pub payload_data_availability_votes: BitVector, /// Whether the execution payload for this block has been received and validated locally. /// Maps to `root in store.payload_states` in the spec. - /// When true, `is_payload_timely` and `is_payload_data_available` return true - /// regardless of PTC vote counts. #[superstruct(only(V29), partial_getter(copy))] pub payload_received: bool, /// The proposer index for this block, used by `should_apply_proposer_boost` From 6cc65848da74fe29a13604d8290445e85a0632bb Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 13:35:51 +1100 Subject: [PATCH 10/20] Remove dead code --- consensus/proto_array/src/proto_array.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index f259ed7e06..14290fd784 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -489,12 +489,10 @@ impl ProtoArray { .ok_or(Error::DeltaOverflow(parent_index))?; } } else { - // V17 child of a V29 parent (fork transition): treat as FULL - // since V17 nodes always have execution payloads inline. - parent_delta.full_delta = parent_delta - .full_delta - .checked_add(delta) - .ok_or(Error::DeltaOverflow(parent_index))?; + // This is a v17 node with a v17 parent. + // There is no empty or full weight for v17 nodes, so nothing to propagate. + // In the tree walk, the v17 nodes have an empty child with 0 weight, which + // wins by default (it is the only child). } } } From 85934b4d5fcd55884abd2580089154dd419d2ad0 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 13:52:21 +1100 Subject: [PATCH 11/20] Remove some noisy TODOs --- consensus/proto_array/src/proto_array.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 14290fd784..392f11a198 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -720,7 +720,6 @@ impl ProtoArray { .nodes .get(block_index) .ok_or(Error::InvalidNodeIndex(block_index))?; - // TODO(gloas): handle parent unknown case? let parent_index = block .parent() .ok_or(Error::NodeUnknown(proposer_boost_root))?; @@ -744,7 +743,6 @@ impl ProtoArray { // the parent's slot from the same proposer. let parent_slot = parent.slot(); let parent_root = parent.root(); - // TODO(gloas): handle proposer index for pre-Gloas blocks? let parent_proposer = parent.proposer_index(); let has_equivocation = self.nodes.iter().any(|node| { From 5284486af08216cdaca1e88c4e02cbce8c0a85f3 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 13:55:29 +1100 Subject: [PATCH 12/20] Break out of invalidation loop on Gloas block --- consensus/proto_array/src/proto_array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 392f11a198..faaf675565 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -960,7 +960,7 @@ impl ProtoArray { // This block is pre-merge, therefore it has no execution status. Nor do its // ancestors. Ok(ExecutionStatus::Irrelevant(_)) => break, - Err(_) => (), + Err(_) => break, } } From 7570fd155503a45d27c2be95d23ab3263d81dc48 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 14:10:11 +1100 Subject: [PATCH 13/20] Remove comment --- consensus/proto_array/src/proto_array.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index faaf675565..20554c963e 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1069,9 +1069,6 @@ impl ProtoArray { }); } - // In the post-Gloas world, always use a virtual tree walk. - // - // Best child/best descendant is dead. let best_fc_node = self.find_head_walk::( justified_index, current_slot, From eceedaf7b69c558a2a8aeb81c25e2547b59b98b3 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 31 Mar 2026 15:47:50 +1100 Subject: [PATCH 14/20] Revert parent->child optimisation AGAIN --- consensus/proto_array/src/proto_array.rs | 70 ++++++++---------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 20554c963e..3bf8994c67 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1104,26 +1104,6 @@ impl ProtoArray { Ok((best_fc_node.root, best_fc_node.payload_status)) } - /// Build a parent->children index. Invalid nodes are excluded - /// (they aren't in store.blocks in the spec). - fn build_children_index(&self) -> Vec> { - let mut children = vec![vec![]; self.nodes.len()]; - for (i, node) in self.nodes.iter().enumerate() { - if node - .execution_status() - .is_ok_and(|status| status.is_invalid()) - { - continue; - } - if let Some(parent) = node.parent() - && parent < children.len() - { - children[parent].push(i); - } - } - children - } - /// Spec: `get_filtered_block_tree`. /// /// Returns the set of node indices on viable branches — those with at least @@ -1134,7 +1114,6 @@ impl ProtoArray { current_slot: Slot, best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, - children_index: &[Vec], ) -> HashSet { let mut viable = HashSet::new(); self.filter_block_tree::( @@ -1142,7 +1121,6 @@ impl ProtoArray { current_slot, best_justified_checkpoint, best_finalized_checkpoint, - children_index, &mut viable, ); viable @@ -1155,17 +1133,25 @@ impl ProtoArray { current_slot: Slot, best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, - children_index: &[Vec], viable: &mut HashSet, ) -> bool { let Some(node) = self.nodes.get(node_index) else { return false; }; - let children = children_index - .get(node_index) - .map(|c| c.as_slice()) - .unwrap_or(&[]); + // Skip invalid children — they aren't in store.blocks in the spec. + let children: Vec = self + .nodes + .iter() + .enumerate() + .filter(|(_, child)| { + child.parent() == Some(node_index) + && !child + .execution_status() + .is_ok_and(|status| status.is_invalid()) + }) + .map(|(i, _)| i) + .collect(); if !children.is_empty() { // Evaluate ALL children (no short-circuit) to mark all viable branches. @@ -1177,7 +1163,6 @@ impl ProtoArray { current_slot, best_justified_checkpoint, best_finalized_checkpoint, - children_index, viable, ) }) @@ -1222,16 +1207,12 @@ impl ProtoArray { payload_status: PayloadStatus::Pending, }; - // Build parent->children index once for O(1) lookups. - let children_index = self.build_children_index(); - // Spec: `get_filtered_block_tree`. let viable_nodes = self.get_filtered_block_tree::( start_index, current_slot, best_justified_checkpoint, best_finalized_checkpoint, - &children_index, ); // Compute once rather than per-child per-level. @@ -1240,7 +1221,7 @@ impl ProtoArray { loop { let children: Vec<_> = self - .get_node_children(&head, &children_index)? + .get_node_children(&head)? .into_iter() .filter(|(fc_node, _)| viable_nodes.contains(&fc_node.proto_node_index)) .collect(); @@ -1403,7 +1384,6 @@ impl ProtoArray { fn get_node_children( &self, node: &IndexedForkChoiceNode, - children_index: &[Vec], ) -> Result, Error> { if node.payload_status == PayloadStatus::Pending { let proto_node = self @@ -1417,25 +1397,23 @@ impl ProtoArray { } Ok(children) } else { - let child_indices = children_index - .get(node.proto_node_index) - .map(|c| c.as_slice()) - .unwrap_or(&[]); - Ok(child_indices + Ok(self + .nodes .iter() - .filter_map(|&child_index| { - let child_node = self.nodes.get(child_index)?; - if child_node.get_parent_payload_status() != node.payload_status { - return None; - } - Some(( + .enumerate() + .filter(|(_, child_node)| { + child_node.parent() == Some(node.proto_node_index) + && child_node.get_parent_payload_status() == node.payload_status + }) + .map(|(child_index, child_node)| { + ( IndexedForkChoiceNode { root: child_node.root(), proto_node_index: child_index, payload_status: PayloadStatus::Pending, }, child_node.clone(), - )) + ) }) .collect()) } From 6df55970aab1b138ebe10cf7f87c4a6148f25462 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 14:24:37 +1100 Subject: [PATCH 15/20] Simplify find_head_walk --- consensus/proto_array/src/proto_array.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 3bf8994c67..39c638f4a8 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1232,11 +1232,7 @@ impl ProtoArray { head = children .into_iter() - .map(|(child, _)| -> Result<_, Error> { - let proto_node = self - .nodes - .get(child.proto_node_index) - .ok_or(Error::InvalidNodeIndex(child.proto_node_index))?; + .map(|(child, ref proto_node)| -> Result<_, Error> { let weight = self.get_weight::( &child, proto_node, From f5413c6e9721e46a9a383e16bd607ec8ce2b6acb Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 15:42:57 +1100 Subject: [PATCH 16/20] Remove useless let _ --- consensus/fork_choice/src/fork_choice.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 0993ae95a3..92fd4c1faf 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -474,7 +474,7 @@ where }; // Ensure that `fork_choice.forkchoice_update_parameters.head_root` is updated. - let _ = fork_choice.get_head(current_slot, spec)?; + fork_choice.get_head(current_slot, spec)?; Ok(fork_choice) } @@ -1743,7 +1743,7 @@ where .set_all_blocks_to_optimistic::()?; // If the second attempt at finding a head fails, return an error since we do not // expect this scenario. - let _ = fork_choice.get_head(current_slot, spec)?; + fork_choice.get_head(current_slot, spec)?; } Ok(fork_choice) From a1296fc0c7df2f28eac8c2c52fce1cdb3e9c51b2 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 16:38:40 +1100 Subject: [PATCH 17/20] Fix block_timeliness_ptc_threshold hardcoded constants --- consensus/proto_array/src/proto_array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 39c638f4a8..f1145598a9 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -636,7 +636,7 @@ impl ProtoArray { // TODO(gloas): use Gloas-specific PTC due threshold once // `get_payload_attestation_due_ms` is on ChainSpec. block_timeliness_ptc_threshold: is_genesis - || (is_current_slot && time_into_slot < spec.get_slot_duration() / 2), + || (is_current_slot && time_into_slot < 3 * spec.get_slot_duration() / 4), equivocating_attestation_score: 0, }) }; From 9db37b8bd3d93917413aa35b880a87137e20eb06 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 2 Apr 2026 00:50:46 -0500 Subject: [PATCH 18/20] Document is_head_weak spec divergence and impact --- consensus/proto_array/src/proto_array.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index f1145598a9..b0f471dd6d 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -672,12 +672,17 @@ impl ProtoArray { Ok(()) } - /// Spec: `is_head_weak`. - /// - /// The spec adds weight from equivocating validators in the head slot's - /// committees. We approximate this with `equivocating_attestation_score` - /// which tracks equivocating validators that voted for this block (close - /// but not identical to committee membership). + // TODO(gloas): the spec adds weight from equivocating validators in the + // head slot's *committees*, regardless of who they voted for. We approximate + // with `equivocating_attestation_score` which only tracks equivocating + // validators whose vote pointed at this block. This under-counts when an + // equivocating validator is in the committee but voted for a different fork, + // which could allow a re-org the spec wouldn't. In practice the deviation + // is small — it requires equivocating validators voting for competing forks + // AND the head weight to be exactly at the reorg threshold boundary. + // Fixing this properly requires committee computation from BeaconState, + // which is not available in proto_array. The fix would be to pass + // pre-computed equivocating committee weight from the beacon_chain caller. fn is_head_weak( &self, head_node: &ProtoNode, From 6763862e0f796cfda42f0781503715c90c14f651 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 2 Apr 2026 00:56:42 -0500 Subject: [PATCH 19/20] Add attestation_due_bps_gloas and payload_attestation_due_bps to ChainSpec Spec: `get_attestation_due_ms(epoch)` uses ATTESTATION_DUE_BPS_GLOAS (2500) for Gloas epochs vs ATTESTATION_DUE_BPS (3333) pre-Gloas. `get_payload_attestation_due_ms` uses PAYLOAD_ATTESTATION_DUE_BPS (7500). - Add both BPS fields to ChainSpec with derived Duration values - Add `get_attestation_due::(slot)` that returns epoch-appropriate threshold matching the spec - Add `get_payload_attestation_due()` matching the spec - Use them in proto_array record_block_timeliness instead of hardcoded values --- consensus/proto_array/src/proto_array.rs | 7 ++- consensus/types/src/core/chain_spec.rs | 54 +++++++++++++++++++++++- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index b0f471dd6d..dfb43f5f34 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -632,11 +632,9 @@ impl ProtoArray { // Anchor gets [True, True]. Others computed from time_into_slot. block_timeliness_attestation_threshold: is_genesis || (is_current_slot - && time_into_slot < spec.get_unaggregated_attestation_due()), - // TODO(gloas): use Gloas-specific PTC due threshold once - // `get_payload_attestation_due_ms` is on ChainSpec. + && time_into_slot < spec.get_attestation_due::(current_slot)), block_timeliness_ptc_threshold: is_genesis - || (is_current_slot && time_into_slot < 3 * spec.get_slot_duration() / 4), + || (is_current_slot && time_into_slot < spec.get_payload_attestation_due()), equivocating_attestation_score: 0, }) }; @@ -672,6 +670,7 @@ impl ProtoArray { Ok(()) } + /// Spec: `is_head_weak`. // TODO(gloas): the spec adds weight from equivocating validators in the // head slot's *committees*, regardless of who they voted for. We approximate // with `equivocating_attestation_score` which only tracks equivocating diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index cc79d3fc29..9ffa7d6f7e 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -107,6 +107,8 @@ pub struct ChainSpec { pub shard_committee_period: u64, pub proposer_reorg_cutoff_bps: u64, pub attestation_due_bps: u64, + pub attestation_due_bps_gloas: u64, + pub payload_attestation_due_bps: u64, pub aggregate_due_bps: u64, pub sync_message_due_bps: u64, pub contribution_due_bps: u64, @@ -115,6 +117,8 @@ pub struct ChainSpec { * Derived time values (computed at startup via `compute_derived_values()`) */ pub unaggregated_attestation_due: Duration, + pub unaggregated_attestation_due_gloas: Duration, + pub payload_attestation_due: Duration, pub aggregate_attestation_due: Duration, pub sync_message_due: Duration, pub contribution_and_proof_due: Duration, @@ -877,6 +881,20 @@ impl ChainSpec { self.unaggregated_attestation_due } + /// Spec: `get_attestation_due_ms`. Returns the epoch-appropriate threshold. + pub fn get_attestation_due(&self, slot: Slot) -> Duration { + if self.fork_name_at_slot::(slot).gloas_enabled() { + self.unaggregated_attestation_due_gloas + } else { + self.unaggregated_attestation_due + } + } + + /// Spec: `get_payload_attestation_due_ms`. + pub fn get_payload_attestation_due(&self) -> Duration { + self.payload_attestation_due + } + /// Get the duration into a slot in which an aggregated attestation is due. /// Returns the pre-computed value from `compute_derived_values()`. pub fn get_aggregate_attestation_due(&self) -> Duration { @@ -949,6 +967,12 @@ impl ChainSpec { self.unaggregated_attestation_due = self .compute_slot_component_duration(self.attestation_due_bps) .expect("invalid chain spec: cannot compute unaggregated_attestation_due"); + self.unaggregated_attestation_due_gloas = self + .compute_slot_component_duration(self.attestation_due_bps_gloas) + .expect("invalid chain spec: cannot compute unaggregated_attestation_due_gloas"); + self.payload_attestation_due = self + .compute_slot_component_duration(self.payload_attestation_due_bps) + .expect("invalid chain spec: cannot compute payload_attestation_due"); self.aggregate_attestation_due = self .compute_slot_component_duration(self.aggregate_due_bps) .expect("invalid chain spec: cannot compute aggregate_attestation_due"); @@ -1079,6 +1103,8 @@ impl ChainSpec { shard_committee_period: 256, proposer_reorg_cutoff_bps: 1667, attestation_due_bps: 3333, + attestation_due_bps_gloas: 2500, + payload_attestation_due_bps: 7500, aggregate_due_bps: 6667, sync_message_due_bps: 3333, contribution_due_bps: 6667, @@ -1087,6 +1113,8 @@ impl ChainSpec { * Derived time values (set by `compute_derived_values()`) */ unaggregated_attestation_due: Duration::from_millis(3999), + unaggregated_attestation_due_gloas: Duration::from_millis(3000), + payload_attestation_due: Duration::from_millis(9000), aggregate_attestation_due: Duration::from_millis(8000), sync_message_due: Duration::from_millis(3999), contribution_and_proof_due: Duration::from_millis(8000), @@ -1479,6 +1507,8 @@ impl ChainSpec { shard_committee_period: 256, proposer_reorg_cutoff_bps: 1667, attestation_due_bps: 3333, + attestation_due_bps_gloas: 2500, + payload_attestation_due_bps: 7500, aggregate_due_bps: 6667, /* @@ -1486,6 +1516,8 @@ impl ChainSpec { * Precomputed for 5000ms slot: 3333 bps = 1666ms, 6667 bps = 3333ms */ unaggregated_attestation_due: Duration::from_millis(1666), + unaggregated_attestation_due_gloas: Duration::from_millis(1250), + payload_attestation_due: Duration::from_millis(3750), aggregate_attestation_due: Duration::from_millis(3333), sync_message_due: Duration::from_millis(1666), contribution_and_proof_due: Duration::from_millis(3333), @@ -2062,6 +2094,12 @@ pub struct Config { #[serde(default = "default_attestation_due_bps")] #[serde(with = "serde_utils::quoted_u64")] attestation_due_bps: u64, + #[serde(default = "default_attestation_due_bps_gloas")] + #[serde(with = "serde_utils::quoted_u64")] + attestation_due_bps_gloas: u64, + #[serde(default = "default_payload_attestation_due_bps")] + #[serde(with = "serde_utils::quoted_u64")] + payload_attestation_due_bps: u64, #[serde(default = "default_aggregate_due_bps")] #[serde(with = "serde_utils::quoted_u64")] aggregate_due_bps: u64, @@ -2288,6 +2326,14 @@ const fn default_attestation_due_bps() -> u64 { 3333 } +const fn default_attestation_due_bps_gloas() -> u64 { + 2500 +} + +const fn default_payload_attestation_due_bps() -> u64 { + 7500 +} + const fn default_aggregate_due_bps() -> u64 { 6667 } @@ -2539,6 +2585,8 @@ impl Config { proposer_reorg_cutoff_bps: spec.proposer_reorg_cutoff_bps, attestation_due_bps: spec.attestation_due_bps, + attestation_due_bps_gloas: spec.attestation_due_bps_gloas, + payload_attestation_due_bps: spec.payload_attestation_due_bps, aggregate_due_bps: spec.aggregate_due_bps, sync_message_due_bps: spec.sync_message_due_bps, contribution_due_bps: spec.contribution_due_bps, @@ -2632,6 +2680,8 @@ impl Config { min_epochs_for_data_column_sidecars_requests, proposer_reorg_cutoff_bps, attestation_due_bps, + attestation_due_bps_gloas, + payload_attestation_due_bps, aggregate_due_bps, sync_message_due_bps, contribution_due_bps, @@ -2731,6 +2781,8 @@ impl Config { proposer_reorg_cutoff_bps, attestation_due_bps, + attestation_due_bps_gloas, + payload_attestation_due_bps, aggregate_due_bps, sync_message_due_bps, contribution_due_bps, @@ -3634,11 +3686,9 @@ mod yaml_tests { "EIP7928_FORK_VERSION", "EIP7928_FORK_EPOCH", // Gloas params not yet in Config - "ATTESTATION_DUE_BPS_GLOAS", "AGGREGATE_DUE_BPS_GLOAS", "SYNC_MESSAGE_DUE_BPS_GLOAS", "CONTRIBUTION_DUE_BPS_GLOAS", - "PAYLOAD_ATTESTATION_DUE_BPS", "MAX_REQUEST_PAYLOADS", // Gloas fork choice params not yet in Config "REORG_HEAD_WEIGHT_THRESHOLD", From 68f18efbe5df5c4e629a9851f951839a927c9444 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 2 Apr 2026 19:24:29 +1100 Subject: [PATCH 20/20] Fix minimal spec --- consensus/types/src/core/chain_spec.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index 9ffa7d6f7e..e612c8b6db 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -1418,6 +1418,8 @@ impl ChainSpec { * Precomputed for 6000ms slot: 3333 bps = 1999ms, 6667 bps = 4000ms */ unaggregated_attestation_due: Duration::from_millis(1999), + unaggregated_attestation_due_gloas: Duration::from_millis(1500), + payload_attestation_due: Duration::from_millis(4500), aggregate_attestation_due: Duration::from_millis(4000), sync_message_due: Duration::from_millis(1999), contribution_and_proof_due: Duration::from_millis(4000),