From 446f5b5c1636f26b5a326d45491930ff0f15dba2 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 18 Jun 2026 12:37:05 +1000 Subject: [PATCH 01/12] Update DB docs for v8.2.0/schema v29 (#9489) - Move info about pre-8.0 schema migrations to the historical migrations section. - Include info about v8.1.x and v8.2.x. Co-Authored-By: Michael Sproul --- book/src/advanced_database_migrations.md | 15 ++++++++------- wordlist.txt | 1 + 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/book/src/advanced_database_migrations.md b/book/src/advanced_database_migrations.md index 115a885878..59a8b7cff5 100644 --- a/book/src/advanced_database_migrations.md +++ b/book/src/advanced_database_migrations.md @@ -17,11 +17,9 @@ validator client or the slasher**. | Lighthouse version | Release date | Schema version | Downgrade available? | |--------------------|--------------|----------------|----------------------| -| v8.0.0 | Nov 2025 | v28 | yes before Fulu | -| v8.0.0-rc.0 | Sep 2025 | v28 | yes before Fulu | -| v7.1.0 | Jul 2025 | v26 | yes | -| v7.0.0 | Apr 2025 | v22 | no | -| v6.0.0 | Nov 2024 | v22 | no | +| v8.2.0 | Jun 2026 | v29 | yes before Gloas | +| v8.1.0 | Feb 2026 | v28 | no | +| v8.0.0 | Nov 2025 | v28 | no | > **Note**: All point releases (e.g. v4.4.1) are schema-compatible with the prior minor release > (e.g. v4.4.0). @@ -209,8 +207,11 @@ Here are the steps to prune historic states: | Lighthouse version | Release date | Schema version | Downgrade available? | |--------------------|--------------|----------------|-------------------------------------| -| v8.0.0-rc.0 | Sep 2025 | v28 | yes before Fulu | -| v7.1.0 | Jul 2025 | v26 | yes | +| v8.2.0 | Jun 2026 | v29 | yes before Gloas | +| v8.1.0 | Feb 2026 | v28 | yes before Fulu using <= v8.1.3 | +| v8.0.0 | Nov 2025 | v28 | yes before Fulu using <= v8.1.3 | +| v8.0.0-rc.0 | Sep 2025 | v28 | yes before Fulu using <= v8.1.3 | +| v7.1.0 | Jul 2025 | v26 | yes using <= v8.1.3 | | v7.0.0 | Apr 2025 | v22 | no | | v6.0.0 | Nov 2024 | v22 | no | | v5.3.0 | Aug 2024 | v21 | yes before Electra using <= v7.0.0 | diff --git a/wordlist.txt b/wordlist.txt index 822e336146..f0076e6332 100644 --- a/wordlist.txt +++ b/wordlist.txt @@ -45,6 +45,7 @@ Fusaka Geth GiB Gitcoin +Gloas Gnosis Goerli Grafana 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 02/12] 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()); From 560f90611ee810dcab0f055daf7dcc50fab891b4 Mon Sep 17 00:00:00 2001 From: Daniel Knopik <107140945+dknopik@users.noreply.github.com> Date: Fri, 19 Jun 2026 02:50:24 +0200 Subject: [PATCH 03/12] Fix and improve handling of empty columns after getBlobs response (#9361) This PR fixes two issues: 1. This condition is inverted: https://github.com/sigp/lighthouse/blob/dfb259171a65cacd6db57b8874af8f543cabcb7a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs#L1507-L1508 We are supposed to filter out incomplete columns when we DON'T have local blobs yet! 2. When the EL returns no blobs, we never store a partial in the assembler, and this code fails to publish our need to the network, as no partials are returned: https://github.com/sigp/lighthouse/blob/dfb259171a65cacd6db57b8874af8f543cabcb7a/beacon_node/network/src/network_beacon_processor/mod.rs#L1038-L1050 The simple fix for 1 would be to invert the condition, but we can improve the flow here: Instead of not publishing anything, we can publish what we got, but not request anything. This ties into the fix for 2: After get blobs completes, we not only publish anything in the partial assembler, but also for every missing custody column in there, publish an empty column and a request for all cells. In particular: - When sending a partial message to `network`, allow specifying a request bitmap instead of hardcoding an all-ones bitmap. - For clarity and to prepare for Gloas integration, add a `PubsubPartialMessage` enum with a `DataColumnFulu` variant. - On republishing after merging a gossip column: always publish, but only request cells if local blobs are known or get blobs is disabled. This also prepares us to request only *some* cells, e.g. in cases where we are aware of the blobs that the EL is going to send us, e.g. via `engine_hasBlobs`. - Move guards in `fetch_engine_blobs_and_publish` to ensure everything works fine if there are no blobs or if get_blobs is disabled. Co-Authored-By: Daniel Knopik --- beacon_node/beacon_chain/src/builder.rs | 2 + .../src/data_availability_checker.rs | 3 + .../beacon_chain/src/fetch_blobs/tests.rs | 2 +- .../src/partial_data_column_assembler.rs | 14 +- beacon_node/beacon_chain/src/test_utils.rs | 1 + beacon_node/http_api/src/publish_blocks.rs | 20 ++- beacon_node/lighthouse_network/src/lib.rs | 4 +- .../lighthouse_network/src/service/mod.rs | 117 ++++++++------- .../lighthouse_network/src/types/mod.rs | 2 +- .../lighthouse_network/src/types/partial.rs | 44 +++++- .../lighthouse_network/src/types/pubsub.rs | 24 ++- .../gossip_methods.rs | 75 ++++++---- .../src/network_beacon_processor/mod.rs | 138 +++++++++++++----- .../network_beacon_processor/sync_methods.rs | 2 +- beacon_node/network/src/service.rs | 13 +- 15 files changed, 313 insertions(+), 148 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index c24c7b6fc6..91a9dcc7c8 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -908,6 +908,7 @@ where let shuffling_cache_size = self.chain_config.shuffling_cache_size; let complete_blob_backfill = self.chain_config.complete_blob_backfill; let enable_partial_columns = self.chain_config.enable_partial_columns; + let disable_get_blobs = self.chain_config.disable_get_blobs; // Calculate the weak subjectivity point in which to backfill blocks to. let genesis_backfill_slot = if self.chain_config.genesis_backfill { @@ -1043,6 +1044,7 @@ where custody_context.clone(), self.spec.clone(), enable_partial_columns, + disable_get_blobs, ) .map_err(|e| format!("Error initializing DataAvailabilityChecker: {:?}", e))?, ), diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 29cbf84235..e559dc7689 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -121,6 +121,7 @@ impl DataAvailabilityChecker { custody_context: Arc>, spec: Arc, enable_partial_columns: bool, + disable_get_blobs: bool, ) -> Result { let inner = DataAvailabilityCheckerInner::new( OVERFLOW_LRU_CAPACITY, @@ -130,6 +131,7 @@ impl DataAvailabilityChecker { let partial_assembler = if enable_partial_columns { Some(Arc::new(PartialDataColumnAssembler::new( OVERFLOW_LRU_CAPACITY, + disable_get_blobs, ))) } else { None @@ -1432,6 +1434,7 @@ mod test { custody_context, spec, true, + false, ) .expect("should initialise data availability checker") } diff --git a/beacon_node/beacon_chain/src/fetch_blobs/tests.rs b/beacon_node/beacon_chain/src/fetch_blobs/tests.rs index 3c0f43fef0..8b805a8da3 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/tests.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/tests.rs @@ -344,7 +344,7 @@ fn mock_beacon_adapter(fork_name: ForkName, get_blobs_v3: bool) -> MockFetchBlob let test_runtime = TestRuntime::default(); let spec = Arc::new(fork_name.make_genesis_spec(E::default_spec())); let kzg = get_kzg(&spec); - let partial_assembler = PartialDataColumnAssembler::new(32); + let partial_assembler = PartialDataColumnAssembler::new(32, false); let mut mock_adapter = MockFetchBlobsBeaconAdapter::default(); mock_adapter.expect_spec().return_const(spec.clone()); diff --git a/beacon_node/beacon_chain/src/partial_data_column_assembler.rs b/beacon_node/beacon_chain/src/partial_data_column_assembler.rs index f8580352b2..0d21cb7621 100644 --- a/beacon_node/beacon_chain/src/partial_data_column_assembler.rs +++ b/beacon_node/beacon_chain/src/partial_data_column_assembler.rs @@ -13,6 +13,9 @@ use types::data::{ColumnIndex, PartialDataColumnHeader}; pub struct PartialDataColumnAssembler { /// Cache of assemblies keyed by block root assemblies: RwLock>>, + /// Whether getBlobs is disabled. If so, always set `has_local_blobs` to true, as we will never + /// retrieve blobs from the EL and therefore should immediately request cells from the network. + disable_get_blobs: bool, } /// Tracks partial columns being assembled for a single block @@ -43,9 +46,10 @@ pub struct PartialMergeResult { } impl PartialDataColumnAssembler { - pub fn new(capacity: usize) -> Self { + pub fn new(capacity: usize, disable_get_blobs: bool) -> Self { Self { assemblies: RwLock::new(LruCache::new(capacity)), + disable_get_blobs, } } @@ -60,7 +64,7 @@ impl PartialDataColumnAssembler { let assembly = PartialAssembly { header, - has_local_blobs: false, + has_local_blobs: self.disable_get_blobs, columns: HashMap::new(), }; @@ -82,7 +86,7 @@ impl PartialDataColumnAssembler { .entry(block_root) .or_insert_with(|| PartialAssembly { header: header.clone(), - has_local_blobs: false, + has_local_blobs: self.disable_get_blobs, columns: HashMap::new(), }); @@ -174,7 +178,7 @@ impl PartialDataColumnAssembler { signed_block_header: fulu.signed_block_header.clone(), kzg_commitments_inclusion_proof: fulu.kzg_commitments_inclusion_proof.clone(), }), - has_local_blobs: false, + has_local_blobs: self.disable_get_blobs, columns: Default::default(), }); let prev = assembly @@ -367,7 +371,7 @@ mod tests { } fn make_assembler() -> PartialDataColumnAssembler { - PartialDataColumnAssembler::new(16) + PartialDataColumnAssembler::new(16, false) } // -- init and get_header tests -- diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 2adfe26d4b..deaae6cba5 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -246,6 +246,7 @@ pub fn test_da_checker( custody_context, spec, true, + false, ) .expect("should initialise data availability checker") } diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 8b45a4b04c..e3e9839b2d 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -14,7 +14,7 @@ use eth2::types::{ }; use execution_layer::{ProvenancedPayload, SubmitBlindedBlockResponse}; use futures::TryFutureExt; -use lighthouse_network::PubsubMessage; +use lighthouse_network::{PubsubMessage, PubsubPartialMessage}; use logging::crit; use network::NetworkMessage; use rand::prelude::SliceRandom; @@ -442,12 +442,22 @@ pub(crate) fn publish_column_sidecars( // Publish partial messages if !partial_columns.is_empty() { if let Some(header) = partial_header { + let header = Arc::new(header); + let messages = partial_columns + .into_iter() + .map(|column| { + let mut request_cells = column.sidecar.cells_present_bitmap.clone(); + request_cells.not_inplace(); + PubsubPartialMessage::DataColumnFulu { + column, + request_cells, + header: header.clone(), + } + }) + .collect(); crate::utils::publish_network_message( sender_clone, - NetworkMessage::PublishPartialColumns { - columns: partial_columns, - header: Arc::new(header), - }, + NetworkMessage::PublishPartialColumns { messages }, ) .map_err(|_| { BlockError::BeaconChainError(Box::new(BeaconChainError::UnableToPublish)) diff --git a/beacon_node/lighthouse_network/src/lib.rs b/beacon_node/lighthouse_network/src/lib.rs index fdb6ff095e..7719ee8540 100644 --- a/beacon_node/lighthouse_network/src/lib.rs +++ b/beacon_node/lighthouse_network/src/lib.rs @@ -98,8 +98,8 @@ impl std::fmt::Display for ClearDialError<'_> { } pub use crate::types::{ - Enr, EnrSyncCommitteeBitfield, GossipTopic, NetworkGlobals, PubsubMessage, Subnet, - SubnetDiscovery, decode_partial, + Enr, EnrSyncCommitteeBitfield, GossipTopic, NetworkGlobals, PubsubMessage, + PubsubPartialMessage, Subnet, SubnetDiscovery, decode_partial, }; pub use prometheus_client; diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 862281c910..9037975a0e 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -20,7 +20,9 @@ use crate::types::{ SubnetDiscovery, all_topics_at_fork, core_topics_to_subscribe, is_fork_non_core_topic, subnet_from_topic_hash, }; -use crate::{Enr, NetworkGlobals, PubsubMessage, TopicHash, decode_partial, metrics}; +use crate::{ + Enr, NetworkGlobals, PubsubMessage, PubsubPartialMessage, TopicHash, decode_partial, metrics, +}; use api_types::{AppRequestId, Response}; use futures::stream::StreamExt; use gossipsub_scoring_parameters::{PeerScoreSettings, lighthouse_gossip_thresholds}; @@ -43,8 +45,9 @@ use std::sync::Arc; use std::time::Duration; use tracing::{debug, error, info, trace, warn}; use types::{ - ChainSpec, DataColumnSubnetId, EnrForkId, EthSpec, ForkContext, ForkName, PartialDataColumn, - PartialDataColumnHeader, Slot, SubnetId, consts::altair::SYNC_COMMITTEE_SUBNET_COUNT, + CellBitmap, ChainSpec, DataColumnSubnetId, EnrForkId, EthSpec, ForkContext, ForkName, + PartialDataColumn, PartialDataColumnHeader, Slot, SubnetId, + consts::altair::SYNC_COMMITTEE_SUBNET_COUNT, }; use utils::{Context as ServiceContext, build_transport, strip_peer_id}; @@ -920,65 +923,73 @@ impl Network { } /// Publishes partial data column sidecars to the gossipsub network. - pub fn publish_partial( - &mut self, - columns: Vec>>, - header: Arc>, - ) { + pub fn publish_partial(&mut self, messages: Vec>) { if !self.network_globals.config.enable_partial_columns { return; } - debug!( - count = columns.len(), - "Sending partial data column sidecars" - ); + debug!(count = messages.len(), "Sending partial messages"); - for column in columns { - let subnet = - DataColumnSubnetId::from_column_index(column.index, &self.fork_context.spec); - let topic = GossipTopic::new( - GossipKind::DataColumnSidecar(subnet), - GossipEncoding::default(), - self.enr_fork_id.fork_digest, - ); - let header_sent_set = self - .partial_column_header_tracker - .get_for_block(column.block_root); - let partial_message = OutgoingPartialColumn::new(column, &header, header_sent_set); - let publish_topic: Topic = topic.clone().into(); - - if let Err(e) = self - .gossipsub_mut() - .publish_partial(publish_topic, partial_message) - { - match e { - PublishError::NoPeersSubscribedToTopic => { - debug!( - kind = %topic.kind(), - "No peers supporting partial messages" - ); - } - ref e => { - warn!( - error = ?e, - kind = %topic.kind(), - "Could not publish partial message" - ); - } - } - - // add to metrics - if let Some(v) = metrics::get_int_gauge( - &metrics::FAILED_PARTIAL_PUBLISHES_PER_MAIN_TOPIC, - &[&format!("{:?}", topic.kind())], - ) { - v.inc() - }; + for message in messages { + match message { + PubsubPartialMessage::DataColumnFulu { + column, + request_cells, + header, + } => self.publish_partial_data_column_fulu(column, request_cells, header), } } } + fn publish_partial_data_column_fulu( + &mut self, + column: Arc>, + request_cells: CellBitmap, + header: Arc>, + ) { + let subnet = DataColumnSubnetId::from_column_index(column.index, &self.fork_context.spec); + let topic = GossipTopic::new( + GossipKind::DataColumnSidecar(subnet), + GossipEncoding::default(), + self.enr_fork_id.fork_digest, + ); + let header_sent_set = self + .partial_column_header_tracker + .get_for_block(column.block_root); + let partial_message = + OutgoingPartialColumn::new(column, &header, header_sent_set, request_cells); + let publish_topic: Topic = topic.clone().into(); + + if let Err(e) = self + .gossipsub_mut() + .publish_partial(publish_topic, partial_message) + { + match e { + PublishError::NoPeersSubscribedToTopic => { + debug!( + kind = %topic.kind(), + "No peers supporting partial messages" + ); + } + ref e => { + warn!( + error = ?e, + kind = %topic.kind(), + "Could not publish partial message" + ); + } + } + + // add to metrics + if let Some(v) = metrics::get_int_gauge( + &metrics::FAILED_PARTIAL_PUBLISHES_PER_MAIN_TOPIC, + &[&format!("{:?}", topic.kind())], + ) { + v.inc() + }; + } + } + /// Informs the gossipsub about the result of a message validation. /// If the message is valid it will get propagated by gossipsub. pub fn report_message_validation_result( diff --git a/beacon_node/lighthouse_network/src/types/mod.rs b/beacon_node/lighthouse_network/src/types/mod.rs index d0173e5b9a..8a20e9da94 100644 --- a/beacon_node/lighthouse_network/src/types/mod.rs +++ b/beacon_node/lighthouse_network/src/types/mod.rs @@ -16,7 +16,7 @@ pub use eth2::lighthouse::sync_state::{BackFillState, CustodyBackFillState, Sync pub use globals::NetworkGlobals; pub use partial::HeaderSentSet; pub use partial::OutgoingPartialColumn; -pub use pubsub::{PubsubMessage, SnappyTransform, decode_partial}; +pub use pubsub::{PubsubMessage, PubsubPartialMessage, SnappyTransform, decode_partial}; pub use subnet::{Subnet, SubnetDiscovery}; pub use topics::{ GossipEncoding, GossipKind, GossipTopic, TopicConfig, all_topics_at_fork, diff --git a/beacon_node/lighthouse_network/src/types/partial.rs b/beacon_node/lighthouse_network/src/types/partial.rs index 4b5dcd8ad6..0dd5c58ac6 100644 --- a/beacon_node/lighthouse_network/src/types/partial.rs +++ b/beacon_node/lighthouse_network/src/types/partial.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use tracing::{error, trace}; use types::core::{EthSpec, Hash256}; use types::data::{ - PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, + CellBitmap, PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, PartialDataColumnSidecar, PartialDataColumnSidecarRef, }; @@ -30,10 +30,29 @@ impl OutgoingPartialColumn { partial_column: Arc>, header: &PartialDataColumnHeader, header_sent_set: HeaderSentSet, + requests: CellBitmap, ) -> Self { - // For now, always request all cells - let mut requests = partial_column.sidecar.cells_present_bitmap.clone_zeroed(); - requests.not_inplace(); + // Always set the request bit for available cells. + // + // Gossipsub applys certain optimisations to avoid sending redundant messages. This + // requires that we stay consistent with our metadata. Gossipsub uses the `Metadata` trait + // impl below to determine whether it can perform these optimisations. + // + // If we request a cell and then receive it, un-setting the request bit in the next + // published message may cause issues: + // Gossipsub tries to avoid the impact of application race conditions by checking newly + // published metadata against previously published metadata. This no longer functions + // correctly if request bits are unset between calls, as Gossipsub will consider a message + // with new requests as new info to be propagated, possibly overwriting previous messages + // with more cells (but fewer request bits). This is because gossipsub will see that both + // metadata have some bits that are not set in the other metadata and therefore cannot + // decide which actually carries more data. By always setting request bits for available + // cells, we avoid this issue, as requests will never be unset between calls. + // + // In other words, gossipsub relies on the fact that metadata is additive. The request bit + // is, therefore, to be seen as a "request if not available" bit. + let requests = requests.union(&partial_column.sidecar.cells_present_bitmap); + let metadata = PartialDataColumnPartsMetadata:: { available: partial_column.sidecar.cells_present_bitmap.clone(), requests, @@ -322,6 +341,14 @@ mod tests { }) } + fn make_all_one_bitmap(len: usize) -> CellBitmap { + let mut request_cells = CellBitmap::::with_capacity(len).unwrap(); + for idx in 0..request_cells.len() { + request_cells.set(idx, true).unwrap(); + } + request_cells + } + fn random_peer_id() -> PeerId { let keypair = Keypair::generate_ed25519(); PeerId::from(keypair.public()) @@ -422,7 +449,8 @@ mod tests { let header = make_header(4); let partial = make_partial_column(root, 4, &[0, 1]); let header_sent_set: HeaderSentSet = Arc::new(Mutex::new(HashSet::new())); - let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set); + let requests = make_all_one_bitmap(4); + let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set, requests); let peer = random_peer_id(); @@ -442,7 +470,8 @@ mod tests { // We have cells [0, 2, 3] let partial = make_partial_column(root, 4, &[0, 2, 3]); let header_sent_set: HeaderSentSet = Arc::new(Mutex::new(HashSet::new())); - let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set); + let requests = make_all_one_bitmap(4); + let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set, requests); let peer = random_peer_id(); @@ -474,7 +503,8 @@ mod tests { // We have cells [0] let partial = make_partial_column(root, 4, &[0]); let header_sent_set: HeaderSentSet = Arc::new(Mutex::new(HashSet::new())); - let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set); + let requests = make_all_one_bitmap(4); + let outgoing = OutgoingPartialColumn::new(partial, &header, header_sent_set, requests); let peer = random_peer_id(); diff --git a/beacon_node/lighthouse_network/src/types/pubsub.rs b/beacon_node/lighthouse_network/src/types/pubsub.rs index d486ca5129..00b2c42629 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -7,10 +7,10 @@ use ssz::{Decode, Encode}; use std::io::{Error, ErrorKind}; use std::sync::Arc; use types::{ - AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, DataColumnSidecar, + AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, CellBitmap, DataColumnSidecar, DataColumnSubnetId, EthSpec, ForkContext, ForkName, Hash256, LightClientFinalityUpdate, - LightClientOptimisticUpdate, PartialDataColumn, PartialDataColumnSidecar, - PayloadAttestationMessage, ProposerSlashing, SignedAggregateAndProof, + LightClientOptimisticUpdate, PartialDataColumn, PartialDataColumnHeader, + PartialDataColumnSidecar, PayloadAttestationMessage, ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase, SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockElectra, @@ -56,6 +56,24 @@ pub enum PubsubMessage { LightClientOptimisticUpdate(Box>), } +/// A message published via the partial gossipsub protocol. +#[derive(Debug, Clone)] +pub enum PubsubPartialMessage { + /// A partial data column sidecar from the Fulu fork. + DataColumnFulu { + /// The column to publish. Libp2p will cache it and treat it as the data to send if any peer + /// asks for data within it. + column: Arc>, + /// The cells we are requesting. Usually, this will be all-ones, as we need all cells. + /// However, while get_blobs is still in progress, blobs we expect from the EL should not be + /// requested to conserve bandwidth. + request_cells: CellBitmap, + /// The header associated with the column above. This is set separately here, as the column + /// to be published does not contain the header - it is stored without. + header: Arc>, + }, +} + // Implements the `DataTransform` trait of gossipsub to employ snappy compression pub struct SnappyTransform { /// Sets the maximum size we allow gossipsub messages to decompress to. diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 20342c1aa9..346e621879 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -33,7 +33,7 @@ use beacon_chain::{ use beacon_processor::{Work, WorkEvent}; use lighthouse_network::{ Client, GossipTopic, MessageAcceptance, MessageId, PeerAction, PeerId, PubsubMessage, - ReportSource, + PubsubPartialMessage, ReportSource, }; use logging::crit; use operation_pool::ReceivedPreCapella; @@ -937,9 +937,15 @@ impl NetworkBeaconProcessor { Ok(mut column) => { let header = column.sidecar.header.take(); if let Some(header) = header { + // Requesting cells is irrelevant as all cells are available, simply clone + // the `cells_present_bitmap`. + let request_cells = column.sidecar.cells_present_bitmap.clone(); self.send_network_message(NetworkMessage::PublishPartialColumns { - columns: vec![Arc::new(column)], - header: Arc::new(header), + messages: vec![PubsubPartialMessage::DataColumnFulu { + column: Arc::new(column), + request_cells, + header: Arc::new(header), + }], }); } else { crit!("Converting from full to partial yielded headerless partial") @@ -1077,8 +1083,10 @@ impl NetworkBeaconProcessor { debug!(block = %block_root, "Triggering getBlobs after receiving partial header"); // We want to publish immediately when this finishes let publish_blobs = true; - self.fetch_engine_blobs_and_publish(header.into_header(), block_root, publish_blobs) - .await + let header = header.into_header(); + self.fetch_engine_blobs_and_publish_full(header.clone(), block_root, publish_blobs) + .await; + self.publish_partial_data_columns(header, block_root).await; } } } @@ -1311,28 +1319,31 @@ impl NetworkBeaconProcessor { }); } - let only_send_completed_partials = - merge_result.local_blobs || self.chain.config.disable_get_blobs; - let columns = merge_result - .updated_partials - .into_iter() - .map(|partial| partial.into_inner()) - .filter(|partial| { - !only_send_completed_partials || partial.sidecar.is_complete() - }) - .collect::>(); - - if !columns.is_empty() { - if only_send_completed_partials { - debug!( - block = %block_root, - "Not publishing incomplete partials before getBlobs" - ); - } - self.send_network_message(NetworkMessage::PublishPartialColumns { - columns, - header: verified_header.into_header(), - }); + if !merge_result.updated_partials.is_empty() { + let header = verified_header.into_header(); + let messages = merge_result + .updated_partials + .into_iter() + .map(|partial| { + let column = partial.into_inner(); + let present_cells = &column.sidecar.cells_present_bitmap; + let request_cells = if merge_result.local_blobs { + // Request all cells that are not available locally. + let mut all_one = present_cells.clone_zeroed(); + all_one.not_inplace(); + all_one + } else { + // Do not request cells if we don't know the local blobs yet. + present_cells.clone_zeroed() + }; + PubsubPartialMessage::DataColumnFulu { + column, + request_cells, + header: header.clone(), + } + }) + .collect(); + self.send_network_message(NetworkMessage::PublishPartialColumns { messages }); } Ok(avail) } @@ -1803,8 +1814,16 @@ impl NetworkBeaconProcessor { self.executor.spawn( async move { if let Ok(header) = PartialDataColumnHeader::try_from(block_clone.as_ref()) { + let header = Arc::new(header); self_clone - .fetch_engine_blobs_and_publish(Arc::new(header), block_root, publish_blobs) + .fetch_engine_blobs_and_publish_full( + header.clone(), + block_root, + publish_blobs, + ) + .await; + self_clone + .publish_partial_data_columns(header, block_root) .await } } diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index 7619f706cc..c5023ed5f4 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -22,9 +22,13 @@ use lighthouse_network::rpc::methods::{ use lighthouse_network::service::api_types::CustodyBackfillBatchId; use lighthouse_network::{ Client, GossipTopic, MessageId, NetworkConfig, NetworkGlobals, PeerId, PubsubMessage, + PubsubPartialMessage, rpc::{BlocksByRangeRequest, BlocksByRootRequest, LightClientBootstrapRequest, StatusMessage}, }; +use logging::crit; use rand::prelude::SliceRandom; +use ssz_types::VariableList; +use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -907,7 +911,7 @@ impl NetworkBeaconProcessor { }); } - pub async fn fetch_engine_blobs_and_publish( + pub async fn fetch_engine_blobs_and_publish_full( self: &Arc, header: Arc>, block_root: Hash256, @@ -931,7 +935,7 @@ impl NetworkBeaconProcessor { match fetch_and_process_engine_blobs( self.chain.clone(), block_root, - header.clone(), + header, custody_columns, publish_fn, ) @@ -975,44 +979,108 @@ impl NetworkBeaconProcessor { ); } } + } - // Publish partial columns without eager send - // TODO(gloas): implement publish partial columns without eager send - if let Some(assembler) = self.chain.data_availability_checker.partial_assembler() { - let columns = assembler.get_columns_and_mark_as_local_fetched(block_root, &header); + pub async fn publish_partial_data_columns( + self: &Arc, + header: Arc>, + block_root: Hash256, + ) { + if header.kzg_commitments.is_empty() { + return; + } + + // TODO(gloas): implement publish partial columns + let Some(assembler) = self.chain.data_availability_checker.partial_assembler() else { + // Partials are disabled. + return; + }; + let epoch = header.slot().epoch(T::EthSpec::slots_per_epoch()); + let custody_columns = self.chain.sampling_columns_for_epoch(epoch); + let columns = assembler.get_columns_and_mark_as_local_fetched(block_root, &header); + + let mut present_indices: HashSet = HashSet::with_capacity(columns.len()); + let mut messages: Vec> = Vec::with_capacity(columns.len()); + for column in columns { // Republish both complete and incomplete columns as partials - let columns: Vec<_> = columns - .into_iter() - .filter_map(|column| match column { - AssemblyColumn::Incomplete(partial) => Some(partial.into_inner()), - AssemblyColumn::Complete(full) => { - let DataColumnSidecar::Fulu(fulu) = full.as_data_column() else { - return None; - }; - match fulu.to_partial() { - Ok(partial) => Some(Arc::new(partial)), - Err(err) => { - error!( - %block_root, - column_index = %full.index(), - ?err, - "Failed to convert complete column to partial for re-seeding" - ); - None - } + let partial_column = match column { + AssemblyColumn::Incomplete(partial) => partial.into_inner(), + AssemblyColumn::Complete(full) => { + let DataColumnSidecar::Fulu(fulu) = full.as_data_column() else { + continue; + }; + match fulu.to_partial() { + Ok(partial) => Arc::new(partial), + Err(err) => { + error!( + %block_root, + column_index = %full.index(), + ?err, + "Failed to convert complete column to partial for re-seeding" + ); + continue; } } - }) - .collect(); - if !columns.is_empty() { - debug!(block = %block_root, "Publishing all partials after getBlobs"); - self.send_network_message(NetworkMessage::PublishPartialColumns { - columns, - header, - }); - } else { - debug!(block = %block_root, "No partials to publish after getBlobs"); + } + }; + + present_indices.insert(partial_column.index); + let mut request_cells = partial_column.sidecar.cells_present_bitmap.clone_zeroed(); + request_cells.not_inplace(); + messages.push(PubsubPartialMessage::DataColumnFulu { + column: partial_column, + request_cells, + header: header.clone(), + }); + } + + // For each custody column without any local partial, send an empty placeholder + // that requests all cells. + let num_cells = header.kzg_commitments.len(); + for col_idx in custody_columns { + if present_indices.contains(col_idx) { + continue; } + // `kzg_commitments.len()` is bounded by `MaxBlobCommitmentsPerBlock`, so the + // bitmap constructor is infallible. + let Ok(cells_present_bitmap) = CellBitmap::::with_capacity(num_cells) + else { + crit!( + %block_root, + num_cells, + column_index = %col_idx, + "CellBitmap construction failed despite being bounded by MaxBlobCommitmentsPerBlock" + ); + continue; + }; + let request_cells = cells_present_bitmap.not(); + messages.push(PubsubPartialMessage::DataColumnFulu { + column: Arc::new(PartialDataColumn { + block_root, + index: *col_idx, + sidecar: PartialDataColumnSidecar { + cells_present_bitmap, + column: VariableList::empty(), + kzg_proofs: VariableList::empty(), + header: None.into(), + }, + }), + request_cells, + header: header.clone(), + }); + } + + if !messages.is_empty() { + debug!( + block = %block_root, + count = messages.len(), + "Publishing all partials" + ); + self.send_network_message(NetworkMessage::PublishPartialColumns { messages }); + } else { + // This should not happen, as any custody columns will have at least an empty + // partial published. + warn!(block = %block_root, "No partials to publish"); } } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index 35437e1a2e..b9e07743eb 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -222,7 +222,7 @@ impl NetworkBeaconProcessor { // to be sent from the peers if we already have them. if let Ok(header) = signed_beacon_block.as_ref().try_into() { let publish_blobs = false; - self.fetch_engine_blobs_and_publish( + self.fetch_engine_blobs_and_publish_full( Arc::new(header), block_root, publish_blobs, diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index c2e79fe9e8..ba4aada352 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -19,7 +19,7 @@ use lighthouse_network::rpc::methods::RpcResponse; use lighthouse_network::service::Network; use lighthouse_network::types::GossipKind; use lighthouse_network::{ - Context, PeerAction, PubsubMessage, ReportSource, Response, Subnet, + Context, PeerAction, PubsubMessage, PubsubPartialMessage, ReportSource, Response, Subnet, rpc::{GoodbyeReason, RpcErrorResponse}, }; use lighthouse_network::{MessageAcceptance, prometheus_client::registry::Registry}; @@ -39,8 +39,8 @@ use tokio::time::Sleep; use tracing::{debug, error, info, trace, warn}; use typenum::Unsigned; use types::{ - EthSpec, ForkContext, PartialDataColumn, PartialDataColumnHeader, Slot, SubnetId, - SyncCommitteeSubscription, SyncSubnetId, ValidatorSubscription, + EthSpec, ForkContext, Slot, SubnetId, SyncCommitteeSubscription, SyncSubnetId, + ValidatorSubscription, }; mod tests; @@ -85,8 +85,7 @@ pub enum NetworkMessage { Publish { messages: Vec> }, /// Publish partial data column sidecars via the partial gossipsub protocol. PublishPartialColumns { - columns: Vec>>, - header: Arc>, + messages: Vec>, }, /// Validates a received gossipsub message. This will propagate the message on the network. ValidationResult { @@ -683,8 +682,8 @@ impl NetworkService { ); self.libp2p.publish(messages); } - NetworkMessage::PublishPartialColumns { columns, header } => { - self.libp2p.publish_partial(columns, header); + NetworkMessage::PublishPartialColumns { messages } => { + self.libp2p.publish_partial(messages); } NetworkMessage::ReportPeer { peer_id, From 477c25db9f39bef5b3afb29643a257a8aa5d249b Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sat, 20 Jun 2026 12:41:30 -0700 Subject: [PATCH 04/12] Fix race condition between validator duties service and proposer preferences (#9309) The proposer preferences service was attempting to publish preferences at the start of each epoch. This caused it to race with the validator duties service, it wouldn't calculate validator duties in time for the proposer preference service. This PR first updates the validator duties service to calculate proposer duties for the current epoch and the next epoch. After Fulu we have the ability to look ahead one epoch for proposer duties, but we never updated the vc to leverage this feature. This PR also updates the proposer preferences service to fire at every slot. We have an `(Epoch, DependentRoot)` map that prevents us from publishing the same preferences twice. The changes here should prevent the race condition between the two services and make the proposer preferences service more robust in general. Co-Authored-By: Eitan Seri- Levi Co-Authored-By: Eitan Seri-Levi Co-Authored-By: Michael Sproul --- book/src/help_vc.md | 5 + lighthouse/tests/validator_client.rs | 15 +++ validator_client/src/cli.rs | 11 ++ validator_client/src/config.rs | 4 + validator_client/src/lib.rs | 1 + .../validator_services/src/duties_service.rs | 127 +++++++++++------- .../src/proposer_preferences_service.rs | 99 +++++++++----- 7 files changed, 182 insertions(+), 80 deletions(-) diff --git a/book/src/help_vc.md b/book/src/help_vc.md index f1a342197c..719b02a5a5 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -200,6 +200,11 @@ Flags: If present, do not configure the system allocator. Providing this flag will generally increase memory usage, it should only be provided when debugging specific memory allocation issues. + --disable-proposer-duties-v2 + Fetch proposer duties using the v1 beacon node endpoint instead of v2. + The v1 endpoint reports an incorrect dependent root which causes + spurious proposer duty re-org warnings. Only enable this flag if your + beacon node does not serve the v2 proposer duties endpoint. --disable-slashing-protection-web3signer Disable Lighthouse's slashing protection for all web3signer keys. This can reduce the I/O burden on the VC but is only safe if slashing diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 945e363ab5..2cbf2aaef0 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -130,6 +130,21 @@ fn disable_auto_discover_flag() { .with_config(|config| assert!(config.disable_auto_discover)); } +#[test] +fn disable_proposer_duties_v2_default() { + CommandLineTest::new() + .run() + .with_config(|config| assert!(!config.disable_proposer_duties_v2)); +} + +#[test] +fn disable_proposer_duties_v2_flag() { + CommandLineTest::new() + .flag("disable-proposer-duties-v2", None) + .run() + .with_config(|config| assert!(config.disable_proposer_duties_v2)); +} + #[test] fn init_slashing_protections_flag() { CommandLineTest::new() diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index e5fe1580da..cf21e276d7 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -105,6 +105,17 @@ pub struct ValidatorClient { )] pub disable_attesting: bool, + #[clap( + long, + help = "Fetch proposer duties using the v1 beacon node endpoint instead of v2. The v1 \ + endpoint reports an incorrect dependent root which causes spurious proposer duty \ + re-org warnings. Only enable this flag if your beacon node does not serve the v2 \ + proposer duties endpoint.", + display_order = 0, + help_heading = FLAG_HEADER + )] + pub disable_proposer_duties_v2: bool, + #[clap( long, help = "If present, the validator client will use longer timeouts for requests \ diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 418cd385da..3e5abebc68 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -92,6 +92,8 @@ pub struct Config { #[serde(flatten)] pub initialized_validators: InitializedValidatorsConfig, pub disable_attesting: bool, + /// Fetch proposer duties using the v1 endpoint instead of v2. + pub disable_proposer_duties_v2: bool, } impl Default for Config { @@ -139,6 +141,7 @@ impl Default for Config { distributed: false, initialized_validators: <_>::default(), disable_attesting: false, + disable_proposer_duties_v2: false, } } } @@ -402,6 +405,7 @@ impl Config { }; config.disable_attesting = validator_client_config.disable_attesting; + config.disable_proposer_duties_v2 = validator_client_config.disable_proposer_duties_v2; Ok(config) } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 71d9333493..9680189b1a 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -502,6 +502,7 @@ impl ProductionValidatorClient { .attestation_selection_proof_config(attestation_selection_proof_config) .sync_selection_proof_config(sync_selection_proof_config) .disable_attesting(config.disable_attesting) + .disable_proposer_duties_v2(config.disable_proposer_duties_v2) .build()?, ); diff --git a/validator_client/validator_services/src/duties_service.rs b/validator_client/validator_services/src/duties_service.rs index 2a371abf62..5fe413a216 100644 --- a/validator_client/validator_services/src/duties_service.rs +++ b/validator_client/validator_services/src/duties_service.rs @@ -305,6 +305,7 @@ pub struct DutiesServiceBuilder { /// Create sync selection proof config sync_selection_proof_config: SelectionProofConfig, disable_attesting: bool, + disable_proposer_duties_v2: bool, } impl Default for DutiesServiceBuilder { @@ -325,6 +326,7 @@ impl DutiesServiceBuilder { attestation_selection_proof_config: SelectionProofConfig::default(), sync_selection_proof_config: SelectionProofConfig::default(), disable_attesting: false, + disable_proposer_duties_v2: false, } } @@ -382,6 +384,11 @@ impl DutiesServiceBuilder { self } + pub fn disable_proposer_duties_v2(mut self, disable_proposer_duties_v2: bool) -> Self { + self.disable_proposer_duties_v2 = disable_proposer_duties_v2; + self + } + pub fn build(self) -> Result, String> { Ok(DutiesService { attesters: Default::default(), @@ -405,6 +412,7 @@ impl DutiesServiceBuilder { enable_high_validator_count_metrics: self.enable_high_validator_count_metrics, selection_proof_config: self.attestation_selection_proof_config, disable_attesting: self.disable_attesting, + disable_proposer_duties_v2: self.disable_proposer_duties_v2, }) } } @@ -437,6 +445,11 @@ pub struct DutiesService { /// Pass the config for distributed or non-distributed mode. pub selection_proof_config: SelectionProofConfig, pub disable_attesting: bool, + /// Use the v1 proposer duties endpoint instead of v2. The v1 endpoint reports an incorrect + /// dependent root, causing spurious "Proposer duties re-org" warnings. This flag exists for + /// compatibility with beacon nodes that do not yet serve the v2 endpoint and can be removed + /// after Gloas. + pub disable_proposer_duties_v2: bool, } impl DutiesService { @@ -1660,54 +1673,8 @@ async fn poll_beacon_proposers( // Only download duties and push out additional block production events if we have some // validators. if !local_pubkeys.is_empty() { - let download_result = duties_service - .beacon_nodes - .first_success(|beacon_node| async move { - let _timer = validator_metrics::start_timer_vec( - &validator_metrics::DUTIES_SERVICE_TIMES, - &[validator_metrics::PROPOSER_DUTIES_HTTP_GET], - ); - beacon_node - .get_validator_duties_proposer(current_epoch) - .await - }) - .await; - - match download_result { - Ok(response) => { - let dependent_root = response.dependent_root; - - let relevant_duties = response - .data - .into_iter() - .filter(|proposer_duty| local_pubkeys.contains(&proposer_duty.pubkey)) - .collect::>(); - - debug!( - %dependent_root, - num_relevant_duties = relevant_duties.len(), - "Downloaded proposer duties" - ); - - if let Some((prior_dependent_root, _)) = duties_service - .proposers - .write() - .insert(current_epoch, (dependent_root, relevant_duties)) - && dependent_root != prior_dependent_root - { - warn!( - %prior_dependent_root, - %dependent_root, - msg = "this may happen from time to time", - "Proposer duties re-org" - ) - } - } - // Don't return early here, we still want to try and produce blocks using the cached values. - Err(e) => error!( - err = %e, - "Failed to download proposer duties" - ), + for epoch in [current_epoch, current_epoch + 1] { + fetch_and_store_proposer_duties(duties_service, epoch, &local_pubkeys).await; } // Compute the block proposers for this slot again, now that we've received an update from @@ -1750,6 +1717,70 @@ async fn poll_beacon_proposers( Ok(()) } +async fn fetch_and_store_proposer_duties( + duties_service: &DutiesService, + epoch: Epoch, + local_pubkeys: &HashSet, +) { + let use_v2 = !duties_service.disable_proposer_duties_v2; + let download_result = duties_service + .beacon_nodes + .first_success(|beacon_node| async move { + let _timer = validator_metrics::start_timer_vec( + &validator_metrics::DUTIES_SERVICE_TIMES, + &[validator_metrics::PROPOSER_DUTIES_HTTP_GET], + ); + // Prefer the v2 endpoint, which reports the correct dependent root. The v1 endpoint + // returns an incorrect dependent root, leading to spurious "Proposer duties re-org" + // warnings. + if use_v2 { + beacon_node.get_validator_duties_proposer_v2(epoch).await + } else { + beacon_node.get_validator_duties_proposer(epoch).await + } + }) + .await; + + match download_result { + Ok(response) => { + let dependent_root = response.dependent_root; + + let relevant_duties = response + .data + .into_iter() + .filter(|proposer_duty| local_pubkeys.contains(&proposer_duty.pubkey)) + .collect::>(); + + debug!( + %dependent_root, + %epoch, + num_relevant_duties = relevant_duties.len(), + "Downloaded proposer duties" + ); + + if let Some((prior_dependent_root, _)) = duties_service + .proposers + .write() + .insert(epoch, (dependent_root, relevant_duties)) + && dependent_root != prior_dependent_root + { + warn!( + %prior_dependent_root, + %dependent_root, + %epoch, + msg = "this may happen from time to time", + "Proposer duties re-org" + ) + } + } + Err(e) => error!( + err = %e, + %epoch, + "Failed to download proposer duties" + ), + } +} + /// Query the beacon node for ptc duties for any known validators. async fn poll_beacon_ptc_attesters( duties_service: &Arc>, diff --git a/validator_client/validator_services/src/proposer_preferences_service.rs b/validator_client/validator_services/src/proposer_preferences_service.rs index 5d5c40a6cd..330517482e 100644 --- a/validator_client/validator_services/src/proposer_preferences_service.rs +++ b/validator_client/validator_services/src/proposer_preferences_service.rs @@ -1,12 +1,14 @@ use crate::duties_service::DutiesService; use beacon_node_fallback::BeaconNodeFallback; +use eth2::types::ProposerData; use slot_clock::SlotClock; +use std::collections::HashMap; use std::ops::Deref; use std::sync::Arc; use task_executor::TaskExecutor; use tokio::time::sleep; use tracing::{debug, error, info, warn}; -use types::{ChainSpec, Epoch, EthSpec, ForkName, ProposerPreferences}; +use types::{ChainSpec, Epoch, EthSpec, ForkName, Hash256, ProposerPreferences}; use validator_store::ValidatorStore; pub struct Inner { @@ -66,6 +68,8 @@ impl ProposerPreferencesSer let executor = self.executor.clone(); let interval_fut = async move { + let mut published_preferences: HashMap = HashMap::new(); + loop { let Some(current_slot) = self.slot_clock.now() else { error!("Failed to read slot clock"); @@ -73,29 +77,16 @@ impl ProposerPreferencesSer continue; }; - if !self - .chain_spec - .fork_name_at_slot::(current_slot) - .gloas_enabled() - { - let duration_to_next_epoch = self - .slot_clock - .duration_to_next_epoch(S::E::slots_per_epoch()) - .unwrap_or_else(|| slot_duration * S::E::slots_per_epoch() as u32); - sleep(duration_to_next_epoch).await; - continue; - } - let current_epoch = current_slot.epoch(S::E::slots_per_epoch()); - let fork_name = self.chain_spec.fork_name_at_slot::(current_slot); - self.publish_proposer_preferences(current_epoch, fork_name) + + self.poll_and_publish_preferences(current_epoch, &mut published_preferences) .await; - let duration_to_next_epoch = self + let duration_to_next_slot = self .slot_clock - .duration_to_next_epoch(S::E::slots_per_epoch()) - .unwrap_or_else(|| slot_duration * S::E::slots_per_epoch() as u32); - sleep(duration_to_next_epoch).await; + .duration_to_next_slot() + .unwrap_or(slot_duration); + sleep(duration_to_next_slot).await; } }; @@ -103,15 +94,57 @@ impl ProposerPreferencesSer Ok(()) } - async fn publish_proposer_preferences(&self, current_epoch: Epoch, fork_name: ForkName) { - let (dependent_root, duties) = { - let proposers = self.duties_service.proposers.read(); - match proposers.get(¤t_epoch) { - Some((root, duties)) => (*root, duties.clone()), - None => return, + /// Publish proposer preferences for `current_epoch` and `current_epoch + 1`. + /// Will only publish preferences for a given epoch once per dependent root. + async fn poll_and_publish_preferences( + &self, + current_epoch: Epoch, + published_preferences: &mut HashMap, + ) { + for (epoch, fork_name) in [ + ( + current_epoch, + self.chain_spec.fork_name_at_epoch(current_epoch), + ), + ( + current_epoch + 1, + self.chain_spec.fork_name_at_epoch(current_epoch + 1), + ), + ] { + if !fork_name.gloas_enabled() { + continue; } - }; + let (dependent_root, duties) = { + let proposers = self.duties_service.proposers.read(); + match proposers.get(&epoch) { + Some((root, duties)) => (*root, duties.clone()), + None => continue, + } + }; + + if published_preferences.get(&epoch) == Some(&dependent_root) { + continue; + } + + if self + .publish_proposer_preferences(epoch, fork_name, dependent_root, duties) + .await + { + published_preferences.insert(epoch, dependent_root); + } + } + + published_preferences.retain(|epoch, _| *epoch >= current_epoch); + } + + async fn publish_proposer_preferences( + &self, + epoch: Epoch, + fork_name: ForkName, + dependent_root: Hash256, + duties: Vec, + ) -> bool { let preferences_to_sign: Vec<_> = { let mut result = vec![]; for duty in &duties { @@ -144,11 +177,11 @@ impl ProposerPreferencesSer }; if preferences_to_sign.is_empty() { - return; + return false; } debug!( - %current_epoch, + %epoch, count = preferences_to_sign.len(), "Signing proposer preferences" ); @@ -172,7 +205,7 @@ impl ProposerPreferencesSer } if signed.is_empty() { - return; + return false; } let count = signed.len(); @@ -213,17 +246,19 @@ impl ProposerPreferencesSer match result { Ok(()) => { info!( - %current_epoch, + %epoch, %count, "Successfully published proposer preferences" ); + true } Err(e) => { error!( error = %e, - %current_epoch, + %epoch, "Failed to publish proposer preferences" ); + false } } } From 10568b139b3f2fc02c3dab2f8de5165349c97b88 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sat, 20 Jun 2026 12:41:34 -0700 Subject: [PATCH 05/12] Add proposer preferences SSE event (#9308) This is needed to connect to buildoor (Kurtosis package, acts as a trustless builder) Co-Authored-By: Eitan Seri- Levi Co-Authored-By: Eitan Seri-Levi Co-Authored-By: Michael Sproul --- beacon_node/beacon_chain/src/events.rs | 15 ++++ .../gossip_verified_proposer_preferences.rs | 14 ++++ beacon_node/beacon_chain/tests/events.rs | 83 ++++++++++++++++++- beacon_node/http_api/src/lib.rs | 3 + common/eth2/src/types.rs | 11 +++ .../types/src/builder/proposer_preferences.rs | 22 +++++ 6 files changed, 145 insertions(+), 3 deletions(-) diff --git a/beacon_node/beacon_chain/src/events.rs b/beacon_node/beacon_chain/src/events.rs index 80667cd399..dd08c59c76 100644 --- a/beacon_node/beacon_chain/src/events.rs +++ b/beacon_node/beacon_chain/src/events.rs @@ -29,6 +29,7 @@ pub struct ServerSentEventHandler { execution_payload_gossip_tx: Sender>, execution_payload_available_tx: Sender>, execution_payload_bid_tx: Sender>, + proposer_preferences_tx: Sender>, payload_attestation_message_tx: Sender>, } @@ -60,6 +61,7 @@ impl ServerSentEventHandler { let (execution_payload_gossip_tx, _) = broadcast::channel(capacity); let (execution_payload_available_tx, _) = broadcast::channel(capacity); let (execution_payload_bid_tx, _) = broadcast::channel(capacity); + let (proposer_preferences_tx, _) = broadcast::channel(capacity); let (payload_attestation_message_tx, _) = broadcast::channel(capacity); Self { @@ -85,6 +87,7 @@ impl ServerSentEventHandler { execution_payload_gossip_tx, execution_payload_available_tx, execution_payload_bid_tx, + proposer_preferences_tx, payload_attestation_message_tx, } } @@ -186,6 +189,10 @@ impl ServerSentEventHandler { .execution_payload_bid_tx .send(kind) .map(|count| log_count("execution payload bid", count)), + EventKind::ProposerPreferences(_) => self + .proposer_preferences_tx + .send(kind) + .map(|count| log_count("proposer preferences", count)), EventKind::PayloadAttestationMessage(_) => self .payload_attestation_message_tx .send(kind) @@ -284,6 +291,10 @@ impl ServerSentEventHandler { self.execution_payload_bid_tx.subscribe() } + pub fn subscribe_proposer_preferences(&self) -> Receiver> { + self.proposer_preferences_tx.subscribe() + } + pub fn subscribe_payload_attestation_message(&self) -> Receiver> { self.payload_attestation_message_tx.subscribe() } @@ -368,6 +379,10 @@ impl ServerSentEventHandler { self.execution_payload_bid_tx.receiver_count() > 0 } + pub fn has_proposer_preferences_subscribers(&self) -> bool { + self.proposer_preferences_tx.receiver_count() > 0 + } + pub fn has_payload_attestation_message_subscribers(&self) -> bool { self.payload_attestation_message_tx.receiver_count() > 0 } diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs index 586721d8c1..4dc1646ec4 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs @@ -6,6 +6,7 @@ use crate::{ ProposerPreferencesError, proposer_preference_cache::GossipVerifiedProposerPreferenceCache, }, }; +use eth2::types::{EventKind, ForkVersionedResponse}; use slot_clock::SlotClock; use state_processing::signature_sets::{get_pubkey_from_state, proposer_preferences_signature_set}; use tracing::debug; @@ -145,6 +146,19 @@ impl BeaconChain { %validator_index, "Successfully verified gossip proposer preferences" ); + + if let Some(event_handler) = self.event_handler.as_ref() + && event_handler.has_proposer_preferences_subscribers() + { + event_handler.register(EventKind::ProposerPreferences(Box::new( + ForkVersionedResponse { + version: self.spec.fork_name_at_slot::(proposal_slot), + metadata: Default::default(), + data: (*verified.signed_preferences).clone(), + }, + ))); + } + Ok(verified) } Err(e) => { diff --git a/beacon_node/beacon_chain/tests/events.rs b/beacon_node/beacon_chain/tests/events.rs index baa6975303..9f0b3675f3 100644 --- a/beacon_node/beacon_chain/tests/events.rs +++ b/beacon_node/beacon_chain/tests/events.rs @@ -7,9 +7,9 @@ use eth2::types::{EventKind, SseBlobSidecar, SseDataColumnSidecar}; use std::sync::Arc; use types::data::FixedBlobSidecarList; use types::{ - BlobSidecar, DataColumnSidecar, DataColumnSidecarFulu, DataColumnSidecarGloas, Domain, EthSpec, - MinimalEthSpec, PayloadAttestationData, PayloadAttestationMessage, SignedExecutionPayloadBid, - SignedRoot, Slot, + Address, BlobSidecar, DataColumnSidecar, DataColumnSidecarFulu, DataColumnSidecarGloas, Domain, + EthSpec, Hash256, MinimalEthSpec, PayloadAttestationData, PayloadAttestationMessage, + ProposerPreferences, SignedExecutionPayloadBid, SignedProposerPreferences, SignedRoot, Slot, }; type E = MinimalEthSpec; @@ -401,3 +401,80 @@ async fn payload_attestation_message_event_on_gossip_verification() { panic!("Expected PayloadAttestationMessage event, got {:?}", event); } } + +/// Verifies that a `proposer_preferences` SSE event is emitted when signed proposer preferences +/// pass gossip verification. +#[tokio::test] +async fn proposer_preferences_event_on_gossip_verification() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + + let harness = BeaconChainHarness::builder(E::default()) + .default_spec() + .deterministic_keypairs(64) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + + let head = harness.chain.canonical_head.cached_head(); + let head_state = &head.snapshot.beacon_state; + let genesis_validators_root = harness.chain.genesis_validators_root; + + // Pick a proposal slot in the next epoch so it is always a valid, future slot. The lookahead + // covers 2 epochs: index = epoch_offset * slots_per_epoch + slot_in_epoch. + let slots_per_epoch = E::slots_per_epoch() as usize; + let proposer_lookahead = head_state + .proposer_lookahead() + .expect("gloas state should have proposer lookahead"); + let next_epoch_start = (head_state.current_epoch() + 1).start_slot(E::slots_per_epoch()); + let proposal_slot = next_epoch_start + 1; + let lookahead_index = slots_per_epoch + 1; + let validator_index = *proposer_lookahead + .get(lookahead_index) + .expect("lookahead index should be in range"); + + // Build and sign proposer preferences for the proposer of `proposal_slot`. + let preferences = ProposerPreferences { + dependent_root: Hash256::ZERO, + proposal_slot, + validator_index, + fee_recipient: Address::repeat_byte(0xaa), + target_gas_limit: 30_000_000, + }; + let domain = harness.spec.get_domain( + proposal_slot.epoch(E::slots_per_epoch()), + Domain::ProposerPreferences, + &head_state.fork(), + genesis_validators_root, + ); + let signature = harness.validator_keypairs[validator_index as usize] + .sk + .sign(preferences.signing_root(domain)); + let signed = SignedProposerPreferences { + message: preferences.clone(), + signature, + }; + + // Subscribe before verification. + let event_handler = harness.chain.event_handler.as_ref().unwrap(); + let mut receiver = event_handler.subscribe_proposer_preferences(); + + // Verify the preferences through the gossip path. + harness + .chain + .verify_proposer_preferences_for_gossip(Arc::new(signed)) + .expect("verification should succeed"); + + // Assert the event was emitted with the expected data. + let event = receiver.try_recv().expect("should receive event"); + if let EventKind::ProposerPreferences(versioned) = event { + assert_eq!(versioned.data.message, preferences); + assert_eq!( + versioned.version, + harness.spec.fork_name_at_slot::(proposal_slot) + ); + } else { + panic!("Expected ProposerPreferences event, got {:?}", event); + } +} diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 94f2e3f1df..7c0959acb9 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -3273,6 +3273,9 @@ pub fn serve( api_types::EventTopic::ExecutionPayloadBid => { event_handler.subscribe_execution_payload_bid() } + api_types::EventTopic::ProposerPreferences => { + event_handler.subscribe_proposer_preferences() + } api_types::EventTopic::PayloadAttestationMessage => { event_handler.subscribe_payload_attestation_message() } diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 049ffba657..4d0bb48f54 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1164,6 +1164,7 @@ pub struct SseExtendedPayloadAttributesGeneric { pub type SseExtendedPayloadAttributes = SseExtendedPayloadAttributesGeneric; pub type VersionedSsePayloadAttributes = ForkVersionedResponse; pub type VersionedSseExecutionPayloadBid = ForkVersionedResponse>; +pub type VersionedSseProposerPreferences = ForkVersionedResponse; pub type VersionedSsePayloadAttestationMessage = ForkVersionedResponse; impl<'de> ContextDeserialize<'de, ForkName> for SsePayloadAttributes { @@ -1245,6 +1246,7 @@ pub enum EventKind { ExecutionPayloadGossip(SseExecutionPayloadGossip), ExecutionPayloadAvailable(SseExecutionPayloadAvailable), ExecutionPayloadBid(Box>), + ProposerPreferences(Box), PayloadAttestationMessage(Box), } @@ -1273,6 +1275,7 @@ impl EventKind { EventKind::ExecutionPayloadGossip(_) => "execution_payload_gossip", EventKind::ExecutionPayloadAvailable(_) => "execution_payload_available", EventKind::ExecutionPayloadBid(_) => "execution_payload_bid", + EventKind::ProposerPreferences(_) => "proposer_preferences", EventKind::PayloadAttestationMessage(_) => "payload_attestation_message", } } @@ -1389,6 +1392,11 @@ impl EventKind { ServerError::InvalidServerSentEvent(format!("Execution Payload Bid: {:?}", e)) })?, ))), + "proposer_preferences" => Ok(EventKind::ProposerPreferences(Box::new( + serde_json::from_str(data).map_err(|e| { + ServerError::InvalidServerSentEvent(format!("Proposer Preferences: {:?}", e)) + })?, + ))), "payload_attestation_message" => Ok(EventKind::PayloadAttestationMessage(Box::new( serde_json::from_str(data).map_err(|e| { ServerError::InvalidServerSentEvent(format!( @@ -1436,6 +1444,7 @@ pub enum EventTopic { ExecutionPayloadGossip, ExecutionPayloadAvailable, ExecutionPayloadBid, + ProposerPreferences, PayloadAttestationMessage, } @@ -1466,6 +1475,7 @@ impl FromStr for EventTopic { "execution_payload_gossip" => Ok(EventTopic::ExecutionPayloadGossip), "execution_payload_available" => Ok(EventTopic::ExecutionPayloadAvailable), "execution_payload_bid" => Ok(EventTopic::ExecutionPayloadBid), + "proposer_preferences" => Ok(EventTopic::ProposerPreferences), "payload_attestation_message" => Ok(EventTopic::PayloadAttestationMessage), _ => Err("event topic cannot be parsed.".to_string()), } @@ -1499,6 +1509,7 @@ impl fmt::Display for EventTopic { write!(f, "execution_payload_available") } EventTopic::ExecutionPayloadBid => write!(f, "execution_payload_bid"), + EventTopic::ProposerPreferences => write!(f, "proposer_preferences"), EventTopic::PayloadAttestationMessage => { write!(f, "payload_attestation_message") } diff --git a/consensus/types/src/builder/proposer_preferences.rs b/consensus/types/src/builder/proposer_preferences.rs index 4f27020105..97ba980d6c 100644 --- a/consensus/types/src/builder/proposer_preferences.rs +++ b/consensus/types/src/builder/proposer_preferences.rs @@ -14,8 +14,10 @@ use tree_hash_derive::TreeHash; pub struct ProposerPreferences { pub dependent_root: Hash256, pub proposal_slot: Slot, + #[serde(with = "serde_utils::quoted_u64")] pub validator_index: u64, pub fee_recipient: Address, + #[serde(with = "serde_utils::quoted_u64")] pub target_gas_limit: u64, } @@ -45,4 +47,24 @@ mod tests { use super::*; ssz_and_tree_hash_tests!(ProposerPreferences); + + /// `validator_index` and `target_gas_limit` must serialize as quoted JSON strings (Beacon API + /// convention) and round-trip back to their numeric values. + #[test] + fn quoted_u64_json_serde() { + let preferences = ProposerPreferences { + dependent_root: Hash256::ZERO, + proposal_slot: Slot::new(7), + validator_index: 42, + fee_recipient: Address::ZERO, + target_gas_limit: 30_000_000, + }; + + let value = serde_json::to_value(&preferences).unwrap(); + assert_eq!(value["validator_index"], serde_json::json!("42")); + assert_eq!(value["target_gas_limit"], serde_json::json!("30000000")); + + let decoded: ProposerPreferences = serde_json::from_value(value).unwrap(); + assert_eq!(decoded, preferences); + } } From b05badb5f2e3aa91bc28fe3ce80fcf4d5cb665ea Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Sun, 21 Jun 2026 18:34:29 +0200 Subject: [PATCH 06/12] Gate sync peer selection on per-protocol concurrent-request limit (#9456) Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> --- beacon_node/lighthouse_network/src/rpc/mod.rs | 2 +- .../network/src/sync/network_context.rs | 119 ++++++++---------- .../src/sync/network_context/custody.rs | 15 ++- 3 files changed, 59 insertions(+), 77 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/mod.rs b/beacon_node/lighthouse_network/src/rpc/mod.rs index 7c43018af8..e9177586cb 100644 --- a/beacon_node/lighthouse_network/src/rpc/mod.rs +++ b/beacon_node/lighthouse_network/src/rpc/mod.rs @@ -46,7 +46,7 @@ mod response_limiter; mod self_limiter; // Maximum number of concurrent requests per protocol ID that a client may issue. -const MAX_CONCURRENT_REQUESTS: usize = 2; +pub const MAX_CONCURRENT_REQUESTS: usize = 2; /// Composite trait for a request id. pub trait ReqId: Send + 'static + std::fmt::Debug + Copy + Clone {} diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index d2ced9fd9d..0db172a21a 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -27,7 +27,9 @@ use fnv::FnvHashMap; use lighthouse_network::rpc::methods::{ BlobsByRangeRequest, DataColumnsByRangeRequest, PayloadEnvelopesByRangeRequest, }; -use lighthouse_network::rpc::{BlocksByRangeRequest, GoodbyeReason, RPCError, RequestType}; +use lighthouse_network::rpc::{ + BlocksByRangeRequest, GoodbyeReason, MAX_CONCURRENT_REQUESTS, RPCError, RequestType, +}; pub use lighthouse_network::service::api_types::RangeRequestId; use lighthouse_network::service::api_types::{ AppRequestId, BlobsByRangeRequestId, BlocksByRangeRequestId, ComponentsByRangeRequestId, @@ -40,8 +42,8 @@ use lighthouse_network::{Client, NetworkGlobals, PeerAction, PeerId, ReportSourc use parking_lot::RwLock; pub use requests::LookupVerifyError; use requests::{ - ActiveRequests, BlobsByRangeRequestItems, BlocksByRangeRequestItems, BlocksByRootRequestItems, - DataColumnsByRangeRequestItems, DataColumnsByRootRequestItems, + ActiveRequestItems, ActiveRequests, BlobsByRangeRequestItems, BlocksByRangeRequestItems, + BlocksByRootRequestItems, DataColumnsByRangeRequestItems, DataColumnsByRootRequestItems, PayloadEnvelopesByRangeRequestItems, PayloadEnvelopesByRootRequestItems, }; #[cfg(test)] @@ -100,6 +102,30 @@ pub type RpcResponseResult = Result<(T, Duration), RpcResponseError>; pub type CustodyByRootResult = Result>, RpcResponseError>; +/// Per-peer count of active requests for a single protocol, to keep peer selection within +/// `MAX_CONCURRENT_REQUESTS` concurrent requests per protocol ID. +struct ActiveRequestsPerPeer { + count_by_peer: HashMap, +} + +impl ActiveRequestsPerPeer { + fn new(requests: &ActiveRequests) -> Self + where + K: Copy + Eq + std::hash::Hash + std::fmt::Display, + T: ActiveRequestItems, + { + let mut count_by_peer = HashMap::::new(); + for peer_id in requests.iter_request_peers() { + *count_by_peer.entry(peer_id).or_default() += 1; + } + Self { count_by_peer } + } + + fn at_concurrency_limit(&self, peer_id: &PeerId) -> bool { + self.count_by_peer.get(peer_id).copied().unwrap_or(0) >= MAX_CONCURRENT_REQUESTS + } +} + #[derive(Debug)] #[allow(private_interfaces)] pub enum RpcResponseError { @@ -440,47 +466,6 @@ impl SyncNetworkContext { } } - fn active_request_count_by_peer(&self) -> HashMap { - let Self { - network_send: _, - request_id: _, - blocks_by_root_requests, - payload_envelopes_by_root_requests, - data_columns_by_root_requests, - blocks_by_range_requests, - blobs_by_range_requests, - data_columns_by_range_requests, - payload_envelopes_by_range_requests, - // custody_by_root_requests is a meta request of data_columns_by_root_requests - custody_by_root_requests: _, - // components_by_range_requests is a meta request of various _by_range requests - components_by_range_requests: _, - custody_backfill_data_column_batch_requests: _, - execution_engine_state: _, - network_beacon_processor: _, - chain: _, - fork_context: _, - // Don't use a fallback match. We want to be sure that all requests are considered when - // adding new ones - } = self; - - let mut active_request_count_by_peer = HashMap::::new(); - - for peer_id in blocks_by_root_requests - .iter_request_peers() - .chain(payload_envelopes_by_root_requests.iter_request_peers()) - .chain(data_columns_by_root_requests.iter_request_peers()) - .chain(blocks_by_range_requests.iter_request_peers()) - .chain(blobs_by_range_requests.iter_request_peers()) - .chain(data_columns_by_range_requests.iter_request_peers()) - .chain(payload_envelopes_by_range_requests.iter_request_peers()) - { - *active_request_count_by_peer.entry(peer_id).or_default() += 1; - } - - active_request_count_by_peer - } - /// Retries only the specified failed columns by requesting them again. /// /// Note: This function doesn't retry the whole batch, but retries specific requests within @@ -507,8 +492,6 @@ impl SyncNetworkContext { return Err("request id not present".to_string()); }; - let active_request_count_by_peer = self.active_request_count_by_peer(); - debug!( ?failed_columns, ?id, @@ -518,12 +501,7 @@ impl SyncNetworkContext { // Attempt to find all required custody peers to request the failed columns from let columns_by_range_peers_to_request = self - .select_columns_by_range_peers_to_request( - failed_columns, - peers, - active_request_count_by_peer, - peers_to_deprioritize, - ) + .select_columns_by_range_peers_to_request(failed_columns, peers, peers_to_deprioritize) .map_err(|e| format!("{:?}", e))?; // Reuse the id for the request that received partially correct responses @@ -581,7 +559,7 @@ impl SyncNetworkContext { column_peers = column_peers.len() ); let _guard = range_request_span.clone().entered(); - let active_request_count_by_peer = self.active_request_count_by_peer(); + let blocks_by_range_per_peer = ActiveRequestsPerPeer::new(&self.blocks_by_range_requests); let Some(block_peer) = block_peers .iter() @@ -589,8 +567,8 @@ impl SyncNetworkContext { ( // If contains -> 1 (order after), not contains -> 0 (order first) peers_to_deprioritize.contains(peer), - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), + // Strictly de-prioritize peers already at the per-protocol concurrency limit + blocks_by_range_per_peer.at_concurrency_limit(peer), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, @@ -620,7 +598,6 @@ impl SyncNetworkContext { Some(self.select_columns_by_range_peers_to_request( &column_indexes, column_peers, - active_request_count_by_peer, peers_to_deprioritize, )?) } else { @@ -692,6 +669,9 @@ impl SyncNetworkContext { let payloads_req_id = if matches!(batch_type, ByRangeRequestType::BlocksAndEnvelopesAndColumns) { Some(self.send_payload_envelopes_by_range_request( + // Peer selection: for a given peer, the count of sent blocks_by_range requests + // equals the count of sent payloads_by_range requests. So we are under the + // concurrency limit for payloads_by_range requests block_peer, PayloadEnvelopesByRangeRequest { start_slot: *request.start_slot(), @@ -731,10 +711,11 @@ impl SyncNetworkContext { &self, custody_indexes: &HashSet, peers: &HashSet, - active_request_count_by_peer: HashMap, peers_to_deprioritize: &HashSet, ) -> Result>, RpcRequestSendError> { let mut columns_to_request_by_peer = HashMap::>::new(); + let data_columns_by_range_per_peer = + ActiveRequestsPerPeer::new(&self.data_columns_by_range_requests); for column_index in custody_indexes { // Strictly consider peers that are custodials of this column AND are part of this @@ -750,12 +731,10 @@ impl SyncNetworkContext { ( // If contains -> 1 (order after), not contains -> 0 (order first) peers_to_deprioritize.contains(peer), - // Prefer peers with less overall requests - // Also account for requests that are not yet issued tracked in peer_id_to_request_map - // We batch requests to the same peer, so count existance in the - // `columns_to_request_by_peer` as a single 1 request. - active_request_count_by_peer.get(peer).copied().unwrap_or(0) - + columns_to_request_by_peer.get(peer).map(|_| 1).unwrap_or(0), + // Strictly de-prioritize peers already at the per-protocol concurrency limit + // Note: do not account for to-be-sent requests on + // `data_columns_by_range_by_peer` as we always send at most one request + data_columns_by_range_per_peer.at_concurrency_limit(peer), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, @@ -881,14 +860,14 @@ impl SyncNetworkContext { lookup_peers: Arc>>, block_root: Hash256, ) -> Result>>, RpcRequestSendError> { - let active_request_count_by_peer = self.active_request_count_by_peer(); + let blocks_by_root_per_peer = ActiveRequestsPerPeer::new(&self.blocks_by_root_requests); let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { ( - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), + // Strictly de-prioritize peers already at the per-protocol concurrency limit + blocks_by_root_per_peer.at_concurrency_limit(peer), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, @@ -1001,13 +980,15 @@ impl SyncNetworkContext { )); } - let active_request_count_by_peer = self.active_request_count_by_peer(); + let payload_envelopes_by_root_per_peer = + ActiveRequestsPerPeer::new(&self.payload_envelopes_by_root_requests); let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { ( - active_request_count_by_peer.get(peer).copied().unwrap_or(0), + // Strictly de-prioritize peers already at the per-protocol concurrency limit + payload_envelopes_by_root_per_peer.at_concurrency_limit(peer), rand::random::(), peer, ) @@ -1757,7 +1738,6 @@ impl SyncNetworkContext { peers: &HashSet, peers_to_deprioritize: &HashSet, ) -> Result { - let active_request_count_by_peer = self.active_request_count_by_peer(); // Attempt to find all required custody peers before sending any request or creating an ID let columns_by_range_peers_to_request = { let column_indexes = self @@ -1770,7 +1750,6 @@ impl SyncNetworkContext { self.select_columns_by_range_peers_to_request( &column_indexes, peers, - active_request_count_by_peer, peers_to_deprioritize, )? }; diff --git a/beacon_node/network/src/sync/network_context/custody.rs b/beacon_node/network/src/sync/network_context/custody.rs index 3518eecf09..3dd3683c42 100644 --- a/beacon_node/network/src/sync/network_context/custody.rs +++ b/beacon_node/network/src/sync/network_context/custody.rs @@ -16,7 +16,9 @@ use tracing::{Span, debug, debug_span, warn}; use types::{DataColumnSidecar, Hash256, Slot, data::ColumnIndex}; use types::{DataColumnSidecarList, EthSpec}; -use super::{LookupRequestResult, PeerGroup, RpcResponseResult, SyncNetworkContext}; +use super::{ + ActiveRequestsPerPeer, LookupRequestResult, PeerGroup, RpcResponseResult, SyncNetworkContext, +}; const MAX_STALE_NO_PEERS_DURATION: Duration = Duration::from_secs(30); @@ -237,7 +239,8 @@ impl ActiveCustodyRequest { ))); } - let active_request_count_by_peer = cx.active_request_count_by_peer(); + let data_columns_by_root_per_peer = + ActiveRequestsPerPeer::new(&cx.data_columns_by_root_requests); let mut columns_to_request_by_peer = HashMap::>::new(); let mut columns_without_peers = vec![]; let lookup_peers = self.lookup_peers.read(); @@ -255,7 +258,7 @@ impl ActiveCustodyRequest { let peer_to_request = self.select_column_peer( cx, - &active_request_count_by_peer, + &data_columns_by_root_per_peer, &lookup_peers, *column_index, &random_state, @@ -360,7 +363,7 @@ impl ActiveCustodyRequest { fn select_column_peer( &self, cx: &mut SyncNetworkContext, - active_request_count_by_peer: &HashMap, + data_columns_by_root_per_peer: &ActiveRequestsPerPeer, lookup_peers: &HashSet, column_index: ColumnIndex, random_state: &RandomState, @@ -377,12 +380,12 @@ impl ActiveCustodyRequest { }) .map(|peer| { ( + // Strictly de-prioritize peers already at the per-protocol concurrency limit + data_columns_by_root_per_peer.at_concurrency_limit(peer), // Prioritize peers that claim to know have imported this block if lookup_peers.contains(peer) { 0 } else { 1 }, // De-prioritize peers that we have already attempted to download from self.peer_attempts.get(peer).copied().unwrap_or(0), - // Prefer peers with fewer requests to load balance across peers. - active_request_count_by_peer.get(peer).copied().unwrap_or(0), // The hash ensures consistent peer ordering within this request // to avoid fragmentation while varying selection across different requests. random_state.hash_one(peer), From e9f55a5e512db62536db04f30728201ffc8a3c85 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 18 Jun 2026 13:43:28 +1000 Subject: [PATCH 07/12] Update deps --- Cargo.lock | 135 +++++++++++++++++++++++++---------------------------- Cargo.toml | 4 ++ 2 files changed, 68 insertions(+), 71 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0344bb7c8..0bc97653d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -447,7 +447,7 @@ dependencies = [ "alloy-rlp", "alloy-serde", "alloy-sol-types", - "itertools 0.14.0", + "itertools 0.13.0", "serde", "serde_json", "serde_with", @@ -1368,7 +1368,7 @@ dependencies = [ "bitflags 2.10.0", "cexpr", "clang-sys", - "itertools 0.12.1", + "itertools 0.10.5", "lazy_static", "lazycell", "proc-macro2", @@ -1969,7 +1969,7 @@ version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fde0e0ec90c9dfb3b4b1a0891a7dcd0e2bffde2f7efed5fe7c9bb00e5bfb915e" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] @@ -2475,7 +2475,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 2.0.117", + "syn 1.0.109", ] [[package]] @@ -3114,7 +3114,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -3441,6 +3441,18 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" +[[package]] +name = "fastbloom" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e7f34442dbe69c60fe8eaf58a8cafff81a1f278816d8ab4db255b3bef4ac3c4" +dependencies = [ + "getrandom 0.3.4", + "libm", + "rand 0.9.2", + "siphasher", +] + [[package]] name = "fastrand" version = "2.3.0" @@ -3654,8 +3666,8 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b604752cefc5aa3ab98992a107a8bd99465d2825c1584e0b60cb6957b21e19d7" dependencies = [ + "futures-timer", "futures-util", - "tokio", ] [[package]] @@ -4409,7 +4421,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.4", + "socket2 0.5.10", "tokio", "tower-service", "tracing", @@ -4783,15 +4795,6 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.13.0" @@ -5086,7 +5089,7 @@ dependencies = [ [[package]] name = "libp2p" version = "0.57.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "bytes", "either", @@ -5117,7 +5120,7 @@ dependencies = [ [[package]] name = "libp2p-allow-block-list" version = "0.7.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "libp2p-core", "libp2p-identity", @@ -5127,7 +5130,7 @@ dependencies = [ [[package]] name = "libp2p-connection-limits" version = "0.7.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "libp2p-core", "libp2p-identity", @@ -5137,7 +5140,7 @@ dependencies = [ [[package]] name = "libp2p-core" version = "0.44.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "either", "fnv", @@ -5161,7 +5164,7 @@ dependencies = [ [[package]] name = "libp2p-dns" version = "0.45.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "hickory-resolver", @@ -5175,7 +5178,7 @@ dependencies = [ [[package]] name = "libp2p-gossipsub" version = "0.50.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "async-channel 2.5.0", "asynchronous-codec", @@ -5205,7 +5208,7 @@ dependencies = [ [[package]] name = "libp2p-identify" version = "0.48.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "asynchronous-codec", "either", @@ -5245,7 +5248,7 @@ dependencies = [ [[package]] name = "libp2p-mdns" version = "0.49.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "hickory-proto", @@ -5263,7 +5266,7 @@ dependencies = [ [[package]] name = "libp2p-metrics" version = "0.18.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "libp2p-core", @@ -5279,7 +5282,7 @@ dependencies = [ [[package]] name = "libp2p-mplex" version = "0.44.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "asynchronous-codec", "bytes", @@ -5297,7 +5300,7 @@ dependencies = [ [[package]] name = "libp2p-noise" version = "0.47.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "asynchronous-codec", "bytes", @@ -5319,7 +5322,7 @@ dependencies = [ [[package]] name = "libp2p-quic" version = "0.14.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "futures-timer", @@ -5328,6 +5331,7 @@ dependencies = [ "libp2p-identity", "libp2p-tls", "quinn", + "quinn-proto", "rand 0.8.5", "ring", "rustls 0.23.40", @@ -5340,7 +5344,7 @@ dependencies = [ [[package]] name = "libp2p-swarm" version = "0.48.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "either", "fnv", @@ -5363,7 +5367,7 @@ dependencies = [ [[package]] name = "libp2p-swarm-derive" version = "0.36.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "heck", "quote", @@ -5373,7 +5377,7 @@ dependencies = [ [[package]] name = "libp2p-tcp" version = "0.45.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "futures-timer", @@ -5388,7 +5392,7 @@ dependencies = [ [[package]] name = "libp2p-tls" version = "0.7.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "futures-rustls", @@ -5406,7 +5410,7 @@ dependencies = [ [[package]] name = "libp2p-upnp" version = "0.7.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "futures-timer", @@ -5420,15 +5424,13 @@ dependencies = [ [[package]] name = "libp2p-yamux" version = "0.48.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ - "either", "futures", "libp2p-core", "thiserror 2.0.17", "tracing", - "yamux 0.12.1", - "yamux 0.13.10", + "yamux", ] [[package]] @@ -6090,7 +6092,7 @@ dependencies = [ [[package]] name = "multistream-select" version = "0.14.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "bytes", "futures", @@ -6296,7 +6298,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -7118,7 +7120,7 @@ dependencies = [ [[package]] name = "prost-codec" version = "0.4.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "asynchronous-codec", "bytes", @@ -7134,7 +7136,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a56d757972c98b346a9b766e3f02746cde6dd1cd1d1d563472929fdd74bec4d" dependencies = [ "anyhow", - "itertools 0.14.0", + "itertools 0.13.0", "proc-macro2", "quote", "syn 2.0.117", @@ -7147,7 +7149,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "27c6023962132f4b30eb4c172c91ce92d933da334c59c23cddee82358ddafb0b" dependencies = [ "anyhow", - "itertools 0.14.0", + "itertools 0.13.0", "proc-macro2", "quote", "syn 2.0.117", @@ -7206,9 +7208,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" [[package]] name = "quinn" -version = "0.11.9" +version = "0.11.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9e20a958963c291dc322d98411f541009df2ced7b5a4f2bd52337638cfccf20" +checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8" dependencies = [ "bytes", "cfg_aliases", @@ -7218,7 +7220,7 @@ dependencies = [ "quinn-udp", "rustc-hash 2.1.1", "rustls 0.23.40", - "socket2 0.6.4", + "socket2 0.5.10", "thiserror 2.0.17", "tokio", "tracing", @@ -7227,11 +7229,12 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.11.14" +version = "0.11.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "434b42fec591c96ef50e21e886936e66d3cc3f737104fdb9b737c40ffb94c098" +checksum = "4fcb935c5bec503c2f0e306bdd3e58bb9029dcb14fa8d9ac76e3a5256ac0763e" dependencies = [ "bytes", + "fastbloom", "getrandom 0.3.4", "lru-slab", "rand 0.9.2", @@ -7255,9 +7258,9 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.6.4", + "socket2 0.5.10", "tracing", - "windows-sys 0.60.2", + "windows-sys 0.59.0", ] [[package]] @@ -7802,7 +7805,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -7908,7 +7911,7 @@ dependencies = [ [[package]] name = "rw-stream-sink" version = "0.5.0" -source = "git+https://github.com/libp2p/rust-libp2p.git#3e72d4c071d5ec8815d2f6f7ee3602600ff51798" +source = "git+https://github.com/sigp/rust-libp2p.git?rev=3563de5ed20e509885592b391aa816992eef55d4#3563de5ed20e509885592b391aa816992eef55d4" dependencies = [ "futures", "pin-project", @@ -8387,6 +8390,12 @@ dependencies = [ "types", ] +[[package]] +name = "siphasher" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ee5873ec9cce0195efcb7a4e9507a04cd49aec9c83d0389df45b1ef7ba2e649" + [[package]] name = "slab" version = "0.4.11" @@ -8870,7 +8879,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -10234,7 +10243,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -10844,31 +10853,15 @@ dependencies = [ [[package]] name = "yamux" -version = "0.12.1" +version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ed0164ae619f2dc144909a9f082187ebb5893693d8c0196e8085283ccd4b776" +checksum = "767113d8f66a81e461362462aa71d8d0108cbf3430d4442fb88a04e31be81165" dependencies = [ "futures", "log", "nohash-hasher", "parking_lot", "pin-project", - "rand 0.8.5", - "static_assertions", -] - -[[package]] -name = "yamux" -version = "0.13.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1991f6690292030e31b0144d73f5e8368936c58e45e7068254f7138b23b00672" -dependencies = [ - "futures", - "log", - "nohash-hasher", - "parking_lot", - "pin-project", - "rand 0.9.2", "static_assertions", "web-time", ] diff --git a/Cargo.toml b/Cargo.toml index 6e0b1ddf3a..f4dba84ff4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -272,3 +272,7 @@ incremental = false inherits = "release" debug = true +[patch."https://github.com/libp2p/rust-libp2p.git"] +libp2p = { git = "https://github.com/sigp/rust-libp2p.git", rev = "3563de5ed20e509885592b391aa816992eef55d4" } +libp2p-mplex = { git = "https://github.com/sigp/rust-libp2p.git", rev = "3563de5ed20e509885592b391aa816992eef55d4" } +libp2p-quic = { git = "https://github.com/sigp/rust-libp2p.git", rev = "3563de5ed20e509885592b391aa816992eef55d4" } From 120c3c6dac9df8ee4d83f055919bd3488abae4f6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 16 Jun 2026 17:19:52 +1000 Subject: [PATCH 08/12] Release v8.2.0 --- Cargo.lock | 14 +++++++------- Cargo.toml | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0bc97653d0..b88b949957 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "account_manager" -version = "8.1.3" +version = "8.2.0" dependencies = [ "account_utils", "bls", @@ -1276,7 +1276,7 @@ dependencies = [ [[package]] name = "beacon_node" -version = "8.1.3" +version = "8.2.0" dependencies = [ "account_utils", "beacon_chain", @@ -1539,7 +1539,7 @@ dependencies = [ [[package]] name = "boot_node" -version = "8.1.3" +version = "8.2.0" dependencies = [ "beacon_node", "bytes", @@ -4984,7 +4984,7 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "lcli" -version = "8.1.3" +version = "8.2.0" dependencies = [ "account_utils", "beacon_chain", @@ -5482,7 +5482,7 @@ dependencies = [ [[package]] name = "lighthouse" -version = "8.1.3" +version = "8.2.0" dependencies = [ "account_manager", "account_utils", @@ -5614,7 +5614,7 @@ dependencies = [ [[package]] name = "lighthouse_version" -version = "8.1.3" +version = "8.2.0" dependencies = [ "regex", ] @@ -9688,7 +9688,7 @@ dependencies = [ [[package]] name = "validator_client" -version = "8.1.3" +version = "8.2.0" dependencies = [ "account_utils", "beacon_node_fallback", diff --git a/Cargo.toml b/Cargo.toml index f4dba84ff4..f43786efe5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,7 @@ resolver = "2" [workspace.package] edition = "2024" -version = "8.1.3" +version = "8.2.0" [workspace.dependencies] account_utils = { path = "common/account_utils" } From 8c4b21c3dbfab4319513b608e5484b0a646794fd Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 22 Jun 2026 17:06:26 -0700 Subject: [PATCH 09/12] Fix ci issues (#9514) - Remove deprecated geth flag - Remove duplicate entries for crate `syn` Co-Authored-By: Eitan Seri-Levi --- Cargo.lock | 2 +- testing/execution_engine_integration/src/geth.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b88b949957..d4a531d26d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2475,7 +2475,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 1.0.109", + "syn 2.0.117", ] [[package]] diff --git a/testing/execution_engine_integration/src/geth.rs b/testing/execution_engine_integration/src/geth.rs index 4b62e68e94..a676cb1a4c 100644 --- a/testing/execution_engine_integration/src/geth.rs +++ b/testing/execution_engine_integration/src/geth.rs @@ -108,7 +108,6 @@ impl GenericExecutionEngine for GethEngine { .arg(http_auth_port.to_string()) .arg("--port") .arg(network_port.to_string()) - .arg("--allow-insecure-unlock") .arg("--authrpc.jwtsecret") .arg(jwt_secret_path.as_path().to_str().unwrap()) // This flag is required to help Geth perform reliably when feeding it blocks From 34e14fd1bc33e5cabaeb3a1fd3eb5e375dbc7092 Mon Sep 17 00:00:00 2001 From: Mac L Date: Tue, 23 Jun 2026 04:09:31 +0400 Subject: [PATCH 10/12] Forbid removed `execution_payload_envelope.rs` file (#9506) I noticed that `beacon_node/http_api/src/beacon/execution_payload_envelope.rs` was recently removed but not added to the forbidden-files.txt. Add the removed file to the forbidden list to ensure it isn't accidentally re-added by a merge or rebase. Co-Authored-By: Mac L --- .github/forbidden-files.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/forbidden-files.txt b/.github/forbidden-files.txt index 1c5e9acab9..eca611585b 100644 --- a/.github/forbidden-files.txt +++ b/.github/forbidden-files.txt @@ -9,6 +9,7 @@ beacon_node/beacon_chain/src/block_reward.rs beacon_node/http_api/src/attestation_performance.rs beacon_node/http_api/src/block_packing_efficiency.rs beacon_node/http_api/src/block_rewards.rs +beacon_node/http_api/src/beacon/execution_payload_envelope.rs common/eth2/src/lighthouse/attestation_performance.rs common/eth2/src/lighthouse/block_packing_efficiency.rs common/eth2/src/lighthouse/block_rewards.rs From e7c027cfa8bd1da66700c51b9486d98cfe3f32ca Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 23 Jun 2026 02:21:25 +0200 Subject: [PATCH 11/12] Run Gloas sync tests instead of skipping them (#9446) Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> Co-Authored-By: Pawan Dhananjay --- .../src/sync/block_sidecar_coupling.rs | 343 ++++++++++-------- beacon_node/network/src/sync/tests/lookups.rs | 123 ++++--- beacon_node/network/src/sync/tests/mod.rs | 6 +- beacon_node/network/src/sync/tests/range.rs | 38 +- 4 files changed, 276 insertions(+), 234 deletions(-) diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index b64ae4a4c5..93c0699ded 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -594,15 +594,113 @@ mod tests { } } - /// The custody-column coupling tests below build Fulu data-column sidecars directly, which is - /// incompatible with a Gloas genesis (Gloas columns have a different structure). Skip them when - /// `FORK_NAME` schedules Gloas at genesis. TODO(gloas): port the harness to build Gloas columns. - fn skip_under_gloas() -> bool { + /// Returns true when `FORK_NAME` schedules Gloas at genesis. Used to make the custody-column + /// coupling tests fork-aware: under Gloas the columns are coupled into the payload envelope, so + /// these tests build Gloas blocks/columns/envelopes and complete the payloads request. + fn is_gloas_env() -> bool { test_spec::() .fork_name_at_epoch(Epoch::new(0)) .gloas_enabled() } + /// The fork to build blocks/columns for in the custody-column coupling tests. Under a Gloas + /// genesis we must build Gloas columns (and matching envelopes); otherwise we use Fulu. + fn custody_test_fork() -> ForkName { + if is_gloas_env() { + ForkName::Gloas + } else { + ForkName::Fulu + } + } + + /// A spec with custody-column (PeerDAS) coupling enabled at genesis, matching the env fork. + /// Under a Gloas env this enables Gloas at genesis (so envelopes are coupled); otherwise it + /// enables Fulu at genesis. + fn custody_test_spec() -> ChainSpec { + let mut spec = test_spec::(); + spec.deneb_fork_epoch = Some(Epoch::new(0)); + spec.fulu_fork_epoch = Some(Epoch::new(0)); + if is_gloas_env() { + spec.gloas_fork_epoch = Some(Epoch::new(0)); + } + spec + } + + /// A block, its data columns, and (under Gloas) its matching payload envelope. + type BlockColumnsEnvelope = ( + Arc>, + DataColumnSidecarList, + Option>>, + ); + + /// Builds `count` blocks with their data columns, plus a matching payload envelope under Gloas. + /// Under Fulu the envelope is `None`. + fn make_blocks_and_columns( + count: usize, + num_blobs: NumBlobs, + spec: &ChainSpec, + ) -> Vec { + let fork = custody_test_fork(); + let mut u = types::test_utils::test_unstructured(); + (0..count) + .map(|_| { + // `NumBlobs` isn't `Clone`, so rebuild a fresh value for each block. + let num_blobs = match &num_blobs { + NumBlobs::Random => NumBlobs::Random, + NumBlobs::Number(n) => NumBlobs::Number(*n), + NumBlobs::None => NumBlobs::None, + }; + let (block, data_columns) = + generate_rand_block_and_data_columns::(fork, num_blobs, &mut u, spec) + .unwrap(); + let block = Arc::new(block); + let envelope = is_gloas_env().then(|| matching_envelope(&block)); + (block, data_columns, envelope) + }) + .collect() + } + + /// Under Gloas, completes the payloads request with the envelopes from `blocks`. Under Fulu this + /// is a no-op (there is no payloads request). Pass the subset of blocks whose envelopes should + /// be supplied. + fn add_envelopes_if_gloas( + info: &mut RangeBlockComponentsRequest, + payloads_req_id: Option, + blocks: &[BlockColumnsEnvelope], + ) { + if let Some(payloads_req_id) = payloads_req_id { + info.add_payload_envelopes( + payloads_req_id, + blocks + .iter() + .filter_map(|(_, _, envelope)| envelope.clone()) + .collect(), + ) + .unwrap(); + } + } + + /// Asserts the coupled `responses` carry the expected data. Pre-Gloas only the count is checked; + /// under Gloas each block must additionally wrap an envelope holding `expected_columns` columns. + fn assert_custody_columns_coupled( + responses: &[RangeSyncBlock], + expected_blocks: usize, + expected_columns: usize, + ) { + assert_eq!(responses.len(), expected_blocks); + if is_gloas_env() { + for response in responses { + match response { + RangeSyncBlock::Gloas { + envelope: Some(envelope), + .. + } => assert_eq!(envelope.columns.len(), expected_columns), + other => panic!("expected Gloas block with envelope, got {other:?}"), + } + } + } + } + fn blocks_id(parent_request_id: ComponentsByRangeRequestId) -> BlocksByRangeRequestId { BlocksByRangeRequestId { id: 1, @@ -798,38 +896,33 @@ mod tests { #[test] fn no_blobs_into_responses() { - // This exercises the pre-Gloas blobs/no-data coupling path. Gloas coupling is covered - // by the dedicated `setup_gloas_coupling` tests below. - if skip_under_gloas() { - return; - } - let spec = Arc::new(test_spec::()); - - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..4) - .map(|_| { - generate_rand_block_and_blobs::( - spec.fork_name_at_epoch(Epoch::new(0)), - NumBlobs::None, - &mut u, - ) - .unwrap() - .0 - .into() - }) - .collect::>>>(); - - let blocks_req_id = blocks_id(components_id()); - let mut info = - RangeBlockComponentsRequest::::new(blocks_req_id, None, None, None, Span::none()); - - // Send blocks and complete terminate response - info.add_blocks(blocks_req_id, blocks).unwrap(); - + // Coupling of blocks that carry no data. Pre-Gloas there is simply no data request; under + // Gloas each block still couples to its (empty-column) payload envelope, so the envelope + // request is driven too. + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); + let blocks = make_blocks_and_columns(4, NumBlobs::None, &spec); - // Assert response is finished and RpcBlocks can be constructed - info.responses(da_checker, spec).unwrap().unwrap(); + let components_id = components_id(); + let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); + let mut info = RangeBlockComponentsRequest::::new( + blocks_req_id, + None, + is_gloas_env().then(|| (vec![], vec![])), + payloads_req_id, + Span::none(), + ); + + info.add_blocks( + blocks_req_id, + blocks.iter().map(|(block, _, _)| block.clone()).collect(), + ) + .unwrap(); + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); + + let responses = info.responses(da_checker, spec).unwrap().unwrap(); + assert_custody_columns_coupled(&responses, blocks.len(), 0); } #[test] @@ -875,33 +968,17 @@ mod tests { #[test] fn rpc_block_with_custody_columns() { - if skip_under_gloas() { - return; - } - let mut spec = test_spec::(); - spec.deneb_fork_epoch = Some(Epoch::new(0)); - spec.fulu_fork_epoch = Some(Epoch::new(0)); - let spec = Arc::new(spec); + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); let expects_custody_columns = da_checker .custody_context() .sampling_columns_for_epoch(Epoch::new(0), &spec) .to_vec(); - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..4) - .map(|_| { - generate_rand_block_and_data_columns::( - ForkName::Fulu, - NumBlobs::Number(1), - &mut u, - &spec, - ) - .unwrap() - }) - .collect::>(); + let blocks = make_blocks_and_columns(4, NumBlobs::Number(1), &spec); let components_id = components_id(); let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); let columns_req_id = expects_custody_columns .iter() .enumerate() @@ -919,13 +996,13 @@ mod tests { blocks_req_id, None, Some((columns_req_id.clone(), expects_custody_columns.clone())), - None, + payloads_req_id, Span::none(), ); // Send blocks and complete terminate response info.add_blocks( blocks_req_id, - blocks.iter().map(|b| b.0.clone().into()).collect(), + blocks.iter().map(|(block, _, _)| block.clone()).collect(), ) .unwrap(); // Assert response is not finished @@ -938,7 +1015,12 @@ mod tests { *req, blocks .iter() - .flat_map(|b| b.1.iter().filter(|d| *d.index() == column_index).cloned()) + .flat_map(|(_, columns, _)| { + columns + .iter() + .filter(|d| *d.index() == column_index) + .cloned() + }) .collect(), ) .unwrap(); @@ -951,19 +1033,18 @@ mod tests { } } + // Under Gloas the columns are coupled into the payload envelope; supply the envelopes so + // the request can complete. + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); + // All completed construct response - info.responses(da_checker, spec).unwrap().unwrap(); + let responses = info.responses(da_checker, spec).unwrap().unwrap(); + assert_custody_columns_coupled(&responses, blocks.len(), expects_custody_columns.len()); } #[test] fn rpc_block_with_custody_columns_batched() { - if skip_under_gloas() { - return; - } - let mut spec = test_spec::(); - spec.deneb_fork_epoch = Some(Epoch::new(0)); - spec.fulu_fork_epoch = Some(Epoch::new(0)); - let spec = Arc::new(spec); + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); let expected_sampling_columns = da_checker .custody_context() @@ -981,6 +1062,7 @@ mod tests { let components_id = components_id(); let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); let columns_req_id = batched_column_requests .iter() .enumerate() @@ -999,27 +1081,16 @@ mod tests { blocks_req_id, None, Some((columns_req_id.clone(), expected_sampling_columns.clone())), - None, + payloads_req_id, Span::none(), ); - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..4) - .map(|_| { - generate_rand_block_and_data_columns::( - ForkName::Fulu, - NumBlobs::Number(1), - &mut u, - &spec, - ) - .unwrap() - }) - .collect::>(); + let blocks = make_blocks_and_columns(4, NumBlobs::Number(1), &spec); // Send blocks and complete terminate response info.add_blocks( blocks_req_id, - blocks.iter().map(|b| b.0.clone().into()).collect(), + blocks.iter().map(|(block, _, _)| block.clone()).collect(), ) .unwrap(); // Assert response is not finished @@ -1032,8 +1103,9 @@ mod tests { *req, blocks .iter() - .flat_map(|b| { - b.1.iter() + .flat_map(|(_, columns, _)| { + columns + .iter() .filter(|d| column_indices.contains(d.index())) .cloned() }) @@ -1049,8 +1121,13 @@ mod tests { } } + // Under Gloas the columns are coupled into the payload envelope; supply the envelopes so + // the request can complete. + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); + // All completed construct response - info.responses(da_checker, spec).unwrap().unwrap(); + let responses = info.responses(da_checker, spec).unwrap().unwrap(); + assert_custody_columns_coupled(&responses, blocks.len(), expected_sampling_columns.len()); } #[test] @@ -1164,31 +1241,18 @@ mod tests { #[test] fn missing_custody_columns_from_faulty_peers() { - if skip_under_gloas() { - return; - } // GIVEN: A request expecting sampling columns from multiple peers - let spec = Arc::new(test_spec::()); + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); let expected_sampling_columns = da_checker .custody_context() .sampling_columns_for_epoch(Epoch::new(0), &spec) .to_vec(); - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..2) - .map(|_| { - generate_rand_block_and_data_columns::( - ForkName::Fulu, - NumBlobs::Number(1), - &mut u, - &spec, - ) - .unwrap() - }) - .collect::>(); + let blocks = make_blocks_and_columns(2, NumBlobs::Number(1), &spec); let components_id = components_id(); let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); let columns_req_id = expected_sampling_columns .iter() .enumerate() @@ -1206,16 +1270,19 @@ mod tests { blocks_req_id, None, Some((columns_req_id.clone(), expected_sampling_columns.clone())), - None, + payloads_req_id, Span::none(), ); // AND: All blocks are received successfully info.add_blocks( blocks_req_id, - blocks.iter().map(|b| b.0.clone().into()).collect(), + blocks.iter().map(|(block, _, _)| block.clone()).collect(), ) .unwrap(); + // Under Gloas the payloads request must be completed for `responses` to proceed; the + // faulty-peer detection happens before the envelope wrap and is fork-independent. + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); // AND: Only the first 2 sampling columns are received successfully for (i, &column_index) in expected_sampling_columns.iter().take(2).enumerate() { @@ -1224,7 +1291,12 @@ mod tests { *req, blocks .iter() - .flat_map(|b| b.1.iter().filter(|d| *d.index() == column_index).cloned()) + .flat_map(|(_, columns, _)| { + columns + .iter() + .filter(|d| *d.index() == column_index) + .cloned() + }) .collect(), ) .unwrap(); @@ -1263,34 +1335,18 @@ mod tests { #[test] fn retry_logic_after_peer_failures() { - if skip_under_gloas() { - return; - } // GIVEN: A request expecting sampling columns where some peers initially fail - let mut spec = test_spec::(); - spec.deneb_fork_epoch = Some(Epoch::new(0)); - spec.fulu_fork_epoch = Some(Epoch::new(0)); - let spec = Arc::new(spec); + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); let expected_sampling_columns = da_checker .custody_context() .sampling_columns_for_epoch(Epoch::new(0), &spec) .to_vec(); - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..2) - .map(|_| { - generate_rand_block_and_data_columns::( - ForkName::Fulu, - NumBlobs::Number(1), - &mut u, - &spec, - ) - .unwrap() - }) - .collect::>(); + let blocks = make_blocks_and_columns(2, NumBlobs::Number(1), &spec); let components_id = components_id(); let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); let columns_req_id = expected_sampling_columns .iter() .enumerate() @@ -1308,16 +1364,18 @@ mod tests { blocks_req_id, None, Some((columns_req_id.clone(), expected_sampling_columns.clone())), - None, + payloads_req_id, Span::none(), ); // AND: All blocks are received info.add_blocks( blocks_req_id, - blocks.iter().map(|b| b.0.clone().into()).collect(), + blocks.iter().map(|(block, _, _)| block.clone()).collect(), ) .unwrap(); + // Under Gloas the payloads request must be completed for `responses` to proceed. + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); // AND: Only partial sampling columns are received (first column but not others) let (req0, _) = columns_req_id.first().unwrap(); @@ -1325,8 +1383,9 @@ mod tests { *req0, blocks .iter() - .flat_map(|b| { - b.1.iter() + .flat_map(|(_, columns, _)| { + columns + .iter() .filter(|d| *d.index() == expected_sampling_columns[0]) .cloned() }) @@ -1363,8 +1422,9 @@ mod tests { new_columns_req_id, blocks .iter() - .flat_map(|b| { - b.1.iter() + .flat_map(|(_, columns, _)| { + columns + .iter() .filter(|d| failed_column_indices.contains(d.index())) .cloned() }) @@ -1383,34 +1443,18 @@ mod tests { #[test] fn max_retries_exceeded_behavior() { - if skip_under_gloas() { - return; - } // GIVEN: A request where peers consistently fail to provide required columns - let mut spec = test_spec::(); - spec.deneb_fork_epoch = Some(Epoch::new(0)); - spec.fulu_fork_epoch = Some(Epoch::new(0)); - let spec = Arc::new(spec); + let spec = Arc::new(custody_test_spec()); let da_checker = Arc::new(test_da_checker(spec.clone(), NodeCustodyType::Fullnode)); let expected_sampling_columns = da_checker .custody_context() .sampling_columns_for_epoch(Epoch::new(0), &spec) .to_vec(); - let mut u = types::test_utils::test_unstructured(); - let blocks = (0..1) - .map(|_| { - generate_rand_block_and_data_columns::( - ForkName::Fulu, - NumBlobs::Number(1), - &mut u, - &spec, - ) - .unwrap() - }) - .collect::>(); + let blocks = make_blocks_and_columns(1, NumBlobs::Number(1), &spec); let components_id = components_id(); let blocks_req_id = blocks_id(components_id); + let payloads_req_id = is_gloas_env().then(|| payloads_id(components_id)); let columns_req_id = expected_sampling_columns .iter() .enumerate() @@ -1428,16 +1472,18 @@ mod tests { blocks_req_id, None, Some((columns_req_id.clone(), expected_sampling_columns.clone())), - None, + payloads_req_id, Span::none(), ); // AND: All blocks are received info.add_blocks( blocks_req_id, - blocks.iter().map(|b| b.0.clone().into()).collect(), + blocks.iter().map(|(block, _, _)| block.clone()).collect(), ) .unwrap(); + // Under Gloas the payloads request must be completed for `responses` to proceed. + add_envelopes_if_gloas(&mut info, payloads_req_id, &blocks); // AND: Only the first sampling column is provided successfully let (req0, _) = columns_req_id.first().unwrap(); @@ -1445,8 +1491,9 @@ mod tests { *req0, blocks .iter() - .flat_map(|b| { - b.1.iter() + .flat_map(|(_, columns, _)| { + columns + .iter() .filter(|d| *d.index() == expected_sampling_columns[0]) .cloned() }) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 621824c7d2..deaf421e39 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -45,6 +45,17 @@ use types::{ const D: Duration = Duration::new(0, 0); +/// Extract the Gloas payload envelope (if any) carried by a stored `RangeSyncBlock`. +fn envelope_of(block: &RangeSyncBlock) -> Option>> { + match block { + RangeSyncBlock::Gloas { + envelope: Some(envelope), + .. + } => Some(envelope.envelope().clone()), + _ => None, + } +} + /// Gloas genesis needs enough validators to populate `proposer_lookahead`. const TEST_RIG_VALIDATOR_COUNT: usize = 8; @@ -331,7 +342,6 @@ impl TestRig { fork_name, network_blocks_by_root: <_>::default(), network_blocks_by_slot: <_>::default(), - network_envelopes_by_root: <_>::default(), penalties: <_>::default(), seen_lookups: <_>::default(), requests: <_>::default(), @@ -669,7 +679,10 @@ impl TestRig { if self.complete_strategy.hold_envelope_for_block == Some(block_root) { return; } - let envelope = self.network_envelopes_by_root.get(&block_root).cloned(); + let envelope = self + .network_blocks_by_root + .get(&block_root) + .and_then(envelope_of); self.send_rpc_envelope_response(req_id, peer_id, envelope); } @@ -843,7 +856,7 @@ impl TestRig { if self.complete_strategy.hold_envelope_for_block == Some(block_root) { return None; } - self.network_envelopes_by_root.get(&block_root).cloned() + envelope_of(block) }) .collect::>(); self.send_rpc_envelopes_response(req_id, peer_id, &envelopes); @@ -1057,13 +1070,7 @@ impl TestRig { .await; let block = external_harness.get_full_block(&block_root); let block_slot = block.slot(); - self.insert_external_block( - block, - external_harness - .chain - .get_payload_envelope(&block_root) - .unwrap(), - ); + self.insert_external_block(block); blocks.push((block_slot, block_root)); } @@ -1171,8 +1178,7 @@ impl TestRig { // Cache every block through the single `get_full_block` + `insert_external_block2` path. for root in [g_root, a_root, c_root, b_root] { let block = external_harness.get_full_block(&root); - let envelope = external_harness.chain.get_payload_envelope(&root).unwrap(); - self.insert_external_block(block, envelope); + self.insert_external_block(block); } self.harness.set_current_slot(child_slot); @@ -1200,21 +1206,12 @@ impl TestRig { Some((r, fork)) } - fn insert_external_block( - &mut self, - block: RangeSyncBlock, - envelope: Option>, - ) { + fn insert_external_block(&mut self, block: RangeSyncBlock) { let block_root = block.canonical_root(); let block_slot = block.slot(); self.network_blocks_by_root .insert(block_root, block.clone()); self.network_blocks_by_slot.insert(block_slot, block); - // Cache Gloas envelopes for lookup RPCs. - if let Some(envelope) = envelope { - self.network_envelopes_by_root - .insert(block_root, envelope.into()); - } self.log(&format!( "Produced block {block_root:?} slot {block_slot} in external harness", )); @@ -1300,9 +1297,9 @@ impl TestRig { let range_sync_block = if block.fork_name_unchecked().gloas_enabled() { // Gloas carries data columns in the payload envelope, not in `block_data`. let envelope = self - .network_envelopes_by_root + .network_blocks_by_root .get(&block_root) - .cloned() + .and_then(envelope_of) .map(|envelope| AvailableEnvelope::new(envelope, columns.unwrap_or_default())); RangeSyncBlock::new_gloas(block, envelope).unwrap() } else { @@ -1370,6 +1367,8 @@ impl TestRig { .unwrap_or_else(|| panic!("No block at slot {slot}")) .clone(); let block_root = rpc_block.canonical_root(); + let block_state_root = rpc_block.as_block().state_root(); + let envelope = envelope_of(&rpc_block); self.harness .chain .process_block( @@ -1381,6 +1380,18 @@ impl TestRig { ) .await .unwrap(); + // Gloas: import the payload envelope so the block counts as full for its children. + if let Some(envelope) = envelope { + let state = self + .harness + .chain + .get_state(&block_state_root, Some(Slot::new(slot)), false) + .expect("should load state") + .expect("state should exist"); + self.harness + .process_envelope(block_root, (*envelope).clone(), &state, block_state_root) + .await; + } } self.harness.chain.recompute_head_at_current_slot().await; } @@ -1495,6 +1506,17 @@ impl TestRig { panic!("Some downscore events: {:?}", self.penalties); } } + + fn assert_no_block_requests(&self) { + assert_eq!( + self.requests + .iter() + .filter(|(request, _)| matches!(request, RequestType::BlocksByRoot(_))) + .collect::>(), + Vec::<&(RequestType, AppRequestId)>::new(), + "There should be no block requests" + ); + } fn assert_failed_lookup_sync(&mut self) { assert!(self.created_lookups() > 0, "no created lookups"); assert_eq!(self.completed_lookups(), 0, "some completed lookups"); @@ -1623,6 +1645,10 @@ impl TestRig { genesis_fork().fulu_enabled().then(Self::default) } + fn new_after_gloas() -> Option { + genesis_fork().gloas_enabled().then(Self::default) + } + pub fn new_fulu_peer_test(fulu_test_type: FuluTestType) -> Option { genesis_fork().fulu_enabled().then(|| { Self::new(TestRigConfig { @@ -2129,7 +2155,8 @@ async fn happy_path_unknown_data_parent(depth: usize) { let Some(mut r) = TestRig::new_after_fulu() else { return; }; - // No unknown-parent data-column trigger post-Gloas. + // Fulu-only: the `UnknownDataColumnParent` trigger doesn't exist post-Gloas (columns ride in + // the payload envelope, not as standalone data columns). if r.is_after_gloas() { return; } @@ -2359,10 +2386,6 @@ async fn test_single_block_lookup_ignored_response() { /// Assert that if the beacon processor returns DuplicateFullyImported, the lookup completes successfully async fn test_single_block_lookup_duplicate_response() { let mut r = TestRig::default(); - // The mock only covers block processing; Gloas also needs real envelope/column results. - if r.is_after_gloas() { - return; - } r.build_chain_and_trigger_last_block(1).await; // Send a DuplicateFullyImported response, the lookup should complete successfully r.simulate( @@ -2427,10 +2450,6 @@ async fn lookups_form_chain() { /// Assert that if a lookup chain (by appending ancestors) is too long we drop it async fn test_parent_lookup_too_deep_grow_ancestor_one() { let mut r = TestRig::default(); - // TODO(gloas): range sync does not fetch payload envelopes yet. - if r.is_after_gloas() { - return; - } r.build_chain(PARENT_DEPTH_TOLERANCE + 1).await; r.trigger_with_last_block(); r.simulate(SimulateConfig::happy_path()).await; @@ -2575,13 +2594,13 @@ async fn test_same_chain_race_condition() { } #[tokio::test] -/// Assert that if the lookup's block is in the da_checker we don't download it again -async fn block_in_da_checker_skips_download() { - // Only post-Fulu, as the block needs custody columns to remain in the da_checker +/// Assert that if the lookup's block is in the da_checker we don't download it again (pre-Gloas). +async fn block_in_da_checker_skips_download_fulu() { + // Only post-Fulu, as the block needs custody columns to remain in the da_checker. let Some(mut r) = TestRig::new_after_fulu() else { return; }; - // TODO(gloas): the helper does not populate the envelope missing-component path yet. + // Pre-Gloas only; the Gloas equivalent is `block_in_da_checker_skips_download_gloas`. if r.is_after_gloas() { return; } @@ -2594,14 +2613,28 @@ async fn block_in_da_checker_skips_download() { r.trigger_with_block_at_slot(1); r.simulate(SimulateConfig::happy_path()).await; r.assert_successful_lookup_sync(); - assert_eq!( - r.requests - .iter() - .filter(|(request, _)| matches!(request, RequestType::BlocksByRoot(_))) - .collect::>(), - Vec::<&(RequestType, AppRequestId)>::new(), - "There should be no block requests" - ); + r.assert_no_block_requests(); +} + +#[tokio::test] +/// Assert that if the lookup's block is in the da_checker we don't download it again (Gloas). +async fn block_in_da_checker_skips_download_gloas() { + let Some(mut r) = TestRig::new_after_gloas() else { + return; + }; + // A Gloas block carries no inline DA, so a lone block never sits in the da_checker awaiting + // components: only a FULL *child* proves the block published a payload and supplies the peers + // that serve its columns/envelope. Build a parent + FULL child, insert the PARENT into the + // da_checker, then trigger via the child (which is provided by the trigger, not downloaded). + // The parent lookup must then skip the parent's block download. + r.build_chain(2).await; + let parent = r.block_at_slot(1); + let child = r.block_at_slot(2); + r.import_block_to_da_checker(parent).await; + r.trigger_unknown_parent_blocks_from_all_peers(&[child]); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); + r.assert_no_block_requests(); } macro_rules! fulu_peer_matrix_tests { diff --git a/beacon_node/network/src/sync/tests/mod.rs b/beacon_node/network/src/sync/tests/mod.rs index 2f318bfb9a..4e185cc081 100644 --- a/beacon_node/network/src/sync/tests/mod.rs +++ b/beacon_node/network/src/sync/tests/mod.rs @@ -21,7 +21,7 @@ use tokio::sync::mpsc; use tracing_subscriber::fmt::MakeWriter; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; -use types::{ForkName, Hash256, MinimalEthSpec as E, SignedExecutionPayloadEnvelope, Slot}; +use types::{ForkName, Hash256, MinimalEthSpec as E, Slot}; mod lookups; mod range; @@ -77,10 +77,6 @@ struct TestRig { /// Blocks that will be used in the test but may not be known to `harness` yet. network_blocks_by_root: HashMap>, network_blocks_by_slot: HashMap>, - /// Gloas execution payload envelopes keyed by block root, populated during `build_chain` - /// from the external harness store. The rig serves these when a lookup issues a - /// `PayloadEnvelopesByRoot` request. - network_envelopes_by_root: HashMap>>, penalties: Vec, /// All seen lookups through the test run seen_lookups: HashMap, diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index e6890cf242..1499ae5016 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -34,13 +34,6 @@ use types::{Epoch, EthSpec, Hash256, MinimalEthSpec as E, Slot}; const SLOTS_PER_EPOCH: usize = 8; impl TestRig { - /// Range sync doesn't yet ingest Gloas blocks in these tests: the range harness doesn't serve - /// payload envelopes, so a Gloas block never becomes fully available and sync can't complete. - /// Skip the affected completion tests under a Gloas genesis. TODO(gloas): support range sync. - fn skip_range_sync_under_gloas(&self) -> bool { - self.fork_name.gloas_enabled() - } - fn add_head_peer(&mut self) -> PeerId { let local_info = self.local_info(); self.add_supernode_peer(SyncInfo { @@ -267,9 +260,6 @@ impl TestRig { #[tokio::test] async fn head_sync_completes() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_head_sync().await; r.simulate(SimulateConfig::happy_path()).await; r.assert_head_sync_completed(); @@ -281,9 +271,6 @@ async fn head_sync_completes() { #[tokio::test] async fn finalized_to_head_transition() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_and_head_sync().await; r.simulate(SimulateConfig::happy_path()).await; r.assert_range_sync_completed(); @@ -295,9 +282,6 @@ async fn finalized_to_head_transition() { #[tokio::test] async fn finalized_sync_completes() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path()).await; r.assert_range_sync_completed(); @@ -309,9 +293,6 @@ async fn finalized_sync_completes() { #[tokio::test] async fn batch_rpc_error_retries() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().return_rpc_error(RPCError::UnsupportedProtocol)) .await; @@ -380,9 +361,6 @@ async fn batch_peer_returns_partial_columns_then_succeeds() { #[tokio::test] async fn batch_non_faulty_failure_retries() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_non_faulty_failures(1)) .await; @@ -394,9 +372,6 @@ async fn batch_non_faulty_failure_retries() { #[tokio::test] async fn batch_faulty_failure_redownloads() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_faulty_failures(1)) .await; @@ -453,9 +428,6 @@ async fn late_response_for_removed_chain() { #[tokio::test] async fn ee_offline_then_online_resumes_sync() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_ee_offline_for_n_range_responses(2)) .await; @@ -468,9 +440,6 @@ async fn ee_offline_then_online_resumes_sync() { #[tokio::test] async fn finalized_sync_with_local_head_partial() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } r.setup_finalized_sync_with_local_head(3).await; r.simulate(SimulateConfig::happy_path()).await; r.assert_range_sync_completed(); @@ -481,9 +450,6 @@ async fn finalized_sync_with_local_head_partial() { #[tokio::test] async fn finalized_sync_with_local_head_near_target() { let mut r = TestRig::default(); - if r.skip_range_sync_under_gloas() { - return; - } let target_epochs = 5; let local_slots = (target_epochs * SLOTS_PER_EPOCH) - 1; // all blocks except last r.build_chain(target_epochs * SLOTS_PER_EPOCH).await; @@ -502,7 +468,7 @@ async fn finalized_sync_with_local_head_near_target() { #[tokio::test] async fn not_enough_custody_peers_then_peers_arrive() { let mut r = TestRig::default(); - if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { + if !r.fork_name.fulu_enabled() { return; } let remote_info = r.setup_finalized_sync_insufficient_peers().await; @@ -529,7 +495,7 @@ async fn not_enough_custody_peers_then_peers_arrive() { #[tokio::test] async fn finalized_sync_not_enough_custody_peers_resume_after_peer_cgc_update() { let mut r = TestRig::default(); - if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { + if !r.fork_name.fulu_enabled() { return; } From 5e54cfbf86f5daed83e315327f4d079fa0c143c8 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 23 Jun 2026 20:29:39 +0200 Subject: [PATCH 12/12] Remove seen_timestamp tracking from sync (#9454) Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> Co-Authored-By: Pawan Dhananjay --- beacon_node/network/src/metrics.rs | 16 ----- .../src/network_beacon_processor/mod.rs | 23 ++---- .../network_beacon_processor/sync_methods.rs | 42 ++--------- .../src/network_beacon_processor/tests.rs | 3 - beacon_node/network/src/router.rs | 7 -- .../sync/block_lookups/single_block_lookup.rs | 13 +--- beacon_node/network/src/sync/manager.rs | 72 +++++-------------- .../network/src/sync/network_context.rs | 54 ++++++-------- .../src/sync/network_context/custody.rs | 35 +++------ .../src/sync/network_context/requests.rs | 15 ++-- beacon_node/network/src/sync/tests/lookups.rs | 12 ---- 11 files changed, 70 insertions(+), 222 deletions(-) diff --git a/beacon_node/network/src/metrics.rs b/beacon_node/network/src/metrics.rs index 1a664662df..07e6d7fdb2 100644 --- a/beacon_node/network/src/metrics.rs +++ b/beacon_node/network/src/metrics.rs @@ -665,22 +665,6 @@ pub static BEACON_BLOB_DELAY_FULL_VERIFICATION: LazyLock> = Laz ) }); -pub static BEACON_BLOB_RPC_SLOT_START_DELAY_TIME: LazyLock> = LazyLock::new( - || { - try_create_histogram_with_buckets( - "beacon_blob_rpc_slot_start_delay_time", - "Duration between when a blob is received over rpc and the start of the slot it belongs to.", - // Create a custom bucket list for greater granularity in block delay - Ok(vec![ - 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 5.0, - 6.0, 7.0, 8.0, 9.0, 10.0, 15.0, 20.0, - ]), // NOTE: Previous values, which we may want to switch back to. - // [0.1, 0.2, 0.5, 1, 2, 5, 10, 20, 50] - //decimal_buckets(-1,2) - ) - }, -); - /* * Light client update reprocessing queue metrics. */ diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index c5023ed5f4..ea5fa3e90b 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -528,15 +528,11 @@ impl NetworkBeaconProcessor { self: &Arc, block_root: Hash256, block: LookupBlock, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> Result<(), Error> { - let process_fn = self.clone().generate_lookup_beacon_block_process_fn( - block_root, - block, - seen_timestamp, - process_type, - ); + let process_fn = + self.clone() + .generate_lookup_beacon_block_process_fn(block_root, block, process_type); self.try_send(BeaconWorkEvent { drop_during_sync: false, work: Work::RpcBlock { @@ -552,14 +548,13 @@ impl NetworkBeaconProcessor { self: &Arc, block_root: Hash256, envelope: Arc>, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> Result<(), Error> { let s = self.clone(); self.try_send(BeaconWorkEvent { drop_during_sync: false, work: Work::RpcEnvelope(Box::pin(async move { - s.process_lookup_envelope(block_root, envelope, seen_timestamp, process_type) + s.process_lookup_envelope(block_root, envelope, process_type) .await; })), }) @@ -571,20 +566,14 @@ impl NetworkBeaconProcessor { self: &Arc, block_root: Hash256, custody_columns: DataColumnSidecarList, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> Result<(), Error> { let s = self.clone(); self.try_send(BeaconWorkEvent { drop_during_sync: false, work: Work::RpcCustodyColumn(Box::pin(async move { - s.process_rpc_custody_columns( - block_root, - custody_columns, - seen_timestamp, - process_type, - ) - .await; + s.process_rpc_custody_columns(block_root, custody_columns, process_type) + .await; })), }) } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index b9e07743eb..c376f1844b 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -14,7 +14,7 @@ use beacon_chain::data_availability_checker::{ use beacon_chain::historical_data_columns::HistoricalDataColumnError; use beacon_chain::{ AvailabilityProcessingStatus, BeaconChainTypes, BlockError, ChainSegmentResult, - HistoricalBlockError, NotifyExecutionLayer, validator_monitor::get_slot_delay_ms, + HistoricalBlockError, NotifyExecutionLayer, }; use beacon_processor::{ AsyncFn, BlockingFn, DuplicateCache, @@ -26,7 +26,6 @@ use lighthouse_network::PeerId; use lighthouse_network::service::api_types::CustodyBackfillBatchId; use logging::crit; use std::sync::Arc; -use std::time::Duration; use tracing::{debug, debug_span, error, info, instrument, warn}; use types::{BlockImportSource, DataColumnSidecarList, Epoch, ExecutionBlockHash, Hash256}; @@ -56,19 +55,12 @@ impl NetworkBeaconProcessor { self: Arc, block_root: Hash256, block: LookupBlock, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> AsyncFn { let process_fn = async move { let duplicate_cache = self.duplicate_cache.clone(); - self.process_lookup_block( - block_root, - block, - seen_timestamp, - process_type, - duplicate_cache, - ) - .await; + self.process_lookup_block(block_root, block, process_type, duplicate_cache) + .await; }; Box::pin(process_fn) } @@ -78,14 +70,12 @@ impl NetworkBeaconProcessor { self: Arc, block_root: Hash256, block: LookupBlock, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> (AsyncFn, BlockingFn) { // An async closure which will import the block. let process_fn = self.clone().generate_lookup_beacon_block_process_fn( block_root, block, - seen_timestamp, process_type.clone(), ); // A closure which will ignore the block. @@ -119,7 +109,6 @@ impl NetworkBeaconProcessor { self: Arc>, block_root: Hash256, block: LookupBlock, - seen_timestamp: Duration, process_type: BlockProcessType, duplicate_cache: DuplicateCache, ) { @@ -133,12 +122,9 @@ impl NetworkBeaconProcessor { ); // Send message to work reprocess queue to retry the block - let (process_fn, ignore_fn) = self.clone().generate_lookup_beacon_block_fns( - block_root, - block, - seen_timestamp, - process_type, - ); + let (process_fn, ignore_fn) = + self.clone() + .generate_lookup_beacon_block_fns(block_root, block, process_type); let reprocess_msg = ReprocessQueueMessage::RpcBlock(QueuedRpcBlock { beacon_block_root: block_root, process_fn, @@ -206,13 +192,6 @@ impl NetworkBeaconProcessor { "Failed to inform block import" ); }; - self.chain.block_times_cache.write().set_time_observed( - *hash, - *slot, - seen_timestamp, - None, - None, - ); self.chain.recompute_head_at_current_slot().await; } @@ -254,7 +233,6 @@ impl NetworkBeaconProcessor { self: Arc>, block_root: Hash256, custody_columns: DataColumnSidecarList, - seen_timestamp: Duration, process_type: BlockProcessType, ) { // custody_columns must always have at least one element @@ -262,13 +240,6 @@ impl NetworkBeaconProcessor { return; }; - if let Ok(current_slot) = self.chain.slot() - && current_slot == slot - { - let delay = get_slot_delay_ms(seen_timestamp, slot, &self.chain.slot_clock); - metrics::observe_duration(&metrics::BEACON_BLOB_RPC_SLOT_START_DELAY_TIME, delay); - } - let mut indices = custody_columns .iter() .map(|d| *d.index()) @@ -332,7 +303,6 @@ impl NetworkBeaconProcessor { self: Arc>, block_root: Hash256, envelope: Arc>, - _seen_timestamp: Duration, process_type: BlockProcessType, ) { debug!( diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index 6b7c623230..89434c878e 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -422,7 +422,6 @@ impl TestRig { .send_lookup_beacon_block( block_root, LookupBlock::new(self.next_block.clone()), - std::time::Duration::default(), BlockProcessType::SingleBlock { id: 0 }, ) .unwrap(); @@ -434,7 +433,6 @@ impl TestRig { .send_lookup_beacon_block( block_root, LookupBlock::new(self.next_block.clone()), - std::time::Duration::default(), BlockProcessType::SingleBlock { id: 1 }, ) .unwrap(); @@ -446,7 +444,6 @@ impl TestRig { .send_rpc_custody_columns( self.next_block.canonical_root(), data_columns, - Duration::default(), BlockProcessType::SingleCustodyColumn(1), ) .unwrap(); diff --git a/beacon_node/network/src/router.rs b/beacon_node/network/src/router.rs index 315ec9387d..d83068e337 100644 --- a/beacon_node/network/src/router.rs +++ b/beacon_node/network/src/router.rs @@ -659,7 +659,6 @@ impl Router { peer_id, sync_request_id, beacon_block, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } @@ -679,7 +678,6 @@ impl Router { peer_id, sync_request_id, blob_sidecar, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } else { crit!("All blobs by range responses should belong to sync"); @@ -716,7 +714,6 @@ impl Router { peer_id, sync_request_id, beacon_block, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } @@ -750,7 +747,6 @@ impl Router { sync_request_id, peer_id, data_column, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } @@ -770,7 +766,6 @@ impl Router { peer_id, sync_request_id, data_column, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } else { crit!("All data columns by range responses should belong to sync"); @@ -796,7 +791,6 @@ impl Router { sync_request_id, peer_id, envelope, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } @@ -819,7 +813,6 @@ impl Router { sync_request_id, peer_id, envelope, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), }); } diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index f03eed1638..346594c2f5 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -381,7 +381,7 @@ impl SingleBlockLookup { if self.awaiting_parent.is_none() && let Some(data) = self.block_request.state.maybe_start_processing() { - cx.send_block_for_processing(self.id, self.block_root, data.value, data.seen_timestamp) + cx.send_block_for_processing(self.id, self.block_root, data.value) .map_err(LookupRequestError::SendFailedProcessor)?; } @@ -422,7 +422,6 @@ impl SingleBlockLookup { self.id, self.block_root, data.value, - data.seen_timestamp, BlockProcessType::SingleCustodyColumn(self.id), ) .map_err(LookupRequestError::SendFailedProcessor)?; @@ -463,7 +462,6 @@ impl SingleBlockLookup { cx.send_payload_for_processing( self.block_root, data.value, - data.seen_timestamp, BlockProcessType::SinglePayloadEnvelope(self.id), ) .map_err(LookupRequestError::SendFailedProcessor)?; @@ -711,17 +709,12 @@ impl SingleBlockLookup { #[derive(Debug, Clone)] pub struct DownloadResult { pub value: T, - pub seen_timestamp: Duration, pub peer_group: PeerGroup, } impl DownloadResult { - pub fn new(value: T, peer_group: PeerGroup, seen_timestamp: Duration) -> Self { - Self { - value, - seen_timestamp, - peer_group, - } + pub fn new(value: T, peer_group: PeerGroup) -> Self { + Self { value, peer_group } } } diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 3282f7f083..b9bea21b8c 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -113,7 +113,6 @@ pub enum SyncMessage { sync_request_id: SyncRequestId, peer_id: PeerId, beacon_block: Option>>, - seen_timestamp: Duration, }, /// A blob has been received from the RPC. @@ -121,7 +120,6 @@ pub enum SyncMessage { sync_request_id: SyncRequestId, peer_id: PeerId, blob_sidecar: Option>>, - seen_timestamp: Duration, }, /// A data columns has been received from the RPC @@ -129,7 +127,6 @@ pub enum SyncMessage { sync_request_id: SyncRequestId, peer_id: PeerId, data_column: Option>>, - seen_timestamp: Duration, }, /// A payload envelope has been received from the RPC. @@ -137,7 +134,6 @@ pub enum SyncMessage { sync_request_id: SyncRequestId, peer_id: PeerId, envelope: Option>>, - seen_timestamp: Duration, }, /// A block with an unknown parent has been received. @@ -835,35 +831,24 @@ impl SyncManager { sync_request_id, peer_id, beacon_block, - seen_timestamp, } => { - self.rpc_block_received(sync_request_id, peer_id, beacon_block, seen_timestamp); + self.rpc_block_received(sync_request_id, peer_id, beacon_block); } SyncMessage::RpcBlob { sync_request_id, peer_id, blob_sidecar, - seen_timestamp, - } => self.rpc_blob_received(sync_request_id, peer_id, blob_sidecar, seen_timestamp), + } => self.rpc_blob_received(sync_request_id, peer_id, blob_sidecar), SyncMessage::RpcDataColumn { sync_request_id, peer_id, data_column, - seen_timestamp, - } => { - self.rpc_data_column_received(sync_request_id, peer_id, data_column, seen_timestamp) - } + } => self.rpc_data_column_received(sync_request_id, peer_id, data_column), SyncMessage::RpcPayloadEnvelope { sync_request_id, peer_id, envelope, - seen_timestamp, - } => self.rpc_payload_envelope_received( - sync_request_id, - peer_id, - envelope, - seen_timestamp, - ), + } => self.rpc_payload_envelope_received(sync_request_id, peer_id, envelope), SyncMessage::UnknownParentBlock(peer_id, block, block_root) => { let block_slot = block.slot(); let parent_root = block.parent_root(); @@ -877,7 +862,6 @@ impl SyncManager { block_slot, BlockComponent::Block(DownloadResult { value: block.block_cloned(), - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), peer_group: PeerGroup::from_single(peer_id), }), ); @@ -1122,19 +1106,14 @@ impl SyncManager { sync_request_id: SyncRequestId, peer_id: PeerId, block: Option>>, - seen_timestamp: Duration, ) { match sync_request_id { - SyncRequestId::SingleBlock { id } => self.on_single_block_response( - id, - peer_id, - RpcEvent::from_chunk(block, seen_timestamp), - ), - SyncRequestId::BlocksByRange(id) => self.on_blocks_by_range_response( - id, - peer_id, - RpcEvent::from_chunk(block, seen_timestamp), - ), + SyncRequestId::SingleBlock { id } => { + self.on_single_block_response(id, peer_id, RpcEvent::from_chunk(block)) + } + SyncRequestId::BlocksByRange(id) => { + self.on_blocks_by_range_response(id, peer_id, RpcEvent::from_chunk(block)) + } _ => { crit!(%peer_id, "bad request id for block"); } @@ -1150,9 +1129,7 @@ impl SyncManager { if let Some(resp) = self.network.on_single_block_response(id, peer_id, block) { self.block_lookups.on_block_download_response( id, - resp.map(|(value, seen_timestamp)| { - DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) - }), + resp.map(|value| DownloadResult::new(value, PeerGroup::from_single(peer_id))), &mut self.network, ) } @@ -1163,14 +1140,11 @@ impl SyncManager { sync_request_id: SyncRequestId, peer_id: PeerId, blob: Option>>, - seen_timestamp: Duration, ) { match sync_request_id { - SyncRequestId::BlobsByRange(id) => self.on_blobs_by_range_response( - id, - peer_id, - RpcEvent::from_chunk(blob, seen_timestamp), - ), + SyncRequestId::BlobsByRange(id) => { + self.on_blobs_by_range_response(id, peer_id, RpcEvent::from_chunk(blob)) + } _ => { crit!(%peer_id, "bad request id for blob"); } @@ -1182,20 +1156,15 @@ impl SyncManager { sync_request_id: SyncRequestId, peer_id: PeerId, envelope: Option>>, - seen_timestamp: Duration, ) { match sync_request_id { SyncRequestId::SinglePayloadEnvelope { id } => self - .on_single_payload_envelope_response( - id, - peer_id, - RpcEvent::from_chunk(envelope, seen_timestamp), - ), + .on_single_payload_envelope_response(id, peer_id, RpcEvent::from_chunk(envelope)), SyncRequestId::PayloadEnvelopesByRange(req_id) => { self.on_payload_envelopes_by_range_response( req_id, peer_id, - RpcEvent::from_chunk(envelope, seen_timestamp), + RpcEvent::from_chunk(envelope), ); } _ => { @@ -1209,21 +1178,20 @@ impl SyncManager { sync_request_id: SyncRequestId, peer_id: PeerId, data_column: Option>>, - seen_timestamp: Duration, ) { match sync_request_id { SyncRequestId::DataColumnsByRoot(req_id) => { self.on_data_columns_by_root_response( req_id, peer_id, - RpcEvent::from_chunk(data_column, seen_timestamp), + RpcEvent::from_chunk(data_column), ); } SyncRequestId::DataColumnsByRange(req_id) => { self.on_data_columns_by_range_response( req_id, peer_id, - RpcEvent::from_chunk(data_column, seen_timestamp), + RpcEvent::from_chunk(data_column), ); } _ => { @@ -1244,9 +1212,7 @@ impl SyncManager { { self.block_lookups.on_payload_download_response( id, - resp.map(|(value, seen_timestamp)| { - DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) - }), + resp.map(|value| DownloadResult::new(value, PeerGroup::from_single(peer_id))), &mut self.network, ) } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 0db172a21a..8b4e3c5694 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -52,7 +52,6 @@ use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::sync::Arc; -use std::time::Duration; #[cfg(test)] use task_executor::TaskExecutor; use tokio::sync::mpsc; @@ -83,22 +82,21 @@ pub const MAX_COLUMN_RETRIES: usize = 3; #[derive(Debug)] pub enum RpcEvent { StreamTermination, - Response(T, Duration), + Response(T), RPCError(RPCError), } impl RpcEvent { - pub fn from_chunk(chunk: Option, seen_timestamp: Duration) -> Self { + pub fn from_chunk(chunk: Option) -> Self { match chunk { - Some(item) => RpcEvent::Response(item, seen_timestamp), + Some(item) => RpcEvent::Response(item), None => RpcEvent::StreamTermination, } } } -pub type RpcResponseResult = Result<(T, Duration), RpcResponseError>; +pub type RpcResponseResult = Result; -/// Duration = latest seen timestamp of all received data columns pub type CustodyByRootResult = Result>, RpcResponseError>; @@ -776,14 +774,14 @@ impl SyncNetworkContext { if let Err(e) = { let request = entry.get_mut(); match range_block_component { - RangeBlockComponent::Block(req_id, resp) => resp.and_then(|(blocks, _)| { + RangeBlockComponent::Block(req_id, resp) => resp.and_then(|blocks| { request.add_blocks(req_id, blocks).map_err(|e| { RpcResponseError::BlockComponentCouplingError(CouplingError::InternalError( e, )) }) }), - RangeBlockComponent::Blob(req_id, resp) => resp.and_then(|(blobs, _)| { + RangeBlockComponent::Blob(req_id, resp) => resp.and_then(|blobs| { request.add_blobs(req_id, blobs).map_err(|e| { RpcResponseError::BlockComponentCouplingError(CouplingError::InternalError( e, @@ -791,7 +789,7 @@ impl SyncNetworkContext { }) }), RangeBlockComponent::CustodyColumns(req_id, resp) => { - resp.and_then(|(custody_columns, _)| { + resp.and_then(|custody_columns| { request .add_custody_columns(req_id, custody_columns) .map_err(|e| { @@ -801,17 +799,15 @@ impl SyncNetworkContext { }) }) } - RangeBlockComponent::PayloadEnvelope(req_id, resp) => { - resp.and_then(|(envelopes, _)| { - request - .add_payload_envelopes(req_id, envelopes) - .map_err(|e| { - RpcResponseError::BlockComponentCouplingError( - CouplingError::InternalError(e), - ) - }) - }) - } + RangeBlockComponent::PayloadEnvelope(req_id, resp) => resp.and_then(|envelopes| { + request + .add_payload_envelopes(req_id, envelopes) + .map_err(|e| { + RpcResponseError::BlockComponentCouplingError( + CouplingError::InternalError(e), + ) + }) + }), } } { entry.remove(); @@ -1479,11 +1475,11 @@ impl SyncNetworkContext { ) -> Option>>> { let resp = self.blocks_by_root_requests.on_response(id, rpc_event); let resp = resp.map(|res| { - res.and_then(|(mut blocks, seen_timestamp)| { + res.and_then(|mut blocks| { // Enforce that exactly one chunk = one block is returned. ReqResp behavior limits the // response count to at most 1. match blocks.pop() { - Some(block) => Ok((block, seen_timestamp)), + Some(block) => Ok(block), // Should never happen, `blocks_by_root_requests` enforces that we receive at least // 1 chunk. None => Err(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }.into()), @@ -1503,9 +1499,9 @@ impl SyncNetworkContext { .payload_envelopes_by_root_requests .on_response(id, rpc_event); let resp = resp.map(|res| { - res.and_then(|(mut envelopes, seen_timestamp)| { + res.and_then(|mut envelopes| { match envelopes.pop() { - Some(envelope) => Ok((envelope, seen_timestamp)), + Some(envelope) => Ok(envelope), // Should never happen, we enforce at least 1 chunk. None => Err(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }.into()), } @@ -1646,7 +1642,6 @@ impl SyncNetworkContext { id: Id, block_root: Hash256, block: Arc>, - seen_timestamp: Duration, ) -> Result<(), SendErrorProcessor> { let beacon_processor = self .beacon_processor_if_enabled() @@ -1661,7 +1656,6 @@ impl SyncNetworkContext { .send_lookup_beacon_block( block_root, lookup_block, - seen_timestamp, BlockProcessType::SingleBlock { id }, ) .map_err(|e| { @@ -1677,7 +1671,6 @@ impl SyncNetworkContext { &self, block_root: Hash256, envelope: Arc>, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> Result<(), SendErrorProcessor> { let beacon_processor = self @@ -1691,7 +1684,7 @@ impl SyncNetworkContext { ); beacon_processor - .send_lookup_envelope(block_root, envelope, seen_timestamp, process_type) + .send_lookup_envelope(block_root, envelope, process_type) .map_err(|e| { error!( error = ?e, @@ -1706,7 +1699,6 @@ impl SyncNetworkContext { _id: Id, block_root: Hash256, custody_columns: DataColumnSidecarList, - seen_timestamp: Duration, process_type: BlockProcessType, ) -> Result<(), SendErrorProcessor> { let beacon_processor = self @@ -1720,7 +1712,7 @@ impl SyncNetworkContext { ); beacon_processor - .send_rpc_custody_columns(block_root, custody_columns, seen_timestamp, process_type) + .send_rpc_custody_columns(block_root, custody_columns, process_type) .map_err(|e| { error!( error = ?e, @@ -1806,7 +1798,7 @@ impl SyncNetworkContext { if let Err(e) = { let request = entry.get_mut(); - data_columns.and_then(|(data_columns, _)| { + data_columns.and_then(|data_columns| { request .add_custody_columns(req_id, data_columns.clone()) .map_err(|e| { diff --git a/beacon_node/network/src/sync/network_context/custody.rs b/beacon_node/network/src/sync/network_context/custody.rs index 3dd3683c42..29cb0a22e5 100644 --- a/beacon_node/network/src/sync/network_context/custody.rs +++ b/beacon_node/network/src/sync/network_context/custody.rs @@ -7,7 +7,6 @@ use fnv::FnvHashMap; use lighthouse_network::PeerId; use lighthouse_network::service::api_types::{CustodyId, DataColumnsByRootRequester}; use parking_lot::RwLock; -use slot_clock::SlotClock; use std::collections::HashSet; use std::hash::{BuildHasher, RandomState}; use std::time::{Duration, Instant}; @@ -119,7 +118,7 @@ impl ActiveCustodyRequest { let _guard = batch_request.span.clone().entered(); match resp { - Ok((data_columns, seen_timestamp)) => { + Ok(data_columns) => { debug!( block_root = ?self.block_root, %req_id, @@ -144,12 +143,7 @@ impl ActiveCustodyRequest { .ok_or(Error::BadState("unknown column_index".to_owned()))?; if let Some(data_column) = data_columns.remove(column_index) { - column_request.on_download_success( - req_id, - peer_id, - data_column, - seen_timestamp, - )?; + column_request.on_download_success(req_id, peer_id, data_column)?; } else { // Peer does not have the requested data. // TODO(das) do not consider this case a success. We know for sure the block has @@ -213,30 +207,20 @@ impl ActiveCustodyRequest { if completed_requests >= total_requests { // All requests have completed successfully. let mut peers = HashMap::>::new(); - let mut seen_timestamps = vec![]; let columns = std::mem::take(&mut self.column_requests) .into_values() .map(|request| { - let (peer, data_column, seen_timestamp) = request.complete()?; + let (peer, data_column) = request.complete()?; peers .entry(peer) .or_default() .push(*data_column.index() as usize); - seen_timestamps.push(seen_timestamp); Ok(data_column) }) .collect::, _>>()?; let peer_group = PeerGroup::from_set(peers); - let max_seen_timestamp = seen_timestamps - .into_iter() - .max() - .unwrap_or_else(|| cx.chain.slot_clock.now_duration().unwrap_or_default()); - return Ok(Some(DownloadResult::new( - columns, - peer_group, - max_seen_timestamp, - ))); + return Ok(Some(DownloadResult::new(columns, peer_group))); } let data_columns_by_root_per_peer = @@ -416,7 +400,7 @@ struct ColumnRequest { enum Status { NotStarted(Instant), Downloading(DataColumnsByRootRequestId), - Downloaded(PeerId, Arc>, Duration), + Downloaded(PeerId, Arc>), } impl ColumnRequest { @@ -485,7 +469,6 @@ impl ColumnRequest { req_id: DataColumnsByRootRequestId, peer_id: PeerId, data_column: Arc>, - seen_timestamp: Duration, ) -> Result<(), Error> { match &self.status { Status::Downloading(expected_req_id) => { @@ -495,7 +478,7 @@ impl ColumnRequest { req_id, }); } - self.status = Status::Downloaded(peer_id, data_column, seen_timestamp); + self.status = Status::Downloaded(peer_id, data_column); Ok(()) } other => Err(Error::BadState(format!( @@ -504,11 +487,9 @@ impl ColumnRequest { } } - fn complete(self) -> Result<(PeerId, Arc>, Duration), Error> { + fn complete(self) -> Result<(PeerId, Arc>), Error> { match self.status { - Status::Downloaded(peer_id, data_column, seen_timestamp) => { - Ok((peer_id, data_column, seen_timestamp)) - } + Status::Downloaded(peer_id, data_column) => Ok((peer_id, data_column)), other => Err(Error::BadState(format!( "bad state complete expected Downloaded got {other:?}" ))), diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index cc74785098..b340064746 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -3,7 +3,6 @@ use std::{collections::hash_map::Entry, hash::Hash}; use fnv::FnvHashMap; use lighthouse_network::PeerId; -use slot_clock::timestamp_now; use strum::IntoStaticStr; use tracing::{Span, debug}; use types::{Hash256, Slot}; @@ -122,7 +121,7 @@ impl ActiveReque let result = match rpc_event { // Handler of a success ReqResp chunk. Adds the item to the request accumulator. // `ActiveRequestItems` validates the item before appending to its internal state. - RpcEvent::Response(item, seen_timestamp) => { + RpcEvent::Response(item) => { let request = &mut entry.get_mut(); let _guard = request.span.clone().entered(); match &mut request.state { @@ -133,7 +132,7 @@ impl ActiveReque Ok(true) => { let items = items.consume(); request.state = State::CompletedEarly; - Some(Ok((items, seen_timestamp, request.start_instant.elapsed()))) + Some(Ok((items, request.start_instant.elapsed()))) } // Received item, but we are still expecting more Ok(false) => None, @@ -170,11 +169,7 @@ impl ActiveReque } .into())) } else { - Some(Ok(( - items.consume(), - timestamp_now(), - request.start_instant.elapsed(), - ))) + Some(Ok((items.consume(), request.start_instant.elapsed()))) } } // Items already returned, ignore stream termination @@ -202,7 +197,7 @@ impl ActiveReque }; result.map(|result| match result { - Ok((items, seen_timestamp, duration)) => { + Ok((items, duration)) => { metrics::inc_counter_vec(&metrics::SYNC_RPC_REQUEST_SUCCESSES, &[self.name]); metrics::observe_timer_vec(&metrics::SYNC_RPC_REQUEST_TIME, &[self.name], duration); debug!( @@ -212,7 +207,7 @@ impl ActiveReque "Sync RPC request completed" ); - Ok((items, seen_timestamp)) + Ok(items) } Err(e) => { let err_str: &'static str = match &e { diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index deaf421e39..d84596cf3c 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -43,8 +43,6 @@ use types::{ SignedExecutionPayloadEnvelope, Slot, }; -const D: Duration = Duration::new(0, 0); - /// Extract the Gloas payload envelope (if any) carried by a stored `RangeSyncBlock`. fn envelope_of(block: &RangeSyncBlock) -> Option>> { match block { @@ -886,14 +884,12 @@ impl TestRig { sync_request_id, peer_id, beacon_block: Some(block.clone()), - seen_timestamp: D, }); } self.push_sync_message(SyncMessage::RpcBlock { sync_request_id, peer_id, beacon_block: None, - seen_timestamp: D, }); } @@ -917,14 +913,12 @@ impl TestRig { sync_request_id, peer_id, blob_sidecar: Some(blob.clone()), - seen_timestamp: D, }); } self.push_sync_message(SyncMessage::RpcBlob { sync_request_id, peer_id, blob_sidecar: None, - seen_timestamp: D, }); } @@ -953,14 +947,12 @@ impl TestRig { sync_request_id, peer_id, data_column: Some(column.clone()), - seen_timestamp: D, }); } self.push_sync_message(SyncMessage::RpcDataColumn { sync_request_id, peer_id, data_column: None, - seen_timestamp: D, }); } @@ -979,14 +971,12 @@ impl TestRig { sync_request_id, peer_id, envelope: envelope.clone(), - seen_timestamp: D, }); // Stream termination self.push_sync_message(SyncMessage::RpcPayloadEnvelope { sync_request_id, peer_id, envelope: None, - seen_timestamp: D, }); } @@ -1006,7 +996,6 @@ impl TestRig { sync_request_id, peer_id, envelope: Some(envelope.clone()), - seen_timestamp: D, }); } // Stream termination @@ -1014,7 +1003,6 @@ impl TestRig { sync_request_id, peer_id, envelope: None, - seen_timestamp: D, }); }