From ddfc26512308aa23a80f701b03f8fc0adfbcdd34 Mon Sep 17 00:00:00 2001 From: hopinheimer <48147533+hopinheimer@users.noreply.github.com> Date: Thu, 18 Jun 2026 04:57:13 -0400 Subject: [PATCH] Enable late re-org and re-org interactive tests (#9405) https://github.com/sigp/lighthouse/issues/8959 WIP still working on adding more re-org tests and refactoring existing. Co-Authored-By: hopinheimer Co-Authored-By: Michael Sproul --- beacon_node/beacon_chain/src/beacon_chain.rs | 21 +- .../beacon_chain/src/block_production/mod.rs | 79 +- .../payload_attestation_verification/tests.rs | 167 +++- beacon_node/beacon_chain/src/test_utils.rs | 298 +++++- .../http_api/tests/gloas_reorg_tests.rs | 946 ++++++++++++++++++ .../http_api/tests/interactive_tests.rs | 13 +- beacon_node/http_api/tests/main.rs | 1 + consensus/proto_array/src/proto_array.rs | 4 + .../src/proto_array_fork_choice.rs | 11 +- 9 files changed, 1480 insertions(+), 60 deletions(-) create mode 100644 beacon_node/http_api/tests/gloas_reorg_tests.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f09b9b0520..d175c54be7 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5138,7 +5138,6 @@ impl BeaconChain { }) } - // TODO(gloas): wrong for Gloas, needs an update pub fn overridden_forkchoice_update_params_or_failure_reason( &self, canonical_forkchoice_params: &ForkchoiceUpdateParameters, @@ -5169,6 +5168,11 @@ impl BeaconChain { ) .map_err(|e| e.map_inner_error(Error::ProposerHeadForkChoiceError))?; + // We don't need to override fork choice updates for Gloas. + if info.head_node.is_gloas() { + return Ok(*canonical_forkchoice_params); + } + // 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(); @@ -5302,9 +5306,7 @@ impl BeaconChain { return Err(Box::new(DoNotReOrg::NotProposing.into())); } - // 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. + // This only works pre-Gloas, but we don't run this code for Gloas anyway. let parent_head_hash = info .parent_node .execution_status() @@ -6341,8 +6343,15 @@ impl BeaconChain { } let canonical_fcu_params = cached_head.forkchoice_update_parameters(); - let fcu_params = - chain.overridden_forkchoice_update_params(canonical_fcu_params)?; + let fcu_params = if chain + .spec + .fork_name_at_slot::(head_slot) + .gloas_enabled() + { + canonical_fcu_params + } else { + chain.overridden_forkchoice_update_params(canonical_fcu_params)? + }; let pre_payload_attributes = chain.get_pre_payload_attributes( prepare_slot, fcu_params.head_root, diff --git a/beacon_node/beacon_chain/src/block_production/mod.rs b/beacon_node/beacon_chain/src/block_production/mod.rs index 84fb886b8f..1f29a47f69 100644 --- a/beacon_node/beacon_chain/src/block_production/mod.rs +++ b/beacon_node/beacon_chain/src/block_production/mod.rs @@ -4,7 +4,7 @@ use fork_choice::PayloadStatus; use proto_array::{ProposerHeadError, ReOrgThreshold}; use slot_clock::SlotClock; use tracing::{debug, error, info, instrument, warn}; -use types::{BeaconState, Epoch, Hash256, SignedExecutionPayloadEnvelope, Slot}; +use types::{BeaconState, Epoch, EthSpec, Hash256, SignedExecutionPayloadEnvelope, Slot}; use crate::{ BeaconChain, BeaconChainTypes, BlockProductionError, StateSkipConfig, @@ -14,13 +14,21 @@ use crate::{ mod gloas; /// State loaded from the database for block production. -pub(crate) struct BlockProductionState { +pub(crate) struct BlockProductionState { pub state: BeaconState, pub state_root: Option, pub parent_payload_status: PayloadStatus, pub parent_envelope: Option>>, } +/// Inputs assembled for producing a block via a proposer re-org. +struct ReOrgInputs { + state: BeaconState, + state_root: Hash256, + parent_payload_status: PayloadStatus, + parent_envelope: Option>>, +} + impl BeaconChain { /// Load a beacon state from the database for block production. This is a long-running process /// that should not be performed in an `async` context. @@ -50,39 +58,32 @@ impl BeaconChain { head.snapshot.execution_envelope.clone(), ) }; + let result = if head_slot < slot { // Attempt an aggressive re-org if configured and the conditions are right. - // TODO(gloas): re-enable reorgs - let gloas_enabled = self - .spec - .fork_name_at_slot::(slot) - .gloas_enabled(); - if !gloas_enabled - && let Some((re_org_state, re_org_state_root)) = - self.get_state_for_re_org(slot, head_slot, head_block_root) - { + if let Some(inputs) = self.get_state_for_re_org(slot, head_slot, head_block_root) { info!( %slot, head_to_reorg = %head_block_root, "Proposing block to re-org current head" ); - // TODO(gloas): ensure we use a sensible payload status when we enable reorgs - // for Gloas BlockProductionState { - state: re_org_state, - state_root: Some(re_org_state_root), - parent_payload_status: PayloadStatus::Pending, - parent_envelope: None, + state: inputs.state, + state_root: Some(inputs.state_root), + parent_payload_status: inputs.parent_payload_status, + parent_envelope: inputs.parent_envelope, } } else { - // Fetch the head state advanced through to `slot`, which should be present in the - // state cache thanks to the state advance timer. + // Continuation: the new block builds on the current head. Fetch the head state + // advanced through to `slot`, which should be present in the state cache thanks to + // the state advance timer. let parent_state_root = head_state_root; let (state_root, state) = self .store .get_advanced_hot_state(head_block_root, slot, parent_state_root) .map_err(BlockProductionError::FailedToLoadState)? .ok_or(BlockProductionError::UnableToProduceAtSlot(slot))?; + BlockProductionState { state, state_root: Some(state_root), @@ -100,13 +101,11 @@ impl BeaconChain { .state_at_slot(slot - 1, StateSkipConfig::WithStateRoots) .map_err(|_| BlockProductionError::UnableToProduceAtSlot(slot))?; - // TODO(gloas): update this to read payload canonicity from fork choice once ready - let parent_payload_status = PayloadStatus::Pending; BlockProductionState { state, state_root: None, - parent_payload_status, - parent_envelope: None, + parent_payload_status: head_payload_status, + parent_envelope: head_envelope, } }; @@ -173,7 +172,7 @@ impl BeaconChain { slot: Slot, head_slot: Slot, canonical_head: Hash256, - ) -> Option<(BeaconState, Hash256)> { + ) -> Option> { let re_org_head_threshold = ReOrgThreshold(self.spec.reorg_head_weight_threshold); let re_org_parent_threshold = ReOrgThreshold(self.spec.reorg_parent_weight_threshold); let re_org_max_epochs_since_finalization = @@ -237,9 +236,15 @@ impl BeaconChain { } }) .ok()?; + drop(proposer_head_timer); let re_org_parent_block = proposer_head.parent_node.root(); + // The head uniquely determines the parent payload status for the re-org block, whichever + // variant (full or empty) it builds on must have more weight, or else we would have already + // re-orged away from this block naturally, and it would not be the head, by definition. + let parent_payload_status = proposer_head.head_node.get_parent_payload_status(); + let (state_root, state) = self .store .get_advanced_hot_state_from_cache(re_org_parent_block, slot) @@ -248,6 +253,25 @@ impl BeaconChain { None })?; + let parent_envelope = if parent_payload_status == PayloadStatus::Full { + let envelope = self + .store + .get_payload_envelope(&re_org_parent_block) + .ok() + .flatten() + .map(Arc::new) + .or_else(|| { + warn!( + reason = "missing execution payload envelope", + "Not attempting re-org" + ); + None + })?; + Some(envelope) + } else { + None + }; + info!( weak_head = ?canonical_head, parent = ?re_org_parent_block, @@ -256,6 +280,11 @@ impl BeaconChain { "Attempting re-org due to weak head" ); - Some((state, state_root)) + Some(ReOrgInputs { + state, + state_root, + parent_payload_status, + parent_envelope, + }) } } diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs index 6c52e5ce2d..01cee2cdb6 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs @@ -15,7 +15,10 @@ use crate::{ GossipVerificationContext, VerifiedPayloadAttestationMessage, }, }, - test_utils::{BeaconChainHarness, EphemeralHarnessType, fork_name_from_env, test_spec}, + test_utils::{ + BeaconChainHarness, EphemeralHarnessType, MakePayloadAttestationOptions, + PayloadAttestationVote, fork_name_from_env, test_spec, + }, }; type E = MinimalEthSpec; @@ -30,6 +33,10 @@ struct TestContext { impl TestContext { fn new() -> Self { + Self::with_validator_count(NUM_VALIDATORS) + } + + fn with_validator_count(num_validators: usize) -> Self { let spec = Arc::new(test_spec::()); let slot_clock = TestingSlotClock::new( Slot::new(0), @@ -38,8 +45,9 @@ impl TestContext { ); let harness = BeaconChainHarness::builder(E::default()) .spec(spec) - .deterministic_keypairs(NUM_VALIDATORS) + .deterministic_keypairs(num_validators) .fresh_ephemeral_store() + .mock_execution_layer() .testing_slot_clock(slot_clock) .build(); @@ -289,6 +297,161 @@ fn duplicate_after_valid() { )); } +#[tokio::test] +async fn harness_builds_and_imports_payload_attestation_messages() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + let slot = Slot::new(1); + let beacon_block_root = ctx.harness.extend_to_slot(slot).await; + let state = &ctx.harness.chain.head_snapshot().beacon_state; + assert_eq!(state.slot(), slot); + let ptc = state.get_ptc(slot, &ctx.harness.spec).unwrap(); + let mut ptc_weights = std::collections::HashMap::new(); + for validator_index in ptc.0.iter().copied() { + *ptc_weights.entry(validator_index).or_insert(0usize) += 1; + } + let votes = vec![ + PayloadAttestationVote { + validator_count: 2, + payload_present: true, + blob_data_available: true, + }, + PayloadAttestationVote { + validator_count: 3, + payload_present: false, + blob_data_available: false, + }, + ]; + + let (messages, attesters) = ctx.harness.make_payload_attestation_messages_with_opts( + &ctx.harness.get_all_validators(), + state, + beacon_block_root, + slot, + MakePayloadAttestationOptions { + votes, + fork: state.fork(), + }, + ); + + assert_eq!(messages.len(), attesters.len()); + assert_eq!( + attesters + .iter() + .copied() + .collect::>() + .len(), + attesters.len() + ); + assert_eq!( + messages + .iter() + .filter(|message| message.data.payload_present && message.data.blob_data_available) + .map(|message| ptc_weights[&(message.validator_index as usize)]) + .sum::(), + 2 + ); + assert_eq!( + messages + .iter() + .filter(|message| !message.data.payload_present && !message.data.blob_data_available) + .map(|message| ptc_weights[&(message.validator_index as usize)]) + .sum::(), + 3 + ); + + let pool_count_before = ctx.harness.chain.op_pool.num_payload_attestation_messages(); + ctx.harness + .import_payload_attestation_messages(messages) + .expect("payload attestation messages should import"); + assert_eq!( + ctx.harness.chain.op_pool.num_payload_attestation_messages(), + pool_count_before + attesters.len() + ); +} + +#[tokio::test] +async fn harness_packs_payload_attestation_messages_by_ptc_weight() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + let slot = Slot::new(1); + let beacon_block_root = ctx.harness.extend_to_slot(slot).await; + let state = &ctx.harness.chain.head_snapshot().beacon_state; + assert_eq!(state.slot(), slot); + let ptc = state.get_ptc(slot, &ctx.harness.spec).unwrap(); + let mut ptc_weights = std::collections::HashMap::new(); + let mut ptc_validator_order = vec![]; + for validator_index in ptc.0.iter().copied() { + if let Some(weight) = ptc_weights.get_mut(&validator_index) { + *weight += 1; + } else { + ptc_weights.insert(validator_index, 1usize); + ptc_validator_order.push(validator_index); + } + } + let mut sorted_ptc_validators = ptc_validator_order + .into_iter() + .enumerate() + .map(|(order, validator_index)| (validator_index, ptc_weights[&validator_index], order)) + .collect::>(); + sorted_ptc_validators.sort_by(|(_, weight_a, order_a), (_, weight_b, order_b)| { + weight_b.cmp(weight_a).then(order_a.cmp(order_b)) + }); + let first_weight = sorted_ptc_validators + .first() + .map(|(_, weight, _)| *weight) + .expect("PTC should have at least one validator"); + assert!(first_weight > 1, "test requires a duplicate PTC member"); + let second_weight = sorted_ptc_validators + .iter() + .skip(1) + .map(|(_, weight, _)| *weight) + .next() + .expect("PTC should have at least two distinct validators"); + let requested_weight = first_weight + second_weight; + + let (messages, attesters) = ctx.harness.make_payload_attestation_messages_with_opts( + &ctx.harness.get_all_validators(), + state, + beacon_block_root, + slot, + MakePayloadAttestationOptions { + votes: vec![PayloadAttestationVote { + validator_count: requested_weight, + payload_present: true, + blob_data_available: true, + }], + fork: state.fork(), + }, + ); + + assert!( + messages.len() < requested_weight, + "duplicate PTC positions should pack into fewer messages" + ); + assert_eq!(messages.len(), attesters.len()); + assert_eq!( + attesters + .iter() + .map(|validator_index| ptc_weights[validator_index]) + .sum::(), + requested_weight + ); + assert!( + attesters + .iter() + .any(|validator_index| ptc_weights[validator_index] > 1) + ); + + ctx.harness + .import_payload_attestation_messages(messages) + .expect("weighted payload attestation messages should import"); +} + #[tokio::test] async fn ptc_cache_is_primed_at_gloas_fork_boundary() { // Only run this test once, when FORK_NAME=gloas exactly. diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 62c7fb3a45..2adfe26d4b 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -43,6 +43,7 @@ use logging::create_test_tracing_subscriber; use merkle_proof::MerkleTree; use operation_pool::ReceivedPreCapella; use parking_lot::{Mutex, RwLockWriteGuard}; +use proto_array::PayloadStatus; use rand::Rng; use rand::SeedableRng; use rand::rngs::StdRng; @@ -752,11 +753,37 @@ pub type HarnessSingleAttestations = Vec<( Option>, )>; +pub type HarnessPayloadAttestationMessages = Vec; + pub type HarnessSyncContributions = Vec<( Vec<(SyncCommitteeMessage, usize)>, Option>, )>; +fn pack_payload_attestation_vote( + available_ptc_validators: &[(usize, usize, usize)], + requested_weight: usize, +) -> Option> { + let mut packs = vec![None::>; requested_weight.checked_add(1)?]; + packs[0] = Some(vec![]); + + for (offset, (_, weight, _)) in available_ptc_validators.iter().enumerate() { + if *weight > requested_weight { + continue; + } + + for weight_so_far in (0..=requested_weight - *weight).rev() { + if packs[weight_so_far].is_some() && packs[weight_so_far + *weight].is_none() { + let mut pack = packs[weight_so_far].as_ref()?.clone(); + pack.push(offset); + packs[weight_so_far + *weight] = Some(pack); + } + } + } + + packs.pop().flatten() +} + impl BeaconChainHarness> where E: EthSpec, @@ -1164,9 +1191,33 @@ where /// /// For pre-Gloas forks, the envelope is `None` and this behaves like `make_block`. pub async fn make_block_with_envelope( + &self, + state: BeaconState, + slot: Slot, + ) -> ( + SignedBlockContentsTuple, + Option>, + BeaconState, + ) { + let parent_payload_status = self + .chain + .canonical_head + .cached_head() + .head_payload_status(); + self.make_block_with_envelope_on(state, slot, parent_payload_status) + .await + } + + /// Returns a newly created block built with the given parent payload status, + /// signed by the proposer for the given slot, along with the execution + /// payload envelope (for Gloas) and the post-block state. + /// + /// For pre-Gloas forks, the envelope is `None` and this behaves like `make_block`. + pub async fn make_block_with_envelope_on( &self, mut state: BeaconState, slot: Slot, + parent_payload_status: PayloadStatus, ) -> ( SignedBlockContentsTuple, Option>, @@ -1189,15 +1240,21 @@ where GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); - // Load the parent's payload envelope and status from the cached head. - // TODO(gloas): we may want to pass these as arguments to support cases where we build - // on alternate chains to the head. - let (parent_payload_status, parent_envelope) = { - let head = self.chain.canonical_head.cached_head(); - ( - head.head_payload_status(), - head.snapshot.execution_envelope.clone(), - ) + let parent_envelope = if parent_payload_status == PayloadStatus::Full { + let parent_root = if state.slot() > 0 { + *state + .get_block_root(state.slot() - 1) + .expect("should get parent block root") + } else { + state.latest_block_header().canonical_root() + }; + self.chain + .store + .get_payload_envelope(&parent_root) + .expect("should load parent payload envelope") + .map(Arc::new) + } else { + None }; let (block, post_block_state, _consensus_block_value) = self @@ -2151,6 +2208,169 @@ where ) } + pub fn make_payload_attestation_message( + &self, + validator_index: usize, + data: PayloadAttestationData, + fork: &Fork, + ) -> PayloadAttestationMessage { + let epoch = data.slot.epoch(E::slots_per_epoch()); + let domain = self.spec.get_domain( + epoch, + Domain::PTCAttester, + fork, + self.chain.genesis_validators_root, + ); + let signing_root = data.signing_root(domain); + let signature = self.validator_keypairs[validator_index] + .sk + .sign(signing_root); + + PayloadAttestationMessage { + validator_index: validator_index as u64, + data, + signature, + } + } + + pub fn make_payload_attestation_messages( + &self, + state: &BeaconState, + beacon_block_root: Hash256, + slot: Slot, + votes: Vec, + ) -> (HarnessPayloadAttestationMessages, Vec) { + let fork = self.spec.fork_at_epoch(slot.epoch(E::slots_per_epoch())); + self.make_payload_attestation_messages_with_opts( + &self.get_all_validators(), + state, + beacon_block_root, + slot, + MakePayloadAttestationOptions { votes, fork }, + ) + } + + pub fn make_payload_attestation_messages_with_opts( + &self, + attesting_validators: &[usize], + state: &BeaconState, + beacon_block_root: Hash256, + slot: Slot, + opts: MakePayloadAttestationOptions, + ) -> (HarnessPayloadAttestationMessages, Vec) { + let MakePayloadAttestationOptions { votes, fork } = opts; + let ptc = state + .get_ptc(slot, &self.spec) + .expect("should get payload timeliness committee"); + + debug!("PTC is {:?}", ptc.0.to_vec()); + + let attesting_validators = attesting_validators.iter().copied().collect::>(); + let mut ptc_weights = HashMap::new(); + let mut ptc_validator_order = vec![]; + for validator_index in ptc + .0 + .iter() + .copied() + .filter(|validator_index| attesting_validators.contains(validator_index)) + { + if let Some(weight) = ptc_weights.get_mut(&validator_index) { + *weight += 1; + } else { + ptc_weights.insert(validator_index, 1usize); + ptc_validator_order.push(validator_index); + } + } + + let mut available_ptc_validators = ptc_validator_order + .into_iter() + .enumerate() + .map(|(order, validator_index)| { + let weight = ptc_weights[&validator_index]; + (validator_index, weight, order) + }) + .collect::>(); + available_ptc_validators.sort_by(|(_, weight_a, order_a), (_, weight_b, order_b)| { + weight_b.cmp(weight_a).then(order_a.cmp(order_b)) + }); + + let mut messages = Vec::new(); + let mut attesters = Vec::new(); + + for vote in votes { + let data = PayloadAttestationData { + beacon_block_root, + slot, + payload_present: vote.payload_present, + blob_data_available: vote.blob_data_available, + }; + + let Some(packed_validator_offsets) = + pack_payload_attestation_vote(&available_ptc_validators, vote.validator_count) + else { + let available_weights = available_ptc_validators + .iter() + .map(|(validator_index, weight, _)| (*validator_index, *weight)) + .collect::>(); + panic!( + "requested packing couldn't be formed for payload attestation vote {vote:?}; \ + requested PTC weight {}, available PTC weights {:?}", + vote.validator_count, available_weights + ); + }; + + for &offset in &packed_validator_offsets { + let validator_index = available_ptc_validators[offset].0; + messages.push(self.make_payload_attestation_message( + validator_index, + data.clone(), + &fork, + )); + attesters.push(validator_index); + } + + for offset in packed_validator_offsets.into_iter().rev() { + available_ptc_validators.remove(offset); + } + } + + (messages, attesters) + } + + pub fn import_payload_attestation_message( + &self, + message: PayloadAttestationMessage, + ) -> Result<(), PayloadAttestationImportError> { + let verified = self + .chain + .verify_payload_attestation_message_for_gossip(message) + .map_err(PayloadAttestationImportError::Verification)?; + + self.chain + .apply_payload_attestation_to_fork_choice( + verified.indexed_payload_attestation(), + verified.ptc(), + ) + .map_err(|e| PayloadAttestationImportError::ForkChoice(Box::new(e)))?; + + self.chain + .add_payload_attestation_to_pool(&verified) + .map_err(|e| PayloadAttestationImportError::Pool(Box::new(e)))?; + + Ok(()) + } + + pub fn import_payload_attestation_messages( + &self, + messages: impl IntoIterator, + ) -> Result<(), PayloadAttestationImportError> { + for message in messages { + self.import_payload_attestation_message(message)?; + } + + Ok(()) + } + pub fn make_sync_contributions( &self, state: &BeaconState, @@ -2158,6 +2378,21 @@ where slot: Slot, relative_sync_committee: RelativeSyncCommittee, ) -> HarnessSyncContributions { + // Resolve the committee for aggregator selection using the same relative committee as the + // messages. Selecting from `current_sync_committee` unconditionally would pick an + // aggregator outside the verifying committee at sync committee period boundaries (where + // `Next` is used), causing `AggregatorNotInCommittee`. + let sync_committee: Arc> = match relative_sync_committee { + RelativeSyncCommittee::Current => state + .current_sync_committee() + .expect("should be called on altair beacon state") + .clone(), + RelativeSyncCommittee::Next => state + .next_sync_committee() + .expect("should be called on altair beacon state") + .clone(), + }; + let sync_messages = self.make_sync_committee_messages(state, block_hash, slot, relative_sync_committee); @@ -2167,10 +2402,7 @@ where .map(|(subnet_id, committee_messages)| { // If there are any sync messages in this committee, create an aggregate. if let Some((sync_message, subcommittee_position)) = committee_messages.first() { - let sync_committee: Arc> = state - .current_sync_committee() - .expect("should be called on altair beacon state") - .clone(); + let sync_committee = sync_committee.clone(); let aggregator_index = sync_committee .get_subcommittee_pubkeys(subnet_id) @@ -3239,14 +3471,24 @@ where if sync_committee_strategy == SyncCommitteeStrategy::AllValidators && new_state.current_sync_committee().is_ok() { + // A sync message for `slot` is verified against the committee of `epoch(slot + 1)` + // (see `BeaconChain::sync_committee_at_next_slot`), so we must sign with `Next` only + // when `slot + 1` crosses into a new sync committee period, not for the whole first + // epoch of the period. + let slots_per_epoch = E::slots_per_epoch(); + let crosses_period = slot + .epoch(slots_per_epoch) + .sync_committee_period(&self.spec) + .unwrap() + != (slot + 1) + .epoch(slots_per_epoch) + .sync_committee_period(&self.spec) + .unwrap(); self.sync_committee_sign_block( &new_state, block_hash.into(), slot, - if (slot + 1).epoch(E::slots_per_epoch()) - % self.spec.epochs_per_sync_committee_period - == 0 - { + if crosses_period { RelativeSyncCommittee::Next } else { RelativeSyncCommittee::Current @@ -3806,6 +4048,28 @@ pub struct MakeAttestationOptions { pub payload_present_override: Option, } +#[derive(Debug, Clone, Copy)] +pub struct PayloadAttestationVote { + /// Amount of PTC weight to produce messages for this vote. + pub validator_count: usize, + pub payload_present: bool, + pub blob_data_available: bool, +} + +pub struct MakePayloadAttestationOptions { + /// Vote groups to produce. Each group becomes `validator_count` individual messages. + pub votes: Vec, + /// Fork to use for signing payload attestation messages. + pub fork: Fork, +} + +#[derive(Debug)] +pub enum PayloadAttestationImportError { + Verification(crate::payload_attestation_verification::Error), + ForkChoice(Box), + Pool(Box), +} + pub enum NumBlobs { Random, Number(usize), diff --git a/beacon_node/http_api/tests/gloas_reorg_tests.rs b/beacon_node/http_api/tests/gloas_reorg_tests.rs new file mode 100644 index 0000000000..6bbca727f9 --- /dev/null +++ b/beacon_node/http_api/tests/gloas_reorg_tests.rs @@ -0,0 +1,946 @@ +//! post-gloas payload re-org tests. +//! +//! These tests are deliberately kept separate from `interactive_tests.rs` because they exercise +//! post-gloas fork-choice behaviour: the head is a `ForkChoiceNode` = (block root, payload status), +//! and a block's *payload* can be re-orged (head flips `FULL` -> `EMPTY`) independently of the +//! beacon block, when later-slot voters attest the block with `payload_present = false`. +//! +use beacon_chain::{ + ChainConfig, + chain_config::DEFAULT_PREPARE_PAYLOAD_LOOKAHEAD_FACTOR, + custody_context::NodeCustodyType, + test_utils::{ + AttestationStrategy, BlockStrategy, LightClientStrategy, MakeAttestationOptions, + MakePayloadAttestationOptions, PayloadAttestationVote, SyncCommitteeStrategy, test_spec, + }, +}; +use eth2::types::ProduceBlockV3Response; +use execution_layer::{ForkchoiceState, PayloadAttributes}; +use fixed_bytes::FixedBytesExtended; +use http_api::test_utils::InteractiveTester; +use parking_lot::Mutex; +use proto_array::PayloadStatus; +use slot_clock::SlotClock; +use state_processing::{ + per_block_processing::get_expected_withdrawals, state_advance::complete_state_advance, +}; +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; +use types::{ + Address, BeaconBlockRef, EthSpec, ExecPayload, ExecutionBlockHash, Hash256, MinimalEthSpec, + ProposerPreparationData, Slot, +}; + +type E = MinimalEthSpec; + +// Must be at least PTC size to simplify PTC reasoning (unique PTC members per slot). +const ATTESTERS_PER_SLOT: usize = 20; + +/// Data structure for tracking fork choice updates received by the mock execution layer. +#[derive(Debug, Default)] +struct ForkChoiceUpdates { + updates: HashMap>, +} + +#[derive(Debug, Clone)] +struct ForkChoiceUpdateMetadata { + received_at: Duration, + state: ForkchoiceState, + payload_attributes: Option, +} + +impl ForkChoiceUpdates { + fn insert(&mut self, update: ForkChoiceUpdateMetadata) { + self.updates + .entry(update.state.head_block_hash) + .or_default() + .push(update); + } + + fn contains_update_for(&self, block_hash: ExecutionBlockHash) -> bool { + self.updates.contains_key(&block_hash) + } + + /// Find the first fork choice update for `head_block_hash` with payload attributes matching + /// the proposal and parent being tested. + fn first_update_with_payload_attributes( + &self, + head_block_hash: ExecutionBlockHash, + proposal_timestamp: u64, + parent_beacon_block_root: Option, + slot_number: Option, + ) -> Option { + self.updates + .get(&head_block_hash)? + .iter() + .find(|update| { + update + .payload_attributes + .as_ref() + .is_some_and(|payload_attributes| { + if payload_attributes.timestamp() != proposal_timestamp { + return false; + } + + if let Some(parent_beacon_block_root) = parent_beacon_block_root + && payload_attributes.parent_beacon_block_root().ok() + != Some(parent_beacon_block_root) + { + return false; + } + + if let Some(slot_number) = slot_number + && payload_attributes.slot_number().ok() != Some(slot_number) + { + return false; + } + + true + }) + }) + .cloned() + } +} + +#[derive(Clone, Copy)] +enum ExpectedFirstUpdateLookahead { + Payload, + ForkChoice, + BlockProduction, +} + +pub struct ReOrgTest { + head_slot: Slot, + /// Number of slots between parent block and canonical head. + parent_distance: u64, + /// Number of slots between head block and block proposal slot. + head_distance: u64, + /// Fraction of parent (A)'s committee that votes for A (always with payload_present=0). + percent_parent_votes: usize, + /// Fraction of B's committee that votes for A with payload_present=0. + percent_skip_empty_votes: usize, + /// Fraction of B's committee that votes for A with payload_present=1. + percent_skip_full_votes: usize, + /// Fraction of B's committee that votes for B (always with payload_present=0). + percent_head_votes: usize, + /// Parent payload status of block B. + head_parent_payload_status: PayloadStatus, + /// Fraction of A's PTC that vote for A's payload being present. + percent_parent_ptc_present_votes: usize, + /// Fraction of A's PTC that vote for A's payload being absent. + percent_parent_ptc_absent_votes: usize, + /// Expected parent payload status of our proposed block (C). + /// + /// This can be the payload status of A or B depending on whether we reorged or not. + expected_parent_payload_status: PayloadStatus, + should_re_org: bool, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead, + /// Whether to expect withdrawals to change on epoch boundaries. + expect_withdrawals_change_on_epoch: bool, +} + +impl Default for ReOrgTest { + /// Default config represents a regular easy re-org. + fn default() -> Self { + Self { + head_slot: Slot::new(E::slots_per_epoch() - 2), + parent_distance: 1, + head_distance: 1, + percent_parent_votes: 100, + percent_skip_empty_votes: 0, + percent_skip_full_votes: 100, + percent_head_votes: 0, + head_parent_payload_status: PayloadStatus::Full, + percent_parent_ptc_present_votes: 100, + percent_parent_ptc_absent_votes: 0, + expected_parent_payload_status: PayloadStatus::Full, + should_re_org: true, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::Payload, + expect_withdrawals_change_on_epoch: false, + } + } +} + +// This test doesn't actually exercise the re-org code path because the chain just naturally +// re-orgs to A-empty at the start of slot C anyway. That only happens after the 500ms +// pre-slot fork choice recompute. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn re_org_parent_is_empty_easy() { + proposer_boost_re_org_test(ReOrgTest { + percent_skip_empty_votes: 100, + percent_skip_full_votes: 0, + expected_parent_payload_status: PayloadStatus::Empty, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::ForkChoice, + ..Default::default() + }) + .await; +} + +// A-Empty chain has 55% of one committee supporting it A-Full chain has 45% of one committee +// supporting it, including 15% for descendant B that is late and re-orgable. +// +// A-Full has 100% PTC support, but this should be completely ignored. +// +// We should re-org B and build on A-Empty. +// +// This test doesn't actually exercise the re-org code path because the chain just naturally +// re-orgs to A-empty at the start of slot C anyway. That only happens after the 500ms +// pre-slot fork choice recompute. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn re_org_parent_is_empty_marginal_win() { + proposer_boost_re_org_test(ReOrgTest { + percent_skip_empty_votes: 55, + percent_skip_full_votes: 30, + percent_head_votes: 15, + percent_parent_ptc_present_votes: 100, + percent_parent_ptc_absent_votes: 0, + expected_parent_payload_status: PayloadStatus::Empty, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::ForkChoice, + ..Default::default() + }) + .await; +} + +// A-Empty chain has 45% of one committee supporting it A-Full chain has 55% of one committee +// supporting it, including 15% for descendant B that is late and re-orgable. +// +// A-Full has 100% PTC support, but this should be completely ignored. +// +// We should re-org B and build on A-Full. +// Since Gloas fork choice updates are not overridden for proposer re-orgs, the first fcU for this +// parent is sent during block production. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn re_org_parent_is_full_marginal_win() { + proposer_boost_re_org_test(ReOrgTest { + percent_skip_empty_votes: 45, + percent_skip_full_votes: 40, + percent_head_votes: 15, + percent_parent_ptc_present_votes: 100, + percent_parent_ptc_absent_votes: 0, + expected_parent_payload_status: PayloadStatus::Full, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_parent_empty() { + proposer_boost_re_org_test(ReOrgTest { + percent_skip_empty_votes: 55, + percent_skip_full_votes: 30, + percent_head_votes: 15, + percent_parent_ptc_present_votes: 100, + percent_parent_ptc_absent_votes: 0, + head_parent_payload_status: PayloadStatus::Empty, + expected_parent_payload_status: PayloadStatus::Empty, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +// Test that the beacon node will try to perform proposer boost re-orgs on late blocks when +// configured. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_zero_weight() { + proposer_boost_re_org_test(ReOrgTest { + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +// Since Fulu, proposer shuffling is stable across epoch boundaries, so re-orgs of the last block +// in an epoch are permitted. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_epoch_boundary() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(E::slots_per_epoch() - 1), + should_re_org: true, + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_epoch_boundary_skip1() { + // Proposing a block on a boundary after a skip will change the set of expected withdrawals + // sent in the payload attributes. + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(2 * E::slots_per_epoch() - 2), + head_distance: 2, + should_re_org: false, + expect_withdrawals_change_on_epoch: true, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_epoch_boundary_skip32() { + // Propose a block at 64 after a whole epoch of skipped slots. + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(E::slots_per_epoch() - 1), + head_distance: E::slots_per_epoch() + 1, + should_re_org: false, + expect_withdrawals_change_on_epoch: true, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_slot_after_epoch_boundary() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(33), + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_bad_ffg() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(64 + 22), + should_re_org: false, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_no_finality() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(96), + percent_parent_votes: 100, + percent_skip_full_votes: 0, + percent_head_votes: 100, + should_re_org: false, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_finality() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(129), + expected_first_update_lookahead: ExpectedFirstUpdateLookahead::BlockProduction, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_parent_distance() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(E::slots_per_epoch() - 2), + parent_distance: 2, + should_re_org: false, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_head_distance() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(E::slots_per_epoch() - 3), + head_distance: 2, + should_re_org: false, + ..Default::default() + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_very_unhealthy() { + proposer_boost_re_org_test(ReOrgTest { + head_slot: Slot::new(E::slots_per_epoch() - 1), + parent_distance: 2, + head_distance: 2, + percent_parent_votes: 10, + percent_skip_full_votes: 10, + percent_head_votes: 10, + should_re_org: false, + ..Default::default() + }) + .await; +} + +/// The head block is late but still receives 30% of the committee vote, making it strong enough +/// that we do not re-org it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +pub async fn proposer_boost_re_org_head_too_strong() { + proposer_boost_re_org_test(ReOrgTest { + percent_skip_full_votes: 70, + percent_head_votes: 30, + should_re_org: false, + ..Default::default() + }) + .await; +} + +/// Run a proposer boost re-org test. +/// +/// - `head_slot`: the slot of the canonical head to be reorged +/// - `reorg_threshold`: committee percentage value for reorging +/// - `num_empty_votes`: percentage of comm of attestations for the parent block +/// - `num_head_votes`: number of attestations for the head block +/// - `should_re_org`: whether the proposer should build on the parent rather than the head +#[allow(clippy::large_stack_frames)] +pub async fn proposer_boost_re_org_test( + ReOrgTest { + head_slot, + parent_distance, + head_distance, + percent_parent_votes, + percent_skip_empty_votes, + percent_skip_full_votes, + percent_head_votes, + head_parent_payload_status, + percent_parent_ptc_present_votes, + percent_parent_ptc_absent_votes, + expected_parent_payload_status, + should_re_org, + expected_first_update_lookahead, + expect_withdrawals_change_on_epoch, + }: ReOrgTest, +) { + assert!(head_slot > 0); + + let spec = test_spec::(); + + if !spec.is_gloas_scheduled() { + return; + } + + // Ensure there are enough validators to have `ATTESTERS_PER_SLOT`. + assert!(ATTESTERS_PER_SLOT >= E::ptc_size()); + let validator_count = E::slots_per_epoch() as usize * ATTESTERS_PER_SLOT; + let all_validators = (0..validator_count).collect::>(); + let num_initial = head_slot.as_u64().checked_sub(parent_distance + 1).unwrap(); + + // Check that the required vote percentages can be satisfied exactly using `ATTESTERS_PER_SLOT`. + assert_eq!(100 % ATTESTERS_PER_SLOT, 0); + let percent_per_attester = 100 / ATTESTERS_PER_SLOT; + assert_eq!(percent_parent_votes % percent_per_attester, 0); + assert_eq!(percent_skip_empty_votes % percent_per_attester, 0); + assert_eq!(percent_skip_full_votes % percent_per_attester, 0); + assert_eq!(percent_head_votes % percent_per_attester, 0); + let num_parent_votes = Some(ATTESTERS_PER_SLOT * percent_parent_votes / 100); + let num_skip_empty_votes = Some(ATTESTERS_PER_SLOT * percent_skip_empty_votes / 100); + let num_skip_full_votes = Some(ATTESTERS_PER_SLOT * percent_skip_full_votes / 100); + let num_head_votes = Some(ATTESTERS_PER_SLOT * percent_head_votes / 100); + + assert_eq!((percent_parent_ptc_present_votes * E::ptc_size()) % 100, 0); + let num_parent_ptc_present_votes = percent_parent_ptc_present_votes * E::ptc_size() / 100; + assert_eq!((percent_parent_ptc_absent_votes * E::ptc_size()) % 100, 0); + let num_parent_ptc_absent_votes = percent_parent_ptc_absent_votes * E::ptc_size() / 100; + + // We must configure the prepare payload lookahead so it scales with the minimal config, + // otherwise the late block reveal for A halfway through the slot can end up being *after* + // the payload lookahead, which messes up our measurement of timings. + let chain_config = ChainConfig { + prepare_payload_lookahead: spec.get_slot_duration() + / DEFAULT_PREPARE_PAYLOAD_LOOKAHEAD_FACTOR, + ..Default::default() + }; + + let tester = InteractiveTester::::new_with_initializer_and_mutator( + Some(spec), + validator_count, + None, + Some(Box::new(move |builder| builder.chain_config(chain_config))), + Default::default(), + false, + NodeCustodyType::Fullnode, + ) + .await; + let harness = &tester.harness; + let mock_el = harness.mock_execution_layer.as_ref().unwrap(); + let execution_ctx = mock_el.server.ctx.clone(); + let slot_clock = &harness.chain.slot_clock; + + mock_el.server.all_payloads_valid(); + + // Send proposer preparation data for all validators. + let proposer_preparation_data = all_validators + .iter() + .map(|i| { + ( + ProposerPreparationData { + validator_index: *i as u64, + fee_recipient: Address::from_low_u64_be(*i as u64), + }, + None, + ) + }) + .collect::>(); + harness + .chain + .execution_layer + .as_ref() + .unwrap() + .update_proposer_preparation( + head_slot.epoch(E::slots_per_epoch()) + 1, + proposer_preparation_data.iter().map(|(a, b)| (a, b)), + ) + .await; + + // Create some chain depth. Sign sync committee signatures so validator balances don't dip + // below 32 ETH and become ineligible for withdrawals. + harness.advance_slot(); + harness + .extend_chain_with_sync( + num_initial as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + SyncCommitteeStrategy::AllValidators, + LightClientStrategy::Disabled, + ) + .await; + + // Start collecting fork choice updates. + let forkchoice_updates = Arc::new(Mutex::new(ForkChoiceUpdates::default())); + let forkchoice_updates_inner = forkchoice_updates.clone(); + let chain_inner = harness.chain.clone(); + + execution_ctx + .hook + .lock() + .set_forkchoice_updated_hook(Box::new(move |state, payload_attributes| { + let received_at = chain_inner.slot_clock.now_duration().unwrap(); + let state = ForkchoiceState::from(state); + let payload_attributes = payload_attributes.map(Into::into); + let update = ForkChoiceUpdateMetadata { + received_at, + state, + payload_attributes, + }; + forkchoice_updates_inner.lock().insert(update); + None + })); + + // We set up the following block graph, where B is a block that arrives late and is re-orged + // by C. + // + // A | B | - | + // ^ | - | C | + + let slot_a = Slot::new(num_initial + 1); + let slot_b = slot_a + parent_distance; + let slot_c = slot_b + head_distance; + + // We need to transition to at least epoch 2 in order to trigger + // `process_rewards_and_penalties`. This allows us to test withdrawals changes at epoch + // boundaries. + if expect_withdrawals_change_on_epoch { + assert!( + slot_c.epoch(E::slots_per_epoch()) >= 2, + "for withdrawals to change, test must end at an epoch >= 2" + ); + } + + harness.advance_slot(); + let (block_a_root, block_a, mut state_a) = harness + .add_block_at_slot(slot_a, harness.get_current_state()) + .await + .unwrap(); + let state_a_root = state_a.canonical_root().unwrap(); + + // Attest to block A during slot A. + let (block_a_parent_votes, _) = harness.make_attestations_with_limit( + &all_validators, + &state_a, + state_a_root, + block_a_root, + slot_a, + num_parent_votes, + ); + harness.process_attestations(block_a_parent_votes, &state_a); + + // Produce PTC messages for slot A. + let a_ptc_votes = vec![ + PayloadAttestationVote { + validator_count: num_parent_ptc_present_votes, + payload_present: true, + blob_data_available: true, + }, + PayloadAttestationVote { + validator_count: num_parent_ptc_absent_votes, + payload_present: false, + blob_data_available: false, + }, + ]; + let (a_ptc_messages, _) = harness.make_payload_attestation_messages_with_opts( + &all_validators, + &state_a, + block_a_root.into(), + slot_a, + MakePayloadAttestationOptions { + votes: a_ptc_votes, + fork: state_a.fork(), + }, + ); + harness + .import_payload_attestation_messages(a_ptc_messages) + .unwrap(); + + // Attest to block A during slot B. + for _ in 0..parent_distance { + harness.advance_slot(); + } + let (block_a_empty_votes, block_a_empty_attesters) = harness.make_attestations_with_opts( + &all_validators, + &state_a, + state_a_root, + block_a_root, + slot_b, + MakeAttestationOptions { + limit: num_skip_empty_votes, + fork: state_a.fork(), + payload_present_override: Some(false), + }, + ); + harness.process_attestations(block_a_empty_votes, &state_a); + let remaining_attesters_after_empty = all_validators + .iter() + .copied() + .filter(|index| !block_a_empty_attesters.contains(index)) + .collect::>(); + let (block_a_full_votes, block_a_full_attesters) = harness.make_attestations_with_opts( + &remaining_attesters_after_empty, + &state_a, + state_a_root, + block_a_root, + slot_b, + MakeAttestationOptions { + limit: num_skip_full_votes, + fork: state_a.fork(), + payload_present_override: Some(true), + }, + ); + harness.process_attestations(block_a_full_votes, &state_a); + + let remaining_attesters = remaining_attesters_after_empty + .iter() + .copied() + .filter(|index| !block_a_full_attesters.contains(index)) + .collect::>(); + + // Produce block B and process it halfway through the slot. + // When B is expected to remain canonical (no re-org), capture its Gloas payload envelope so we + // can reveal B's execution payload to fork choice below. Without this, B's payload status stays + // `Empty`/`Pending` and the forkchoiceUpdated head hash falls back to B's parent rather than B's + // own execution block hash. We skip this when B will be re-orged, since the execution layer + // must never be told about a block that is about to be re-orged away. + let is_gloas = harness + .chain + .spec + .fork_name_at_slot::(slot_b) + .gloas_enabled(); + let reveal_block_b_payload = is_gloas && !should_re_org; + let (block_b, block_b_envelope, mut state_b) = if is_gloas { + let (block_b, block_b_envelope, state_b) = harness + .make_block_with_envelope_on(state_a.clone(), slot_b, head_parent_payload_status) + .await; + let block_b_envelope = if reveal_block_b_payload { + block_b_envelope + } else { + None + }; + (block_b, block_b_envelope, state_b) + } else { + let (block_b, state_b) = harness.make_block(state_a.clone(), slot_b).await; + (block_b, None, state_b) + }; + let state_b_root = state_b.canonical_root().unwrap(); + let block_b_root = block_b.0.canonical_root(); + + let obs_time = slot_clock.start_of(slot_b).unwrap() + slot_clock.slot_duration() / 2; + slot_clock.set_current_time(obs_time); + harness.chain.block_times_cache.write().set_time_observed( + block_b_root, + slot_b, + obs_time, + None, + None, + ); + harness.process_block_result(block_b.clone()).await.unwrap(); + + // Reveal B's execution payload so fork choice marks the payload as received and the + // forkchoiceUpdated head hash references B's own execution block hash. + if let Some(block_b_envelope) = block_b_envelope { + harness + .process_envelope(block_b_root, block_b_envelope, &state_b, state_b_root) + .await; + } + + // Add attestations to block B. + let (block_b_head_votes, _) = harness.make_attestations_with_limit( + &remaining_attesters, + &state_b, + state_b_root, + block_b_root.into(), + slot_b, + num_head_votes, + ); + harness.process_attestations(block_b_head_votes, &state_b); + + let payload_lookahead = harness.chain.config.prepare_payload_lookahead; + let fork_choice_lookahead = Duration::from_millis(500); + while harness.get_current_slot() != slot_c { + let current_slot = harness.get_current_slot(); + let next_slot = current_slot + 1; + + // Simulate the scheduled call to prepare proposers at 8 seconds into the slot. + harness.advance_to_slot_lookahead(next_slot, payload_lookahead); + harness + .chain + .prepare_beacon_proposer(current_slot) + .await + .unwrap(); + + // Simulate the scheduled call to fork choice + prepare proposers 500ms before the + // next slot. + harness.advance_to_slot_lookahead(next_slot, fork_choice_lookahead); + harness.chain.recompute_head_at_slot(next_slot).await; + harness + .chain + .prepare_beacon_proposer(current_slot) + .await + .unwrap(); + + harness.advance_slot(); + harness.chain.per_slot_task().await; + } + + // Produce block C. + // Advance state_b so we can get the proposer. + assert_eq!(state_b.slot(), slot_b); + let pre_advance_withdrawals = get_expected_withdrawals(&state_b, &harness.chain.spec) + .unwrap() + .withdrawals() + .to_vec(); + complete_state_advance(&mut state_b, None, slot_c, &harness.chain.spec).unwrap(); + + let proposer_index = state_b + .get_beacon_proposer_index(slot_c, &harness.chain.spec) + .unwrap(); + let randao_reveal = harness + .sign_randao_reveal(&state_b, proposer_index, slot_c) + .into(); + let is_gloas = harness + .chain + .spec + .fork_name_at_slot::(slot_c) + .gloas_enabled(); + + let (block_c, block_c_blobs) = if is_gloas { + let (response, _) = tester + .client + .get_validator_blocks_v4::(slot_c, &randao_reveal, None, None, None, None) + .await + .unwrap(); + ( + Arc::new(harness.sign_beacon_block(response.data, &state_b)), + None, + ) + } else { + let (unsigned_block_type, _) = tester + .client + .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None, None) + .await + .unwrap(); + + let (unsigned_block_c, block_c_blobs) = match unsigned_block_type.data { + ProduceBlockV3Response::Full(unsigned_block_contents_c) => { + unsigned_block_contents_c.deconstruct() + } + ProduceBlockV3Response::Blinded(_) => { + panic!("Should not be a blinded block"); + } + }; + ( + Arc::new(harness.sign_beacon_block(unsigned_block_c, &state_b)), + block_c_blobs, + ) + }; + + // Post-Gloas the execution payload is decoupled from the beacon block: the payload hash + // lives in the execution payload bid, and the payload timestamp is derived from the slot. + let exec_block_hash = |block: BeaconBlockRef| -> ExecutionBlockHash { + if is_gloas { + block + .body() + .signed_execution_payload_bid() + .unwrap() + .message + .block_hash + } else { + block.execution_payload().unwrap().block_hash() + } + }; + let exec_parent_hash = |block: BeaconBlockRef| -> ExecutionBlockHash { + if is_gloas { + block + .body() + .signed_execution_payload_bid() + .unwrap() + .message + .parent_block_hash + } else { + block.execution_payload().unwrap().parent_hash() + } + }; + + let block_a_exec_hash = exec_block_hash(block_a.0.message()); + let block_b_exec_hash = exec_block_hash(block_b.0.message()); + + if is_gloas { + assert_eq!( + block_b.0.is_parent_block_full(block_a_exec_hash), + head_parent_payload_status == PayloadStatus::Full + ); + } + + if should_re_org { + // Block C should build on A. + assert_eq!(block_c.parent_root(), Hash256::from(block_a_root)); + + if is_gloas { + assert_eq!( + block_c.is_parent_block_full(block_a_exec_hash), + expected_parent_payload_status == PayloadStatus::Full + ); + } + } else { + // Block C should build on B. + assert_eq!(block_c.parent_root(), block_b_root); + + if is_gloas { + assert_eq!( + block_c.is_parent_block_full(block_b_exec_hash), + expected_parent_payload_status == PayloadStatus::Full + ); + } + } + + // Applying block C should cause it to become head regardless (re-org or continuation). + let block_root_c = Hash256::from( + harness + .process_block_result((block_c.clone(), block_c_blobs)) + .await + .unwrap(), + ); + + assert_eq!(harness.head_block_root(), block_root_c); + + // Check the fork choice updates that were sent. + let forkchoice_updates = forkchoice_updates.lock(); + + let block_c_timestamp = if is_gloas { + harness.chain.slot_clock.start_of(slot_c).unwrap().as_secs() + } else { + block_c.message().execution_payload().unwrap().timestamp() + }; + + // If we re-orged then no fork choice update for B should have been sent. + assert_eq!( + should_re_org, + !forkchoice_updates.contains_update_for(block_b_exec_hash), + "{block_b_exec_hash:?}" + ); + + // Check the timing of the first fork choice update with payload attributes for block C. + let c_parent_block = if should_re_org { + block_a.0.message() + } else { + block_b.0.message() + }; + let c_parent_hash = if expected_parent_payload_status == PayloadStatus::Full { + exec_block_hash(c_parent_block) + } else { + exec_parent_hash(c_parent_block) + }; + let first_update = forkchoice_updates + .first_update_with_payload_attributes( + c_parent_hash, + block_c_timestamp, + is_gloas.then(|| block_c.parent_root()), + is_gloas.then(|| slot_c.as_u64()), + ) + .unwrap(); + let payload_attribs = first_update.payload_attributes.as_ref().unwrap(); + + // Check that withdrawals from the payload attributes match those computed from the state used + // by the path that produced the matching fcU. + let parent_state_advanced = if should_re_org { + let mut state = state_a.clone(); + complete_state_advance(&mut state, None, slot_c, &harness.chain.spec).unwrap(); + state + } else { + state_b.clone() + }; + let expected_withdrawals = if is_gloas + && matches!( + expected_first_update_lookahead, + ExpectedFirstUpdateLookahead::BlockProduction + ) + && expected_parent_payload_status == PayloadStatus::Empty + { + parent_state_advanced + .payload_expected_withdrawals() + .unwrap() + .to_vec() + } else { + get_expected_withdrawals(&parent_state_advanced, &harness.chain.spec) + .unwrap() + .withdrawals() + .to_vec() + }; + let payload_attribs_withdrawals = payload_attribs.withdrawals().unwrap(); + assert_eq!(expected_withdrawals, *payload_attribs_withdrawals); + // The validator withdrawal sweep is positional: it scans a rotating window of + // `max_validators_per_withdrawals_sweep` validators starting at `next_withdrawal_validator_index`. + // For a given proposal slot that window can legitimately contain no withdrawal-eligible + // validators (with empty partial/builder withdrawal queues), so an empty withdrawals list is + // valid. Withdrawal correctness is covered by the equality check above; we only assert the + // re-org/epoch-boundary withdrawals change when there are withdrawals to compare. + if !expected_withdrawals.is_empty() + && (should_re_org + || expect_withdrawals_change_on_epoch + && slot_c.epoch(E::slots_per_epoch()) != slot_b.epoch(E::slots_per_epoch())) + { + assert_ne!(expected_withdrawals, pre_advance_withdrawals); + } + + // Check that the `parent_beacon_block_root` of the payload attributes are correct. + if let Ok(parent_beacon_block_root) = payload_attribs.parent_beacon_block_root() { + assert_eq!(parent_beacon_block_root, block_c.parent_root()); + } + + let lookahead = slot_clock + .start_of(slot_c) + .unwrap() + .checked_sub(first_update.received_at) + .unwrap(); + + let expected_lookahead = match expected_first_update_lookahead { + ExpectedFirstUpdateLookahead::Payload => payload_lookahead, + ExpectedFirstUpdateLookahead::ForkChoice => fork_choice_lookahead, + ExpectedFirstUpdateLookahead::BlockProduction => Duration::ZERO, + }; + assert_eq!( + lookahead, + expected_lookahead, + "observed_lookahead={lookahead:?}, expected={expected_lookahead:?}, timestamp={}, prev_randao={:?}", + payload_attribs.timestamp(), + payload_attribs.prev_randao(), + ); +} diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 9258dab1af..d2a28da0f5 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -3,7 +3,8 @@ use beacon_chain::custody_context::NodeCustodyType; use beacon_chain::{ ChainConfig, test_utils::{ - AttestationStrategy, BlockStrategy, LightClientStrategy, SyncCommitteeStrategy, test_spec, + AttestationStrategy, BlockStrategy, LightClientStrategy, SyncCommitteeStrategy, + fork_name_from_env, test_spec, }, }; use beacon_processor::{Work, WorkEvent, work_reprocessing_queue::ReprocessQueueMessage}; @@ -366,9 +367,13 @@ pub async fn proposer_boost_re_org_test( ) { assert!(head_slot > 0); - // TODO(EIP-7732): extend test for Gloas — `get_validator_blocks_v3` is missing the - // `Eth-Execution-Payload-Blinded` header for Gloas block production responses. - let spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); + // We don't run these test for post-Gloas forks because of the FcU changes that were + // applied in the gloas. Gloas adopted tests can be found in `gloas_re_org_test.rs` + if fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + + let spec = test_spec::(); // Ensure there are enough validators to have `attesters_per_slot`. let attesters_per_slot = 10; diff --git a/beacon_node/http_api/tests/main.rs b/beacon_node/http_api/tests/main.rs index e0636424e4..35400a912e 100644 --- a/beacon_node/http_api/tests/main.rs +++ b/beacon_node/http_api/tests/main.rs @@ -2,6 +2,7 @@ pub mod broadcast_validation_tests; pub mod fork_tests; +pub mod gloas_reorg_tests; pub mod interactive_tests; pub mod status_tests; pub mod tests; diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index d45412c608..04113e2c0e 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -174,6 +174,10 @@ pub struct ProtoNode { } impl ProtoNode { + pub fn is_gloas(&self) -> bool { + self.as_v29().is_ok() + } + /// Generic version of spec's `parent_payload_status` that works for pre-Gloas nodes by /// considering their parents Empty. pub fn get_parent_payload_status(&self) -> PayloadStatus { diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 90143f1dd1..c6a7829c27 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -723,15 +723,14 @@ impl ProtoArrayForkChoice { .into()); } - // 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); + // Spec: `is_parent_strong`. Use `PayloadStatus::Pending` to avoid weight split + // between payload statuses. https://github.com/ethereum/consensus-specs/issues/5305 + let parent_pending_weight = info.parent_node.attestation_score(PayloadStatus::Pending); let re_org_parent_weight_threshold = info.re_org_parent_weight_threshold; - let parent_strong = parent_weight > re_org_parent_weight_threshold; + let parent_strong = parent_pending_weight > re_org_parent_weight_threshold; if !parent_strong { return Err(DoNotReOrg::ParentNotStrong { - parent_weight, + parent_weight: parent_pending_weight, re_org_parent_weight_threshold, } .into());