diff --git a/Cargo.lock b/Cargo.lock index 3ba431d62e..726929e9ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3583,6 +3583,7 @@ name = "fork_choice" version = "0.1.0" dependencies = [ "beacon_chain", + "bls", "ethereum_ssz", "ethereum_ssz_derive", "fixed_bytes", @@ -7023,7 +7024,9 @@ dependencies = [ "fixed_bytes", "safe_arith", "serde", + "smallvec", "superstruct", + "typenum", "types", "yaml_serde", ] diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 81735bdd9d..3c8ea30779 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1468,7 +1468,7 @@ impl BeaconChain { .proto_array() .heads_descended_from_finalization::(fork_choice.finalized_checkpoint()) .iter() - .map(|node| (node.root, node.slot)) + .map(|node| (node.root(), node.slot())) .collect() } @@ -2298,6 +2298,7 @@ impl BeaconChain { self.slot()?, verified.indexed_attestation().to_ref(), AttestationFromBlock::False, + &self.spec, ) .map_err(Into::into) } @@ -3951,7 +3952,7 @@ impl BeaconChain { let fork_choice_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_FORK_CHOICE); match fork_choice.get_head(current_slot, &self.spec) { // This block became the head, add it to the early attester cache. - Ok(new_head_root) if new_head_root == block_root => { + Ok((new_head_root, _)) if new_head_root == block_root => { if let Some(proto_block) = fork_choice.get_block(&block_root) { let new_head_is_optimistic = proto_block.execution_status.is_optimistic_or_invalid(); @@ -4756,6 +4757,7 @@ impl BeaconChain { }) } + // TODO(gloas): wrong for Gloas, needs an update pub fn overridden_forkchoice_update_params_or_failure_reason( &self, canonical_forkchoice_params: &ForkchoiceUpdateParameters, @@ -4790,7 +4792,7 @@ impl BeaconChain { // The slot of our potential re-org block is always 1 greater than the head block because we // only attempt single-slot re-orgs. - let head_slot = info.head_node.slot; + let head_slot = info.head_node.slot(); let re_org_block_slot = head_slot + 1; let fork_choice_slot = info.current_slot; @@ -4825,9 +4827,9 @@ impl BeaconChain { .fork_name_at_slot::(re_org_block_slot) .fulu_enabled() { - info.head_node.current_epoch_shuffling_id + info.head_node.current_epoch_shuffling_id() } else { - info.head_node.next_epoch_shuffling_id + info.head_node.next_epoch_shuffling_id() } .shuffling_decision_block; let proposer_index = self @@ -4853,13 +4855,15 @@ impl BeaconChain { return Err(Box::new(DoNotReOrg::NotProposing.into())); } - // 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. + // TODO(gloas): reorg weight logic needs updating for GLOAS. For now use + // total weight which is correct for pre-GLOAS and conservative for post-GLOAS. + let head_weight = info.head_node.weight(); + let parent_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) @@ -4867,7 +4871,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(), @@ -4876,7 +4880,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(), @@ -4894,9 +4898,16 @@ impl BeaconChain { return Err(Box::new(DoNotReOrg::HeadNotLate.into())); } - let parent_head_hash = info.parent_node.execution_status.block_hash(); + // TODO(gloas): V29 nodes don't carry execution_status, so this returns + // None for post-Gloas re-orgs. Need to source the EL block hash from + // the bid's block_hash instead. Re-org is disabled for Gloas for now. + let parent_head_hash = info + .parent_node + .execution_status() + .ok() + .and_then(|execution_status| execution_status.block_hash()); let forkchoice_update_params = ForkchoiceUpdateParameters { - head_root: info.parent_node.root, + head_root: info.parent_node.root(), head_hash: parent_head_hash, justified_hash: canonical_forkchoice_params.justified_hash, finalized_hash: canonical_forkchoice_params.finalized_hash, @@ -4904,7 +4915,7 @@ impl BeaconChain { debug!( canonical_head = ?head_block_root, - ?info.parent_node.root, + parent_root = ?info.parent_node.root(), slot = %fork_choice_slot, "Fork choice update overridden" ); diff --git a/beacon_node/beacon_chain/src/block_production/mod.rs b/beacon_node/beacon_chain/src/block_production/mod.rs index b33323f527..60cc3e919a 100644 --- a/beacon_node/beacon_chain/src/block_production/mod.rs +++ b/beacon_node/beacon_chain/src/block_production/mod.rs @@ -227,7 +227,7 @@ impl BeaconChain { }) .ok()?; drop(proposer_head_timer); - let re_org_parent_block = proposer_head.parent_node.root; + let re_org_parent_block = proposer_head.parent_node.root(); let (state_root, state) = self .store @@ -244,7 +244,7 @@ impl BeaconChain { info!( weak_head = ?canonical_head, parent = ?re_org_parent_block, - head_weight = proposer_head.head_node.weight, + head_weight = proposer_head.head_node.weight(), threshold_weight = proposer_head.re_org_head_weight_threshold, "Attempting re-org due to weak head" ); diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 916a207e62..ae9acdefd5 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1677,6 +1677,7 @@ impl ExecutionPendingBlock { current_slot, indexed_attestation, AttestationFromBlock::True, + &chain.spec, ) { Ok(()) => Ok(()), // Ignore invalid attestations whilst importing attestations from a block. The @@ -1685,6 +1686,31 @@ impl ExecutionPendingBlock { Err(e) => Err(BlockError::BeaconChainError(Box::new(e.into()))), }?; } + + // Register each payload attestation in the block with fork choice. + if let Ok(payload_attestations) = block.message().body().payload_attestations() { + for (i, payload_attestation) in payload_attestations.iter().enumerate() { + let indexed_payload_attestation = consensus_context + .get_indexed_payload_attestation(&state, payload_attestation, &chain.spec) + .map_err(|e| BlockError::PerBlockProcessingError(e.into_with_index(i)))?; + + let ptc = state + .get_ptc(indexed_payload_attestation.data.slot, &chain.spec) + .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + + // Ignore invalid payload attestations from blocks (same as + // regular attestations — the block may be old). + if let Err(e) = fork_choice.on_payload_attestation( + current_slot, + indexed_payload_attestation, + AttestationFromBlock::True, + &ptc.0, + ) && !matches!(e, ForkChoiceError::InvalidAttestation(_)) + { + return Err(BlockError::BeaconChainError(Box::new(e.into()))); + } + } + } drop(fork_choice); Ok(Self { @@ -1941,25 +1967,29 @@ fn load_parent>( // Post-Gloas we must also fetch a state with the correct payload status. If the current // block builds upon the payload of its parent block, then we know the parent block is FULL // and we need to load the full state. - let (payload_status, parent_state_root) = - if block.as_block().fork_name_unchecked().gloas_enabled() - && let Ok(parent_bid_block_hash) = parent_block.payload_bid_block_hash() - { - if block.as_block().is_parent_block_full(parent_bid_block_hash) { - // If the parent's execution payload envelope hasn't arrived yet, - // return an unknown parent error so the block gets sent to the - // reprocess queue. - let envelope = chain - .store - .get_payload_envelope(&root)? - .ok_or(BlockError::ParentEnvelopeUnknown { parent_root: root })?; - (StatePayloadStatus::Full, envelope.message.state_root) - } else { - (StatePayloadStatus::Pending, parent_block.state_root()) - } - } else { - (StatePayloadStatus::Pending, parent_block.state_root()) - }; + let (payload_status, parent_state_root) = if parent_block.slot() == chain.spec.genesis_slot + { + // Genesis state is always pending, there is no such thing as a "genesis envelope". + // See: https://github.com/ethereum/consensus-specs/issues/5043 + (StatePayloadStatus::Pending, parent_block.state_root()) + } else if !block.as_block().fork_name_unchecked().gloas_enabled() { + // All pre-Gloas parent states are pending. + (StatePayloadStatus::Pending, parent_block.state_root()) + } else if let Ok(parent_bid_block_hash) = parent_block.payload_bid_block_hash() + && block.as_block().is_parent_block_full(parent_bid_block_hash) + { + // Post-Gloas Full block case. + // TODO(gloas): loading the envelope here is not very efficient + let envelope = chain + .store + .get_payload_envelope(&root)? + .ok_or(BlockError::ParentEnvelopeUnknown { parent_root: root })?; + let state_root = envelope.message.state_root; + (StatePayloadStatus::Full, state_root) + } else { + // Post-Gloas empty block case (also covers the Gloas fork transition). + (StatePayloadStatus::Pending, parent_block.state_root()) + }; let (parent_state_root, state) = chain .store .get_advanced_hot_state(root, payload_status, block.slot(), parent_state_root)? diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 7eb92060a2..11b87351b1 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -45,7 +45,7 @@ use tree_hash::TreeHash; use types::data::CustodyIndex; use types::{ BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, ColumnIndex, DataColumnSidecarList, - Epoch, EthSpec, Hash256, SignedBeaconBlock, Slot, StatePayloadStatus, + Epoch, EthSpec, Hash256, SignedBeaconBlock, Slot, }; /// An empty struct used to "witness" all the `BeaconChainTypes` traits. It has no user-facing @@ -776,7 +776,7 @@ where slot_clock.now().ok_or("Unable to read slot")? }; - let initial_head_block_root = fork_choice + let (initial_head_block_root, head_payload_status) = fork_choice .get_head(current_slot, &self.spec) .map_err(|e| format!("Unable to get fork choice head: {:?}", e))?; @@ -786,13 +786,12 @@ where .map_err(|e| descriptive_db_error("head block", &e))? .ok_or("Head block not found in store")?; - // TODO(gloas): update head loading to load Full block once fork choice works - let payload_status = StatePayloadStatus::Pending; + let state_payload_status = head_payload_status.as_state_payload_status(); let (_head_state_root, head_state) = store .get_advanced_hot_state( head_block_root, - payload_status, + state_payload_status, current_slot, head_block.state_root(), ) @@ -923,7 +922,8 @@ where let genesis_validators_root = head_snapshot.beacon_state.genesis_validators_root(); let genesis_time = head_snapshot.beacon_state.genesis_time(); - let canonical_head = CanonicalHead::new(fork_choice, Arc::new(head_snapshot)); + let canonical_head = + CanonicalHead::new(fork_choice, Arc::new(head_snapshot), head_payload_status); let shuffling_cache_size = self.chain_config.shuffling_cache_size; let complete_blob_backfill = self.chain_config.complete_blob_backfill; diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 3a429bdb8a..bfd3d79512 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -108,6 +108,8 @@ pub struct CachedHead { /// This value may be distinct to the `self.snapshot.beacon_state.finalized_checkpoint`. /// This value should be used over the beacon state value in practically all circumstances. finalized_checkpoint: Checkpoint, + /// The payload status of the head block, as determined by fork choice. + head_payload_status: proto_array::PayloadStatus, /// The `execution_payload.block_hash` of the block at the head of the chain. Set to `None` /// before Bellatrix. head_hash: Option, @@ -232,6 +234,10 @@ impl CachedHead { finalized_hash: self.finalized_hash, } } + + pub fn head_payload_status(&self) -> proto_array::PayloadStatus { + self.head_payload_status + } } /// Represents the "canonical head" of the beacon chain. @@ -262,6 +268,7 @@ impl CanonicalHead { pub fn new( fork_choice: BeaconForkChoice, snapshot: Arc>, + head_payload_status: proto_array::PayloadStatus, ) -> Self { let fork_choice_view = fork_choice.cached_fork_choice_view(); let forkchoice_update_params = fork_choice.get_forkchoice_update_parameters(); @@ -269,6 +276,7 @@ impl CanonicalHead { snapshot, justified_checkpoint: fork_choice_view.justified_checkpoint, finalized_checkpoint: fork_choice_view.finalized_checkpoint, + head_payload_status, head_hash: forkchoice_update_params.head_hash, justified_hash: forkchoice_update_params.justified_hash, finalized_hash: forkchoice_update_params.finalized_hash, @@ -296,9 +304,11 @@ impl CanonicalHead { store: &BeaconStore, spec: &ChainSpec, ) -> Result<(), Error> { - let fork_choice = + let mut fork_choice = >::load_fork_choice(store.clone(), reset_payload_statuses, spec)? .ok_or(Error::MissingPersistedForkChoice)?; + let current_slot_for_head = fork_choice.fc_store().get_current_slot(); + let (_, head_payload_status) = fork_choice.get_head(current_slot_for_head, spec)?; let fork_choice_view = fork_choice.cached_fork_choice_view(); let beacon_block_root = fork_choice_view.head_block_root; let beacon_block = store @@ -329,6 +339,7 @@ impl CanonicalHead { snapshot: Arc::new(snapshot), justified_checkpoint: fork_choice_view.justified_checkpoint, finalized_checkpoint: fork_choice_view.finalized_checkpoint, + head_payload_status, head_hash: forkchoice_update_params.head_hash, justified_hash: forkchoice_update_params.justified_hash, finalized_hash: forkchoice_update_params.finalized_hash, @@ -609,11 +620,12 @@ impl BeaconChain { justified_checkpoint: old_cached_head.justified_checkpoint(), finalized_checkpoint: old_cached_head.finalized_checkpoint(), }; + let old_payload_status = old_cached_head.head_payload_status(); let mut fork_choice_write_lock = self.canonical_head.fork_choice_write_lock(); // Recompute the current head via the fork choice algorithm. - fork_choice_write_lock.get_head(current_slot, &self.spec)?; + let (_, new_payload_status) = fork_choice_write_lock.get_head(current_slot, &self.spec)?; // Downgrade the fork choice write-lock to a read lock, without allowing access to any // other writers. @@ -658,9 +670,8 @@ impl BeaconChain { }); } - // Exit early if the head or justified/finalized checkpoints have not changed, there's - // nothing to do. - if new_view == old_view { + // Exit early if the head, checkpoints, and payload status have not changed. + if new_view == old_view && new_payload_status == old_payload_status { debug!( head = ?new_view.head_block_root, "No change in canonical head" @@ -717,6 +728,7 @@ impl BeaconChain { snapshot: Arc::new(new_snapshot), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, + head_payload_status: new_payload_status, head_hash: new_forkchoice_update_parameters.head_hash, justified_hash: new_forkchoice_update_parameters.justified_hash, finalized_hash: new_forkchoice_update_parameters.finalized_hash, @@ -744,6 +756,7 @@ impl BeaconChain { snapshot: old_cached_head.snapshot.clone(), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, + head_payload_status: new_payload_status, head_hash: new_forkchoice_update_parameters.head_hash, justified_hash: new_forkchoice_update_parameters.justified_hash, finalized_hash: new_forkchoice_update_parameters.finalized_hash, 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/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs index 86f9293c8f..a538f35689 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/execution_pending_envelope.rs @@ -12,7 +12,7 @@ use crate::{ PayloadVerificationOutcome, block_verification::PayloadVerificationHandle, payload_envelope_verification::{ - EnvelopeError, EnvelopeImportData, MaybeAvailableEnvelope, + AvailableEnvelope, EnvelopeError, EnvelopeImportData, MaybeAvailableEnvelope, gossip_verified_envelope::GossipVerifiedEnvelope, load_snapshot_from_state_root, payload_notifier::PayloadNotifier, }, @@ -32,7 +32,6 @@ impl GossipVerifiedEnvelope { ) -> Result, EnvelopeError> { let signed_envelope = self.signed_envelope; let envelope = &signed_envelope.message; - let payload = &envelope.payload; // Define a future that will verify the execution payload with an execution engine. // @@ -91,10 +90,13 @@ impl GossipVerifiedEnvelope { )?; Ok(ExecutionPendingEnvelope { - signed_envelope: MaybeAvailableEnvelope::AvailabilityPending { - block_hash: payload.block_hash, - envelope: signed_envelope, - }, + signed_envelope: MaybeAvailableEnvelope::Available(AvailableEnvelope::new( + signed_envelope.block_hash(), + signed_envelope.clone(), + vec![], + None, + chain.spec.clone(), + )), import_data: EnvelopeImportData { block_root, post_state: Box::new(state), 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 2ee315e559..ed121ccb94 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -239,16 +239,14 @@ impl BeaconChain { // Note that a duplicate cache/payload status table should prevent this from happening // but it doesnt hurt to be defensive. - // TODO(gloas) when the code below is implemented we can delete this drop - drop(fork_choice_reader); - - // TODO(gloas) no fork choice logic yet // Take an exclusive write-lock on fork choice. It's very important to prevent deadlocks by // avoiding taking other locks whilst holding this lock. - // let fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); + let mut fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); - // TODO(gloas) Do we need this check? Do not import a block that doesn't descend from the finalized root. - // let signed_block = check_block_is_finalized_checkpoint_or_descendant(self, &fork_choice, signed_block)?; + // Update the node's payload_status from PENDING to FULL in fork choice. + fork_choice + .on_execution_payload(block_root) + .map_err(|e| EnvelopeError::InternalError(format!("{e:?}")))?; // TODO(gloas) emit SSE event if the payload became the new head payload @@ -302,10 +300,9 @@ impl BeaconChain { drop(db_span); - // TODO(gloas) drop fork choice lock // The fork choice write-lock is dropped *after* the on-disk database has been updated. // This prevents inconsistency between the two at the expense of concurrency. - // drop(fork_choice); + drop(fork_choice); // We're declaring the envelope "imported" at this point, since fork choice and the DB know // about it. 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..8ca6871dda 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/mod.rs @@ -59,6 +59,22 @@ pub struct AvailableEnvelope { } impl AvailableEnvelope { + pub fn new( + execution_block_hash: ExecutionBlockHash, + envelope: Arc>, + columns: DataColumnSidecarList, + columns_available_timestamp: Option, + spec: Arc, + ) -> Self { + Self { + execution_block_hash, + envelope, + columns, + columns_available_timestamp, + spec, + } + } + pub fn message(&self) -> &ExecutionPayloadEnvelope { &self.envelope.message } diff --git a/beacon_node/beacon_chain/src/persisted_fork_choice.rs b/beacon_node/beacon_chain/src/persisted_fork_choice.rs index 6229544e81..592ea9ecd7 100644 --- a/beacon_node/beacon_chain/src/persisted_fork_choice.rs +++ b/beacon_node/beacon_chain/src/persisted_fork_choice.rs @@ -6,15 +6,44 @@ use superstruct::superstruct; use types::Hash256; // If adding a new version you should update this type alias and fix the breakages. -pub type PersistedForkChoice = PersistedForkChoiceV28; +pub type PersistedForkChoice = PersistedForkChoiceV29; -#[superstruct(variants(V28), variant_attributes(derive(Encode, Decode)), no_enum)] +#[superstruct( + variants(V28, V29), + variant_attributes(derive(Encode, Decode)), + no_enum +)] pub struct PersistedForkChoice { - pub fork_choice: fork_choice::PersistedForkChoiceV28, + #[superstruct(only(V28))] + pub fork_choice_v28: fork_choice::PersistedForkChoiceV28, + #[superstruct(only(V29))] + pub fork_choice: fork_choice::PersistedForkChoiceV29, + #[superstruct(only(V28, V29))] pub fork_choice_store: PersistedForkChoiceStoreV28, } -impl PersistedForkChoiceV28 { +macro_rules! impl_store_item { + ($type:ty) => { + impl store::StoreItem for $type { + fn db_column() -> DBColumn { + DBColumn::ForkChoice + } + + fn as_store_bytes(&self) -> Vec { + self.as_ssz_bytes() + } + + fn from_store_bytes(bytes: &[u8]) -> std::result::Result { + Self::from_ssz_bytes(bytes).map_err(Into::into) + } + } + }; +} + +impl_store_item!(PersistedForkChoiceV28); +impl_store_item!(PersistedForkChoiceV29); + +impl PersistedForkChoiceV29 { pub fn from_bytes(bytes: &[u8], store_config: &StoreConfig) -> Result { let decompressed_bytes = store_config .decompress_bytes(bytes) @@ -45,3 +74,12 @@ impl PersistedForkChoiceV28 { )) } } + +impl From for PersistedForkChoiceV29 { + fn from(v28: PersistedForkChoiceV28) -> Self { + Self { + fork_choice: v28.fork_choice_v28.into(), + fork_choice_store: v28.fork_choice_store, + } + } +} diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index ed82143c38..fa2ab70d21 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -1,5 +1,8 @@ //! Utilities for managing database schema changes. +mod migration_schema_v29; + use crate::beacon_chain::BeaconChainTypes; +use migration_schema_v29::{downgrade_from_v29, upgrade_to_v29}; use std::sync::Arc; use store::Error as StoreError; use store::hot_cold_store::{HotColdDB, HotColdDBError}; @@ -10,13 +13,23 @@ use store::metadata::{CURRENT_SCHEMA_VERSION, SchemaVersion}; /// All migrations for schema versions up to and including v28 have been removed. Nodes on live /// networks are already running v28, so only the current version check remains. pub fn migrate_schema( - _db: Arc>, + db: Arc>, from: SchemaVersion, to: SchemaVersion, ) -> Result<(), StoreError> { match (from, to) { // Migrating from the current schema version to itself is always OK, a no-op. (_, _) if from == to && to == CURRENT_SCHEMA_VERSION => Ok(()), + // Upgrade from v28 to v29. + (SchemaVersion(28), SchemaVersion(29)) => { + upgrade_to_v29::(&db)?; + db.store_schema_version_atomically(to, vec![]) + } + // Downgrade from v29 to v28. + (SchemaVersion(29), SchemaVersion(28)) => { + downgrade_from_v29::(&db)?; + db.store_schema_version_atomically(to, vec![]) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, 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 new file mode 100644 index 0000000000..6c82e8a737 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs @@ -0,0 +1,115 @@ +use crate::beacon_chain::BeaconChainTypes; +use crate::persisted_fork_choice::{PersistedForkChoiceV28, PersistedForkChoiceV29}; +use ssz::Decode; +use store::hot_cold_store::HotColdDB; +use store::{DBColumn, Error as StoreError, KeyValueStore, KeyValueStoreOp}; +use types::{EthSpec, Hash256}; + +/// The key used to store the fork choice in the database. +const FORK_CHOICE_DB_KEY: Hash256 = Hash256::ZERO; + +/// Upgrade from schema v28 to v29. +/// +/// - Clears `best_child` and `best_descendant` on all nodes (replaced by +/// virtual tree walk). +/// - Fails if the persisted fork choice contains any V17 (pre-Gloas) proto +/// nodes at or after the Gloas fork slot. +pub fn upgrade_to_v29( + db: &HotColdDB, +) -> Result<(), StoreError> { + let gloas_fork_slot = db + .spec + .gloas_fork_epoch + .map(|epoch| epoch.start_slot(T::EthSpec::slots_per_epoch())); + + // Load the persisted fork choice (v28 format, uncompressed SSZ). + let Some(fc_bytes) = db + .hot_db + .get_bytes(DBColumn::ForkChoice, FORK_CHOICE_DB_KEY.as_slice())? + else { + return Ok(()); + }; + + let mut persisted_v28 = + PersistedForkChoiceV28::from_ssz_bytes(&fc_bytes).map_err(StoreError::SszDecodeError)?; + + // Check for V17 nodes at/after the Gloas fork slot. + if let Some(gloas_fork_slot) = gloas_fork_slot { + let bad_node = persisted_v28 + .fork_choice_v28 + .proto_array_v28 + .nodes + .iter() + .find(|node| node.slot >= gloas_fork_slot); + + if let Some(node) = bad_node { + return Err(StoreError::MigrationError(format!( + "cannot upgrade from v28 to v29: found V17 proto node at slot {} (root: {:?}) \ + which is at or after the Gloas fork slot {}. This node has synced a chain with \ + Gloas disabled and cannot be upgraded. Please resync from scratch.", + node.slot, node.root, gloas_fork_slot, + ))); + } + } + + // Clear best_child/best_descendant — replaced by the virtual tree walk. + for node in &mut persisted_v28.fork_choice_v28.proto_array_v28.nodes { + node.best_child = None; + node.best_descendant = None; + } + + // Convert to v29 and write back. + let persisted_v29 = PersistedForkChoiceV29::from(persisted_v28); + let fc_bytes = persisted_v29 + .as_bytes(db.get_config()) + .map_err(|e| StoreError::MigrationError(format!("failed to encode v29: {:?}", e)))?; + db.hot_db.do_atomically(vec![KeyValueStoreOp::PutKeyValue( + DBColumn::ForkChoice, + FORK_CHOICE_DB_KEY.as_slice().to_vec(), + fc_bytes, + )])?; + + Ok(()) +} + +/// Downgrade from schema v29 to v28 (no-op). +/// +/// Fails if the persisted fork choice contains any V29 proto nodes, as these contain +/// payload-specific fields that cannot be losslessly converted back to V17 format. +pub fn downgrade_from_v29( + db: &HotColdDB, +) -> Result<(), StoreError> { + // Load the persisted fork choice (v29 format, compressed). + let Some(fc_bytes) = db + .hot_db + .get_bytes(DBColumn::ForkChoice, FORK_CHOICE_DB_KEY.as_slice())? + else { + return Ok(()); + }; + + let persisted_v29 = + PersistedForkChoiceV29::from_bytes(&fc_bytes, db.get_config()).map_err(|e| { + StoreError::MigrationError(format!( + "cannot downgrade from v29 to v28: failed to decode fork choice: {:?}", + e + )) + })?; + + let has_v29_node = persisted_v29 + .fork_choice + .proto_array + .nodes + .iter() + .any(|node| matches!(node, proto_array::core::ProtoNode::V29(_))); + + if has_v29_node { + return Err(StoreError::MigrationError( + "cannot downgrade from v29 to v28: the persisted fork choice contains V29 proto \ + nodes which cannot be losslessly converted to V17 format. The Gloas-specific \ + payload data would be lost." + .to_string(), + )); + } + + Ok(()) +} diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 3ed8f59838..13672bbb63 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1350,7 +1350,7 @@ async fn recover_from_invalid_head_by_importing_blocks() { "the fork block should become the head" ); - let manual_get_head = rig + let (manual_get_head, _) = rig .harness .chain .canonical_head @@ -1428,7 +1428,7 @@ async fn weights_after_resetting_optimistic_status() { .fork_choice_read_lock() .proto_array() .iter_nodes(&head.head_block_root()) - .map(|node| (node.root, node.weight)) + .map(|node| (node.root(), node.weight())) .collect::>(); rig.invalidate_manually(roots[1]).await; @@ -1448,7 +1448,7 @@ async fn weights_after_resetting_optimistic_status() { .fork_choice_read_lock() .proto_array() .iter_nodes(&head.head_block_root()) - .map(|node| (node.root, node.weight)) + .map(|node| (node.root(), node.weight())) .collect::>(); assert_eq!(original_weights, new_weights); diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index fb5262b893..c6e13bd160 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3995,7 +3995,7 @@ async fn schema_downgrade_to_min_version(store_config: StoreConfig, archive: boo ) .await; - let min_version = CURRENT_SCHEMA_VERSION; + let min_version = SchemaVersion(28); // Save the slot clock so that the new harness doesn't revert in time. let slot_clock = harness.chain.slot_clock.clone(); @@ -5426,10 +5426,12 @@ fn assert_chains_pretty_much_the_same(a: &BeaconChain, b .fork_choice_write_lock() .get_head(slot, &spec) .unwrap() + .0 == b.canonical_head .fork_choice_write_lock() .get_head(slot, &spec) - .unwrap(), + .unwrap() + .0, "fork_choice heads should be equal" ); } diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index b052ba66f1..10c0b429a9 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -590,7 +590,10 @@ async fn unaggregated_attestations_added_to_fork_choice_some_none() { if slot <= num_blocks_produced && slot != 0 { assert_eq!( - latest_message.unwrap().1, + latest_message + .expect("latest message should be present") + .slot + .epoch(MinimalEthSpec::slots_per_epoch()), slot.epoch(MinimalEthSpec::slots_per_epoch()), "Latest message epoch for {} should be equal to epoch {}.", validator, @@ -700,10 +703,12 @@ async fn unaggregated_attestations_added_to_fork_choice_all_updated() { let validator_slots: Vec<(&usize, Slot)> = validators.iter().zip(slots).collect(); for (validator, slot) in validator_slots { - let latest_message = fork_choice.latest_message(*validator); + let latest_message = fork_choice + .latest_message(*validator) + .expect("latest message should be present"); assert_eq!( - latest_message.unwrap().1, + latest_message.slot.epoch(MinimalEthSpec::slots_per_epoch()), slot.epoch(MinimalEthSpec::slots_per_epoch()), "Latest message slot should be equal to attester duty." ); @@ -714,8 +719,7 @@ async fn unaggregated_attestations_added_to_fork_choice_all_updated() { .expect("Should get block root at slot"); assert_eq!( - latest_message.unwrap().0, - *block_root, + latest_message.root, *block_root, "Latest message block root should be equal to block at slot." ); } diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 17d41cfbcd..5df1078617 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -2098,52 +2098,62 @@ pub fn serve( .nodes .iter() .map(|node| { - let execution_status = if node.execution_status.is_execution_enabled() { - Some(node.execution_status.to_string()) + let execution_status = if node + .execution_status() + .is_ok_and(|status| status.is_execution_enabled()) + { + node.execution_status() + .ok() + .map(|status| status.to_string()) } else { None }; + let execution_status_string = node + .execution_status() + .map_or_else(|_| "irrelevant".to_string(), |s| s.to_string()); + ForkChoiceNode { - slot: node.slot, - block_root: node.root, + slot: node.slot(), + block_root: node.root(), parent_root: node - .parent + .parent() .and_then(|index| proto_array.nodes.get(index)) - .map(|parent| parent.root), - justified_epoch: node.justified_checkpoint.epoch, - finalized_epoch: node.finalized_checkpoint.epoch, - weight: node.weight, + .map(|parent| parent.root()), + justified_epoch: node.justified_checkpoint().epoch, + finalized_epoch: node.finalized_checkpoint().epoch, + weight: node.weight(), validity: execution_status, execution_block_hash: node - .execution_status - .block_hash() + .execution_status() + .ok() + .and_then(|status| status.block_hash()) .map(|block_hash| block_hash.into_root()), extra_data: ForkChoiceExtraData { - target_root: node.target_root, - justified_root: node.justified_checkpoint.root, - finalized_root: node.finalized_checkpoint.root, + target_root: node.target_root(), + justified_root: node.justified_checkpoint().root, + finalized_root: node.finalized_checkpoint().root, unrealized_justified_root: node - .unrealized_justified_checkpoint + .unrealized_justified_checkpoint() .map(|checkpoint| checkpoint.root), unrealized_finalized_root: node - .unrealized_finalized_checkpoint + .unrealized_finalized_checkpoint() .map(|checkpoint| checkpoint.root), unrealized_justified_epoch: node - .unrealized_justified_checkpoint + .unrealized_justified_checkpoint() .map(|checkpoint| checkpoint.epoch), unrealized_finalized_epoch: node - .unrealized_finalized_checkpoint + .unrealized_finalized_checkpoint() .map(|checkpoint| checkpoint.epoch), - execution_status: node.execution_status.to_string(), + execution_status: execution_status_string, best_child: node - .best_child + .best_child() .and_then(|index| proto_array.nodes.get(index)) - .map(|child| child.root), + .map(|child| child.root()), best_descendant: node - .best_descendant + .best_descendant() .and_then(|index| proto_array.nodes.get(index)) - .map(|descendant| descendant.root), + .map(|descendant| descendant.root()), }, } }) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index c9086dd876..14bfb5ce92 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -33,7 +33,7 @@ use lighthouse_network::{Enr, PeerId, types::SyncState}; use network::NetworkReceivers; use network_utils::enr_ext::EnrExt; use operation_pool::attestation_storage::CheckpointKey; -use proto_array::ExecutionStatus; +use proto_array::{ExecutionStatus, core::ProtoNode}; use reqwest::{RequestBuilder, Response, StatusCode}; use sensitive_url::SensitiveUrl; use slot_clock::SlotClock; @@ -3130,51 +3130,61 @@ impl ApiTester { .nodes .iter() .map(|node| { - let execution_status = if node.execution_status.is_execution_enabled() { - Some(node.execution_status.to_string()) + let execution_status = if node + .execution_status() + .is_ok_and(|status| status.is_execution_enabled()) + { + node.execution_status() + .ok() + .map(|status| status.to_string()) } else { None }; ForkChoiceNode { - slot: node.slot, - block_root: node.root, + slot: node.slot(), + block_root: node.root(), parent_root: node - .parent + .parent() .and_then(|index| expected_proto_array.nodes.get(index)) - .map(|parent| parent.root), - justified_epoch: node.justified_checkpoint.epoch, - finalized_epoch: node.finalized_checkpoint.epoch, - weight: node.weight, + .map(|parent| parent.root()), + justified_epoch: node.justified_checkpoint().epoch, + finalized_epoch: node.finalized_checkpoint().epoch, + weight: node.weight(), validity: execution_status, execution_block_hash: node - .execution_status - .block_hash() + .execution_status() + .ok() + .and_then(|status| status.block_hash()) .map(|block_hash| block_hash.into_root()), extra_data: ForkChoiceExtraData { - target_root: node.target_root, - justified_root: node.justified_checkpoint.root, - finalized_root: node.finalized_checkpoint.root, + target_root: node.target_root(), + justified_root: node.justified_checkpoint().root, + finalized_root: node.finalized_checkpoint().root, unrealized_justified_root: node - .unrealized_justified_checkpoint + .unrealized_justified_checkpoint() .map(|checkpoint| checkpoint.root), unrealized_finalized_root: node - .unrealized_finalized_checkpoint + .unrealized_finalized_checkpoint() .map(|checkpoint| checkpoint.root), unrealized_justified_epoch: node - .unrealized_justified_checkpoint + .unrealized_justified_checkpoint() .map(|checkpoint| checkpoint.epoch), unrealized_finalized_epoch: node - .unrealized_finalized_checkpoint + .unrealized_finalized_checkpoint() .map(|checkpoint| checkpoint.epoch), - execution_status: node.execution_status.to_string(), + execution_status: node + .execution_status() + .ok() + .map(|status| status.to_string()) + .unwrap_or_else(|| "irrelevant".to_string()), best_child: node - .best_child + .best_child() .and_then(|index| expected_proto_array.nodes.get(index)) - .map(|child| child.root), + .map(|child| child.root()), best_descendant: node - .best_descendant + .best_descendant() .and_then(|index| expected_proto_array.nodes.get(index)) - .map(|descendant| descendant.root), + .map(|descendant| descendant.root()), }, } }) @@ -7180,6 +7190,7 @@ impl ApiTester { .core_proto_array_mut() .nodes .last_mut() + && let ProtoNode::V17(head_node) = head_node { head_node.execution_status = ExecutionStatus::Optimistic(ExecutionBlockHash::zero()) } diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index cf49468451..215cdb2b64 100644 --- a/beacon_node/store/src/metadata.rs +++ b/beacon_node/store/src/metadata.rs @@ -4,7 +4,7 @@ use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use types::{Hash256, Slot}; -pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(28); +pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(29); // All the keys that get stored under the `BeaconMeta` column. // diff --git a/consensus/fork_choice/Cargo.toml b/consensus/fork_choice/Cargo.toml index a07aa38aa5..df47a5c9d1 100644 --- a/consensus/fork_choice/Cargo.toml +++ b/consensus/fork_choice/Cargo.toml @@ -19,5 +19,6 @@ types = { workspace = true } [dev-dependencies] beacon_chain = { workspace = true } +bls = { workspace = true } store = { workspace = true } tokio = { workspace = true } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 74b287975e..3b13cd4429 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -3,8 +3,8 @@ use crate::{ForkChoiceStore, InvalidationOperation}; use fixed_bytes::FixedBytesExtended; use logging::crit; use proto_array::{ - Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances, - ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, + Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances, LatestMessage, + PayloadStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; use ssz_derive::{Decode, Encode}; use state_processing::{ @@ -19,7 +19,8 @@ use tracing::{debug, instrument, warn}; use types::{ AbstractExecPayload, AttestationShufflingId, AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecPayload, ExecutionBlockHash, - Hash256, IndexedAttestationRef, RelativeEpoch, SignedBeaconBlock, Slot, + Hash256, IndexedAttestationRef, IndexedPayloadAttestation, RelativeEpoch, SignedBeaconBlock, + Slot, }; #[derive(Debug)] @@ -137,6 +138,10 @@ pub enum InvalidBlock { finalized_root: Hash256, block_ancestor: Option, }, + MissingExecutionPayloadBid { + block_slot: Slot, + block_root: Hash256, + }, } #[derive(Debug)] @@ -169,6 +174,21 @@ 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 }, + /// A gossip payload attestation must be for the current slot. + PayloadAttestationNotCurrentSlot { + attestation_slot: Slot, + current_slot: Slot, + }, } impl From for Error { @@ -240,6 +260,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 { @@ -249,10 +271,24 @@ 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, } } } +/// Used for queuing payload attestations (PTC votes) from the current slot. +/// Payload attestations have different dequeue timing than regular attestations: +/// gossiped payload attestations need an extra slot of delay (slot + 1 < current_slot). +#[derive(Clone, PartialEq, Encode, Decode)] +pub struct QueuedPayloadAttestation { + slot: Slot, + /// Resolved PTC committee positions (not validator indices). + ptc_indices: Vec, + block_root: Hash256, + payload_present: bool, + blob_data_available: bool, +} + /// Returns all values in `self.queued_attestations` that have a slot that is earlier than the /// current slot. Also removes those values from `self.queued_attestations`. fn dequeue_attestations( @@ -274,6 +310,22 @@ fn dequeue_attestations( std::mem::replace(queued_attestations, remaining) } +/// Returns all values in `queued` that have `slot + 1 < current_slot`. +/// Payload attestations need an extra slot of delay compared to regular attestations. +fn dequeue_payload_attestations( + current_slot: Slot, + queued: &mut Vec, +) -> Vec { + let remaining = queued.split_off( + queued + .iter() + .position(|a| a.slot.saturating_add(1_u64) >= current_slot) + .unwrap_or(queued.len()), + ); + + std::mem::replace(queued, remaining) +} + /// Denotes whether an attestation we are processing was received from a block or from gossip. /// Equivalent to the `is_from_block` `bool` in: /// @@ -318,6 +370,9 @@ pub struct ForkChoice { proto_array: ProtoArrayForkChoice, /// Attestations that arrived at the current slot and must be queued for later processing. queued_attestations: Vec, + /// Payload attestations (PTC votes) that must be queued for later processing. + /// These have different dequeue timing than regular attestations. + queued_payload_attestations: Vec, /// Stores a cache of the values required to be sent to the execution layer. forkchoice_update_parameters: ForkchoiceUpdateParameters, _phantom: PhantomData, @@ -332,6 +387,7 @@ where self.fc_store == other.fc_store && self.proto_array == other.proto_array && self.queued_attestations == other.queued_attestations + && self.queued_payload_attestations == other.queued_payload_attestations } } @@ -366,21 +422,34 @@ where AttestationShufflingId::new(anchor_block_root, anchor_state, RelativeEpoch::Next) .map_err(Error::BeaconStateError)?; - let execution_status = anchor_block.message().execution_payload().map_or_else( - // If the block doesn't have an execution payload then it can't have - // execution enabled. - |_| ExecutionStatus::irrelevant(), - |execution_payload| { + let (execution_status, execution_payload_parent_hash, execution_payload_block_hash) = + if let Ok(execution_payload) = anchor_block.message().execution_payload() { + // Pre-Gloas forks: hashes come from the execution payload. if execution_payload.is_default_with_empty_roots() { - // A default payload does not have execution enabled. - ExecutionStatus::irrelevant() + (ExecutionStatus::irrelevant(), None, None) } else { - // Assume that this payload is valid, since the anchor should be a trusted block and - // state. - ExecutionStatus::Valid(execution_payload.block_hash()) + // Assume that this payload is valid, since the anchor should be a + // trusted block and state. + ( + ExecutionStatus::Valid(execution_payload.block_hash()), + Some(execution_payload.parent_hash()), + Some(execution_payload.block_hash()), + ) } - }, - ); + } else if let Ok(signed_bid) = + anchor_block.message().body().signed_execution_payload_bid() + { + // Gloas: execution status is irrelevant post-Gloas; payload validation + // is decoupled from beacon blocks. + ( + ExecutionStatus::irrelevant(), + Some(signed_bid.message.parent_block_hash), + Some(signed_bid.message.block_hash), + ) + } else { + // Pre-merge: no execution payload at all. + (ExecutionStatus::irrelevant(), None, None) + }; // If the current slot is not provided, use the value that was last provided to the store. let current_slot = current_slot.unwrap_or_else(|| fc_store.get_current_slot()); @@ -394,25 +463,29 @@ where current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, + execution_payload_parent_hash, + execution_payload_block_hash, + spec, )?; let mut fork_choice = Self { fc_store, proto_array, queued_attestations: vec![], + queued_payload_attestations: vec![], // This will be updated during the next call to `Self::get_head`. forkchoice_update_parameters: ForkchoiceUpdateParameters { head_hash: None, justified_hash: None, finalized_hash: None, - // This will be updated during the next call to `Self::get_head`. + // These will be updated during the next call to `Self::get_head`. head_root: Hash256::zero(), }, _phantom: PhantomData, }; // Ensure that `fork_choice.forkchoice_update_parameters.head_root` is updated. - fork_choice.get_head(current_slot, spec)?; + let _ = fork_choice.get_head(current_slot, spec)?; Ok(fork_choice) } @@ -479,7 +552,7 @@ where &mut self, system_time_current_slot: Slot, spec: &ChainSpec, - ) -> Result> { + ) -> Result<(Hash256, PayloadStatus), Error> { // Provide the slot (as per the system clock) to the `fc_store` and then return its view of // the current slot. The `fc_store` will ensure that the `current_slot` is never // decreasing, a property which we must maintain. @@ -487,7 +560,7 @@ where let store = &mut self.fc_store; - let head_root = self.proto_array.find_head::( + let (head_root, head_payload_status) = self.proto_array.find_head::( *store.justified_checkpoint(), *store.finalized_checkpoint(), store.justified_balances(), @@ -516,7 +589,7 @@ where finalized_hash, }; - Ok(head_root) + Ok((head_root, head_payload_status)) } /// Get the block to build on as proposer, taking into account proposer re-orgs. @@ -729,6 +802,11 @@ where let attestation_threshold = spec.get_unaggregated_attestation_due(); // Add proposer score boost if the block is timely. + // TODO(gloas): the spec's `update_proposer_boost_root` additionally checks that + // `block.proposer_index == get_beacon_proposer_index(head_state)` — i.e. that + // the block's proposer matches the expected proposer on the canonical chain. + // This requires calling `get_head` and advancing the head state to the current + // slot, which is expensive. Implement once we have a cached proposer index. let is_before_attesting_interval = block_delay < attestation_threshold; let is_first_block = self.fc_store.proposer_boost_root().is_zero(); @@ -881,6 +959,24 @@ where ExecutionStatus::irrelevant() }; + let (execution_payload_parent_hash, execution_payload_block_hash) = + if let Ok(signed_bid) = block.body().signed_execution_payload_bid() { + ( + Some(signed_bid.message.parent_block_hash), + Some(signed_bid.message.block_hash), + ) + } else { + if spec.fork_name_at_slot::(block.slot()).gloas_enabled() { + return Err(Error::InvalidBlock( + InvalidBlock::MissingExecutionPayloadBid { + block_slot: block.slot(), + block_root, + }, + )); + } + (None, None) + }; + // This does not apply a vote to the block, it just makes fork choice aware of the block so // it can still be identified as the head even if it doesn't have any votes. self.proto_array.process_block::( @@ -907,15 +1003,24 @@ where execution_status, unrealized_justified_checkpoint: Some(unrealized_justified_checkpoint), unrealized_finalized_checkpoint: Some(unrealized_finalized_checkpoint), + execution_payload_parent_hash, + execution_payload_block_hash, + proposer_index: Some(block.proposer_index()), }, current_slot, - self.justified_checkpoint(), - self.finalized_checkpoint(), + spec, + block_delay, )?; 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, @@ -979,6 +1084,7 @@ where &self, indexed_attestation: IndexedAttestationRef, is_from_block: AttestationFromBlock, + spec: &ChainSpec, ) -> Result<(), InvalidAttestation> { // There is no point in processing an attestation with an empty bitfield. Reject // it immediately. @@ -1051,6 +1157,90 @@ where }); } + if spec + .fork_name_at_slot::(indexed_attestation.data().slot) + .gloas_enabled() + { + 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(()) + } + + /// Validates a payload attestation for application to fork choice. + fn validate_on_payload_attestation( + &self, + indexed_payload_attestation: &IndexedPayloadAttestation, + is_from_block: AttestationFromBlock, + ) -> Result<(), InvalidAttestation> { + if indexed_payload_attestation.attesting_indices.is_empty() { + return Err(InvalidAttestation::EmptyAggregationBitfield); + } + + let block = self + .proto_array + .get_block(&indexed_payload_attestation.data.beacon_block_root) + .ok_or(InvalidAttestation::UnknownHeadBlock { + beacon_block_root: indexed_payload_attestation.data.beacon_block_root, + })?; + + if block.slot > indexed_payload_attestation.data.slot { + return Err(InvalidAttestation::AttestsToFutureBlock { + block: block.slot, + attestation: indexed_payload_attestation.data.slot, + }); + } + + // Spec: `if data.slot != state.slot: return` — PTC votes can only + // change the vote for their assigned beacon block. + if block.slot != indexed_payload_attestation.data.slot { + return Ok(()); + } + + // Gossip payload attestations must be for the current slot. + // 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() + { + return Err(InvalidAttestation::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 + // 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(InvalidAttestation::PayloadPresentDuringSameSlot { slot: block.slot }); + } + Ok(()) } @@ -1076,6 +1266,7 @@ where system_time_current_slot: Slot, attestation: IndexedAttestationRef, is_from_block: AttestationFromBlock, + spec: &ChainSpec, ) -> Result<(), Error> { let _timer = metrics::start_timer(&metrics::FORK_CHOICE_ON_ATTESTATION_TIMES); @@ -1098,14 +1289,21 @@ where return Ok(()); } - self.validate_on_attestation(attestation, is_from_block)?; + self.validate_on_attestation(attestation, is_from_block, spec)?; + + // Per Gloas spec: `payload_present = attestation.data.index == 1`. + let payload_present = spec + .fork_name_at_slot::(attestation.data().slot) + .gloas_enabled() + && 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().target.epoch, + attestation.data().slot, + payload_present, )?; } } else { @@ -1122,6 +1320,65 @@ where Ok(()) } + /// Register a payload attestation with the fork choice DAG. + /// + /// `ptc` is the PTC committee for the attestation's slot: a list of validator indices + /// ordered by committee position. Each attesting validator index is resolved to its + /// position within `ptc` (its `ptc_index`) before being applied to the proto-array. + pub fn on_payload_attestation( + &mut self, + system_time_current_slot: Slot, + attestation: &IndexedPayloadAttestation, + is_from_block: AttestationFromBlock, + ptc: &[usize], + ) -> Result<(), Error> { + self.update_time(system_time_current_slot)?; + + if attestation.data.beacon_block_root == Hash256::zero() { + return Ok(()); + } + + self.validate_on_payload_attestation(attestation, is_from_block)?; + + // Resolve validator indices to PTC committee positions. + let ptc_indices: Vec = attestation + .attesting_indices_iter() + .filter_map(|vi| ptc.iter().position(|&p| p == *vi as usize)) + .collect(); + + let processing_slot = self.fc_store.get_current_slot(); + // Payload attestations from blocks can be applied in the next slot (S+1 for data.slot=S), + // while gossiped payload attestations are delayed one extra slot. + let should_process_now = match is_from_block { + AttestationFromBlock::True => attestation.data.slot < processing_slot, + AttestationFromBlock::False => { + attestation.data.slot.saturating_add(1_u64) < processing_slot + } + }; + + if should_process_now { + for &ptc_index in &ptc_indices { + self.proto_array.process_payload_attestation( + attestation.data.beacon_block_root, + ptc_index, + attestation.data.payload_present, + attestation.data.blob_data_available, + )?; + } + } else { + self.queued_payload_attestations + .push(QueuedPayloadAttestation { + slot: attestation.data.slot, + ptc_indices, + block_root: attestation.data.beacon_block_root, + payload_present: attestation.data.payload_present, + blob_data_available: attestation.data.blob_data_available, + }); + } + + Ok(()) + } + /// Apply an attester slashing to fork choice. /// /// We assume that the attester slashing provided to this function has already been verified. @@ -1151,6 +1408,7 @@ where // Process any attestations that might now be eligible. self.process_attestation_queue()?; + self.process_payload_attestation_queue()?; Ok(self.fc_store.get_current_slot()) } @@ -1228,7 +1486,28 @@ where self.proto_array.process_attestation( *validator_index as usize, attestation.block_root, - attestation.target_epoch, + attestation.slot, + attestation.payload_present, + )?; + } + } + + Ok(()) + } + + /// Processes and removes from the queue any queued payload attestations which may now be + /// eligible for processing. Payload attestations use `slot + 1 < current_slot` timing. + fn process_payload_attestation_queue(&mut self) -> Result<(), Error> { + let current_slot = self.fc_store.get_current_slot(); + for attestation in + dequeue_payload_attestations(current_slot, &mut self.queued_payload_attestations) + { + for &ptc_index in &attestation.ptc_indices { + self.proto_array.process_payload_attestation( + attestation.block_root, + ptc_index, + attestation.payload_present, + attestation.blob_data_available, )?; } } @@ -1358,13 +1637,15 @@ where /// Returns the latest message for a given validator, if any. /// - /// Returns `(block_root, block_slot)`. + /// Returns `block_root, block_slot, payload_present`. /// /// ## Notes /// /// It may be prudent to call `Self::update_time` before calling this function, /// since some attestations might be queued and awaiting processing. - pub fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Epoch)> { + /// + /// This function is only used in tests. + pub fn latest_message(&self, validator_index: usize) -> Option { self.proto_array.latest_message(validator_index) } @@ -1389,6 +1670,11 @@ where &self.queued_attestations } + /// Returns a reference to the currently queued payload attestations. + pub fn queued_payload_attestations(&self) -> &[QueuedPayloadAttestation] { + &self.queued_payload_attestations + } + /// Returns the store's `proposer_boost_root`. pub fn proposer_boost_root(&self) -> Hash256 { self.fc_store.proposer_boost_root() @@ -1473,6 +1759,7 @@ where fc_store, proto_array, queued_attestations: persisted.queued_attestations, + queued_payload_attestations: persisted.queued_payload_attestations, // Will be updated in the following call to `Self::get_head`. forkchoice_update_parameters: ForkchoiceUpdateParameters { head_hash: None, @@ -1501,7 +1788,7 @@ where .set_all_blocks_to_optimistic::(spec)?; // If the second attempt at finding a head fails, return an error since we do not // expect this scenario. - fork_choice.get_head(current_slot, spec)?; + let _ = fork_choice.get_head(current_slot, spec)?; } Ok(fork_choice) @@ -1511,10 +1798,9 @@ where /// be instantiated again later. pub fn to_persisted(&self) -> PersistedForkChoice { PersistedForkChoice { - proto_array: self - .proto_array() - .as_ssz_container(self.justified_checkpoint(), self.finalized_checkpoint()), + proto_array: self.proto_array().as_ssz_container(), queued_attestations: self.queued_attestations().to_vec(), + queued_payload_attestations: self.queued_payload_attestations.clone(), } } @@ -1528,16 +1814,31 @@ where /// /// This is used when persisting the state of the fork choice to disk. #[superstruct( - variants(V28), + variants(V28, V29), variant_attributes(derive(Encode, Decode, Clone)), no_enum )] pub struct PersistedForkChoice { - pub proto_array: proto_array::core::SszContainerV28, + #[superstruct(only(V28))] + pub proto_array_v28: proto_array::core::SszContainerV28, + #[superstruct(only(V29))] + pub proto_array: proto_array::core::SszContainerV29, pub queued_attestations: Vec, + #[superstruct(only(V29))] + pub queued_payload_attestations: Vec, } -pub type PersistedForkChoice = PersistedForkChoiceV28; +pub type PersistedForkChoice = PersistedForkChoiceV29; + +impl From for PersistedForkChoiceV29 { + fn from(v28: PersistedForkChoiceV28) -> Self { + Self { + proto_array: v28.proto_array_v28.into(), + queued_attestations: v28.queued_attestations, + queued_payload_attestations: vec![], + } + } +} #[cfg(test)] mod tests { @@ -1574,6 +1875,7 @@ mod tests { attesting_indices: vec![], block_root: Hash256::zero(), target_epoch: Epoch::new(0), + payload_present: false, }) .collect() } diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index 8cf2936db4..70f1dbc215 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -5,9 +5,10 @@ mod metrics; pub use crate::fork_choice::{ AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice, - PersistedForkChoiceV28, QueuedAttestation, ResetPayloadStatuses, + PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation, QueuedPayloadAttestation, + ResetPayloadStatuses, }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::{ - Block as ProtoBlock, ExecutionStatus, InvalidationOperation, ProposerHeadError, + Block as ProtoBlock, ExecutionStatus, InvalidationOperation, PayloadStatus, ProposerHeadError, }; diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index d3a84ee85b..839d0f4c5c 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -7,9 +7,11 @@ use beacon_chain::{ BeaconChain, BeaconChainError, BeaconForkChoiceStore, ChainConfig, ForkChoiceError, StateSkipConfig, WhenSlotSkipped, }; +use bls::AggregateSignature; use fixed_bytes::FixedBytesExtended; use fork_choice::{ - ForkChoiceStore, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, QueuedAttestation, + AttestationFromBlock, ForkChoiceStore, InvalidAttestation, InvalidBlock, + PayloadVerificationStatus, QueuedAttestation, }; use state_processing::state_advance::complete_state_advance; use std::fmt; @@ -19,8 +21,8 @@ use store::MemoryStore; use types::SingleAttestation; use types::{ BeaconBlockRef, BeaconState, ChainSpec, Checkpoint, Epoch, EthSpec, ForkName, Hash256, - IndexedAttestation, MainnetEthSpec, RelativeEpoch, SignedBeaconBlock, Slot, SubnetId, - test_utils::generate_deterministic_keypair, + IndexedAttestation, IndexedPayloadAttestation, MainnetEthSpec, PayloadAttestationData, + RelativeEpoch, SignedBeaconBlock, Slot, SubnetId, test_utils::generate_deterministic_keypair, }; pub type E = MainnetEthSpec; @@ -71,6 +73,9 @@ impl ForkChoiceTest { Self { harness } } + /// Creates a new tester with the GLOAS fork active at epoch 1. + /// Genesis is a standard Fulu block (epoch 0), so block production works normally. + /// Tests that need GLOAS semantics should advance the chain into epoch 1 first. /// Get a value from the `ForkChoice` instantiation. fn get(&self, func: T) -> U where @@ -923,6 +928,56 @@ async fn invalid_attestation_future_block() { .await; } +/// Gossip payload attestations must be for the current slot. A payload attestation for slot S +/// received at slot S+1 should be rejected per the spec. +#[tokio::test] +async fn non_block_payload_attestation_for_previous_slot_is_rejected() { + let test = ForkChoiceTest::new() + .apply_blocks_without_new_attestations(1) + .await; + + let chain = &test.harness.chain; + let block_a = chain + .block_at_slot(Slot::new(1), WhenSlotSkipped::Prev) + .expect("lookup should succeed") + .expect("block A should exist"); + let block_a_root = block_a.canonical_root(); + let s_plus_1 = block_a.slot().saturating_add(1_u64); + + let payload_attestation = IndexedPayloadAttestation:: { + attesting_indices: vec![0_u64].try_into().expect("valid attesting indices"), + data: PayloadAttestationData { + beacon_block_root: block_a_root, + slot: Slot::new(1), + payload_present: true, + blob_data_available: true, + }, + signature: AggregateSignature::empty(), + }; + + let ptc = &[0_usize]; + + let result = chain + .canonical_head + .fork_choice_write_lock() + .on_payload_attestation( + s_plus_1, + &payload_attestation, + AttestationFromBlock::False, + ptc, + ); + assert!( + matches!( + result, + Err(ForkChoiceError::InvalidAttestation( + InvalidAttestation::PayloadAttestationNotCurrentSlot { .. } + )) + ), + "gossip payload attestation for previous slot should be rejected, got: {:?}", + result + ); +} + /// Specification v0.12.1: /// /// assert target.root == get_ancestor(store, attestation.data.beacon_block_root, target_slot) diff --git a/consensus/proto_array/Cargo.toml b/consensus/proto_array/Cargo.toml index 7419ad813b..ee86277f9c 100644 --- a/consensus/proto_array/Cargo.toml +++ b/consensus/proto_array/Cargo.toml @@ -14,6 +14,8 @@ ethereum_ssz_derive = { workspace = true } fixed_bytes = { workspace = true } safe_arith = { workspace = true } serde = { workspace = true } +smallvec = { workspace = true } superstruct = { workspace = true } +typenum = { workspace = true } types = { workspace = true } yaml_serde = { workspace = true } diff --git a/consensus/proto_array/src/error.rs b/consensus/proto_array/src/error.rs index 35cb4007b7..bb47af97d9 100644 --- a/consensus/proto_array/src/error.rs +++ b/consensus/proto_array/src/error.rs @@ -54,6 +54,14 @@ pub enum Error { }, InvalidEpochOffset(u64), Arith(ArithError), + InvalidNodeVariant { + block_root: Hash256, + }, + BrokenBlock { + block_root: Hash256, + }, + NoViableChildren, + OnBlockRequiresProposerIndex, } impl From for Error { diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index e9deb6759f..34d7f2e48e 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -1,20 +1,24 @@ mod execution_status; mod ffg_updates; +mod gloas_payload; mod no_votes; mod votes; -use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; +use crate::proto_array_fork_choice::{Block, ExecutionStatus, PayloadStatus, ProtoArrayForkChoice}; use crate::{InvalidationOperation, JustifiedBalances}; use fixed_bytes::FixedBytesExtended; use serde::{Deserialize, Serialize}; +use ssz::BitVector; use std::collections::BTreeSet; +use std::time::Duration; use types::{ - AttestationShufflingId, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, + AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, MainnetEthSpec, Slot, }; pub use execution_status::*; pub use ffg_updates::*; +pub use gloas_payload::*; pub use no_votes::*; pub use votes::*; @@ -25,6 +29,9 @@ pub enum Operation { finalized_checkpoint: Checkpoint, justified_state_balances: Vec, expected_head: Hash256, + current_slot: Slot, + #[serde(default)] + expected_payload_status: Option, }, ProposerBoostFindHead { justified_checkpoint: Checkpoint, @@ -44,11 +51,23 @@ pub enum Operation { parent_root: Hash256, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, + #[serde(default)] + execution_payload_parent_hash: Option, + #[serde(default)] + execution_payload_block_hash: Option, }, ProcessAttestation { validator_index: usize, block_root: Hash256, - target_epoch: Epoch, + attestation_slot: Slot, + }, + ProcessPayloadAttestation { + validator_index: usize, + block_root: Hash256, + attestation_slot: Slot, + payload_present: bool, + #[serde(default)] + blob_data_available: bool, }, Prune { finalized_root: Hash256, @@ -63,6 +82,29 @@ pub enum Operation { block_root: Hash256, weight: u64, }, + AssertPayloadWeights { + block_root: Hash256, + expected_full_weight: u64, + expected_empty_weight: u64, + }, + AssertParentPayloadStatus { + block_root: Hash256, + expected_status: PayloadStatus, + }, + SetPayloadTiebreak { + block_root: Hash256, + 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)] @@ -71,12 +113,23 @@ pub struct ForkChoiceTestDefinition { pub justified_checkpoint: Checkpoint, pub finalized_checkpoint: Checkpoint, pub operations: Vec, + #[serde(default)] + pub execution_payload_parent_hash: Option, + #[serde(default)] + pub execution_payload_block_hash: Option, + #[serde(skip)] + pub spec: Option, } impl ForkChoiceTestDefinition { pub fn run(self) { - let mut spec = MainnetEthSpec::default_spec(); - spec.proposer_score_boost = Some(50); + let spec = self.spec.unwrap_or_else(|| { + let mut spec = MainnetEthSpec::default_spec(); + spec.proposer_score_boost = Some(50); + // Legacy test definitions target pre-Gloas behaviour unless explicitly overridden. + spec.gloas_fork_epoch = None; + spec + }); let junk_shuffling_id = AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero()); @@ -89,6 +142,9 @@ impl ForkChoiceTestDefinition { junk_shuffling_id.clone(), junk_shuffling_id, ExecutionStatus::Optimistic(ExecutionBlockHash::zero()), + self.execution_payload_parent_hash, + self.execution_payload_block_hash, + &spec, ) .expect("should create fork choice struct"); let equivocating_indices = BTreeSet::new(); @@ -100,18 +156,20 @@ impl ForkChoiceTestDefinition { finalized_checkpoint, justified_state_balances, expected_head, + current_slot, + expected_payload_status, } => { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) .unwrap(); - let head = fork_choice + let (head, payload_status) = fork_choice .find_head::( justified_checkpoint, finalized_checkpoint, &justified_balances, Hash256::zero(), &equivocating_indices, - Slot::new(0), + current_slot, &spec, ) .unwrap_or_else(|e| { @@ -123,6 +181,13 @@ impl ForkChoiceTestDefinition { "Operation at index {} failed head check. Operation: {:?}", op_index, op ); + if let Some(expected_status) = expected_payload_status { + assert_eq!( + payload_status, expected_status, + "Operation at index {} failed payload status check. Operation: {:?}", + op_index, op + ); + } check_bytes_round_trip(&fork_choice); } Operation::ProposerBoostFindHead { @@ -135,7 +200,7 @@ impl ForkChoiceTestDefinition { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) .unwrap(); - let head = fork_choice + let (head, _payload_status) = fork_choice .find_head::( justified_checkpoint, finalized_checkpoint, @@ -188,6 +253,8 @@ impl ForkChoiceTestDefinition { parent_root, justified_checkpoint, finalized_checkpoint, + execution_payload_parent_hash, + execution_payload_block_hash, } => { let block = Block { slot, @@ -211,14 +278,12 @@ impl ForkChoiceTestDefinition { ), unrealized_justified_checkpoint: None, unrealized_finalized_checkpoint: None, + execution_payload_parent_hash, + execution_payload_block_hash, + proposer_index: Some(0), }; fork_choice - .process_block::( - block, - slot, - self.justified_checkpoint, - self.finalized_checkpoint, - ) + .process_block::(block, slot, &spec, Duration::ZERO) .unwrap_or_else(|e| { panic!( "process_block op at index {} returned error: {:?}", @@ -230,10 +295,10 @@ impl ForkChoiceTestDefinition { Operation::ProcessAttestation { validator_index, block_root, - target_epoch, + attestation_slot, } => { fork_choice - .process_attestation(validator_index, block_root, target_epoch) + .process_attestation(validator_index, block_root, attestation_slot, false) .unwrap_or_else(|_| { panic!( "process_attestation op at index {} returned error", @@ -242,6 +307,28 @@ impl ForkChoiceTestDefinition { }); check_bytes_round_trip(&fork_choice); } + Operation::ProcessPayloadAttestation { + validator_index, + block_root, + attestation_slot: _, + payload_present, + blob_data_available, + } => { + fork_choice + .process_payload_attestation( + block_root, + validator_index, + payload_present, + blob_data_available, + ) + .unwrap_or_else(|_| { + panic!( + "process_payload_attestation op at index {} returned error", + op_index + ) + }); + check_bytes_round_trip(&fork_choice); + } Operation::Prune { finalized_root, prune_threshold, @@ -287,8 +374,153 @@ impl ForkChoiceTestDefinition { Operation::AssertWeight { block_root, weight } => assert_eq!( fork_choice.get_weight(&block_root).unwrap(), weight, - "block weight" + "block weight at op index {}", + op_index ), + Operation::AssertPayloadWeights { + block_root, + expected_full_weight, + expected_empty_weight, + } => { + let block_index = fork_choice + .proto_array + .indices + .get(&block_root) + .unwrap_or_else(|| { + panic!( + "AssertPayloadWeights: block root not found at op index {}", + op_index + ) + }); + let node = fork_choice + .proto_array + .nodes + .get(*block_index) + .unwrap_or_else(|| { + panic!( + "AssertPayloadWeights: node not found at op index {}", + op_index + ) + }); + let v29 = node.as_v29().unwrap_or_else(|_| { + panic!( + "AssertPayloadWeights: node is not V29 at op index {}", + op_index + ) + }); + assert_eq!( + v29.full_payload_weight, expected_full_weight, + "full_payload_weight mismatch at op index {}", + op_index + ); + assert_eq!( + v29.empty_payload_weight, expected_empty_weight, + "empty_payload_weight mismatch at op index {}", + op_index + ); + } + Operation::AssertParentPayloadStatus { + block_root, + expected_status, + } => { + let block_index = fork_choice + .proto_array + .indices + .get(&block_root) + .unwrap_or_else(|| { + panic!( + "AssertParentPayloadStatus: block root not found at op index {}", + op_index + ) + }); + let node = fork_choice + .proto_array + .nodes + .get(*block_index) + .unwrap_or_else(|| { + panic!( + "AssertParentPayloadStatus: node not found at op index {}", + op_index + ) + }); + let v29 = node.as_v29().unwrap_or_else(|_| { + panic!( + "AssertParentPayloadStatus: node is not V29 at op index {}", + op_index + ) + }); + assert_eq!( + v29.parent_payload_status, expected_status, + "parent_payload_status mismatch at op index {}", + op_index + ); + } + Operation::SetPayloadTiebreak { + block_root, + is_timely, + is_data_available, + } => { + let block_index = fork_choice + .proto_array + .indices + .get(&block_root) + .unwrap_or_else(|| { + panic!( + "SetPayloadTiebreak: block root not found at op index {}", + op_index + ) + }); + let node = fork_choice + .proto_array + .nodes + .get_mut(*block_index) + .unwrap_or_else(|| { + panic!( + "SetPayloadTiebreak: node not found at op index {}", + op_index + ) + }); + let node_v29 = node.as_v29_mut().unwrap_or_else(|_| { + panic!( + "SetPayloadTiebreak: node is not V29 at op index {}", + op_index + ) + }); + // Set all bits (exceeds any threshold) or clear all bits. + let fill = if is_timely { 0xFF } else { 0x00 }; + 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 = + 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 + ); + } } } } @@ -314,8 +546,7 @@ fn get_checkpoint(i: u64) -> Checkpoint { } fn check_bytes_round_trip(original: &ProtoArrayForkChoice) { - // The checkpoint are ignored `ProtoArrayForkChoice::from_bytes` so any value is ok - let bytes = original.as_bytes(Checkpoint::default(), Checkpoint::default()); + let bytes = original.as_bytes(); let decoded = ProtoArrayForkChoice::from_bytes(&bytes, original.balances.clone()) .expect("fork choice should decode from bytes"); assert!( diff --git a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs index aa26a84306..794310ef89 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs @@ -16,6 +16,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -35,6 +37,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is 2 @@ -53,6 +57,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -73,6 +79,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -91,6 +99,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -101,7 +111,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(1), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is now 1, because 1 has a vote. @@ -120,6 +130,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -143,7 +155,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(2), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is 2 since 1 and 2 both have a vote @@ -162,6 +174,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -196,6 +210,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -216,6 +232,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -245,7 +263,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(3), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Ensure that the head is still 2 @@ -266,6 +284,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -315,6 +335,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Invalidation of 3 should have removed upstream weight. @@ -347,7 +369,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(1), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Ensure that the head has switched back to 1 @@ -368,6 +390,8 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -399,6 +423,9 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { root: get_root(0), }, operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } @@ -418,6 +445,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -437,6 +466,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is 2 @@ -455,6 +486,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -475,6 +508,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -493,6 +528,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -503,7 +540,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(1), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is now 1, because 1 has a vote. @@ -522,6 +559,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -545,7 +584,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(2), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is 2 since 1 and 2 both have a vote @@ -564,6 +603,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -598,6 +639,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -618,6 +661,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -647,7 +692,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(3), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Move validator #1 vote from 2 to 3 @@ -660,7 +705,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(3), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Ensure that the head is now 3. @@ -681,6 +726,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -730,6 +777,8 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Invalidation of 3 should have removed upstream weight. @@ -763,6 +812,9 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { root: get_root(0), }, operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } @@ -782,6 +834,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -801,6 +855,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is 2 @@ -819,6 +875,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -839,6 +897,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -857,6 +917,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -867,7 +929,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(1), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is now 1, because 1 has a vote. @@ -886,6 +948,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -909,7 +973,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(1), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is 1. @@ -928,6 +992,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -962,6 +1028,8 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is now 3, applying a proposer boost to 3 as well. @@ -985,13 +1053,15 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { proposer_boost_root: get_root(3), }); + // Stored weights are pure attestation scores (proposer boost is applied + // on-the-fly in the walk's `get_weight`, not baked into `node.weight()`). ops.push(Operation::AssertWeight { block_root: get_root(0), - weight: 33_250, + weight: 2_000, }); ops.push(Operation::AssertWeight { block_root: get_root(1), - weight: 33_250, + weight: 2_000, }); ops.push(Operation::AssertWeight { block_root: get_root(2), @@ -999,8 +1069,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }); ops.push(Operation::AssertWeight { block_root: get_root(3), - // This is a "magic number" generated from `calculate_committee_fraction`. - weight: 31_250, + weight: 0, }); // Invalidate the payload of 3. @@ -1065,6 +1134,9 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { root: get_root(0), }, operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } diff --git a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs index 3b31616145..76f9a95315 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs @@ -10,6 +10,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Build the following tree (stick? lol). @@ -27,6 +29,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(0), justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), @@ -34,6 +38,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(1), justified_checkpoint: get_checkpoint(1), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), @@ -41,6 +47,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(2), justified_checkpoint: get_checkpoint(2), finalized_checkpoint: get_checkpoint(1), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that with justified epoch 0 we find 3 @@ -57,6 +65,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that with justified epoch 1 we find 3 @@ -77,6 +87,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that with justified epoch 2 we find 3 @@ -93,6 +105,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(1), justified_state_balances: balances, expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, }); // END OF TESTS @@ -101,6 +115,9 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } @@ -114,6 +131,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Build the following tree. @@ -137,6 +156,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(0), justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), @@ -147,6 +168,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(1), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), @@ -157,6 +180,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(1), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(4), @@ -167,6 +192,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(1), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(5), @@ -177,6 +204,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(3), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Right branch @@ -186,6 +215,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(0), justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), @@ -193,6 +224,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(2), justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), @@ -200,6 +233,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { parent_root: get_root(4), justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(4), @@ -210,6 +245,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(2), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(5), @@ -220,6 +257,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { root: get_root(4), }, finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that if we start at 0 we find 10 (just: 0, fin: 0). @@ -240,6 +279,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above, but with justified epoch 2. ops.push(Operation::FindHead { @@ -250,6 +291,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above, but with justified epoch 3. // @@ -264,6 +307,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to 1. @@ -282,7 +327,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(1), - target_epoch: Epoch::new(0), + attestation_slot: Slot::new(0), }); // Ensure that if we start at 0 we find 9 (just: 0, fin: 0). @@ -303,6 +348,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Save as above but justified epoch 2. ops.push(Operation::FindHead { @@ -313,6 +360,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Save as above but justified epoch 3. // @@ -327,6 +376,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to 2. @@ -345,7 +396,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(2), - target_epoch: Epoch::new(0), + attestation_slot: Slot::new(0), }); // Ensure that if we start at 0 we find 10 (just: 0, fin: 0). @@ -366,6 +417,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -376,6 +429,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -390,6 +445,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that if we start at 1 we find 9 (just: 0, fin: 0). @@ -413,6 +470,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -423,6 +482,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -437,6 +498,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that if we start at 2 we find 10 (just: 0, fin: 0). @@ -457,6 +520,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -467,6 +532,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -481,6 +548,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances, expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // END OF TESTS @@ -489,6 +558,9 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } 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 new file mode 100644 index 0000000000..0fb120328c --- /dev/null +++ b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs @@ -0,0 +1,762 @@ +use super::*; + +fn gloas_spec() -> ChainSpec { + let mut spec = MainnetEthSpec::default_spec(); + spec.proposer_score_boost = Some(50); + spec.gloas_fork_epoch = Some(Epoch::new(0)); + spec +} + +pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Build two branches off genesis where one child extends parent's payload chain (Full) + // and the other does not (Empty). + 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)), + }); + 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(2)), + }); + + // Extend both branches to verify that head selection follows the selected chain. + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(3), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(4), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(100)), + execution_payload_block_hash: Some(get_hash(4)), + }); + + // 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 { + block_root: get_root(1), + }); + + 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, + }); + + // With equal full/empty parent weights, tiebreak decides which chain to follow. + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: false, + is_data_available: false, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(4), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + 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()), + } +} + +pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + 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)), + }); + + // One Full and one Empty vote for the same head block: tie probes via runtime tiebreak, + // which defaults to Empty unless timely+data-available evidence is set. + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(2), + payload_present: true, + blob_data_available: false, + }); + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 1, + block_root: get_root(1), + attestation_slot: Slot::new(2), + payload_present: false, + blob_data_available: false, + }); + 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(0), + // With MainnetEthSpec PTC_SIZE=512, 1 bit set out of 256 threshold → not timely → Empty. + expected_payload_status: Some(PayloadStatus::Empty), + }); + // PTC votes write to bitfields only, not to full/empty weight. + // Weight is 0 because no CL attestations target this block. + ops.push(Operation::AssertPayloadWeights { + block_root: get_root(1), + expected_full_weight: 0, + expected_empty_weight: 0, + }); + + // Flip validator 0 to Empty; both bits now clear. + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(3), + payload_present: false, + blob_data_available: false, + }); + 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(0), + expected_payload_status: Some(PayloadStatus::Empty), + }); + ops.push(Operation::AssertPayloadWeights { + block_root: get_root(1), + expected_full_weight: 0, + expected_empty_weight: 0, + }); + + // Same-slot attestation to a new head candidate should be Pending (no payload bucket change). + // Root 5 is an Empty child of root_1 (parent_hash doesn't match root_1's block_hash), + // so it's reachable through root_1's Empty direction (root_1 has no payload_received). + ops.push(Operation::ProcessBlock { + slot: Slot::new(3), + root: get_root(5), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(101)), + execution_payload_block_hash: Some(get_hash(5)), + }); + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 2, + block_root: get_root(5), + attestation_slot: Slot::new(3), + payload_present: true, + blob_data_available: false, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1, 1], + expected_head: get_root(5), + current_slot: Slot::new(0), + expected_payload_status: Some(PayloadStatus::Empty), + }); + ops.push(Operation::AssertPayloadWeights { + block_root: get_root(5), + expected_full_weight: 0, + expected_empty_weight: 0, + }); + + 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()), + } +} + +/// Test that CL attestation weight can flip the head between Full/Empty branches, +/// overriding the tiebreaker. +pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Competing branches with distinct payload ancestry (Full vs Empty from genesis). + 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)), + }); + 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(2)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(3), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(4), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(100)), + execution_payload_block_hash: Some(get_hash(4)), + }); + + // 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 { + block_root: get_root(1), + }); + + // Equal branch weights: tiebreak FULL picks branch rooted at 3. + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + // CL attestation to Empty branch (root 4) from validator 0 → head flips to 4. + ops.push(Operation::ProcessAttestation { + validator_index: 0, + block_root: get_root(4), + attestation_slot: Slot::new(3), + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(4), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + // CL attestation back to Full branch (root 3) → head returns to 3. + ops.push(Operation::ProcessAttestation { + validator_index: 0, + block_root: get_root(3), + attestation_slot: Slot::new(4), + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + 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()), + } +} + +/// CL attestation weight overrides payload preference tiebreaker. +pub fn get_gloas_weight_priority_over_payload_preference_test_definition() +-> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Build two branches where one child extends payload (Full) and the other doesn't (Empty). + 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)), + }); + 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(2)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(3), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(4), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(100)), + execution_payload_block_hash: Some(get_hash(4)), + }); + + // 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 { + block_root: get_root(1), + }); + + // Parent prefers Full on equal branch weights (tiebreaker). + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1], + expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + // Two CL attestations to the Empty branch make it strictly heavier, + // overriding the Full tiebreaker. + ops.push(Operation::ProcessAttestation { + validator_index: 0, + block_root: get_root(4), + attestation_slot: Slot::new(3), + }); + ops.push(Operation::ProcessAttestation { + validator_index: 1, + block_root: get_root(4), + attestation_slot: Slot::new(3), + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1], + expected_head: get_root(4), + current_slot: Slot::new(0), + expected_payload_status: None, + }); + + 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()), + } +} + +pub fn get_gloas_parent_empty_when_child_points_to_grandparent_test_definition() +-> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Build a three-block chain A -> B -> C (CL parent links). + // A: EL parent = genesis hash(0), EL hash = hash(1). + 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)), + }); + + // B: EL parent = hash(1), EL hash = hash(2). + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(2), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(2)), + }); + + // C: CL parent is B, but EL parent points to A (hash 1), not B (hash 2). + // This models B's payload not arriving in time, so C records parent status as Empty. + ops.push(Operation::ProcessBlock { + slot: Slot::new(3), + root: get_root(3), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + + ops.push(Operation::AssertParentPayloadStatus { + block_root: get_root(3), + expected_status: PayloadStatus::Empty, + }); + + 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()), + } +} + +/// Test interleaving of blocks, regular attestations, and tiebreaker. +/// +/// genesis → block 1 (Full) → block 3 +/// → block 2 (Empty) → block 4 +/// +/// With equal CL weight, tiebreaker determines which branch wins. +/// An extra CL attestation can override the tiebreaker. +pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Step 1: Two competing blocks at slot 1. + 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)), + }); + 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(2)), + }); + + // Step 2: Regular attestations arrive, one per branch (equal CL 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), + }); + + // Step 3: Child blocks at slot 2. + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(3), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(4), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(100)), + execution_payload_block_hash: Some(get_hash(4)), + }); + + // 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 { + block_root: get_root(1), + }); + + // Step 4: Set tiebreaker to Empty on genesis → Empty branch wins. + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: false, + is_data_available: false, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1], + expected_head: get_root(4), + current_slot: Slot::new(1), + expected_payload_status: None, + }); + + // Step 5: Flip tiebreaker to Full → Full branch wins. + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1], + expected_head: get_root(3), + current_slot: Slot::new(100), + expected_payload_status: None, + }); + + // Step 6: Add extra CL weight to Empty branch → overrides Full tiebreaker. + ops.push(Operation::ProcessAttestation { + validator_index: 2, + block_root: get_root(4), + attestation_slot: Slot::new(3), + }); + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1, 1], + expected_head: get_root(4), + current_slot: Slot::new(100), + expected_payload_status: None, + }); + + 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()), + } +} + +/// 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, + }); + + // Per spec `get_forkchoice_store`: genesis starts with payload_received=true + // (anchor block is in `payload_states`). + ops.push(Operation::AssertPayloadReceived { + block_root: get_root(0), + expected: true, + }); + + // 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, payload_received=true on genesis → tiebreaker uses + // payload_received (not previous slot, equal payload weights) → 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), + expected_payload_status: None, + }); + + // ProcessExecutionPayload on genesis is a no-op (already received at init). + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(0), + }); + + ops.push(Operation::AssertPayloadReceived { + block_root: get_root(0), + expected: true, + }); + + // Set PTC votes on genesis as timely + data available (simulates PTC voting). + // This doesn't change the preference since genesis is not the previous slot + // (slot 0 + 1 != current_slot 100). + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: true, + is_data_available: true, + }); + + // Still prefers Full via payload_received tiebreaker → Block 1 (Full) wins. + 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), + expected_payload_status: None, + }); + + 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::*; + + #[test] + fn chain_following() { + let test = get_gloas_chain_following_test_definition(); + test.run(); + } + + #[test] + fn payload_probe() { + let test = get_gloas_payload_probe_test_definition(); + test.run(); + } + + #[test] + fn find_head_vote_transition() { + let test = get_gloas_find_head_vote_transition_test_definition(); + test.run(); + } + + #[test] + fn weight_priority_over_payload_preference() { + let test = get_gloas_weight_priority_over_payload_preference_test_definition(); + test.run(); + } + + #[test] + fn parent_empty_when_child_points_to_grandparent() { + let test = get_gloas_parent_empty_when_child_points_to_grandparent_test_definition(); + test.run(); + } + + #[test] + fn interleaved_attestations() { + 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/fork_choice_test_definition/no_votes.rs b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs index d20eaacb99..7b5ee31c64 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs @@ -18,6 +18,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: Hash256::zero(), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 2 // @@ -36,6 +38,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure the head is 2 // @@ -53,6 +57,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 1 // @@ -71,6 +77,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure the head is still 2 // @@ -88,6 +96,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 3 // @@ -108,6 +118,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure 2 is still the head // @@ -127,6 +139,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 4 // @@ -147,6 +161,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure the head is 4. // @@ -166,6 +182,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(4), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 5 with a justified epoch of 2 // @@ -185,6 +203,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure the head is now 5 whilst the justified epoch is 0. // @@ -206,6 +226,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Ensure there is no error when starting from a block that has the // wrong justified epoch. @@ -232,6 +254,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head. // @@ -250,6 +274,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 6 // @@ -271,6 +297,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: Hash256::zero(), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }, // Ensure 6 is the head // @@ -291,6 +319,8 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(6), + current_slot: Slot::new(0), + expected_payload_status: None, }, ]; @@ -305,6 +335,9 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { root: Hash256::zero(), }, operations, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } diff --git a/consensus/proto_array/src/fork_choice_test_definition/votes.rs b/consensus/proto_array/src/fork_choice_test_definition/votes.rs index 01994fff9b..ac97a592b7 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/votes.rs @@ -16,6 +16,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -35,6 +37,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is 2 @@ -53,6 +57,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -73,6 +79,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -91,6 +99,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -101,7 +111,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(1), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is now 1, because 1 has a vote. @@ -120,6 +130,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 2 @@ -130,7 +142,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(2), - target_epoch: Epoch::new(2), + attestation_slot: Slot::new(2), }); // Ensure that the head is 2 since 1 and 2 both have a vote @@ -149,6 +161,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 3. @@ -170,6 +184,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is still 2 @@ -190,6 +206,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Move validator #0 vote from 1 to 3 @@ -202,7 +220,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(3), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Ensure that the head is still 2 @@ -223,6 +241,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Move validator #1 vote from 2 to 1 (this is an equivocation, but fork choice doesn't @@ -236,7 +256,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(1), - target_epoch: Epoch::new(3), + attestation_slot: Slot::new(3), }); // Ensure that the head is now 3 @@ -257,6 +277,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 4. @@ -280,6 +302,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that the head is now 4 @@ -302,6 +326,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(4), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 5, which has a justified epoch of 2. @@ -327,19 +353,22 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(1), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); - // Ensure that 5 is filtered out and the head stays at 4. + // Block 5 has incompatible finalized checkpoint, so `get_filtered_block_tree` + // excludes the entire 1->3->4->5 branch (no viable leaf). Head moves to 2. // // 0 // / \ - // 2 1 + // head-> 2 1 // | // 3 // | - // 4 <- head + // 4 // / - // 5 + // 5 <- incompatible finalized checkpoint ops.push(Operation::FindHead { justified_checkpoint: Checkpoint { epoch: Epoch::new(1), @@ -350,7 +379,9 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { root: get_root(0), }, justified_state_balances: balances.clone(), - expected_head: get_root(4), + expected_head: get_root(2), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 6, which has a justified epoch of 0. @@ -376,6 +407,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(1), root: get_root(0), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Move both votes to 5. @@ -392,12 +425,12 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(5), - target_epoch: Epoch::new(4), + attestation_slot: Slot::new(4), }); ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(5), - target_epoch: Epoch::new(4), + attestation_slot: Slot::new(4), }); // Add blocks 7, 8 and 9. Adding these blocks helps test the `best_descendant` @@ -430,6 +463,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(5), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(0), @@ -443,6 +478,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(5), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); ops.push(Operation::ProcessBlock { slot: Slot::new(0), @@ -456,6 +493,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(5), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure that 6 is the head, even though 5 has all the votes. This is testing to ensure @@ -487,6 +526,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(6), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -520,6 +561,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -545,12 +588,12 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 0, block_root: get_root(9), - target_epoch: Epoch::new(5), + attestation_slot: Slot::new(5), }); ops.push(Operation::ProcessAttestation { validator_index: 1, block_root: get_root(9), - target_epoch: Epoch::new(5), + attestation_slot: Slot::new(5), }); // Add block 10 @@ -582,6 +625,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(5), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Double-check the head is still 9 (no diagram this time) @@ -596,6 +641,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Introduce 2 more validators into the system @@ -621,12 +668,12 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { ops.push(Operation::ProcessAttestation { validator_index: 2, block_root: get_root(10), - target_epoch: Epoch::new(5), + attestation_slot: Slot::new(5), }); ops.push(Operation::ProcessAttestation { validator_index: 3, block_root: get_root(10), - target_epoch: Epoch::new(5), + attestation_slot: Slot::new(5), }); // Check the head is now 10. @@ -657,6 +704,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Set the balances of the last two validators to zero @@ -682,6 +731,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Set the balances of the last two validators back to 1 @@ -707,6 +758,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Remove the last two validators @@ -733,6 +786,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that pruning below the prune threshold does not prune. @@ -754,6 +809,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that pruning above the prune threshold does prune. @@ -792,6 +849,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 11 @@ -817,6 +876,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { epoch: Epoch::new(2), root: get_root(5), }, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, }); // Ensure the head is now 11 @@ -841,6 +902,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(11), + current_slot: Slot::new(0), + expected_payload_status: None, }); ForkChoiceTestDefinition { @@ -854,6 +917,9 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { root: get_root(0), }, operations: ops, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + spec: None, } } diff --git a/consensus/proto_array/src/lib.rs b/consensus/proto_array/src/lib.rs index 04e57d791b..702c014f07 100644 --- a/consensus/proto_array/src/lib.rs +++ b/consensus/proto_array/src/lib.rs @@ -8,13 +8,13 @@ mod ssz_container; pub use crate::justified_balances::JustifiedBalances; pub use crate::proto_array::{InvalidationOperation, calculate_committee_fraction}; pub use crate::proto_array_fork_choice::{ - Block, DisallowedReOrgOffsets, DoNotReOrg, ExecutionStatus, ProposerHeadError, - ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, + Block, DisallowedReOrgOffsets, DoNotReOrg, ExecutionStatus, LatestMessage, PayloadStatus, + ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; pub use error::Error; pub mod core { pub use super::proto_array::{ProposerBoost, ProtoArray, ProtoNode}; pub use super::proto_array_fork_choice::VoteTracker; - pub use super::ssz_container::{SszContainer, SszContainerV28}; + pub use super::ssz_container::{SszContainer, SszContainerV28, SszContainerV29}; } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 5bfcdae463..933e9eb078 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1,12 +1,18 @@ use crate::error::InvalidBestNodeInfo; -use crate::{Block, ExecutionStatus, JustifiedBalances, error::Error}; +use crate::proto_array_fork_choice::IndexedForkChoiceNode; +use crate::{ + Block, ExecutionStatus, JustifiedBalances, LatestMessage, 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 std::time::Duration; use superstruct::superstruct; +use typenum::U512; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, Slot, @@ -17,6 +23,14 @@ use types::{ four_byte_option_impl!(four_byte_option_usize, usize); four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); +fn all_true_bitvector() -> BitVector { + let mut bv = BitVector::new(); + for i in 0..bv.len() { + let _ = bv.set(i, true); + } + bv +} + /// Defines an operation which may invalidate the `execution_status` of some nodes. #[derive(Clone, Debug)] pub enum InvalidationOperation { @@ -68,47 +82,152 @@ impl InvalidationOperation { } } -pub type ProtoNode = ProtoNodeV17; - #[superstruct( - variants(V17), - variant_attributes(derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)), - no_enum + variants(V17, V29), + variant_attributes(derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)) )] +#[derive(PartialEq, Debug, Encode, Decode, Serialize, Deserialize, Clone)] +#[ssz(enum_behaviour = "union")] pub struct ProtoNode { /// The `slot` is not necessary for `ProtoArray`, it just exists so external components can /// easily query the block slot. This is useful for upstream fork choice logic. + #[superstruct(getter(copy))] pub slot: Slot, /// The `state_root` is not necessary for `ProtoArray` either, it also just exists for upstream /// components (namely attestation verification). + #[superstruct(getter(copy))] pub state_root: Hash256, /// The root that would be used for the `attestation.data.target.root` if a LMD vote was cast /// for this block. /// /// The `target_root` is not necessary for `ProtoArray` either, it also just exists for upstream /// components (namely fork choice attestation verification). + #[superstruct(getter(copy))] pub target_root: Hash256, pub current_epoch_shuffling_id: AttestationShufflingId, pub next_epoch_shuffling_id: AttestationShufflingId, + #[superstruct(getter(copy))] pub root: Hash256, + #[superstruct(getter(copy))] #[ssz(with = "four_byte_option_usize")] pub parent: Option, - #[superstruct(only(V17))] + #[superstruct(only(V17, V29), partial_getter(copy))] pub justified_checkpoint: Checkpoint, - #[superstruct(only(V17))] + #[superstruct(only(V17, V29), partial_getter(copy))] pub finalized_checkpoint: Checkpoint, + #[superstruct(getter(copy))] pub weight: u64, + #[superstruct(getter(copy))] #[ssz(with = "four_byte_option_usize")] pub best_child: Option, + #[superstruct(getter(copy))] #[ssz(with = "four_byte_option_usize")] pub best_descendant: Option, /// Indicates if an execution node has marked this block as valid. Also contains the execution - /// block hash. + /// block hash. This is only used pre-Gloas. + #[superstruct(only(V17), partial_getter(copy))] pub execution_status: ExecutionStatus, + #[superstruct(getter(copy))] #[ssz(with = "four_byte_option_checkpoint")] pub unrealized_justified_checkpoint: Option, + #[superstruct(getter(copy))] #[ssz(with = "four_byte_option_checkpoint")] pub unrealized_finalized_checkpoint: Option, + + /// We track the parent payload status from which the current node was extended. + #[superstruct(only(V29), partial_getter(copy))] + pub parent_payload_status: PayloadStatus, + #[superstruct(only(V29), partial_getter(copy))] + pub empty_payload_weight: u64, + #[superstruct(only(V29), partial_getter(copy))] + pub full_payload_weight: u64, + #[superstruct(only(V29), partial_getter(copy))] + pub execution_payload_block_hash: ExecutionBlockHash, + /// Equivalent to spec's `block_timeliness[root][ATTESTATION_TIMELINESS_INDEX]`. + #[superstruct(only(V29), partial_getter(copy))] + pub block_timeliness_attestation_threshold: bool, + /// Equivalent to spec's `block_timeliness[root][PTC_TIMELINESS_INDEX]`. + #[superstruct(only(V29), partial_getter(copy))] + pub block_timeliness_ptc_threshold: bool, + /// Equivalent to spec's `store.payload_timeliness_vote[root]`. + /// PTC timeliness vote bitfield, indexed by PTC committee position. + /// Bit i set means PTC member i voted `payload_present = true`. + /// Tiebreak derived as: `num_set_bits() > ptc_size / 2`. + #[superstruct(only(V29))] + pub payload_timeliness_votes: BitVector, + /// Equivalent to spec's `store.payload_data_availability_vote[root]`. + /// 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: `num_set_bits() > ptc_size / 2`. + #[superstruct(only(V29))] + 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` + /// to detect equivocations at the parent's slot. + #[superstruct(only(V29), partial_getter(copy))] + pub proposer_index: u64, + /// Weight from equivocating validators that voted for this block. + /// Used by `is_head_weak` to match the spec's monotonicity guarantee: + /// more attestations can only increase head weight, never decrease it. + #[superstruct(only(V29), partial_getter(copy))] + pub equivocating_attestation_score: u64, +} + +impl ProtoNode { + /// Generic version of spec's `parent_payload_status` that works for pre-Gloas nodes by + /// considering their parents Empty. + /// Pre-Gloas nodes have no ePBS, default to Empty. + pub fn get_parent_payload_status(&self) -> PayloadStatus { + self.parent_payload_status().unwrap_or(PayloadStatus::Empty) + } + + pub fn is_parent_node_full(&self) -> bool { + self.get_parent_payload_status() == PayloadStatus::Full + } + + pub fn attestation_score(&self, payload_status: PayloadStatus) -> u64 { + match payload_status { + PayloadStatus::Pending => self.weight(), + // Pre-Gloas (V17) nodes have no payload separation — all weight + // is in `weight()`. Post-Gloas (V29) nodes track per-status weights. + PayloadStatus::Empty => self + .empty_payload_weight() + .unwrap_or_else(|_| self.weight()), + PayloadStatus::Full => self.full_payload_weight().unwrap_or_else(|_| self.weight()), + } + } + + pub fn is_payload_timely(&self) -> bool { + let Ok(node) = self.as_v29() else { + return false; + }; + + // Equivalent to `if root not in store.payload_states` in the spec. + if !node.payload_received { + return false; + } + + node.payload_timeliness_votes.num_set_bits() > E::ptc_size() / 2 + } + + pub fn is_payload_data_available(&self) -> bool { + let Ok(node) = self.as_v29() else { + return false; + }; + + // Equivalent to `if root not in store.payload_states` in the spec. + if !node.payload_received { + return false; + } + + // TODO(gloas): add function on EthSpec for DATA_AVAILABILITY_TIMELY_THRESHOLD + node.payload_data_availability_votes.num_set_bits() > E::ptc_size() / 2 + } } #[derive(PartialEq, Debug, Encode, Decode, Serialize, Deserialize, Copy, Clone)] @@ -126,6 +245,122 @@ impl Default for ProposerBoost { } } +/// Accumulated score changes for a single proto-array node during a `find_head` pass. +/// +/// `delta` tracks the ordinary LMD-GHOST balance change applied to the concrete block node. +/// This is the same notion of weight that pre-gloas fork choice used. +/// +/// +/// Under gloas we also need to track how votes contribute to the parent's virtual payload +/// branches: +/// +/// - `empty_delta` is the balance change attributable to votes that support the `Empty` payload +/// interpretation of the node +/// - `full_delta` is the balance change attributable to votes that support the `Full` payload +/// interpretation of the node +/// +/// Votes in `Pending` state only affect `delta`; they do not contribute to either payload bucket. +/// During score application these payload deltas are propagated independently up the tree so that +/// ancestors can compare children using payload-aware tie breaking. +#[derive(Clone, PartialEq, Debug, Copy)] +pub struct NodeDelta { + /// Total weight change for the node. All votes contribute regardless of payload status. + pub delta: i64, + /// Weight change from `PayloadStatus::Empty` votes. + pub empty_delta: i64, + /// Weight change from `PayloadStatus::Full` votes. + pub full_delta: i64, + /// Weight from equivocating validators that voted for this node. + pub equivocating_attestation_delta: u64, +} + +impl NodeDelta { + /// Classify a vote into the payload bucket it contributes to for `block_slot`. + /// + /// Per the gloas model: + /// + /// - a same-slot vote is `Pending` + /// - a later vote with `payload_present = true` is `Full` + /// - a later vote with `payload_present = false` is `Empty` + /// + /// This classification is used only for payload-aware accounting; all votes still contribute to + /// the aggregate `delta`. + pub fn payload_status( + vote_slot: Slot, + payload_present: bool, + block_slot: Slot, + ) -> PayloadStatus { + if vote_slot == block_slot { + PayloadStatus::Pending + } else if payload_present { + PayloadStatus::Full + } else { + PayloadStatus::Empty + } + } + + /// Add `balance` to the payload bucket selected by `status`. + /// + /// `Pending` votes do not affect payload buckets, so this becomes a no-op for that case. + pub fn add_payload_delta( + &mut self, + status: PayloadStatus, + balance: u64, + index: usize, + ) -> Result<(), Error> { + let field = match status { + PayloadStatus::Full => &mut self.full_delta, + PayloadStatus::Empty => &mut self.empty_delta, + PayloadStatus::Pending => return Ok(()), + }; + *field = field + .checked_add(balance as i64) + .ok_or(Error::DeltaOverflow(index))?; + Ok(()) + } + + /// Create a delta that only affects the aggregate block weight. + /// + /// This is useful for callers or tests that only care about ordinary LMD-GHOST weight changes + /// and do not need payload-aware accounting. + pub fn from_delta(delta: i64) -> Self { + Self { + delta, + empty_delta: 0, + full_delta: 0, + equivocating_attestation_delta: 0, + } + } + + /// Subtract `balance` from the payload bucket selected by `status`. + /// + /// `Pending` votes do not affect payload buckets, so this becomes a no-op for that case. + pub fn sub_payload_delta( + &mut self, + status: PayloadStatus, + balance: u64, + index: usize, + ) -> Result<(), Error> { + let field = match status { + PayloadStatus::Full => &mut self.full_delta, + PayloadStatus::Empty => &mut self.empty_delta, + PayloadStatus::Pending => return Ok(()), + }; + *field = field + .checked_sub(balance as i64) + .ok_or(Error::DeltaOverflow(index))?; + Ok(()) + } +} + +/// Compare NodeDelta with i64 by comparing the aggregate `delta` field. +/// This is used by tests that only care about the total weight delta. +impl PartialEq for NodeDelta { + fn eq(&self, other: &i64) -> bool { + self.delta == *other + } +} + #[derive(PartialEq, Debug, Serialize, Deserialize, Clone)] pub struct ProtoArray { /// Do not attempt to prune the tree unless it has at least this many nodes. Small prunes @@ -153,13 +388,7 @@ impl ProtoArray { #[allow(clippy::too_many_arguments)] pub fn apply_score_changes( &mut self, - mut deltas: Vec, - best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, - new_justified_balances: &JustifiedBalances, - proposer_boost_root: Hash256, - current_slot: Slot, - spec: &ChainSpec, + mut deltas: Vec, ) -> Result<(), Error> { if deltas.len() != self.indices.len() { return Err(Error::InvalidDeltaLen { @@ -168,9 +397,6 @@ impl ProtoArray { }); } - // Default the proposer boost score to zero. - let mut proposer_score = 0; - // Iterate backwards through all indices in `self.nodes`. for node_index in (0..self.nodes.len()).rev() { let node = self @@ -181,118 +407,103 @@ impl ProtoArray { // There is no need to adjust the balances or manage parent of the zero hash since it // is an alias to the genesis block. The weight applied to the genesis block is // irrelevant as we _always_ choose it and it's impossible for it to have a parent. - if node.root == Hash256::zero() { + if node.root() == Hash256::zero() { continue; } - let execution_status_is_invalid = node.execution_status.is_invalid(); - - let mut node_delta = if execution_status_is_invalid { - // If the node has an invalid execution payload, reduce its weight to zero. - 0_i64 - .checked_sub(node.weight as i64) - .ok_or(Error::InvalidExecutionDeltaOverflow(node_index))? + let execution_status_is_invalid = if let Ok(proto_node) = node.as_v17() + && proto_node.execution_status.is_invalid() + { + true } else { - deltas - .get(node_index) - .copied() - .ok_or(Error::InvalidNodeDelta(node_index))? + false }; - // If we find the node for which the proposer boost was previously applied, decrease - // the delta by the previous score amount. - if self.previous_proposer_boost.root != Hash256::zero() - && self.previous_proposer_boost.root == node.root - // Invalid nodes will always have a weight of zero so there's no need to subtract - // the proposer boost delta. - && !execution_status_is_invalid - { - node_delta = node_delta - .checked_sub(self.previous_proposer_boost.score as i64) - .ok_or(Error::DeltaOverflow(node_index))?; - } - // If we find the node matching the current proposer boost root, increase - // the delta by the new score amount (unless the block has an invalid execution status). - // - // https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_latest_attesting_balance - if let Some(proposer_score_boost) = spec.proposer_score_boost - && proposer_boost_root != Hash256::zero() - && proposer_boost_root == node.root - // Invalid nodes (or their ancestors) should not receive a proposer boost. - && !execution_status_is_invalid - { - proposer_score = - calculate_committee_fraction::(new_justified_balances, proposer_score_boost) - .ok_or(Error::ProposerBoostOverflow(node_index))?; - node_delta = node_delta - .checked_add(proposer_score as i64) - .ok_or(Error::DeltaOverflow(node_index))?; - } + let node_delta = deltas + .get(node_index) + .copied() + .ok_or(Error::InvalidNodeDelta(node_index))?; + + let delta = if execution_status_is_invalid { + // If the node has an invalid execution payload, reduce its weight to zero. + 0_i64 + .checked_sub(node.weight() as i64) + .ok_or(Error::InvalidExecutionDeltaOverflow(node_index))? + } else { + node_delta.delta + }; + + let (node_empty_delta, node_full_delta) = if node.as_v29().is_ok() { + (node_delta.empty_delta, node_delta.full_delta) + } else { + (0, 0) + }; + + // Proposer boost is NOT applied here. It is computed on-the-fly + // during the virtual tree walk in `get_weight`, matching the spec's + // `get_weight` which adds boost separately from `get_attestation_score`. // Apply the delta to the node. if execution_status_is_invalid { // Invalid nodes always have a weight of 0. - node.weight = 0 - } else if node_delta < 0 { - // Note: I am conflicted about whether to use `saturating_sub` or `checked_sub` - // here. - // - // I can't think of any valid reason why `node_delta.abs()` should be greater than - // `node.weight`, so I have chosen `checked_sub` to try and fail-fast if there is - // some error. - // - // However, I am not fully convinced that some valid case for `saturating_sub` does - // not exist. - node.weight = node - .weight - .checked_sub(node_delta.unsigned_abs()) - .ok_or(Error::DeltaOverflow(node_index))?; + *node.weight_mut() = 0; } else { - node.weight = node - .weight - .checked_add(node_delta as u64) - .ok_or(Error::DeltaOverflow(node_index))?; + *node.weight_mut() = apply_delta(node.weight(), delta, node_index)?; + } + + // Apply post-Gloas score deltas. + if let Ok(node) = node.as_v29_mut() { + node.empty_payload_weight = + apply_delta(node.empty_payload_weight, node_empty_delta, node_index)?; + node.full_payload_weight = + apply_delta(node.full_payload_weight, node_full_delta, node_index)?; + node.equivocating_attestation_score = node + .equivocating_attestation_score + .saturating_add(node_delta.equivocating_attestation_delta); } // Update the parent delta (if any). - if let Some(parent_index) = node.parent { + if let Some(parent_index) = node.parent() { let parent_delta = deltas .get_mut(parent_index) .ok_or(Error::InvalidParentDelta(parent_index))?; - // Back-propagate the nodes delta to its parent. - *parent_delta += node_delta; + // Back-propagate the node's delta to its parent. + parent_delta.delta = parent_delta + .delta + .checked_add(delta) + .ok_or(Error::DeltaOverflow(parent_index))?; + + // Route ALL child weight into the parent's FULL or EMPTY bucket + // based on the child's `parent_payload_status` (the ancestor path + // direction). If this child is on the FULL path from the parent, + // all weight supports the parent's FULL virtual node, and vice versa. + if let Ok(child_v29) = node.as_v29() { + if child_v29.parent_payload_status == PayloadStatus::Full { + parent_delta.full_delta = parent_delta + .full_delta + .checked_add(delta) + .ok_or(Error::DeltaOverflow(parent_index))?; + } else { + parent_delta.empty_delta = parent_delta + .empty_delta + .checked_add(delta) + .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))?; + } } } - // After applying all deltas, update the `previous_proposer_boost`. - self.previous_proposer_boost = ProposerBoost { - root: proposer_boost_root, - score: proposer_score, - }; - - // A second time, iterate backwards through all indices in `self.nodes`. - // - // We _must_ perform these functions separate from the weight-updating loop above to ensure - // that we have a fully coherent set of weights before updating parent - // best-child/descendant. - for node_index in (0..self.nodes.len()).rev() { - let node = self - .nodes - .get_mut(node_index) - .ok_or(Error::InvalidNodeIndex(node_index))?; - - // If the node has a parent, try to update its best-child and best-descendant. - if let Some(parent_index) = node.parent { - self.maybe_update_best_child_and_descendant::( - parent_index, - node_index, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )?; - } - } + // 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(()) } @@ -304,71 +515,286 @@ impl ProtoArray { &mut self, block: Block, current_slot: Slot, - best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + spec: &ChainSpec, + time_into_slot: Duration, ) -> Result<(), Error> { // If the block is already known, simply ignore it. if self.indices.contains_key(&block.root) { return Ok(()); } - let node_index = self.nodes.len(); - - let node = ProtoNode { - slot: block.slot, - root: block.root, - target_root: block.target_root, - current_epoch_shuffling_id: block.current_epoch_shuffling_id, - next_epoch_shuffling_id: block.next_epoch_shuffling_id, - state_root: block.state_root, - parent: block - .parent_root - .and_then(|parent| self.indices.get(&parent).copied()), - justified_checkpoint: block.justified_checkpoint, - finalized_checkpoint: block.finalized_checkpoint, - weight: 0, - best_child: None, - best_descendant: None, - execution_status: block.execution_status, - unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, - unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + // We do not allow `proposer_index=None` for calls to `on_block`, it is only non-optional + // for backwards-compatibility with pre-Gloas V17 proto nodes. + let Some(proposer_index) = block.proposer_index else { + return Err(Error::OnBlockRequiresProposerIndex); }; - // If the parent has an invalid execution status, return an error before adding the block to - // `self`. - if let Some(parent_index) = node.parent { + let node_index = self.nodes.len(); + + let parent_index = block + .parent_root + .and_then(|parent| self.indices.get(&parent).copied()); + + let node = if !spec.fork_name_at_slot::(current_slot).gloas_enabled() { + ProtoNode::V17(ProtoNodeV17 { + slot: block.slot, + root: block.root, + target_root: block.target_root, + current_epoch_shuffling_id: block.current_epoch_shuffling_id, + next_epoch_shuffling_id: block.next_epoch_shuffling_id, + state_root: block.state_root, + parent: parent_index, + justified_checkpoint: block.justified_checkpoint, + finalized_checkpoint: block.finalized_checkpoint, + weight: 0, + best_child: None, + best_descendant: None, + execution_status: block.execution_status, + unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + }) + } else { + let is_current_slot = current_slot == block.slot; + + let execution_payload_block_hash = + block + .execution_payload_block_hash + .ok_or(Error::BrokenBlock { + block_root: block.root, + })?; + + let execution_payload_parent_hash = + block + .execution_payload_parent_hash + .ok_or(Error::BrokenBlock { + block_root: block.root, + })?; + + let parent_payload_status: PayloadStatus = if let Some(parent_node) = + parent_index.and_then(|idx| self.nodes.get(idx)) + { + // Get the parent's execution block hash, handling both V17 and V29 nodes. + // V17 parents occur during the Gloas fork transition. + // TODO(gloas): the spec's `get_parent_payload_status` assumes all blocks are + // post-Gloas with bids. Revisit once the spec clarifies fork-transition behavior. + let parent_el_block_hash = match parent_node { + ProtoNode::V29(v29) => Some(v29.execution_payload_block_hash), + ProtoNode::V17(v17) => v17.execution_status.block_hash(), + }; + // Per spec's `is_parent_node_full`: if the child's EL parent hash + // matches the parent's EL block hash, the child extends the parent's + // payload chain, meaning the parent was Full. + if parent_el_block_hash.is_some_and(|hash| execution_payload_parent_hash == hash) { + PayloadStatus::Full + } else { + PayloadStatus::Empty + } + } else { + // Parent is missing (genesis or pruned due to finalization). Default to Full + // since this path should only be hit at Gloas genesis, and extending the payload + // chain is the safe default. + PayloadStatus::Full + }; + + // Per spec `get_forkchoice_store`: the anchor (genesis) block has + // its payload state initialized (`payload_states = {anchor_root: ...}`). + // Without `payload_received = true` on genesis, the FULL virtual + // child doesn't exist in the spec's `get_node_children`, making all + // Full concrete children of genesis unreachable in `get_head`. + let is_genesis = parent_index.is_none(); + + ProtoNode::V29(ProtoNodeV29 { + slot: block.slot, + root: block.root, + target_root: block.target_root, + current_epoch_shuffling_id: block.current_epoch_shuffling_id, + next_epoch_shuffling_id: block.next_epoch_shuffling_id, + state_root: block.state_root, + parent: parent_index, + justified_checkpoint: block.justified_checkpoint, + finalized_checkpoint: block.finalized_checkpoint, + weight: 0, + best_child: None, + best_descendant: None, + unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + parent_payload_status, + empty_payload_weight: 0, + full_payload_weight: 0, + execution_payload_block_hash, + // Per spec `get_forkchoice_store`: the anchor block's PTC votes are + // initialized to all-True, ensuring `is_payload_timely` and + // `is_payload_data_available` return true for the anchor. + payload_timeliness_votes: if is_genesis { + all_true_bitvector() + } else { + BitVector::default() + }, + payload_data_availability_votes: if is_genesis { + all_true_bitvector() + } else { + BitVector::default() + }, + payload_received: is_genesis, + proposer_index, + // Spec: `record_block_timeliness` + `get_forkchoice_store`. + // 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. + block_timeliness_ptc_threshold: is_genesis + || (is_current_slot && time_into_slot < spec.get_slot_duration() / 2), + equivocating_attestation_score: 0, + }) + }; + + // If the parent has an invalid execution status, return an error before adding the + // block to `self`. This applies only when the parent is a V17 node with execution tracking. + if let Some(parent_index) = node.parent() { let parent = self .nodes .get(parent_index) .ok_or(Error::InvalidNodeIndex(parent_index))?; - if parent.execution_status.is_invalid() { + + // Execution status tracking only exists on V17 (pre-Gloas) nodes. + if let Ok(v17) = parent.as_v17() + && v17.execution_status.is_invalid() + { return Err(Error::ParentExecutionStatusIsInvalid { block_root: block.root, - parent_root: parent.root, + parent_root: parent.root(), }); } } - self.indices.insert(node.root, node_index); + self.indices.insert(node.root(), node_index); self.nodes.push(node.clone()); - if let Some(parent_index) = node.parent { - self.maybe_update_best_child_and_descendant::( - parent_index, - node_index, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )?; - - if matches!(block.execution_status, ExecutionStatus::Valid(_)) { - self.propagate_execution_payload_validation_by_index(parent_index)?; - } + if let Some(parent_index) = node.parent() + && matches!(block.execution_status, ExecutionStatus::Valid(_)) + { + self.propagate_execution_payload_validation_by_index(parent_index)?; } 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). + fn is_head_weak( + &self, + head_node: &ProtoNode, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> bool { + let reorg_threshold = calculate_committee_fraction::( + justified_balances, + spec.reorg_head_weight_threshold.unwrap_or(20), + ) + .unwrap_or(0); + + let head_weight = head_node + .attestation_score(PayloadStatus::Pending) + .saturating_add(head_node.equivocating_attestation_score().unwrap_or(0)); + + head_weight < reorg_threshold + } + + /// Spec's `should_apply_proposer_boost` for Gloas. + /// + /// Returns `true` if the proposer boost should be kept. Returns `false` if the + /// boost should be subtracted (invalidated) because the parent is weak and there + /// are no equivocating blocks at the parent's slot. + fn should_apply_proposer_boost( + &self, + proposer_boost_root: Hash256, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result { + if proposer_boost_root.is_zero() { + return Ok(false); + } + + let block_index = *self + .indices + .get(&proposer_boost_root) + .ok_or(Error::NodeUnknown(proposer_boost_root))?; + let block = self + .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))?; + let parent = self + .nodes + .get(parent_index) + .ok_or(Error::InvalidNodeIndex(parent_index))?; + let slot = block.slot(); + + // Apply proposer boost if `parent` is not from the previous slot + if parent.slot().saturating_add(1_u64) < slot { + return Ok(true); + } + + // Apply proposer boost if `parent` is not weak + if !self.is_head_weak::(parent, justified_balances, spec) { + return Ok(true); + } + + // Parent is weak. Apply boost unless there's an equivocating block at + // 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| { + if let Ok(timeliness) = node.block_timeliness_ptc_threshold() + && let Ok(proposer_index) = node.proposer_index() + { + timeliness + && Ok(proposer_index) == parent_proposer + && node.slot() == parent_slot + && node.root() != parent_root + } else { + // Pre-Gloas. + false + } + }); + + Ok(!has_equivocation) + } + + /// Process an execution payload 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> { + let index = *self + .indices + .get(&block_root) + .ok_or(Error::NodeUnknown(block_root))?; + let node = self + .nodes + .get_mut(index) + .ok_or(Error::InvalidNodeIndex(index))?; + let v29 = node + .as_v29_mut() + .map_err(|_| Error::InvalidNodeVariant { block_root })?; + v29.payload_received = true; + + Ok(()) + } + /// Updates the `block_root` and all ancestors to have validated execution payloads. /// /// Returns an error if: @@ -402,33 +828,48 @@ impl ProtoArray { .nodes .get_mut(index) .ok_or(Error::InvalidNodeIndex(index))?; - let parent_index = match node.execution_status { - // We have reached a node that we already know is valid. No need to iterate further - // since we assume an ancestors have already been set to valid. - ExecutionStatus::Valid(_) => return Ok(()), - // We have reached an irrelevant node, this node is prior to a terminal execution - // block. There's no need to iterate further, it's impossible for this block to have - // any relevant ancestors. - ExecutionStatus::Irrelevant(_) => return Ok(()), - // The block has an unknown status, set it to valid since any ancestor of a valid - // payload can be considered valid. - ExecutionStatus::Optimistic(payload_block_hash) => { - node.execution_status = ExecutionStatus::Valid(payload_block_hash); + let parent_index = match node { + ProtoNode::V17(node) => match node.execution_status { + // We have reached a node that we already know is valid. No need to iterate further + // since we assume an ancestors have already been set to valid. + ExecutionStatus::Valid(_) => return Ok(()), + // We have reached an irrelevant node, this node is prior to a terminal execution + // block. There's no need to iterate further, it's impossible for this block to have + // any relevant ancestors. + ExecutionStatus::Irrelevant(_) => return Ok(()), + // The block has an unknown status, set it to valid since any ancestor of a valid + // payload can be considered valid. + ExecutionStatus::Optimistic(payload_block_hash) => { + node.execution_status = ExecutionStatus::Valid(payload_block_hash); + if let Some(parent_index) = node.parent { + parent_index + } else { + // We have reached the root block, iteration complete. + return Ok(()); + } + } + // An ancestor of the valid payload was invalid. This is a serious error which + // indicates a consensus failure in the execution node. This is unrecoverable. + ExecutionStatus::Invalid(ancestor_payload_block_hash) => { + return Err(Error::InvalidAncestorOfValidPayload { + ancestor_block_root: node.root, + ancestor_payload_block_hash, + }); + } + }, + // 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 { - // We have reached the root block, iteration complete. return Ok(()); } } - // An ancestor of the valid payload was invalid. This is a serious error which - // indicates a consensus failure in the execution node. This is unrecoverable. - ExecutionStatus::Invalid(ancestor_payload_block_hash) => { - return Err(Error::InvalidAncestorOfValidPayload { - ancestor_block_root: node.root, - ancestor_payload_block_hash, - }); - } }; index = parent_index; @@ -484,10 +925,11 @@ impl ProtoArray { .get_mut(index) .ok_or(Error::InvalidNodeIndex(index))?; - match node.execution_status { - ExecutionStatus::Valid(hash) - | ExecutionStatus::Invalid(hash) - | ExecutionStatus::Optimistic(hash) => { + let node_execution_status = node.execution_status(); + match node_execution_status { + Ok(ExecutionStatus::Valid(hash)) + | Ok(ExecutionStatus::Invalid(hash)) + | Ok(ExecutionStatus::Optimistic(hash)) => { // If we're no longer processing the `head_block_root` and the last valid // ancestor is unknown, exit this loop and proceed to invalidate and // descendants of `head_block_root`/`latest_valid_ancestor_root`. @@ -496,74 +938,51 @@ impl ProtoArray { // supplied, don't validate any ancestors. The alternative is to invalidate // *all* ancestors, which would likely involve shutting down the client due to // an invalid justified checkpoint. - if !latest_valid_ancestor_is_descendant && node.root != head_block_root { + if !latest_valid_ancestor_is_descendant && node.root() != head_block_root { break; } else if op.latest_valid_ancestor() == Some(hash) { - // If the `best_child` or `best_descendant` of the latest valid hash was - // invalidated, set those fields to `None`. - // - // In theory, an invalid `best_child` necessarily infers an invalid - // `best_descendant`. However, we check each variable independently to - // defend against errors which might result in an invalid block being set as - // head. - if node - .best_child - .is_some_and(|i| invalidated_indices.contains(&i)) - { - node.best_child = None - } - if node - .best_descendant - .is_some_and(|i| invalidated_indices.contains(&i)) - { - node.best_descendant = None - } - + // Reached latest valid block, stop invalidating further. break; } } - ExecutionStatus::Irrelevant(_) => break, + Ok(ExecutionStatus::Irrelevant(_)) => break, + Err(_) => break, } // Only invalidate the head block if either: // // - The head block was specifically indicated to be invalidated. // - The latest valid hash is a known ancestor. - if node.root != head_block_root + if node.root() != head_block_root || op.invalidate_block_root() || latest_valid_ancestor_is_descendant { - match &node.execution_status { + match node.execution_status() { // It's illegal for an execution client to declare that some previously-valid block // is now invalid. This is a consensus failure on their behalf. - ExecutionStatus::Valid(hash) => { + Ok(ExecutionStatus::Valid(hash)) => { return Err(Error::ValidExecutionStatusBecameInvalid { - block_root: node.root, - payload_block_hash: *hash, + block_root: node.root(), + payload_block_hash: hash, }); } - ExecutionStatus::Optimistic(hash) => { + Ok(ExecutionStatus::Optimistic(hash)) => { invalidated_indices.insert(index); - node.execution_status = ExecutionStatus::Invalid(*hash); - - // It's impossible for an invalid block to lead to a "best" block, so set these - // fields to `None`. - // - // Failing to set these values will result in `Self::node_leads_to_viable_head` - // returning `false` for *valid* ancestors of invalid blocks. - node.best_child = None; - node.best_descendant = None; + if let ProtoNode::V17(node) = node { + node.execution_status = ExecutionStatus::Invalid(hash); + } } // The block is already invalid, but keep going backwards to ensure all ancestors // are updated. - ExecutionStatus::Invalid(_) => (), + Ok(ExecutionStatus::Invalid(_)) => (), // This block is pre-merge, therefore it has no execution status. Nor do its // ancestors. - ExecutionStatus::Irrelevant(_) => break, + Ok(ExecutionStatus::Irrelevant(_)) => break, + Err(_) => (), } } - if let Some(parent_index) = node.parent { + if let Some(parent_index) = node.parent() { index = parent_index } else { // The root of the block tree has been reached (aka the finalized block), without @@ -597,24 +1016,27 @@ impl ProtoArray { .get_mut(index) .ok_or(Error::InvalidNodeIndex(index))?; - if let Some(parent_index) = node.parent + if let Some(parent_index) = node.parent() && invalidated_indices.contains(&parent_index) { - match &node.execution_status { - ExecutionStatus::Valid(hash) => { + match node.execution_status() { + Ok(ExecutionStatus::Valid(hash)) => { return Err(Error::ValidExecutionStatusBecameInvalid { - block_root: node.root, - payload_block_hash: *hash, + block_root: node.root(), + payload_block_hash: hash, }); } - ExecutionStatus::Optimistic(hash) | ExecutionStatus::Invalid(hash) => { - node.execution_status = ExecutionStatus::Invalid(*hash) + Ok(ExecutionStatus::Optimistic(hash)) | Ok(ExecutionStatus::Invalid(hash)) => { + if let ProtoNode::V17(node) = node { + node.execution_status = ExecutionStatus::Invalid(hash) + } } - ExecutionStatus::Irrelevant(_) => { + Ok(ExecutionStatus::Irrelevant(_)) => { return Err(Error::IrrelevantDescendant { - block_root: node.root, + block_root: node.root(), }); } + Err(_) => (), } invalidated_indices.insert(index); @@ -632,13 +1054,17 @@ impl ProtoArray { /// been called without a subsequent `Self::apply_score_changes` call. This is because /// `on_new_block` does not attempt to walk backwards through the tree and update the /// best-child/best-descendant links. + #[allow(clippy::too_many_arguments)] pub fn find_head( &self, justified_root: &Hash256, current_slot: Slot, best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, - ) -> Result { + proposer_boost_root: Hash256, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result<(Hash256, PayloadStatus), Error> { let justified_index = self .indices .get(justified_root) @@ -652,25 +1078,33 @@ impl ProtoArray { // Since there are no valid descendants of a justified block with an invalid execution // payload, there would be no head to choose from. - // - // Fork choice is effectively broken until a new justified root is set. It might not be - // practically possible to set a new justified root if we are unable to find a new head. - // - // This scenario is *unsupported*. It represents a serious consensus failure. - if justified_node.execution_status.is_invalid() { + // Execution status tracking only exists on V17 (pre-Gloas) nodes. + if let Ok(v17) = justified_node.as_v17() + && v17.execution_status.is_invalid() + { return Err(Error::InvalidJustifiedCheckpointExecutionStatus { justified_root: *justified_root, }); } - let best_descendant_index = justified_node.best_descendant.unwrap_or(justified_index); - - let best_node = self - .nodes - .get(best_descendant_index) - .ok_or(Error::InvalidBestDescendant(best_descendant_index))?; + // 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, + best_justified_checkpoint, + best_finalized_checkpoint, + proposer_boost_root, + justified_balances, + spec, + )?; // Perform a sanity check that the node is indeed valid to be the head. + let best_node = self + .nodes + .get(best_fc_node.proto_node_index) + .ok_or(Error::InvalidNodeIndex(best_fc_node.proto_node_index))?; if !self.node_is_viable_for_head::( best_node, current_slot, @@ -682,13 +1116,444 @@ impl ProtoArray { start_root: *justified_root, justified_checkpoint: best_justified_checkpoint, finalized_checkpoint: best_finalized_checkpoint, - head_root: best_node.root, - head_justified_checkpoint: best_node.justified_checkpoint, - head_finalized_checkpoint: best_node.finalized_checkpoint, + head_root: best_node.root(), + head_justified_checkpoint: *best_node.justified_checkpoint(), + head_finalized_checkpoint: *best_node.finalized_checkpoint(), }))); } - Ok(best_node.root) + 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 + /// one leaf descendant with correct justified/finalized checkpoints. + fn get_filtered_block_tree( + &self, + start_index: usize, + current_slot: Slot, + best_justified_checkpoint: Checkpoint, + best_finalized_checkpoint: Checkpoint, + children_index: &[Vec], + ) -> HashSet { + let mut viable = HashSet::new(); + self.filter_block_tree::( + start_index, + current_slot, + best_justified_checkpoint, + best_finalized_checkpoint, + children_index, + &mut viable, + ); + viable + } + + /// Spec: `filter_block_tree`. + fn filter_block_tree( + &self, + node_index: usize, + 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(&[]); + + if !children.is_empty() { + // Evaluate ALL children (no short-circuit) to mark all viable branches. + let any_viable = children + .iter() + .map(|&child_index| { + self.filter_block_tree::( + child_index, + current_slot, + best_justified_checkpoint, + best_finalized_checkpoint, + children_index, + viable, + ) + }) + .collect::>() + .into_iter() + .any(|v| v); + if any_viable { + viable.insert(node_index); + return true; + } + return false; + } + + // Leaf node: check viability. + if self.node_is_viable_for_head::( + node, + current_slot, + best_justified_checkpoint, + best_finalized_checkpoint, + ) { + viable.insert(node_index); + return true; + } + false + } + + /// Spec: `get_head`. + #[allow(clippy::too_many_arguments)] + fn find_head_walk( + &self, + start_index: usize, + current_slot: Slot, + best_justified_checkpoint: Checkpoint, + best_finalized_checkpoint: Checkpoint, + proposer_boost_root: Hash256, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result { + let mut head = IndexedForkChoiceNode { + root: best_justified_checkpoint.root, + proto_node_index: start_index, + 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. + let apply_proposer_boost = + self.should_apply_proposer_boost::(proposer_boost_root, justified_balances, spec)?; + + loop { + let children: Vec<_> = self + .get_node_children(&head, &children_index)? + .into_iter() + .filter(|(fc_node, _)| viable_nodes.contains(&fc_node.proto_node_index)) + .collect(); + + if children.is_empty() { + return Ok(head); + } + + 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))?; + let weight = self.get_weight::( + &child, + proto_node, + apply_proposer_boost, + proposer_boost_root, + current_slot, + justified_balances, + spec, + )?; + let payload_status_tiebreaker = self.get_payload_status_tiebreaker::( + &child, + proto_node, + current_slot, + proposer_boost_root, + )?; + Ok((child, weight, payload_status_tiebreaker)) + }) + .collect::, Error>>()? + .into_iter() + .max_by_key(|(child, weight, payload_status_tiebreaker)| { + (*weight, child.root, *payload_status_tiebreaker) + }) + .map(|(child, _, _)| child) + .ok_or(Error::NoViableChildren)?; + } + } + + /// Spec: `get_weight`. + #[allow(clippy::too_many_arguments)] + fn get_weight( + &self, + fc_node: &IndexedForkChoiceNode, + proto_node: &ProtoNode, + apply_proposer_boost: bool, + proposer_boost_root: Hash256, + current_slot: Slot, + justified_balances: &JustifiedBalances, + spec: &ChainSpec, + ) -> Result { + if fc_node.payload_status == PayloadStatus::Pending + || proto_node.slot().saturating_add(1_u64) != current_slot + { + let attestation_score = proto_node.attestation_score(fc_node.payload_status); + + if !apply_proposer_boost { + return Ok(attestation_score); + } + + // Spec: proposer boost is treated as a synthetic vote. + let message = LatestMessage { + slot: current_slot, + root: proposer_boost_root, + payload_present: false, + }; + let proposer_score = if self.is_supporting_vote(fc_node, &message)? { + get_proposer_score::(justified_balances, spec)? + } else { + 0 + }; + + Ok(attestation_score.saturating_add(proposer_score)) + } else { + Ok(0) + } + } + + /// Spec: `is_supporting_vote`. + fn is_supporting_vote( + &self, + node: &IndexedForkChoiceNode, + message: &LatestMessage, + ) -> Result { + let block = self + .nodes + .get(node.proto_node_index) + .ok_or(Error::InvalidNodeIndex(node.proto_node_index))?; + + if node.root == message.root { + if node.payload_status == PayloadStatus::Pending { + return Ok(true); + } + // For the proposer boost case: message.slot == current_slot == block.slot, + // so this returns false — boost does not support EMPTY/FULL of the + // boosted block itself, only its ancestors. + if message.slot <= block.slot() { + return Ok(false); + } + if message.payload_present { + Ok(node.payload_status == PayloadStatus::Full) + } else { + Ok(node.payload_status == PayloadStatus::Empty) + } + } else { + let ancestor = self.get_ancestor_node(message.root, block.slot())?; + Ok(node.root == ancestor.root + && (node.payload_status == PayloadStatus::Pending + || node.payload_status == ancestor.payload_status)) + } + } + + /// Spec: `get_ancestor` (modified to return ForkChoiceNode with payload_status). + fn get_ancestor_node(&self, root: Hash256, slot: Slot) -> Result { + let index = *self.indices.get(&root).ok_or(Error::NodeUnknown(root))?; + let block = self + .nodes + .get(index) + .ok_or(Error::InvalidNodeIndex(index))?; + + if block.slot() <= slot { + return Ok(IndexedForkChoiceNode { + root, + proto_node_index: index, + payload_status: PayloadStatus::Pending, + }); + } + + // Walk up until we find the ancestor at `slot`. + let mut child_index = index; + let mut current_index = block.parent().ok_or(Error::NodeUnknown(block.root()))?; + + loop { + let current = self + .nodes + .get(current_index) + .ok_or(Error::InvalidNodeIndex(current_index))?; + + if current.slot() <= slot { + let child = self + .nodes + .get(child_index) + .ok_or(Error::InvalidNodeIndex(child_index))?; + return Ok(IndexedForkChoiceNode { + root: current.root(), + proto_node_index: current_index, + payload_status: child.get_parent_payload_status(), + }); + } + + child_index = current_index; + current_index = current.parent().ok_or(Error::NodeUnknown(current.root()))?; + } + } + + /// Spec: `get_node_children`. + fn get_node_children( + &self, + node: &IndexedForkChoiceNode, + children_index: &[Vec], + ) -> Result, Error> { + if node.payload_status == PayloadStatus::Pending { + let proto_node = self + .nodes + .get(node.proto_node_index) + .ok_or(Error::InvalidNodeIndex(node.proto_node_index))?; + + // V17 (pre-GLOAS) nodes don't have payload_received or parent_payload_status. + // Skip the virtual Empty/Full split and return real children directly. + if proto_node.as_v17().is_ok() { + let child_indices = children_index + .get(node.proto_node_index) + .map(|c| c.as_slice()) + .unwrap_or(&[]); + return Ok(child_indices + .iter() + .filter_map(|&child_index| { + let child_node = self.nodes.get(child_index)?; + Some(( + IndexedForkChoiceNode { + root: child_node.root(), + proto_node_index: child_index, + payload_status: PayloadStatus::Pending, + }, + child_node.clone(), + )) + }) + .collect()); + } + + // TODO(gloas) this is the actual change we want to keep once PTC is implemented + // let mut children = vec![(node.with_status(PayloadStatus::Empty), proto_node.clone())]; + // // The FULL virtual child only exists if the payload has been received. + // if proto_node.payload_received().is_ok_and(|received| received) { + // children.push((node.with_status(PayloadStatus::Full), proto_node.clone())); + // } + + // TODO(gloas) remove this and uncomment the code above once we implement PTC + // Skip Empty/Full split: go straight to Full when payload received, + // giving full payload weight 100% without PTC votes. + let children = if proto_node.payload_received().is_ok_and(|received| received) { + vec![(node.with_status(PayloadStatus::Full), proto_node.clone())] + } else { + vec![(node.with_status(PayloadStatus::Empty), proto_node.clone())] + }; + // TODO(gloas) delete up to here + + Ok(children) + } else { + let child_indices = children_index + .get(node.proto_node_index) + .map(|c| c.as_slice()) + .unwrap_or(&[]); + Ok(child_indices + .iter() + .filter_map(|&child_index| { + let child_node = self.nodes.get(child_index)?; + // Skip parent_payload_status filter for V17 children (they don't have it) + if child_node.get_parent_payload_status() != node.payload_status { + return None; + } + Some(( + IndexedForkChoiceNode { + root: child_node.root(), + proto_node_index: child_index, + payload_status: PayloadStatus::Pending, + }, + child_node.clone(), + )) + }) + .collect()) + } + } + + fn get_payload_status_tiebreaker( + &self, + fc_node: &IndexedForkChoiceNode, + proto_node: &ProtoNode, + current_slot: Slot, + proposer_boost_root: Hash256, + ) -> Result { + if fc_node.payload_status == PayloadStatus::Pending + || proto_node.slot().saturating_add(1_u64) != current_slot + { + Ok(fc_node.payload_status as u8) + } else if fc_node.payload_status == PayloadStatus::Empty { + Ok(1) + } else if self.should_extend_payload::(fc_node, proto_node, proposer_boost_root)? { + Ok(2) + } else { + Ok(0) + } + } + + fn should_extend_payload( + &self, + fc_node: &IndexedForkChoiceNode, + proto_node: &ProtoNode, + proposer_boost_root: Hash256, + ) -> Result { + // Per spec: `proposer_root == Root()` is one of the `or` conditions that + // makes `should_extend_payload` return True. + if proposer_boost_root.is_zero() { + return Ok(true); + } + + let proposer_boost_node_index = *self + .indices + .get(&proposer_boost_root) + .ok_or(Error::NodeUnknown(proposer_boost_root))?; + let proposer_boost_node = self + .nodes + .get(proposer_boost_node_index) + .ok_or(Error::InvalidNodeIndex(proposer_boost_node_index))?; + + let parent_index = proposer_boost_node + .parent() + .ok_or(Error::NodeUnknown(proposer_boost_root))?; + let proposer_boost_parent_root = self + .nodes + .get(parent_index) + .ok_or(Error::InvalidNodeIndex(parent_index))? + .root(); + + Ok( + (proto_node.is_payload_timely::() && proto_node.is_payload_data_available::()) + || proposer_boost_parent_root != fc_node.root + || proposer_boost_node.is_parent_node_full(), + ) } /// Update the tree with new finalization information. The tree is only actually pruned if both @@ -721,7 +1586,7 @@ impl ProtoArray { .nodes .get(node_index) .ok_or(Error::InvalidNodeIndex(node_index))? - .root; + .root(); self.indices.remove(root); } @@ -738,176 +1603,15 @@ impl ProtoArray { // Iterate through all the existing nodes and adjust their indices to match the new layout // of `self.nodes`. for node in self.nodes.iter_mut() { - if let Some(parent) = node.parent { + if let Some(parent) = node.parent() { // If `node.parent` is less than `finalized_index`, set it to `None`. - node.parent = parent.checked_sub(finalized_index); - } - if let Some(best_child) = node.best_child { - node.best_child = Some( - best_child - .checked_sub(finalized_index) - .ok_or(Error::IndexOverflow("best_child"))?, - ); - } - if let Some(best_descendant) = node.best_descendant { - node.best_descendant = Some( - best_descendant - .checked_sub(finalized_index) - .ok_or(Error::IndexOverflow("best_descendant"))?, - ); + *node.parent_mut() = parent.checked_sub(finalized_index); } } Ok(()) } - /// Observe the parent at `parent_index` with respect to the child at `child_index` and - /// potentially modify the `parent.best_child` and `parent.best_descendant` values. - /// - /// ## Detail - /// - /// There are four outcomes: - /// - /// - The child is already the best child but it's now invalid due to a FFG change and should be removed. - /// - The child is already the best child and the parent is updated with the new - /// best-descendant. - /// - The child is not the best child but becomes the best child. - /// - The child is not the best child and does not become the best child. - fn maybe_update_best_child_and_descendant( - &mut self, - parent_index: usize, - child_index: usize, - current_slot: Slot, - best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, - ) -> Result<(), Error> { - let child = self - .nodes - .get(child_index) - .ok_or(Error::InvalidNodeIndex(child_index))?; - - let parent = self - .nodes - .get(parent_index) - .ok_or(Error::InvalidNodeIndex(parent_index))?; - - let child_leads_to_viable_head = self.node_leads_to_viable_head::( - child, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )?; - - // These three variables are aliases to the three options that we may set the - // `parent.best_child` and `parent.best_descendant` to. - // - // I use the aliases to assist readability. - let change_to_none = (None, None); - let change_to_child = ( - Some(child_index), - child.best_descendant.or(Some(child_index)), - ); - let no_change = (parent.best_child, parent.best_descendant); - - let (new_best_child, new_best_descendant) = - if let Some(best_child_index) = parent.best_child { - if best_child_index == child_index && !child_leads_to_viable_head { - // If the child is already the best-child of the parent but it's not viable for - // the head, remove it. - change_to_none - } else if best_child_index == child_index { - // If the child is the best-child already, set it again to ensure that the - // best-descendant of the parent is updated. - change_to_child - } else { - let best_child = self - .nodes - .get(best_child_index) - .ok_or(Error::InvalidBestDescendant(best_child_index))?; - - let best_child_leads_to_viable_head = self.node_leads_to_viable_head::( - best_child, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )?; - - if child_leads_to_viable_head && !best_child_leads_to_viable_head { - // The child leads to a viable head, but the current best-child doesn't. - change_to_child - } else if !child_leads_to_viable_head && best_child_leads_to_viable_head { - // The best child leads to a viable head, but the child doesn't. - no_change - } else if child.weight == best_child.weight { - // Tie-breaker of equal weights by root. - if child.root >= best_child.root { - change_to_child - } else { - no_change - } - } else { - // Choose the winner by weight. - if child.weight > best_child.weight { - change_to_child - } else { - no_change - } - } - } - } else if child_leads_to_viable_head { - // There is no current best-child and the child is viable. - change_to_child - } else { - // There is no current best-child but the child is not viable. - no_change - }; - - let parent = self - .nodes - .get_mut(parent_index) - .ok_or(Error::InvalidNodeIndex(parent_index))?; - - parent.best_child = new_best_child; - parent.best_descendant = new_best_descendant; - - Ok(()) - } - - /// Indicates if the node itself is viable for the head, or if its best descendant is viable - /// for the head. - fn node_leads_to_viable_head( - &self, - node: &ProtoNode, - current_slot: Slot, - best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, - ) -> Result { - let best_descendant_is_viable_for_head = - if let Some(best_descendant_index) = node.best_descendant { - let best_descendant = self - .nodes - .get(best_descendant_index) - .ok_or(Error::InvalidBestDescendant(best_descendant_index))?; - - self.node_is_viable_for_head::( - best_descendant, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - ) - } else { - false - }; - - Ok(best_descendant_is_viable_for_head - || self.node_is_viable_for_head::( - node, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )) - } - /// This is the equivalent to the `filter_block_tree` function in the eth2 spec: /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.10.0/specs/phase0/fork-choice.md#filter_block_tree @@ -921,25 +1625,27 @@ impl ProtoArray { best_justified_checkpoint: Checkpoint, best_finalized_checkpoint: Checkpoint, ) -> bool { - if node.execution_status.is_invalid() { + if let Ok(proto_node) = node.as_v17() + && proto_node.execution_status.is_invalid() + { return false; } let genesis_epoch = Epoch::new(0); let current_epoch = current_slot.epoch(E::slots_per_epoch()); - let node_epoch = node.slot.epoch(E::slots_per_epoch()); - let node_justified_checkpoint = node.justified_checkpoint; + let node_epoch = node.slot().epoch(E::slots_per_epoch()); + let node_justified_checkpoint = node.justified_checkpoint(); let voting_source = if current_epoch > node_epoch { // The block is from a prior epoch, the voting source will be pulled-up. - node.unrealized_justified_checkpoint + node.unrealized_justified_checkpoint() // Sometimes we don't track the unrealized justification. In // that case, just use the fully-realized justified checkpoint. - .unwrap_or(node_justified_checkpoint) + .unwrap_or(*node_justified_checkpoint) } else { // The block is not from a prior epoch, therefore the voting source // is not pulled up. - node_justified_checkpoint + *node_justified_checkpoint }; let correct_justified = best_justified_checkpoint.epoch == genesis_epoch @@ -948,7 +1654,7 @@ impl ProtoArray { let correct_finalized = best_finalized_checkpoint.epoch == genesis_epoch || self - .is_finalized_checkpoint_or_descendant::(node.root, best_finalized_checkpoint); + .is_finalized_checkpoint_or_descendant::(node.root(), best_finalized_checkpoint); correct_justified && correct_finalized } @@ -970,7 +1676,7 @@ impl ProtoArray { block_root: &Hash256, ) -> impl Iterator + 'a { self.iter_nodes(block_root) - .map(|node| (node.root, node.slot)) + .map(|node| (node.root(), node.slot())) } /// Returns `true` if the `descendant_root` has an ancestor with `ancestor_root`. Always @@ -991,8 +1697,8 @@ impl ProtoArray { .and_then(|ancestor_index| self.nodes.get(*ancestor_index)) .and_then(|ancestor| { self.iter_block_roots(&descendant_root) - .take_while(|(_root, slot)| *slot >= ancestor.slot) - .find(|(_root, slot)| *slot == ancestor.slot) + .take_while(|(_root, slot)| *slot >= ancestor.slot()) + .find(|(_root, slot)| *slot == ancestor.slot()) .map(|(root, _slot)| root == ancestor_root) }) .unwrap_or(false) @@ -1031,15 +1737,15 @@ impl ProtoArray { // Run this check once, outside of the loop rather than inside the loop. // If the conditions don't match for this node then they're unlikely to // start matching for its ancestors. - for checkpoint in &[node.finalized_checkpoint, node.justified_checkpoint] { - if checkpoint == &best_finalized_checkpoint { + for checkpoint in &[node.finalized_checkpoint(), node.justified_checkpoint()] { + if **checkpoint == best_finalized_checkpoint { return true; } } for checkpoint in &[ - node.unrealized_finalized_checkpoint, - node.unrealized_justified_checkpoint, + node.unrealized_finalized_checkpoint(), + node.unrealized_justified_checkpoint(), ] { if checkpoint.is_some_and(|cp| cp == best_finalized_checkpoint) { return true; @@ -1049,13 +1755,13 @@ impl ProtoArray { loop { // If `node` is less than or equal to the finalized slot then `node` // must be the finalized block. - if node.slot <= finalized_slot { - return node.root == finalized_root; + if node.slot() <= finalized_slot { + return node.root() == finalized_root; } // Since `node` is from a higher slot that the finalized checkpoint, // replace `node` with the parent of `node`. - if let Some(parent) = node.parent.and_then(|index| self.nodes.get(index)) { + if let Some(parent) = node.parent().and_then(|index| self.nodes.get(index)) { node = parent } else { // If `node` is not the finalized block and its parent does not @@ -1077,11 +1783,12 @@ impl ProtoArray { .iter() .rev() .find(|node| { - node.execution_status - .block_hash() + node.execution_status() + .ok() + .and_then(|execution_status| execution_status.block_hash()) .is_some_and(|node_block_hash| node_block_hash == *block_hash) }) - .map(|node| node.root) + .map(|node| node.root()) } /// Returns all nodes that have zero children and are descended from the finalized checkpoint. @@ -1095,13 +1802,17 @@ impl ProtoArray { ) -> Vec<&ProtoNode> { self.nodes .iter() - .filter(|node| { - node.best_child.is_none() + .enumerate() + .filter(|(i, node)| { + // TODO(gloas): we unoptimized this for Gloas fork choice, could re-optimize. + let num_children = self.nodes.iter().filter(|n| n.parent() == Some(*i)).count(); + num_children == 0 && self.is_finalized_checkpoint_or_descendant::( - node.root, + node.root(), best_finalized_checkpoint, ) }) + .map(|(_, node)| node) .collect() } } @@ -1121,6 +1832,31 @@ pub fn calculate_committee_fraction( .checked_div(100) } +/// Spec: `get_proposer_score`. +fn get_proposer_score( + justified_balances: &JustifiedBalances, + spec: &ChainSpec, +) -> Result { + let Some(proposer_score_boost) = spec.proposer_score_boost else { + return Ok(0); + }; + calculate_committee_fraction::(justified_balances, proposer_score_boost) + .ok_or(Error::ProposerBoostOverflow(0)) +} + +/// Apply a signed delta to an unsigned weight, returning an error on overflow. +fn apply_delta(weight: u64, delta: i64, index: usize) -> Result { + if delta < 0 { + weight + .checked_sub(delta.unsigned_abs()) + .ok_or(Error::DeltaOverflow(index)) + } else { + weight + .checked_add(delta as u64) + .ok_or(Error::DeltaOverflow(index)) + } +} + /// Reverse iterator over one path through a `ProtoArray`. pub struct Iter<'a> { next_node_index: Option, @@ -1133,7 +1869,7 @@ impl<'a> Iterator for Iter<'a> { fn next(&mut self) -> Option { let next_node_index = self.next_node_index?; let node = self.proto_array.nodes.get(next_node_index)?; - self.next_node_index = node.parent; + self.next_node_index = node.parent(); Some(node) } } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 3edf1e0644..cb467f2531 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -2,7 +2,7 @@ use crate::{ JustifiedBalances, error::Error, proto_array::{ - InvalidationOperation, Iter, ProposerBoost, ProtoArray, ProtoNode, + InvalidationOperation, Iter, NodeDelta, ProposerBoost, ProtoArray, ProtoNode, calculate_committee_fraction, }, ssz_container::SszContainer, @@ -14,10 +14,11 @@ use ssz_derive::{Decode, Encode}; use std::{ collections::{BTreeSet, HashMap}, fmt, + time::Duration, }; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, - Slot, + Slot, StatePayloadStatus, }; pub const DEFAULT_PRUNE_THRESHOLD: usize = 256; @@ -26,10 +27,19 @@ pub const DEFAULT_PRUNE_THRESHOLD: usize = 256; pub struct VoteTracker { current_root: Hash256, next_root: Hash256, - next_epoch: Epoch, + current_slot: Slot, + next_slot: Slot, + current_payload_present: bool, + next_payload_present: bool, } -/// Represents the verification status of an execution payload. +pub struct LatestMessage { + pub slot: Slot, + pub root: Hash256, + pub payload_present: bool, +} + +/// Represents the verification status of an execution payload pre-Gloas. #[derive(Clone, Copy, Debug, PartialEq, Encode, Decode, Serialize, Deserialize)] #[ssz(enum_behaviour = "union")] pub enum ExecutionStatus { @@ -49,6 +59,46 @@ pub enum ExecutionStatus { Irrelevant(bool), } +/// Represents the status of an execution payload post-Gloas. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Encode, Decode, Serialize, Deserialize)] +#[ssz(enum_behaviour = "tag")] +#[repr(u8)] +pub enum PayloadStatus { + Empty = 0, + Full = 1, + Pending = 2, +} + +impl PayloadStatus { + /// Convert a `PayloadStatus` into the equivalent `StatePayloadStatus`. + /// + /// This maps `Empty` onto `StatePayloadStatus::Pending` because empty and pending fork choice + /// nodes correspond to the exact same state. + pub fn as_state_payload_status(self) -> StatePayloadStatus { + match self { + Self::Empty | Self::Pending => StatePayloadStatus::Pending, + Self::Full => StatePayloadStatus::Full, + } + } +} + +/// Spec's `ForkChoiceNode` augmented with ProtoNode index. +pub struct IndexedForkChoiceNode { + pub root: Hash256, + pub proto_node_index: usize, + pub payload_status: PayloadStatus, +} + +impl IndexedForkChoiceNode { + pub fn with_status(&self, payload_status: PayloadStatus) -> Self { + Self { + root: self.root, + proto_node_index: self.proto_node_index, + payload_status, + } + } +} + impl ExecutionStatus { pub fn is_execution_enabled(&self) -> bool { !matches!(self, ExecutionStatus::Irrelevant(_)) @@ -159,6 +209,11 @@ pub struct Block { pub execution_status: ExecutionStatus, pub unrealized_justified_checkpoint: Option, pub unrealized_finalized_checkpoint: Option, + + /// post-Gloas fields + pub execution_payload_parent_hash: Option, + pub execution_payload_block_hash: Option, + pub proposer_index: Option, } impl Block { @@ -422,6 +477,9 @@ impl ProtoArrayForkChoice { current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, execution_status: ExecutionStatus, + execution_payload_parent_hash: Option, + execution_payload_block_hash: Option, + spec: &ChainSpec, ) -> Result { let mut proto_array = ProtoArray { prune_threshold: DEFAULT_PRUNE_THRESHOLD, @@ -445,14 +503,20 @@ impl ProtoArrayForkChoice { execution_status, unrealized_justified_checkpoint: Some(justified_checkpoint), unrealized_finalized_checkpoint: Some(finalized_checkpoint), + execution_payload_parent_hash, + execution_payload_block_hash, + proposer_index: Some(0), }; proto_array .on_block::( block, current_slot, - justified_checkpoint, - finalized_checkpoint, + spec, + // Anchor block is always timely (delay=0 ensures both timeliness + // checks pass). Combined with `is_genesis` override in on_block, + // this matches spec's `block_timeliness = {anchor: [True, True]}`. + Duration::ZERO, ) .map_err(|e| format!("Failed to add finalized block to proto_array: {:?}", e))?; @@ -463,6 +527,11 @@ impl ProtoArrayForkChoice { }) } + pub fn on_execution_payload(&mut self, block_root: Hash256) -> Result<(), String> { + self.proto_array + .on_valid_execution_payload(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, @@ -488,36 +557,71 @@ impl ProtoArrayForkChoice { &mut self, validator_index: usize, block_root: Hash256, - target_epoch: Epoch, + attestation_slot: Slot, + payload_present: bool, ) -> Result<(), String> { let vote = self.votes.get_mut(validator_index); - if target_epoch > vote.next_epoch || *vote == VoteTracker::default() { + if attestation_slot > vote.next_slot || *vote == VoteTracker::default() { vote.next_root = block_root; - vote.next_epoch = target_epoch; + vote.next_slot = attestation_slot; + vote.next_payload_present = payload_present; } Ok(()) } + /// Process a PTC vote by setting the appropriate bits on the target block's V29 node. + /// + /// `ptc_index` is the voter's position in the PTC committee (resolved by the caller). + /// This writes directly to the node's bitfields, bypassing the delta pipeline. + pub fn process_payload_attestation( + &mut self, + block_root: Hash256, + ptc_index: usize, + payload_present: bool, + blob_data_available: bool, + ) -> Result<(), String> { + let node_index = self + .proto_array + .indices + .get(&block_root) + .copied() + .ok_or_else(|| { + format!("process_payload_attestation: unknown block root {block_root:?}") + })?; + let node = self.proto_array.nodes.get_mut(node_index).ok_or_else(|| { + format!("process_payload_attestation: invalid node index {node_index}") + })?; + let v29 = node + .as_v29_mut() + .map_err(|_| format!("process_payload_attestation: node {block_root:?} is not V29"))?; + + 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(()) + } + pub fn process_block( &mut self, block: Block, current_slot: Slot, - justified_checkpoint: Checkpoint, - finalized_checkpoint: Checkpoint, + spec: &ChainSpec, + time_into_slot: Duration, ) -> Result<(), String> { if block.parent_root.is_none() { return Err("Missing parent root".to_string()); } self.proto_array - .on_block::( - block, - current_slot, - justified_checkpoint, - finalized_checkpoint, - ) + .on_block::(block, current_slot, spec, time_into_slot) .map_err(|e| format!("process_block_error: {:?}", e)) } @@ -531,12 +635,19 @@ impl ProtoArrayForkChoice { equivocating_indices: &BTreeSet, current_slot: Slot, spec: &ChainSpec, - ) -> Result { + ) -> Result<(Hash256, PayloadStatus), String> { let old_balances = &mut self.balances; let new_balances = justified_state_balances; + let node_slots = self + .proto_array + .nodes + .iter() + .map(|node| node.slot()) + .collect::>(); let deltas = compute_deltas( &self.proto_array.indices, + &node_slots, &mut self.votes, &old_balances.effective_balances, &new_balances.effective_balances, @@ -545,15 +656,7 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("find_head compute_deltas failed: {:?}", e))?; self.proto_array - .apply_score_changes::( - deltas, - justified_checkpoint, - finalized_checkpoint, - new_balances, - proposer_boost_root, - current_slot, - spec, - ) + .apply_score_changes::(deltas) .map_err(|e| format!("find_head apply_score_changes failed: {:?}", e))?; *old_balances = new_balances.clone(); @@ -564,6 +667,9 @@ impl ProtoArrayForkChoice { current_slot, justified_checkpoint, finalized_checkpoint, + proposer_boost_root, + new_balances, + spec, ) .map_err(|e| format!("find_head failed: {:?}", e)) } @@ -593,13 +699,13 @@ impl ProtoArrayForkChoice { )?; // Only re-org a single slot. This prevents cascading failures during asynchrony. - let head_slot_ok = info.head_node.slot + 1 == current_slot; + let head_slot_ok = info.head_node.slot().saturating_add(1_u64) == current_slot; if !head_slot_ok { return Err(DoNotReOrg::HeadDistance.into()); } // Only re-org if the head's weight is less than the heads configured committee fraction. - let head_weight = info.head_node.weight; + let head_weight = info.head_node.weight(); let re_org_head_weight_threshold = info.re_org_head_weight_threshold; let weak_head = head_weight < re_org_head_weight_threshold; if !weak_head { @@ -610,8 +716,10 @@ impl ProtoArrayForkChoice { .into()); } - // Only re-org if the parent's weight is greater than the parents configured committee fraction. - let parent_weight = info.parent_node.weight; + // Spec: `is_parent_strong`. Use payload-aware weight matching the + // payload path the head node is on from its parent. + let parent_payload_status = info.head_node.get_parent_payload_status(); + let parent_weight = info.parent_node.attestation_score(parent_payload_status); let re_org_parent_weight_threshold = info.re_org_parent_weight_threshold; let parent_strong = parent_weight > re_org_parent_weight_threshold; if !parent_strong { @@ -650,14 +758,14 @@ impl ProtoArrayForkChoice { let parent_node = nodes.pop().ok_or(DoNotReOrg::MissingHeadOrParentNode)?; let head_node = nodes.pop().ok_or(DoNotReOrg::MissingHeadOrParentNode)?; - let parent_slot = parent_node.slot; - let head_slot = head_node.slot; - let re_org_block_slot = head_slot + 1; + let parent_slot = parent_node.slot(); + let head_slot = head_node.slot(); + let re_org_block_slot = head_slot.saturating_add(1_u64); // Check finalization distance. let proposal_epoch = re_org_block_slot.epoch(E::slots_per_epoch()); let finalized_epoch = head_node - .unrealized_finalized_checkpoint + .unrealized_finalized_checkpoint() .ok_or(DoNotReOrg::MissingHeadFinalizedCheckpoint)? .epoch; let epochs_since_finalization = proposal_epoch.saturating_sub(finalized_epoch).as_u64(); @@ -689,10 +797,10 @@ impl ProtoArrayForkChoice { } // Check FFG. - let ffg_competitive = parent_node.unrealized_justified_checkpoint - == head_node.unrealized_justified_checkpoint - && parent_node.unrealized_finalized_checkpoint - == head_node.unrealized_finalized_checkpoint; + let ffg_competitive = parent_node.unrealized_justified_checkpoint() + == head_node.unrealized_justified_checkpoint() + && parent_node.unrealized_finalized_checkpoint() + == head_node.unrealized_finalized_checkpoint(); if !ffg_competitive { return Err(DoNotReOrg::JustificationAndFinalizationNotCompetitive.into()); } @@ -720,10 +828,10 @@ impl ProtoArrayForkChoice { /// This will operate on *all* blocks, even those that do not descend from the finalized /// ancestor. pub fn contains_invalid_payloads(&mut self) -> bool { - self.proto_array - .nodes - .iter() - .any(|node| node.execution_status.is_invalid()) + self.proto_array.nodes.iter().any(|node| { + node.execution_status() + .is_ok_and(|status| status.is_invalid()) + }) } /// For all nodes, regardless of their relationship to the finalized block, set their execution @@ -748,9 +856,11 @@ impl ProtoArrayForkChoice { .get_mut(node_index) .ok_or("unreachable index out of bounds in proto_array nodes")?; - match node.execution_status { - ExecutionStatus::Invalid(block_hash) => { - node.execution_status = ExecutionStatus::Optimistic(block_hash); + match node.execution_status() { + Ok(ExecutionStatus::Invalid(block_hash)) => { + if let ProtoNode::V17(node) = node { + node.execution_status = ExecutionStatus::Optimistic(block_hash); + } // Restore the weight of the node, it would have been set to `0` in // `apply_score_changes` when it was invalidated. @@ -760,7 +870,7 @@ impl ProtoArrayForkChoice { .iter() .enumerate() .filter_map(|(validator_index, vote)| { - if vote.current_root == node.root { + if vote.current_root == node.root() { // Any voting validator that does not have a balance should be // ignored. This is consistent with `compute_deltas`. self.balances.effective_balances.get(validator_index) @@ -773,7 +883,7 @@ impl ProtoArrayForkChoice { // 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 + && 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 @@ -794,12 +904,12 @@ impl ProtoArrayForkChoice { if restored_weight > 0 { let mut node_or_ancestor = node; loop { - node_or_ancestor.weight = node_or_ancestor - .weight + *node_or_ancestor.weight_mut() = node_or_ancestor + .weight() .checked_add(restored_weight) .ok_or("Overflow when adding weight to ancestor")?; - if let Some(parent_index) = node_or_ancestor.parent { + if let Some(parent_index) = node_or_ancestor.parent() { node_or_ancestor = self .proto_array .nodes @@ -815,11 +925,14 @@ impl ProtoArrayForkChoice { } // There are no balance changes required if the node was either valid or // optimistic. - ExecutionStatus::Valid(block_hash) | ExecutionStatus::Optimistic(block_hash) => { - node.execution_status = ExecutionStatus::Optimistic(block_hash) + Ok(ExecutionStatus::Valid(block_hash)) + | Ok(ExecutionStatus::Optimistic(block_hash)) => { + if let ProtoNode::V17(node) = node { + node.execution_status = ExecutionStatus::Optimistic(block_hash) + } } // An irrelevant node cannot become optimistic, this is a no-op. - ExecutionStatus::Irrelevant(_) => (), + Ok(ExecutionStatus::Irrelevant(_)) | Err(_) => (), } } @@ -856,30 +969,47 @@ impl ProtoArrayForkChoice { pub fn get_block(&self, block_root: &Hash256) -> Option { let block = self.get_proto_node(block_root)?; let parent_root = block - .parent + .parent() .and_then(|i| self.proto_array.nodes.get(i)) - .map(|parent| parent.root); + .map(|parent| parent.root()); Some(Block { - slot: block.slot, - root: block.root, + slot: block.slot(), + root: block.root(), parent_root, - state_root: block.state_root, - target_root: block.target_root, - current_epoch_shuffling_id: block.current_epoch_shuffling_id.clone(), - next_epoch_shuffling_id: block.next_epoch_shuffling_id.clone(), - justified_checkpoint: block.justified_checkpoint, - finalized_checkpoint: block.finalized_checkpoint, - execution_status: block.execution_status, - unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, - unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + state_root: block.state_root(), + target_root: block.target_root(), + current_epoch_shuffling_id: block.current_epoch_shuffling_id().clone(), + next_epoch_shuffling_id: block.next_epoch_shuffling_id().clone(), + justified_checkpoint: *block.justified_checkpoint(), + finalized_checkpoint: *block.finalized_checkpoint(), + execution_status: block + .execution_status() + .unwrap_or_else(|_| ExecutionStatus::irrelevant()), + unrealized_justified_checkpoint: block.unrealized_justified_checkpoint(), + unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint(), + execution_payload_parent_hash: None, + execution_payload_block_hash: block.execution_payload_block_hash().ok(), + proposer_index: block.proposer_index().ok(), }) } /// Returns the `block.execution_status` field, if the block is present. pub fn get_block_execution_status(&self, block_root: &Hash256) -> Option { let block = self.get_proto_node(block_root)?; - Some(block.execution_status) + Some( + block + .execution_status() + .unwrap_or_else(|_| ExecutionStatus::irrelevant()), + ) + } + + /// 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. @@ -888,9 +1018,11 @@ impl ProtoArrayForkChoice { self.proto_array .nodes .get(*block_index) - .map(|node| node.weight) + .map(|node| node.weight()) } + /// Returns the payload status of the head node based on accumulated weights and tiebreaker. + /// /// See `ProtoArray` documentation. pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool { self.proto_array @@ -907,14 +1039,18 @@ impl ProtoArrayForkChoice { .is_finalized_checkpoint_or_descendant::(descendant_root, best_finalized_checkpoint) } - pub fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Epoch)> { + pub fn latest_message(&self, validator_index: usize) -> Option { if validator_index < self.votes.0.len() { let vote = &self.votes.0[validator_index]; if *vote == VoteTracker::default() { None } else { - Some((vote.next_root, vote.next_epoch)) + Some(LatestMessage { + root: vote.next_root, + slot: vote.next_slot, + payload_present: vote.next_payload_present, + }) } } else { None @@ -934,21 +1070,12 @@ impl ProtoArrayForkChoice { self.proto_array.iter_block_roots(block_root) } - pub fn as_ssz_container( - &self, - justified_checkpoint: Checkpoint, - finalized_checkpoint: Checkpoint, - ) -> SszContainer { - SszContainer::from_proto_array(self, justified_checkpoint, finalized_checkpoint) + pub fn as_ssz_container(&self) -> SszContainer { + SszContainer::from_proto_array(self) } - pub fn as_bytes( - &self, - justified_checkpoint: Checkpoint, - finalized_checkpoint: Checkpoint, - ) -> Vec { - self.as_ssz_container(justified_checkpoint, finalized_checkpoint) - .as_ssz_bytes() + pub fn as_bytes(&self) -> Vec { + self.as_ssz_container().as_ssz_bytes() } pub fn from_bytes(bytes: &[u8], balances: JustifiedBalances) -> Result { @@ -1002,12 +1129,28 @@ impl ProtoArrayForkChoice { /// always valid). fn compute_deltas( indices: &HashMap, + node_slots: &[Slot], votes: &mut ElasticList, old_balances: &[u64], new_balances: &[u64], equivocating_indices: &BTreeSet, -) -> Result, Error> { - let mut deltas = vec![0_i64; indices.len()]; +) -> Result, Error> { + let block_slot = |index: usize| -> Result { + node_slots + .get(index) + .copied() + .ok_or(Error::InvalidNodeDelta(index)) + }; + + let mut deltas = vec![ + NodeDelta { + delta: 0, + empty_delta: 0, + full_delta: 0, + equivocating_attestation_delta: 0, + }; + indices.len() + ]; for (val_index, vote) in votes.iter_mut().enumerate() { // There is no need to create a score change if the validator has never voted or both their @@ -1032,17 +1175,30 @@ fn compute_deltas( let old_balance = old_balances.get(val_index).copied().unwrap_or(0); if let Some(current_delta_index) = indices.get(&vote.current_root).copied() { - let delta = deltas - .get(current_delta_index) - .ok_or(Error::InvalidNodeDelta(current_delta_index))? + let node_delta = deltas + .get_mut(current_delta_index) + .ok_or(Error::InvalidNodeDelta(current_delta_index))?; + node_delta.delta = node_delta + .delta .checked_sub(old_balance as i64) .ok_or(Error::DeltaOverflow(current_delta_index))?; - // Array access safe due to check on previous line. - deltas[current_delta_index] = delta; + let status = NodeDelta::payload_status( + vote.current_slot, + vote.current_payload_present, + block_slot(current_delta_index)?, + ); + node_delta.sub_payload_delta(status, old_balance, current_delta_index)?; + + // Track equivocating weight for `is_head_weak` monotonicity. + node_delta.equivocating_attestation_delta = node_delta + .equivocating_attestation_delta + .saturating_add(old_balance); } vote.current_root = Hash256::zero(); + vote.current_slot = Slot::new(0); + vote.current_payload_present = false; } // We've handled this slashed validator, continue without applying an ordinary delta. continue; @@ -1059,34 +1215,52 @@ fn compute_deltas( // on-boarded less validators than the prior fork. let new_balance = new_balances.get(val_index).copied().unwrap_or(0); - if vote.current_root != vote.next_root || old_balance != new_balance { + if vote.current_root != vote.next_root + || old_balance != new_balance + || vote.current_payload_present != vote.next_payload_present + || vote.current_slot != vote.next_slot + { // We ignore the vote if it is not known in `indices`. We assume that it is outside // of our tree (i.e., pre-finalization) and therefore not interesting. if let Some(current_delta_index) = indices.get(&vote.current_root).copied() { - let delta = deltas - .get(current_delta_index) - .ok_or(Error::InvalidNodeDelta(current_delta_index))? + let node_delta = deltas + .get_mut(current_delta_index) + .ok_or(Error::InvalidNodeDelta(current_delta_index))?; + node_delta.delta = node_delta + .delta .checked_sub(old_balance as i64) .ok_or(Error::DeltaOverflow(current_delta_index))?; - // Array access safe due to check on previous line. - deltas[current_delta_index] = delta; + let status = NodeDelta::payload_status( + vote.current_slot, + vote.current_payload_present, + block_slot(current_delta_index)?, + ); + node_delta.sub_payload_delta(status, old_balance, current_delta_index)?; } // We ignore the vote if it is not known in `indices`. We assume that it is outside // of our tree (i.e., pre-finalization) and therefore not interesting. if let Some(next_delta_index) = indices.get(&vote.next_root).copied() { - let delta = deltas - .get(next_delta_index) - .ok_or(Error::InvalidNodeDelta(next_delta_index))? + let node_delta = deltas + .get_mut(next_delta_index) + .ok_or(Error::InvalidNodeDelta(next_delta_index))?; + node_delta.delta = node_delta + .delta .checked_add(new_balance as i64) .ok_or(Error::DeltaOverflow(next_delta_index))?; - // Array access safe due to check on previous line. - deltas[next_delta_index] = delta; + let status = NodeDelta::payload_status( + vote.next_slot, + vote.next_payload_present, + block_slot(next_delta_index)?, + ); + node_delta.add_payload_delta(status, new_balance, next_delta_index)?; } vote.current_root = vote.next_root; + vote.current_slot = vote.next_slot; + vote.current_payload_present = vote.next_payload_present; } } @@ -1104,8 +1278,13 @@ mod test_compute_deltas { Hash256::from_low_u64_be(i as u64 + 1) } + fn test_node_slots(count: usize) -> Vec { + vec![Slot::new(0); count] + } + #[test] fn finalized_descendant() { + let spec = MainnetEthSpec::default_spec(); let genesis_slot = Slot::new(0); let genesis_epoch = Epoch::new(0); @@ -1136,6 +1315,9 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, + None, + None, + &spec, ) .unwrap(); @@ -1152,13 +1334,16 @@ mod test_compute_deltas { next_epoch_shuffling_id: junk_shuffling_id.clone(), justified_checkpoint: genesis_checkpoint, finalized_checkpoint: genesis_checkpoint, - execution_status, unrealized_justified_checkpoint: Some(genesis_checkpoint), + execution_status, unrealized_finalized_checkpoint: Some(genesis_checkpoint), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + proposer_index: Some(0), }, genesis_slot + 1, - genesis_checkpoint, - genesis_checkpoint, + &spec, + Duration::ZERO, ) .unwrap(); @@ -1180,10 +1365,13 @@ mod test_compute_deltas { execution_status, unrealized_justified_checkpoint: None, unrealized_finalized_checkpoint: None, + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + proposer_index: Some(0), }, genesis_slot + 1, - genesis_checkpoint, - genesis_checkpoint, + &spec, + Duration::ZERO, ) .unwrap(); @@ -1259,6 +1447,7 @@ mod test_compute_deltas { /// *checkpoint*, not just the finalized *block*. #[test] fn finalized_descendant_edge_case() { + let spec = MainnetEthSpec::default_spec(); let get_block_root = Hash256::from_low_u64_be; let genesis_slot = Slot::new(0); let junk_state_root = Hash256::zero(); @@ -1280,6 +1469,9 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, + None, + None, + &spec, ) .unwrap(); @@ -1308,10 +1500,13 @@ mod test_compute_deltas { execution_status, unrealized_justified_checkpoint: Some(genesis_checkpoint), unrealized_finalized_checkpoint: Some(genesis_checkpoint), + execution_payload_parent_hash: None, + execution_payload_block_hash: None, + proposer_index: Some(0), }, Slot::from(block.slot), - genesis_checkpoint, - genesis_checkpoint, + &spec, + Duration::ZERO, ) .unwrap(); }; @@ -1414,7 +1609,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: Hash256::zero(), next_root: Hash256::zero(), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); old_balances.push(0); new_balances.push(0); @@ -1422,6 +1620,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1465,7 +1664,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: Hash256::zero(), next_root: hash_from_index(0), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); old_balances.push(BALANCE); new_balances.push(BALANCE); @@ -1473,6 +1675,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1523,7 +1726,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: Hash256::zero(), next_root: hash_from_index(i), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); old_balances.push(BALANCE); new_balances.push(BALANCE); @@ -1531,6 +1737,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1576,7 +1783,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(0), next_root: hash_from_index(1), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); old_balances.push(BALANCE); new_balances.push(BALANCE); @@ -1584,6 +1794,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1640,18 +1851,25 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(1), next_root: Hash256::zero(), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); // One validator moves their vote from the block to something outside the tree. votes.0.push(VoteTracker { current_root: hash_from_index(1), next_root: Hash256::from_low_u64_be(1337), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1693,7 +1911,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(0), next_root: hash_from_index(1), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); old_balances.push(OLD_BALANCE); new_balances.push(NEW_BALANCE); @@ -1701,6 +1922,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1762,12 +1984,16 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(1), next_root: hash_from_index(2), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); } let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1818,12 +2044,16 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(1), next_root: hash_from_index(2), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); } let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1872,7 +2102,10 @@ mod test_compute_deltas { votes.0.push(VoteTracker { current_root: hash_from_index(1), next_root: hash_from_index(2), - next_epoch: Epoch::new(0), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: false, }); } @@ -1881,6 +2114,7 @@ mod test_compute_deltas { let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &old_balances, &new_balances, @@ -1910,6 +2144,7 @@ mod test_compute_deltas { // Re-computing the deltas should be a no-op (no repeat deduction for the slashed validator). let deltas = compute_deltas( &indices, + &test_node_slots(indices.len()), &mut votes, &new_balances, &new_balances, @@ -1918,4 +2153,68 @@ mod test_compute_deltas { .expect("should compute deltas"); assert_eq!(deltas, vec![0, 0]); } + + #[test] + fn payload_bucket_changes_on_non_pending_vote() { + const BALANCE: u64 = 42; + + let mut indices = HashMap::new(); + indices.insert(hash_from_index(1), 0); + + let node_slots = vec![Slot::new(0)]; + let mut votes = ElasticList(vec![VoteTracker { + current_root: hash_from_index(1), + next_root: hash_from_index(1), + current_slot: Slot::new(1), + next_slot: Slot::new(1), + current_payload_present: false, + next_payload_present: true, + }]); + + let deltas = compute_deltas( + &indices, + &node_slots, + &mut votes, + &[BALANCE], + &[BALANCE], + &BTreeSet::new(), + ) + .expect("should compute deltas"); + + assert_eq!(deltas[0].delta, 0); + assert_eq!(deltas[0].empty_delta, -(BALANCE as i64)); + assert_eq!(deltas[0].full_delta, BALANCE as i64); + } + + #[test] + fn pending_vote_only_updates_regular_weight() { + const BALANCE: u64 = 42; + + let mut indices = HashMap::new(); + indices.insert(hash_from_index(1), 0); + + let node_slots = vec![Slot::new(0)]; + let mut votes = ElasticList(vec![VoteTracker { + current_root: hash_from_index(1), + next_root: hash_from_index(1), + current_slot: Slot::new(0), + next_slot: Slot::new(0), + current_payload_present: false, + next_payload_present: true, + }]); + + let deltas = compute_deltas( + &indices, + &node_slots, + &mut votes, + &[BALANCE], + &[BALANCE], + &BTreeSet::new(), + ) + .expect("should compute deltas"); + + assert_eq!(deltas[0].delta, 0); + assert_eq!(deltas[0].empty_delta, 0); + assert_eq!(deltas[0].full_delta, 0); + } } diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 42696256f7..5edc1cd313 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -1,7 +1,7 @@ use crate::proto_array::ProposerBoost; use crate::{ Error, JustifiedBalances, - proto_array::{ProtoArray, ProtoNodeV17}, + proto_array::{ProtoArray, ProtoNode, ProtoNodeV17}, proto_array_fork_choice::{ElasticList, ProtoArrayForkChoice, VoteTracker}, }; use ssz::{Encode, four_byte_option_impl}; @@ -14,10 +14,10 @@ use types::{Checkpoint, Hash256}; // selector. four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); -pub type SszContainer = SszContainerV28; +pub type SszContainer = SszContainerV29; #[superstruct( - variants(V28), + variants(V28, V29), variant_attributes(derive(Encode, Decode, Clone)), no_enum )] @@ -25,27 +25,26 @@ pub struct SszContainer { pub votes: Vec, pub prune_threshold: usize, // Deprecated, remove in a future schema migration + #[superstruct(only(V28))] justified_checkpoint: Checkpoint, // Deprecated, remove in a future schema migration + #[superstruct(only(V28))] finalized_checkpoint: Checkpoint, + #[superstruct(only(V28))] pub nodes: Vec, + #[superstruct(only(V29))] + pub nodes: Vec, pub indices: Vec<(Hash256, usize)>, pub previous_proposer_boost: ProposerBoost, } -impl SszContainer { - pub fn from_proto_array( - from: &ProtoArrayForkChoice, - justified_checkpoint: Checkpoint, - finalized_checkpoint: Checkpoint, - ) -> Self { +impl SszContainerV29 { + pub fn from_proto_array(from: &ProtoArrayForkChoice) -> Self { let proto_array = &from.proto_array; Self { votes: from.votes.0.clone(), prune_threshold: proto_array.prune_threshold, - justified_checkpoint, - finalized_checkpoint, nodes: proto_array.nodes.clone(), indices: proto_array.indices.iter().map(|(k, v)| (*k, *v)).collect(), previous_proposer_boost: proto_array.previous_proposer_boost, @@ -53,10 +52,10 @@ impl SszContainer { } } -impl TryFrom<(SszContainer, JustifiedBalances)> for ProtoArrayForkChoice { +impl TryFrom<(SszContainerV29, JustifiedBalances)> for ProtoArrayForkChoice { type Error = Error; - fn try_from((from, balances): (SszContainer, JustifiedBalances)) -> Result { + fn try_from((from, balances): (SszContainerV29, JustifiedBalances)) -> Result { let proto_array = ProtoArray { prune_threshold: from.prune_threshold, nodes: from.nodes, @@ -71,3 +70,40 @@ impl TryFrom<(SszContainer, JustifiedBalances)> for ProtoArrayForkChoice { }) } } + +// Convert legacy V28 to current V29. +impl From for SszContainerV29 { + fn from(v28: SszContainerV28) -> Self { + Self { + votes: v28.votes, + prune_threshold: v28.prune_threshold, + nodes: v28.nodes.into_iter().map(ProtoNode::V17).collect(), + indices: v28.indices, + previous_proposer_boost: v28.previous_proposer_boost, + } + } +} + +// Downgrade current V29 to legacy V28 (lossy: V29 nodes lose payload-specific fields). +impl From for SszContainerV28 { + fn from(v29: SszContainerV29) -> Self { + Self { + votes: v29.votes, + prune_threshold: v29.prune_threshold, + // These checkpoints are not consumed in v28 paths since the upgrade from v17, + // we can safely default the values. + justified_checkpoint: Checkpoint::default(), + finalized_checkpoint: Checkpoint::default(), + nodes: v29 + .nodes + .into_iter() + .filter_map(|node| match node { + ProtoNode::V17(v17) => Some(v17), + ProtoNode::V29(_) => None, + }) + .collect(), + indices: v29.indices, + previous_proposer_boost: v29.previous_proposer_boost, + } + } +} diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 07a7d4c6b6..22e8453e14 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -30,7 +30,8 @@ use types::{ Attestation, AttestationRef, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, BlobSidecar, BlobsList, BlockImportSource, Checkpoint, DataColumnSidecar, DataColumnSidecarList, DataColumnSubnetId, ExecutionBlockHash, Hash256, IndexedAttestation, - KzgProof, ProposerPreparationData, SignedBeaconBlock, Slot, Uint256, + KzgProof, ProposerPreparationData, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, + Uint256, }; // When set to true, cache any states fetched from the db. @@ -72,6 +73,7 @@ pub struct Checks { proposer_boost_root: Option, get_proposer_head: Option, should_override_forkchoice_update: Option, + head_payload_status: Option, } #[derive(Debug, Clone, Deserialize)] @@ -94,7 +96,15 @@ impl From for PayloadStatusV1 { #[derive(Debug, Clone, Deserialize)] #[serde(untagged, deny_unknown_fields)] -pub enum Step { +pub enum Step< + TBlock, + TBlobs, + TColumns, + TAttestation, + TAttesterSlashing, + TPowBlock, + TExecutionPayload = String, +> { Tick { tick: u64, }, @@ -128,6 +138,10 @@ pub enum Step, valid: bool, }, + OnExecutionPayload { + execution_payload: TExecutionPayload, + valid: bool, + }, } #[derive(Debug, Clone, Deserialize)] @@ -151,6 +165,7 @@ pub struct ForkChoiceTest { Attestation, AttesterSlashing, PowBlock, + SignedExecutionPayloadEnvelope, >, >, } @@ -271,6 +286,17 @@ impl LoadCase for ForkChoiceTest { valid, }) } + Step::OnExecutionPayload { + execution_payload, + valid, + } => { + let envelope = + ssz_decode_file(&path.join(format!("{execution_payload}.ssz_snappy")))?; + Ok(Step::OnExecutionPayload { + execution_payload: envelope, + valid, + }) + } }) .collect::>()?; let anchor_state = ssz_decode_state(&path.join("anchor_state.ssz_snappy"), spec)?; @@ -359,6 +385,7 @@ impl Case for ForkChoiceTest { proposer_boost_root, get_proposer_head, should_override_forkchoice_update: should_override_fcu, + head_payload_status, } = checks.as_ref(); if let Some(expected_head) = head { @@ -405,6 +432,10 @@ impl Case for ForkChoiceTest { if let Some(expected_proposer_head) = get_proposer_head { tester.check_expected_proposer_head(*expected_proposer_head)?; } + + if let Some(expected_status) = head_payload_status { + tester.check_head_payload_status(*expected_status)?; + } } Step::MaybeValidBlockAndColumns { @@ -414,6 +445,12 @@ impl Case for ForkChoiceTest { } => { tester.process_block_and_columns(block.clone(), columns.clone(), *valid)?; } + Step::OnExecutionPayload { + execution_payload, + valid, + } => { + tester.process_execution_payload(execution_payload, *valid)?; + } } } @@ -584,6 +621,18 @@ impl Tester { self.apply_invalid_block(&block)?; } + // Per spec test runner: an on_block step implies receiving block's attestations + // and attester slashings. + if success { + for attestation in block.message().body().attestations() { + let att = attestation.clone_as_attestation(); + let _ = self.process_attestation(&att); + } + for attester_slashing in block.message().body().attester_slashings() { + self.process_attester_slashing(attester_slashing); + } + } + Ok(()) } @@ -674,6 +723,18 @@ impl Tester { self.apply_invalid_block(&block)?; } + // Per spec test runner: an on_block step implies receiving block's attestations + // and attester slashings. + if success { + for attestation in block.message().body().attestations() { + let att = attestation.clone_as_attestation(); + let _ = self.process_attestation(&att); + } + for attester_slashing in block.message().body().attester_slashings() { + self.process_attester_slashing(attester_slashing); + } + } + Ok(()) } @@ -913,7 +974,7 @@ impl Tester { ) -> Result<(), Error> { let mut fc = self.harness.chain.canonical_head.fork_choice_write_lock(); let slot = self.harness.chain.slot().unwrap(); - let canonical_head = fc.get_head(slot, &self.harness.spec).unwrap(); + let (canonical_head, _) = fc.get_head(slot, &self.harness.spec).unwrap(); let proposer_head_result = fc.get_proposer_head( slot, canonical_head, @@ -923,7 +984,7 @@ impl Tester { DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, ); let proposer_head = match proposer_head_result { - Ok(head) => head.parent_node.root, + Ok(head) => head.parent_node.root(), Err(ProposerHeadError::DoNotReOrg(_)) => canonical_head, _ => panic!("Unexpected error in get proposer head"), }; @@ -931,6 +992,58 @@ impl Tester { check_equal("proposer_head", proposer_head, expected_proposer_head) } + pub fn process_execution_payload( + &self, + signed_envelope: &SignedExecutionPayloadEnvelope, + valid: bool, + ) -> Result<(), Error> { + let block_root = signed_envelope.message.beacon_block_root; + + // Store the envelope in the database so that child blocks extending + // the FULL path can load the parent's post-payload state. + if valid { + self.harness + .chain + .store + .put_payload_envelope(&block_root, signed_envelope.clone()) + .map_err(|e| { + Error::InternalError(format!( + "Failed to store payload envelope for {block_root:?}: {e:?}", + )) + })?; + } + + let result = self + .harness + .chain + .canonical_head + .fork_choice_write_lock() + .on_execution_payload(block_root); + + if valid { + result.map_err(|e| { + Error::InternalError(format!( + "on_execution_payload for block root {} failed: {:?}", + block_root, e + )) + })?; + } else if result.is_ok() { + return Err(Error::DidntFail(format!( + "on_execution_payload for block root {} should have failed", + block_root + ))); + } + + Ok(()) + } + + pub fn check_head_payload_status(&self, expected_status: u8) -> Result<(), Error> { + let head = self.find_head()?; + // PayloadStatus repr: Empty=0, Full=1, Pending=2 (matches spec constants). + let actual = head.head_payload_status() as u8; + check_equal("head_payload_status", actual, expected_status) + } + pub fn check_should_override_fcu( &self, expected_should_override_fcu: ShouldOverrideFcu, diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index f8c16aec0b..4373d6b7d1 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -704,15 +704,27 @@ impl Handler for ForkChoiceHandler { return false; } - // No FCU override tests prior to bellatrix. + // No FCU override tests prior to bellatrix, and removed in Gloas. if self.handler_name == "should_override_forkchoice_update" - && !fork_name.bellatrix_enabled() + && (!fork_name.bellatrix_enabled() || fork_name.gloas_enabled()) { return false; } - // Deposit tests exist only after Electra. - if self.handler_name == "deposit_with_reorg" && !fork_name.electra_enabled() { + // Deposit tests exist only for Electra and Fulu (not Gloas). + if self.handler_name == "deposit_with_reorg" + && (!fork_name.electra_enabled() || fork_name.gloas_enabled()) + { + return false; + } + + // Proposer head tests removed in Gloas. + if self.handler_name == "get_proposer_head" && fork_name.gloas_enabled() { + return false; + } + + // on_execution_payload tests exist only for Gloas. + if self.handler_name == "on_execution_payload" && !fork_name.gloas_enabled() { return false; } @@ -722,8 +734,7 @@ impl Handler for ForkChoiceHandler { } fn disabled_forks(&self) -> Vec { - // TODO(gloas): remove once we have Gloas fork choice tests - vec![ForkName::Gloas] + vec![] } } diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 3254bb6e90..62eb2dd038 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -1038,6 +1038,12 @@ fn fork_choice_deposit_with_reorg() { // There is no mainnet variant for this test. } +#[test] +fn fork_choice_on_execution_payload() { + ForkChoiceHandler::::new("on_execution_payload").run(); + ForkChoiceHandler::::new("on_execution_payload").run(); +} + #[test] fn optimistic_sync() { OptimisticSyncHandler::::default().run();