From f7cf8fca8d9d40dde95318c58daab49de828f4d1 Mon Sep 17 00:00:00 2001 From: Eitan Seri- Levi Date: Mon, 6 Apr 2026 00:16:02 -0700 Subject: [PATCH] Temp fixes --- beacon_node/beacon_chain/src/beacon_chain.rs | 50 +++++++++++++++- .../network_beacon_processor/sync_methods.rs | 57 ++++++------------- .../network/src/sync/backfill_sync/mod.rs | 3 +- beacon_node/network/src/sync/manager.rs | 39 +------------ .../network/src/sync/range_sync/chain.rs | 17 +----- .../src/sync/range_sync/chain_collection.rs | 26 +-------- .../network/src/sync/range_sync/range.rs | 9 --- 7 files changed, 69 insertions(+), 132 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 723d64489e..9588aa7a2b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2866,7 +2866,15 @@ impl BeaconChain { // In the case of (2), skipping the block is valid since we should never import it. // However, we will potentially get a `ParentUnknown` on a later block. The sync // protocol will need to ensure this is handled gracefully. - Err(BlockError::WouldRevertFinalizedSlot { .. }) => continue, + Err(BlockError::WouldRevertFinalizedSlot { .. }) => { + // Gloas: keep blocks at finalized slots so their envelopes can + // still be processed. This handles the checkpoint sync case where + // the checkpoint block is already finalized but its envelope hasn't + // been stored yet. + if block.as_block().fork_name_unchecked().gloas_enabled() { + filtered_chain_segment.push((block_root, block)); + } + } // The block has a known parent that does not descend from the finalized block. // There is no need to process this block or any children. Err(BlockError::NotFinalizedDescendant { block_parent_root }) => { @@ -2935,6 +2943,39 @@ impl BeaconChain { } }; + // Strip already-known blocks (e.g. the checkpoint sync anchor) from the + // front of the segment and process only their envelopes. These blocks + // can't go through signature_verify_chain_segment because their parents + // may not be available. + while let Some((root, _)) = filtered_chain_segment.first() { + if !self + .canonical_head + .fork_choice_read_lock() + .contains_block(root) + { + break; + } + let (block_root, block) = filtered_chain_segment.remove(0); + let maybe_envelope = match block { + RangeSyncBlock::Gloas { envelope, .. } => envelope, + _ => None, + }; + if let Some(envelope) = maybe_envelope + && let Err(error) = self + .process_range_sync_envelope( + block_root, + envelope, + notify_execution_layer, + ) + .await + { + return ChainSegmentResult::Failed { + imported_blocks, + error: BlockError::EnvelopeError(Box::new(error)), + }; + } + } + while let Some((_root, block)) = filtered_chain_segment.first() { // Determine the epoch of the first block in the remaining segment. let start_epoch = block.epoch(); @@ -2978,6 +3019,7 @@ impl BeaconChain { for (signature_verified_block, maybe_envelope) in signature_verified_blocks { let block_root = signature_verified_block.block_root(); let block_slot = signature_verified_block.slot(); + match self .process_block( block_root, @@ -3025,13 +3067,15 @@ impl BeaconChain { } } } - Err(BlockError::DuplicateFullyImported(block_root)) => { + Err(BlockError::DuplicateFullyImported(_)) + | Err(BlockError::WouldRevertFinalizedSlot { .. }) => { debug!( ?block_root, "Ignoring already known block while processing chain segment" ); // Gloas: still process the envelope for duplicate blocks. The envelope - // may not have been persisted before a restart. + // may not have been persisted before a restart, or the block may be the + // checkpoint sync anchor whose envelope was never stored. if let Some(envelope) = maybe_envelope && let Err(error) = self .process_range_sync_envelope( 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 3f98b6c28f..57d3d7d220 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -41,7 +41,6 @@ pub enum ChainSegmentProcessId { BackSyncBatchId(Epoch), } -/// Returned when a chain segment import fails. /// Returned when a chain segment import fails. struct ChainSegmentFailed { /// To be displayed in logs. @@ -50,13 +49,6 @@ struct ChainSegmentFailed { peer_action: Option, } -/// Result of processing a batch of blocks. -enum BlockBatchResult { - Ok { imported_blocks: usize }, - ParentEnvelopeUnknown { parent_root: Hash256 }, - Err { imported_blocks: usize, failed: Option }, -} - impl NetworkBeaconProcessor { /// Returns an async closure which processes a beacon block received via RPC. /// @@ -641,7 +633,7 @@ impl NetworkBeaconProcessor { .process_blocks(downloaded_blocks.iter(), notify_execution_layer) .await { - BlockBatchResult::Ok { imported_blocks } => { + (imported_blocks, Ok(_)) => { debug!( batch_epoch = %epoch, first_block_slot = start_slot, @@ -655,27 +647,17 @@ impl NetworkBeaconProcessor { imported_blocks, } } - BlockBatchResult::ParentEnvelopeUnknown { parent_root } => { - warn!( - batch_epoch = %epoch, - ?parent_root, - "Batch processing paused: parent envelope unknown" - ); - BatchProcessResult::ParentEnvelopeUnknown { parent_root } - } - BlockBatchResult::Err { imported_blocks, failed } => { - if let Some(e) = &failed { - debug!( - batch_epoch = %epoch, - first_block_slot = start_slot, - chain = chain_id, - last_block_slot = end_slot, - imported_blocks, - error = %e.message, - service = "sync", - "Batch processing failed"); - } - match failed.and_then(|e| e.peer_action) { + (imported_blocks, Err(e)) => { + debug!( + batch_epoch = %epoch, + first_block_slot = start_slot, + chain = chain_id, + last_block_slot = end_slot, + imported_blocks, + error = %e.message, + service = "sync", + "Batch processing failed"); + match e.peer_action { Some(penalty) => BatchProcessResult::FaultyFailure { imported_blocks, penalty, @@ -776,7 +758,7 @@ impl NetworkBeaconProcessor { &self, downloaded_blocks: impl Iterator>, notify_execution_layer: NotifyExecutionLayer, - ) -> BlockBatchResult { + ) -> (usize, Result<(), ChainSegmentFailed>) { let blocks: Vec<_> = downloaded_blocks.cloned().collect(); match self .chain @@ -788,25 +770,18 @@ impl NetworkBeaconProcessor { if !imported_blocks.is_empty() { self.chain.recompute_head_at_current_slot().await; } - BlockBatchResult::Ok { imported_blocks: imported_blocks.len() } + (imported_blocks.len(), Ok(())) } ChainSegmentResult::Failed { imported_blocks, error, } => { metrics::inc_counter(&metrics::BEACON_PROCESSOR_CHAIN_SEGMENT_FAILED_TOTAL); + let r = self.handle_failed_chain_segment(error); if !imported_blocks.is_empty() { self.chain.recompute_head_at_current_slot().await; } - // Intercept ParentEnvelopeUnknown before normal error handling. - if let BlockError::ParentEnvelopeUnknown { parent_root } = error { - return BlockBatchResult::ParentEnvelopeUnknown { parent_root }; - } - let r = self.handle_failed_chain_segment(error); - BlockBatchResult::Err { - imported_blocks: imported_blocks.len(), - failed: r.err(), - } + (imported_blocks.len(), r) } } } diff --git a/beacon_node/network/src/sync/backfill_sync/mod.rs b/beacon_node/network/src/sync/backfill_sync/mod.rs index 4fef78a47a..29beb96e5a 100644 --- a/beacon_node/network/src/sync/backfill_sync/mod.rs +++ b/beacon_node/network/src/sync/backfill_sync/mod.rs @@ -672,8 +672,7 @@ impl BackFillSync { } } } - BatchProcessResult::NonFaultyFailure - | BatchProcessResult::ParentEnvelopeUnknown { .. } => { + BatchProcessResult::NonFaultyFailure => { if let Err(e) = batch.processing_completed(BatchProcessingResult::NonFaultyFailure) { self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0))?; diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index da67dd5dc4..c1c1029446 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -233,11 +233,6 @@ pub enum BatchProcessResult { penalty: PeerAction, }, NonFaultyFailure, - /// The batch processing failed because the parent block's execution payload envelope - /// is not yet available. The chain should pause until the envelope is fetched. - ParentEnvelopeUnknown { - parent_root: Hash256, - }, } /// The result of processing multiple data columns. @@ -977,20 +972,9 @@ impl SyncManager { SyncMessage::BlockComponentProcessed { process_type, result, - } => { - // If a payload envelope was successfully imported, resume any range - // sync chains that were waiting for it. - if let BlockProcessType::SinglePayloadEnvelope { block_root, .. } = &process_type { - if matches!(&result, BlockProcessingResult::Ok(_)) { - self.range_sync.resume_chains_awaiting_envelope( - *block_root, - &mut self.network, - ); - } - } - self.block_lookups - .on_processing_result(process_type, result, &mut self.network) - } + } => self + .block_lookups + .on_processing_result(process_type, result, &mut self.network), SyncMessage::GossipBlockProcessResult { block_root, imported, @@ -1001,23 +985,6 @@ impl SyncManager { ), SyncMessage::BatchProcessed { sync_type, result } => match sync_type { ChainSegmentProcessId::RangeBatchId(chain_id, epoch) => { - // If the batch failed due to a missing parent envelope, trigger - // an envelope lookup before pausing the chain. - if let BatchProcessResult::ParentEnvelopeUnknown { parent_root } = &result { - let peers: Vec<_> = self - .network - .network_globals() - .peers - .read() - .synced_peers() - .cloned() - .collect(); - let _ = self.block_lookups.search_parent_envelope_of_child( - *parent_root, - &peers, - &mut self.network, - ); - } self.range_sync.handle_block_process_result( &mut self.network, chain_id, diff --git a/beacon_node/network/src/sync/range_sync/chain.rs b/beacon_node/network/src/sync/range_sync/chain.rs index bd2c7af385..d533d8ed0d 100644 --- a/beacon_node/network/src/sync/range_sync/chain.rs +++ b/beacon_node/network/src/sync/range_sync/chain.rs @@ -156,8 +156,6 @@ pub enum ChainSyncingState { Stopped, /// The chain is undergoing syncing. Syncing, - /// The chain is paused waiting for a parent envelope to be fetched. - AwaitingEnvelope { parent_root: Hash256 }, } impl SyncingChain { @@ -641,19 +639,6 @@ impl SyncingChain { // Simply re-download all batches in `AwaitingDownload` state. self.attempt_send_awaiting_download_batches(network, "non-faulty-failure") } - BatchProcessResult::ParentEnvelopeUnknown { parent_root } => { - batch.processing_completed(BatchProcessingResult::NonFaultyFailure)?; - - // Pause the chain until the missing parent envelope is fetched. - debug!( - ?parent_root, - "Chain paused: awaiting parent envelope" - ); - self.state = ChainSyncingState::AwaitingEnvelope { - parent_root: *parent_root, - }; - Ok(KeepChain) - } } } @@ -1190,7 +1175,7 @@ impl SyncingChain { pub fn is_syncing(&self) -> bool { match self.state { ChainSyncingState::Syncing => true, - ChainSyncingState::Stopped | ChainSyncingState::AwaitingEnvelope { .. } => false, + ChainSyncingState::Stopped => false, } } diff --git a/beacon_node/network/src/sync/range_sync/chain_collection.rs b/beacon_node/network/src/sync/range_sync/chain_collection.rs index 7a642f1e02..a087fdecdf 100644 --- a/beacon_node/network/src/sync/range_sync/chain_collection.rs +++ b/beacon_node/network/src/sync/range_sync/chain_collection.rs @@ -3,7 +3,7 @@ //! Each chain type is stored in it's own map. A variety of helper functions are given along with //! this struct to simplify the logic of the other layers of sync. -use super::chain::{ChainId, ChainSyncingState, ProcessingResult, RemoveChain, SyncingChain}; +use super::chain::{ChainId, ProcessingResult, RemoveChain, SyncingChain}; use super::sync_type::RangeSyncType; use crate::metrics; use crate::sync::batch::BatchMetricsState; @@ -562,30 +562,6 @@ impl ChainCollection { } } - /// Resume any chains that were paused waiting for the given parent envelope. - pub fn resume_chains_awaiting_envelope( - &mut self, - parent_root: Hash256, - network: &mut SyncNetworkContext, - ) { - for chain in self - .finalized_chains - .values_mut() - .chain(self.head_chains.values_mut()) - { - if chain.state - == (ChainSyncingState::AwaitingEnvelope { parent_root }) - { - debug!( - ?parent_root, - "Resuming chain after parent envelope received" - ); - chain.state = ChainSyncingState::Syncing; - let _ = chain.resume(network); - } - } - } - fn update_metrics(&self) { metrics::set_gauge_vec( &metrics::SYNCING_CHAINS_COUNT, diff --git a/beacon_node/network/src/sync/range_sync/range.rs b/beacon_node/network/src/sync/range_sync/range.rs index 80915df86a..6509ac3cb3 100644 --- a/beacon_node/network/src/sync/range_sync/range.rs +++ b/beacon_node/network/src/sync/range_sync/range.rs @@ -266,15 +266,6 @@ where } } - /// Resume any chains that were paused waiting for the given parent envelope. - pub fn resume_chains_awaiting_envelope( - &mut self, - parent_root: Hash256, - network: &mut SyncNetworkContext, - ) { - self.chains.resume_chains_awaiting_envelope(parent_root, network); - } - /// A peer has disconnected. This removes the peer from any ongoing chains and mappings. A /// disconnected peer could remove a chain pub fn peer_disconnect(&mut self, network: &mut SyncNetworkContext, peer_id: &PeerId) {