From d7d56e6312e3e4d2fe6e3988853dd08f21c6a888 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 2 Jun 2026 16:57:03 +0200 Subject: [PATCH] Delete unnecessary SyncMessage variants (#9379) - Simplification from https://github.com/sigp/lighthouse/pull/9155 Lookup sync does not cache sidecars, so sending the full network object adds unnecessary complexity. Sync only needs to know: We have received a header that has an unknown parent. Replace `UnknownParentDataColumn` and `UnknownParentPartialDataColumn` for `UnknownParentSidecarHeader` Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> Co-Authored-By: Eitan Seri-Levi --- .../gossip_methods.rs | 12 +++-- .../network/src/sync/block_lookups/mod.rs | 13 ++--- .../sync/block_lookups/single_block_lookup.rs | 2 +- beacon_node/network/src/sync/manager.rs | 53 +++---------------- beacon_node/network/src/sync/tests/lookups.rs | 13 ++++- 5 files changed, 31 insertions(+), 62 deletions(-) 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 65c95eff35..df94b473a8 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -719,17 +719,19 @@ impl NetworkBeaconProcessor { MessageAcceptance::Accept, ); } - GossipDataColumnError::ParentUnknown { parent_root, .. } => { + GossipDataColumnError::ParentUnknown { parent_root, slot } => { debug!( action = "requesting parent", %block_root, %parent_root, "Unknown parent hash for column" ); - self.send_sync_message(SyncMessage::UnknownParentDataColumn( + self.send_sync_message(SyncMessage::UnknownParentSidecarHeader { peer_id, - column_sidecar, - )); + block_root, + parent_root, + slot, + }); } GossipDataColumnError::BlockRootUnknown { block_root: unknown_block_root, @@ -1047,7 +1049,7 @@ impl NetworkBeaconProcessor { %parent_root, "Unknown parent hash for partial column" ); - self.send_sync_message(SyncMessage::UnknownParentPartialDataColumn { + self.send_sync_message(SyncMessage::UnknownParentSidecarHeader { peer_id, block_root, parent_root, diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index ecaee7c0ec..0c3cb988a9 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -74,26 +74,23 @@ const LOOKUP_MAX_DURATION_NO_PEERS_SECS: u64 = 10; /// take at most 2 GB. 200 lookups allow 3 parallel chains of depth 64 (current maximum). const MAX_LOOKUPS: usize = 200; -/// The values for `Blob`, `DataColumn` and `PartialDataColumn` is the parent root of the column. +/// The value for `Sidecar` is the parent root of the sidecar. pub enum BlockComponent { Block(DownloadResult>>), - DataColumn(DownloadResult), - PartialDataColumn(DownloadResult), + Sidecar { parent_root: Hash256 }, } impl BlockComponent { fn parent_root(&self) -> Hash256 { match self { BlockComponent::Block(block) => block.value.parent_root(), - BlockComponent::DataColumn(parent_root) - | BlockComponent::PartialDataColumn(parent_root) => parent_root.value, + BlockComponent::Sidecar { parent_root } => *parent_root, } } fn get_type(&self) -> &'static str { match self { BlockComponent::Block(_) => "block", - BlockComponent::DataColumn(_) => "data_column", - BlockComponent::PartialDataColumn(_) => "partial_data_column", + BlockComponent::Sidecar { .. } => "sidecar", } } } @@ -207,7 +204,7 @@ impl BlockLookups { block_root, Some(block_component), Some(parent_root), - // On a `UnknownParentBlock` or `UnknownParentDataColumn` event the peer is not + // On a `UnknownParentBlock` or `UnknownParentSidecarHeader` event the peer is not // required to have the rest of the block components. Create the lookup with zero // peers to house the block components. &[], 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 dda58023be..38d6b2216d 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 @@ -151,7 +151,7 @@ impl SingleBlockLookup { .block_request_state .state .insert_verified_response(block), - BlockComponent::DataColumn(_) | BlockComponent::PartialDataColumn(_) => { + BlockComponent::Sidecar { .. } => { // For now ignore single blobs and columns, as the blob request state assumes all blobs are // attributed to the same peer = the peer serving the remaining blobs. Ignoring this // block component has a minor effect, causing the node to re-request this blob diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index ecbe6227cc..37d13c2eae 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -144,11 +144,9 @@ pub enum SyncMessage { /// A block with an unknown parent has been received. UnknownParentBlock(PeerId, Arc>, Hash256), - /// A data column with an unknown parent has been received. - UnknownParentDataColumn(PeerId, Arc>), - - /// A partial data column with an unknown parent has been received. - UnknownParentPartialDataColumn { + /// A sidecar (full/partial data column) with an unknown parent has been received. Carries only the header + /// info needed to trigger a parent lookup, decoupled from the concrete sidecar type. + UnknownParentSidecarHeader { peer_id: PeerId, block_root: Hash256, parent_root: Hash256, @@ -875,58 +873,19 @@ impl SyncManager { }), ); } - SyncMessage::UnknownParentDataColumn(peer_id, data_column) => { - let data_column_slot = data_column.slot(); - let block_root = data_column.block_root(); - match data_column.as_ref() { - DataColumnSidecar::Fulu(column) => { - let parent_root = column.block_parent_root(); - debug!(%block_root, %parent_root, "Received unknown parent data column message"); - self.handle_unknown_parent( - peer_id, - block_root, - parent_root, - data_column_slot, - BlockComponent::DataColumn(DownloadResult { - value: parent_root, - block_root, - seen_timestamp: self - .chain - .slot_clock - .now_duration() - .unwrap_or_default(), - peer_group: PeerGroup::from_single(peer_id), - }), - ); - } - DataColumnSidecar::Gloas(_) => { - // TODO(gloas): proper lookup sync for Gloas. Routing into - // `handle_unknown_block_root` here mixes column processing with the - // single-block-lookup path; the Gloas column-arrives-before-block - // case wants its own queue/wakeup. - debug!(%block_root, "Received unknown block data column message"); - self.handle_unknown_block_root(peer_id, block_root); - } - } - } - SyncMessage::UnknownParentPartialDataColumn { + SyncMessage::UnknownParentSidecarHeader { peer_id, block_root, parent_root, slot, } => { - debug!(%block_root, %parent_root, "Received unknown parent partial column message"); + debug!(%block_root, %parent_root, "Received unknown parent sidecar header message"); self.handle_unknown_parent( peer_id, block_root, parent_root, slot, - BlockComponent::PartialDataColumn(DownloadResult { - value: parent_root, - block_root, - seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), - peer_group: PeerGroup::from_single(peer_id), - }), + BlockComponent::Sidecar { parent_root }, ); } SyncMessage::UnknownBlockHashFromAttestation(peer_id, block_root) => { diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 1a60e4f243..3ec4d11da2 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1365,7 +1365,18 @@ impl TestRig { peer_id: PeerId, data_column: Arc>, ) { - self.send_sync_message(SyncMessage::UnknownParentDataColumn(peer_id, data_column)); + let block_root = data_column.block_root(); + let slot = data_column.slot(); + let parent_root = match data_column.as_ref() { + DataColumnSidecar::Fulu(column) => column.block_parent_root(), + DataColumnSidecar::Gloas(_) => panic!("Gloas data column not supported in this test"), + }; + self.send_sync_message(SyncMessage::UnknownParentSidecarHeader { + peer_id, + block_root, + parent_root, + slot, + }); } fn trigger_unknown_block_from_attestation(&mut self, block_root: Hash256, peer_id: PeerId) {