diff --git a/Cargo.lock b/Cargo.lock index 653be9351e..a14aacc0a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7020,7 +7020,9 @@ dependencies = [ "safe_arith", "serde", "serde_yaml", + "smallvec", "superstruct", + "typenum", "types", ] diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d9a1e46b03..29cd437c43 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4840,10 +4840,53 @@ impl BeaconChain { // If the current slot is already equal to the proposal slot (or we are in the tail end of // the prior slot), then check the actual weight of the head against the head re-org threshold // and the actual weight of the parent against the parent re-org threshold. + // Per spec `is_head_weak`: uses get_attestation_score(head, PENDING) which is + // the total weight. Per spec `is_parent_strong`: uses + // get_attestation_score(parent, parent_payload_status) where parent_payload_status + // is determined by the head block's relationship to its parent. + let head_weight = info.head_node.weight(); + let parent_weight = if let Ok(head_payload_status) = info.head_node.parent_payload_status() + { + // Post-GLOAS: use the payload-filtered weight matching how the head + // extends from its parent. + match head_payload_status { + proto_array::PayloadStatus::Full => { + info.parent_node.full_payload_weight().map_err(|()| { + Box::new(ProposerHeadError::Error( + Error::ProposerHeadForkChoiceError( + fork_choice::Error::ProtoArrayError( + proto_array::Error::InvalidNodeVariant { + block_root: info.parent_node.root(), + }, + ), + ), + )) + })? + } + proto_array::PayloadStatus::Empty => { + info.parent_node.empty_payload_weight().map_err(|()| { + Box::new(ProposerHeadError::Error( + Error::ProposerHeadForkChoiceError( + fork_choice::Error::ProtoArrayError( + proto_array::Error::InvalidNodeVariant { + block_root: info.parent_node.root(), + }, + ), + ), + )) + })? + } + proto_array::PayloadStatus::Pending => info.parent_node.weight(), + } + } else { + // Pre-GLOAS (V17): use total weight. + info.parent_node.weight() + }; + let (head_weak, parent_strong) = if fork_choice_slot == re_org_block_slot { ( - info.head_node.weight() < info.re_org_head_weight_threshold, - info.parent_node.weight() > info.re_org_parent_weight_threshold, + head_weight < info.re_org_head_weight_threshold, + parent_weight > info.re_org_parent_weight_threshold, ) } else { (true, true) @@ -4851,7 +4894,7 @@ impl BeaconChain { if !head_weak { return Err(Box::new( DoNotReOrg::HeadNotWeak { - head_weight: info.head_node.weight(), + head_weight, re_org_head_weight_threshold: info.re_org_head_weight_threshold, } .into(), @@ -4860,7 +4903,7 @@ impl BeaconChain { if !parent_strong { return Err(Box::new( DoNotReOrg::ParentNotStrong { - parent_weight: info.parent_node.weight(), + parent_weight, re_org_parent_weight_threshold: info.re_org_parent_weight_threshold, } .into(), diff --git a/beacon_node/beacon_chain/src/invariants.rs b/beacon_node/beacon_chain/src/invariants.rs index 7bcec7b0b4..b365f37a0a 100644 --- a/beacon_node/beacon_chain/src/invariants.rs +++ b/beacon_node/beacon_chain/src/invariants.rs @@ -23,9 +23,9 @@ impl BeaconChain { // Only check blocks that are descendants of the finalized checkpoint. // Pruned non-canonical fork blocks may linger in the proto-array but // are legitimately absent from the database. - fc.is_finalized_checkpoint_or_descendant(node.root) + fc.is_finalized_checkpoint_or_descendant(node.root()) }) - .map(|node| (node.root, node.slot)) + .map(|node| (node.root(), node.slot())) .collect() }; diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 63220f0bc6..30c56c9775 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -175,8 +175,13 @@ pub enum InvalidAttestation { /// The attestation is attesting to a state that is later than itself. (Viz., attesting to the /// future). AttestsToFutureBlock { block: Slot, attestation: Slot }, + /// Post-GLOAS: attestation index must be 0 or 1. + InvalidAttestationIndex { index: u64 }, /// A same-slot attestation has a non-zero index, which is invalid post-GLOAS. InvalidSameSlotAttestationIndex { slot: Slot }, + /// 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 }, @@ -256,6 +261,8 @@ pub struct QueuedAttestation { attesting_indices: Vec, block_root: Hash256, target_epoch: Epoch, + /// Per GLOAS spec: `payload_present = attestation.data.index == 1`. + payload_present: bool, } impl<'a, E: EthSpec> From> for QueuedAttestation { @@ -265,6 +272,7 @@ impl<'a, E: EthSpec> From> for QueuedAttestation { attesting_indices: a.attesting_indices_to_vec(), block_root: a.data().beacon_block_root, target_epoch: a.data().target.epoch, + payload_present: a.data().index == 1, } } } @@ -1136,15 +1144,34 @@ where }); } - // Post-GLOAS: same-slot attestations must have index == 0. Attestations with - // index != 0 during the same slot as the block are invalid. if spec .fork_name_at_slot::(indexed_attestation.data().slot) .gloas_enabled() - && indexed_attestation.data().slot == block.slot - && indexed_attestation.data().index != 0 { - return Err(InvalidAttestation::InvalidSameSlotAttestationIndex { slot: block.slot }); + let index = indexed_attestation.data().index; + + // Post-GLOAS: attestation index must be 0 or 1. + if index > 1 { + return Err(InvalidAttestation::InvalidAttestationIndex { index }); + } + + // Same-slot attestations must have index == 0. + if indexed_attestation.data().slot == block.slot && index != 0 { + return Err(InvalidAttestation::InvalidSameSlotAttestationIndex { + slot: block.slot, + }); + } + + // index == 1 (payload_present) requires the block's payload to have been received. + if index == 1 + && !self + .proto_array + .is_payload_received(&indexed_attestation.data().beacon_block_root) + { + return Err(InvalidAttestation::PayloadNotReceived { + beacon_block_root: indexed_attestation.data().beacon_block_root, + }); + } } Ok(()) @@ -1245,12 +1272,16 @@ where self.validate_on_attestation(attestation, is_from_block, spec)?; + // Per GLOAS spec: `payload_present = attestation.data.index == 1`. + let payload_present = attestation.data().index == 1; + if attestation.data().slot < self.fc_store.get_current_slot() { for validator_index in attestation.attesting_indices_iter() { self.proto_array.process_attestation( *validator_index as usize, attestation.data().beacon_block_root, attestation.data().slot, + payload_present, )?; } } else { @@ -1433,6 +1464,7 @@ where *validator_index as usize, attestation.block_root, attestation.slot, + attestation.payload_present, )?; } } @@ -1850,6 +1882,7 @@ mod tests { attesting_indices: vec![], block_root: Hash256::zero(), target_epoch: Epoch::new(0), + payload_present: false, }) .collect() } diff --git a/consensus/proto_array/Cargo.toml b/consensus/proto_array/Cargo.toml index 782610e0d3..f9c35bb585 100644 --- a/consensus/proto_array/Cargo.toml +++ b/consensus/proto_array/Cargo.toml @@ -15,5 +15,7 @@ fixed_bytes = { workspace = true } safe_arith = { workspace = true } serde = { workspace = true } serde_yaml = { workspace = true } +smallvec = { workspace = true } superstruct = { workspace = true } +typenum = { workspace = true } types = { workspace = true } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 45aed23b29..16c7df4ca2 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -8,6 +8,7 @@ use crate::proto_array_fork_choice::{Block, ExecutionStatus, PayloadStatus, Prot use crate::{InvalidationOperation, JustifiedBalances}; use fixed_bytes::FixedBytesExtended; use serde::{Deserialize, Serialize}; +use ssz::BitVector; use std::collections::BTreeSet; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, @@ -96,6 +97,15 @@ pub enum Operation { is_timely: bool, is_data_available: bool, }, + /// Simulate receiving and validating an execution payload for `block_root`. + /// Sets `payload_received = true` on the V29 node via the live validation path. + ProcessExecutionPayload { + block_root: Hash256, + }, + AssertPayloadReceived { + block_root: Hash256, + expected: bool, + }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -286,7 +296,7 @@ impl ForkChoiceTestDefinition { attestation_slot, } => { fork_choice - .process_attestation(validator_index, block_root, attestation_slot) + .process_attestation(validator_index, block_root, attestation_slot, false) .unwrap_or_else(|_| { panic!( "process_attestation op at index {} returned error", @@ -494,9 +504,38 @@ impl ForkChoiceTestDefinition { }); // Set all bits (exceeds any threshold) or clear all bits. let fill = if is_timely { 0xFF } else { 0x00 }; - node_v29.payload_timeliness_votes.fill(fill); + node_v29.payload_timeliness_votes = + BitVector::from_bytes(smallvec::smallvec![fill; 64]) + .expect("valid 512-bit bitvector"); let fill = if is_data_available { 0xFF } else { 0x00 }; - node_v29.payload_data_availability_votes.fill(fill); + node_v29.payload_data_availability_votes = + BitVector::from_bytes(smallvec::smallvec![fill; 64]) + .expect("valid 512-bit bitvector"); + // Per spec, is_payload_timely/is_payload_data_available require + // the payload to be in payload_states (payload_received). + node_v29.payload_received = is_timely || is_data_available; + } + Operation::ProcessExecutionPayload { block_root } => { + fork_choice + .on_execution_payload(block_root) + .unwrap_or_else(|e| { + panic!( + "on_execution_payload op at index {} returned error: {}", + op_index, e + ) + }); + check_bytes_round_trip(&fork_choice); + } + Operation::AssertPayloadReceived { + block_root, + expected, + } => { + let actual = fork_choice.is_payload_received(&block_root); + assert_eq!( + actual, expected, + "payload_received mismatch at op index {}", + op_index + ); } } } 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 9a0043a467..84e2878d32 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 @@ -571,6 +571,120 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef } } +/// Test interleaving of blocks, payload validation, and attestations. +/// +/// Scenario: +/// - Genesis block (slot 0) +/// - Block 1 (slot 1) extends genesis, Full chain +/// - Block 2 (slot 1) extends genesis, Empty chain +/// - Before payload arrives: payload_received is false for block 1 +/// - Process execution payload for block 1 → payload_received becomes true +/// - Payload attestations arrive voting block 1's payload as timely + available +/// - Head should follow block 1 because the PTC votes now count (payload_received = true) +pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Block 1 at slot 1: extends genesis Full chain. + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(1), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(0)), + execution_payload_block_hash: Some(get_hash(1)), + }); + + // Block 2 at slot 1: extends genesis Empty chain (parent_hash doesn't match genesis EL hash). + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(2), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(99)), + execution_payload_block_hash: Some(get_hash(100)), + }); + + // Both children have parent_payload_status set correctly. + ops.push(Operation::AssertParentPayloadStatus { + block_root: get_root(1), + expected_status: PayloadStatus::Full, + }); + ops.push(Operation::AssertParentPayloadStatus { + block_root: get_root(2), + expected_status: PayloadStatus::Empty, + }); + + // Before payload arrives: payload_received is false on genesis. + ops.push(Operation::AssertPayloadReceived { + block_root: get_root(0), + expected: false, + }); + + // Give one vote to each child so they have equal weight. + ops.push(Operation::ProcessAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(1), + }); + ops.push(Operation::ProcessAttestation { + validator_index: 1, + block_root: get_root(2), + attestation_slot: Slot::new(1), + }); + + // Equal weight, no payload received on genesis → tiebreaker uses PTC votes which + // require payload_received. Without it, is_payload_timely returns false → prefers Empty. + // Block 2 (Empty) wins because it matches the Empty preference. + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1], + expected_head: get_root(2), + current_slot: Slot::new(100), + }); + + // Now the execution payload for genesis arrives and is validated. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(0), + }); + + // payload_received is now true. + ops.push(Operation::AssertPayloadReceived { + block_root: get_root(0), + expected: true, + }); + + // Set PTC votes on genesis as timely + data available (simulates PTC voting). + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + + // Now with payload_received=true and PTC votes exceeding threshold: + // is_payload_timely=true, is_payload_data_available=true → prefers Full. + // Block 1 (Full) wins because it matches the Full preference. + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1], + expected_head: get_root(1), + current_slot: Slot::new(100), + }); + + ForkChoiceTestDefinition { + finalized_block_slot: Slot::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + operations: ops, + execution_payload_parent_hash: Some(get_hash(42)), + execution_payload_block_hash: Some(get_hash(0)), + spec: Some(gloas_spec()), + } +} + #[cfg(test)] mod tests { use super::*; @@ -610,4 +724,10 @@ mod tests { let test = get_gloas_interleaved_attestations_test_definition(); test.run(); } + + #[test] + fn payload_received_interleaving() { + let test = get_gloas_payload_received_interleaving_test_definition(); + test.run(); + } } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 4f89e6084f..908d391401 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -2,11 +2,13 @@ use crate::error::InvalidBestNodeInfo; use crate::{Block, ExecutionStatus, JustifiedBalances, PayloadStatus, error::Error}; use fixed_bytes::FixedBytesExtended; use serde::{Deserialize, Serialize}; +use ssz::BitVector; use ssz::Encode; use ssz::four_byte_option_impl; use ssz_derive::{Decode, Encode}; use std::collections::{HashMap, HashSet}; use superstruct::superstruct; +use typenum::U512; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, Slot, @@ -131,14 +133,20 @@ pub struct ProtoNode { pub execution_payload_block_hash: ExecutionBlockHash, /// PTC timeliness vote bitfield, indexed by PTC committee position. /// Bit i set means PTC member i voted `payload_present = true`. - /// Tiebreak derived as: `count_ones() > ptc_size / 2`. + /// Tiebreak derived as: `num_set_bits() > ptc_size / 2`. #[superstruct(only(V29))] - pub payload_timeliness_votes: Vec, + pub payload_timeliness_votes: BitVector, /// PTC data availability vote bitfield, indexed by PTC committee position. /// Bit i set means PTC member i voted `blob_data_available = true`. - /// Tiebreak derived as: `count_ones() > ptc_size / 2`. + /// Tiebreak derived as: `num_set_bits() > ptc_size / 2`. #[superstruct(only(V29))] - pub payload_data_availability_votes: Vec, + 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, } #[derive(PartialEq, Debug, Encode, Decode, Serialize, Deserialize, Copy, Clone)] @@ -385,26 +393,18 @@ impl ProtoArray { .checked_add(delta) .ok_or(Error::DeltaOverflow(parent_index))?; - // Per spec's `is_supporting_vote`: a vote for descendant B supports - // ancestor A's payload status based on B's `parent_payload_status`. - // Route the child's *total* weight delta to the parent's appropriate - // payload bucket. - match node.parent_payload_status() { - Ok(PayloadStatus::Full) => { - parent_delta.full_delta = parent_delta - .full_delta - .checked_add(delta) - .ok_or(Error::DeltaOverflow(parent_index))?; - } - Ok(PayloadStatus::Empty) => { - parent_delta.empty_delta = parent_delta - .empty_delta - .checked_add(delta) - .ok_or(Error::DeltaOverflow(parent_index))?; - } - // Pending or V17 nodes: no payload propagation. - _ => {} - } + // Per spec's `is_supporting_vote`: a vote supports a parent's + // FULL/EMPTY virtual node based on the voter's `payload_present` + // flag, NOT based on which child the vote goes through. + // Propagate each child's full/empty deltas independently. + parent_delta.full_delta = parent_delta + .full_delta + .checked_add(node_full_delta) + .ok_or(Error::DeltaOverflow(parent_index))?; + parent_delta.empty_delta = parent_delta + .empty_delta + .checked_add(node_empty_delta) + .ok_or(Error::DeltaOverflow(parent_index))?; } } @@ -540,8 +540,9 @@ impl ProtoArray { empty_payload_weight: 0, full_payload_weight: 0, execution_payload_block_hash, - payload_timeliness_votes: empty_ptc_bitfield(E::ptc_size()), - payload_data_availability_votes: empty_ptc_bitfield(E::ptc_size()), + payload_timeliness_votes: BitVector::default(), + payload_data_availability_votes: BitVector::default(), + payload_received: false, }) }; @@ -584,9 +585,11 @@ impl ProtoArray { Ok(()) } - /// Process an excution payload for a Gloas block. + /// Process an execution payload for a Gloas block. /// - /// this function assumes the + /// 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> { let index = *self .indices @@ -599,10 +602,7 @@ impl ProtoArray { let v29 = node .as_v29_mut() .map_err(|_| Error::InvalidNodeVariant { block_root })?; - // A valid execution payload means the payload is timely and data is available. - // Set all bits to ensure the threshold is met regardless of PTC size. - v29.payload_timeliness_votes.fill(0xFF); - v29.payload_data_availability_votes.fill(0xFF); + v29.payload_received = true; Ok(()) } @@ -669,8 +669,13 @@ impl ProtoArray { }); } }, - // Gloas nodes don't carry `ExecutionStatus`. + // 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 { @@ -1057,6 +1062,22 @@ impl ProtoArray { best_finalized_checkpoint, )?; + // Per spec `should_extend_payload`: if the proposer-boosted block is a child of + // this parent and extends Empty, force Empty preference regardless of + // weights/tiebreaker. + let proposer_boost_root = self.previous_proposer_boost.root; + let proposer_boost = !proposer_boost_root.is_zero() + && self + .indices + .get(&proposer_boost_root) + .and_then(|&idx| self.nodes.get(idx)) + .is_some_and(|boost_node| { + boost_node.parent() == Some(parent_index) + && boost_node + .parent_payload_status() + .map_or(false, |s| s != PayloadStatus::Full) + }); + // These three variables are aliases to the three options that we may set the // `parent.best_child` and `parent.best_descendant` to. // @@ -1112,12 +1133,14 @@ impl ProtoArray { child, current_slot, E::ptc_size(), + proposer_boost, ); let best_child_matches = child_matches_parent_payload_preference( parent, best_child, current_slot, E::ptc_size(), + proposer_boost, ); if child_matches && !best_child_matches { @@ -1390,27 +1413,30 @@ impl ProtoArray { } /// For V29 parents, returns `true` if the child's `parent_payload_status` matches the parent's -/// preferred payload status. When full and empty weights are unequal, the higher weight wins. -/// When equal, the tiebreaker uses the parent's `payload_tiebreak`: prefer Full if the block -/// was timely and data is available; otherwise prefer Empty. +/// preferred payload status per spec `should_extend_payload`. +/// +/// If `proposer_boost` is set, the parent unconditionally prefers Empty (the proposer-boosted +/// block is a child of this parent and extends Empty). Otherwise, when full and empty weights +/// are unequal the higher weight wins; when equal, the tiebreaker uses PTC votes. +/// /// For V17 parents (or mixed), always returns `true` (no payload preference). -/// -/// TODO(gloas): the spec's `should_extend_payload` has additional conditions beyond the -/// tiebreaker: it also checks proposer_boost_root (empty, different parent, or extends full). -/// See: https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md#new-should_extend_payload -/// -/// TODO(gloas): the spec's `should_extend_payload` has additional conditions beyond the -/// tiebreaker: it also checks proposer_boost_root (empty, different parent, or extends full). -/// See: https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md#new-should_extend_payload fn child_matches_parent_payload_preference( parent: &ProtoNode, child: &ProtoNode, current_slot: Slot, ptc_size: usize, + proposer_boost: bool, ) -> bool { let (Ok(parent_v29), Ok(child_v29)) = (parent.as_v29(), child.as_v29()) else { return true; }; + + // Per spec `should_extend_payload`: if the proposer-boosted block extends Empty from + // this parent, unconditionally prefer Empty. + if proposer_boost { + return child_v29.parent_payload_status == PayloadStatus::Empty; + } + // Per spec `get_weight`: FULL/EMPTY virtual nodes at `current_slot - 1` have weight 0. // The PTC is still voting, so payload preference is determined solely by the tiebreaker. let use_tiebreaker_only = parent.slot() + 1 == current_slot; @@ -1424,8 +1450,15 @@ fn child_matches_parent_payload_preference( false } else { // Equal weights (or current-slot parent): tiebreaker per spec. - is_payload_timely(&parent_v29.payload_timeliness_votes, ptc_size) - && is_payload_data_available(&parent_v29.payload_data_availability_votes, ptc_size) + is_payload_timely( + &parent_v29.payload_timeliness_votes, + ptc_size, + parent_v29.payload_received, + ) && is_payload_data_available( + &parent_v29.payload_data_availability_votes, + ptc_size, + parent_v29.payload_received, + ) }; if prefers_full { child_v29.parent_payload_status == PayloadStatus::Full @@ -1434,24 +1467,30 @@ fn child_matches_parent_payload_preference( } } -/// Count the number of set bits in a byte-slice bitfield. -pub fn count_set_bits(bitfield: &[u8]) -> usize { - bitfield.iter().map(|b| b.count_ones() as usize).sum() -} - -/// Create a zero-initialized bitfield for the given PTC size. -pub fn empty_ptc_bitfield(ptc_size: usize) -> Vec { - vec![0u8; ptc_size.div_ceil(8)] -} - /// Derive `is_payload_timely` from the timeliness vote bitfield. -pub fn is_payload_timely(timeliness_votes: &[u8], ptc_size: usize) -> bool { - count_set_bits(timeliness_votes) > ptc_size / 2 +/// +/// Per spec: returns false if the payload has not been received locally +/// (`payload_received == false`, i.e. `root not in store.payload_states`), +/// regardless of PTC votes. Both local receipt and PTC threshold are required. +pub fn is_payload_timely( + timeliness_votes: &BitVector, + ptc_size: usize, + payload_received: bool, +) -> bool { + payload_received && timeliness_votes.num_set_bits() > ptc_size / 2 } /// Derive `is_payload_data_available` from the data availability vote bitfield. -pub fn is_payload_data_available(availability_votes: &[u8], ptc_size: usize) -> bool { - count_set_bits(availability_votes) > ptc_size / 2 +/// +/// Per spec: returns false if the payload has not been received locally +/// (`payload_received == false`, i.e. `root not in store.payload_states`), +/// regardless of PTC votes. Both local receipt and PTC threshold are required. +pub fn is_payload_data_available( + availability_votes: &BitVector, + ptc_size: usize, + payload_received: bool, +) -> bool { + payload_received && availability_votes.num_set_bits() > ptc_size / 2 } /// A helper method to calculate the proposer boost based on the given `justified_balances`. diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 021d62e63f..e1b8c43ff1 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -63,9 +63,9 @@ pub enum ExecutionStatus { #[ssz(enum_behaviour = "tag")] #[repr(u8)] pub enum PayloadStatus { - Pending = 0, - Empty = 1, - Full = 2, + Empty = 0, + Full = 1, + Pending = 2, } impl ExecutionStatus { @@ -523,12 +523,14 @@ impl ProtoArrayForkChoice { validator_index: usize, block_root: Hash256, attestation_slot: Slot, + payload_present: bool, ) -> Result<(), String> { let vote = self.votes.get_mut(validator_index); if attestation_slot > vote.next_slot || *vote == VoteTracker::default() { vote.next_root = block_root; vote.next_slot = attestation_slot; + vote.next_payload_present = payload_present; } Ok(()) @@ -560,23 +562,14 @@ impl ProtoArrayForkChoice { .as_v29_mut() .map_err(|_| format!("process_payload_attestation: node {block_root:?} is not V29"))?; - let byte_index = ptc_index / 8; - let bit_mask = 1u8 << (ptc_index % 8); - - if let Some(byte) = v29.payload_timeliness_votes.get_mut(byte_index) { - if payload_present { - *byte |= bit_mask; - } else { - *byte &= !bit_mask; - } - } - if let Some(byte) = v29.payload_data_availability_votes.get_mut(byte_index) { - if blob_data_available { - *byte |= bit_mask; - } else { - *byte &= !bit_mask; - } - } + v29.payload_timeliness_votes + .set(ptc_index, payload_present) + .map_err(|e| format!("process_payload_attestation: timeliness set failed: {e:?}"))?; + v29.payload_data_availability_votes + .set(ptc_index, blob_data_available) + .map_err(|e| { + format!("process_payload_attestation: data availability set failed: {e:?}") + })?; Ok(()) } @@ -981,6 +974,14 @@ impl ProtoArrayForkChoice { block.execution_status().ok() } + /// Returns whether the execution payload for a block has been received. + /// Returns `false` for pre-GLOAS (V17) nodes or unknown blocks. + pub fn is_payload_received(&self, block_root: &Hash256) -> bool { + self.get_proto_node(block_root) + .and_then(|node| node.payload_received().ok()) + .unwrap_or(false) + } + /// Returns the weight of a given block. pub fn get_weight(&self, block_root: &Hash256) -> Option { let block_index = self.proto_array.indices.get(block_root)?; @@ -1004,9 +1005,15 @@ impl ProtoArrayForkChoice { Some(PayloadStatus::Full) } else if v29.empty_payload_weight > v29.full_payload_weight { Some(PayloadStatus::Empty) - } else if is_payload_timely(&v29.payload_timeliness_votes, E::ptc_size()) - && is_payload_data_available(&v29.payload_data_availability_votes, E::ptc_size()) - { + } else if is_payload_timely( + &v29.payload_timeliness_votes, + E::ptc_size(), + v29.payload_received, + ) && is_payload_data_available( + &v29.payload_data_availability_votes, + E::ptc_size(), + v29.payload_received, + ) { Some(PayloadStatus::Full) } else { Some(PayloadStatus::Empty)