From 0b3989c9e9ddc40f75cc9c11b3bc59afee1497c2 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:47:32 +0200 Subject: [PATCH 01/27] Consumer ancestor blocks in lookup sync --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 ++ .../beacon_chain/src/canonical_head.rs | 7 ++ .../network/src/sync/block_lookups/mod.rs | 74 ++++++++++++++++--- .../sync/block_lookups/single_block_lookup.rs | 67 ++++++++++++++++- beacon_node/network/src/sync/manager.rs | 8 +- .../network/src/sync/network_context.rs | 15 ++++ 6 files changed, 162 insertions(+), 15 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 569f0eea502..325ef0275ab 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -6294,6 +6294,12 @@ impl BeaconChain { Ok(()) } + pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { + self.canonical_head + .fork_choice_read_lock() + .is_parent_imported(block) + } + pub fn block_is_known_to_fork_choice(&self, root: &Hash256) -> bool { self.canonical_head .fork_choice_read_lock() diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 1eab7ccf7ac..6ee4b1bc2ec 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -210,6 +210,13 @@ impl CachedHead { self.finalized_checkpoint } + /// Returns the finalized slot from the fork choice finalized checkpoint + pub fn finalized_slot(&self) -> Slot { + self.finalized_checkpoint + .epoch + .start_slot(E::slots_per_epoch()) + } + /// Returns the justified checkpoint, as determined by fork choice. /// /// ## Note diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index d403382e9e9..d587d065e8b 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -24,7 +24,7 @@ use self::parent_chain::{NodeChain, compute_parent_chains}; pub use self::single_block_lookup::DownloadResult; use self::single_block_lookup::{LookupRequestError, PeerType, SingleBlockLookup}; use super::manager::{BlockProcessType, SLOT_IMPORT_TOLERANCE}; -use super::network_context::{RpcResponseError, SyncNetworkContext}; +use super::network_context::{AncestorBlocks, RpcResponseError, SyncNetworkContext}; use crate::metrics; use crate::network_beacon_processor::BlockProcessingResult; use crate::sync::SyncMessage; @@ -37,6 +37,7 @@ use fnv::FnvHashMap; use lighthouse_network::PeerId; use lighthouse_network::service::api_types::SingleLookupReqId; use lru_cache::LRUTimeCache; +use std::collections::HashMap; use std::collections::hash_map::Entry; use std::sync::Arc; use std::time::Duration; @@ -77,6 +78,7 @@ const LOOKUP_MAX_DURATION_NO_PEERS_SECS: u64 = 10; const MAX_LOOKUPS: usize = 200; type BlockDownloadResponse = Result>>, RpcResponseError>; +type BlocksDownloadResponse = Result>, RpcResponseError>; type CustodyDownloadResponse = Result>, RpcResponseError>; type PayloadDownloadResponse = @@ -87,6 +89,8 @@ pub enum BlockComponent { Sidecar, } +type KnownParents = HashMap>>>; + pub type SingleLookupId = u32; pub struct BlockLookups { @@ -183,6 +187,7 @@ impl BlockLookups { ) -> bool { let parent_lookup_exists = self.search_parent_of_child( parent_root, + None, &PeerType::new(parent_block_hash), block_root, &[peer_id], @@ -230,6 +235,7 @@ impl BlockLookups { pub fn search_parent_of_child( &mut self, block_root_to_search: Hash256, + block_component: Option>, peer_type: &PeerType, child_block_root_trigger: Hash256, peers: &[PeerId], @@ -323,7 +329,14 @@ impl BlockLookups { } // `block_root_to_search` is a failed chain check happens inside new_current_lookup - self.new_current_lookup(block_root_to_search, None, None, peers, peer_type, cx) + self.new_current_lookup( + block_root_to_search, + block_component, + None, + peers, + peer_type, + cx, + ) } /// Searches for a single block hash. If the blocks parent is unknown, a chain of blocks is @@ -416,7 +429,7 @@ impl BlockLookups { self.metrics.created_lookups += 1; let result = lookup.continue_requests(cx); - if self.on_lookup_result(id, result, "new_current_lookup", cx) { + if self.on_lookup_result(id, result, None, "new_current_lookup", cx) { self.update_metrics(); true } else { @@ -430,15 +443,49 @@ impl BlockLookups { pub fn on_block_download_response( &mut self, id: SingleLookupReqId, - response: BlockDownloadResponse, + response: BlocksDownloadResponse, cx: &mut SyncNetworkContext, ) { let Some(lookup) = self.single_block_lookups.get_mut(&id.lookup_id) else { debug!(?id, "Block returned for single block lookup not present"); return; }; - let result = lookup.on_block_download_response(id.req_id, response, cx); - self.on_lookup_result(id.lookup_id, result, "block_download_response", cx); + // If response is successful, `on_block_download_response` will advance the block request + // state to `AwaitingProcess` then `continue_requests` will check if the block can be + // processed: + // - If yes: sends block for processing + // - If not: marks the lookup as awaiting parent + let known_ancestors = response.as_ref().ok().map(|d| { + let mut known_ancestors = HashMap::new(); + for block in &d.value.ancestor_blocks { + known_ancestors.insert( + block.canonical_root(), + DownloadResult { + value: block.clone(), + seen_timestamp: d.seen_timestamp, + peer_group: d.peer_group.clone(), + }, + ); + } + known_ancestors + }); + + let result = lookup.on_block_download_response( + id.req_id, + response.map(|d| DownloadResult { + value: d.value.first_block, + seen_timestamp: d.seen_timestamp, + peer_group: d.peer_group, + }), + cx, + ); + self.on_lookup_result( + id.lookup_id, + result, + known_ancestors, + "block_download_response", + cx, + ); } pub fn on_custody_download_response( @@ -452,7 +499,7 @@ impl BlockLookups { return; }; let result = lookup.on_custody_download_response(id.req_id, response, cx); - self.on_lookup_result(id.lookup_id, result, "custody_download_response", cx); + self.on_lookup_result(id.lookup_id, result, None, "custody_download_response", cx); } pub fn on_payload_download_response( @@ -469,7 +516,7 @@ impl BlockLookups { return; }; let result = lookup.on_payload_download_response(id.req_id, response, cx); - self.on_lookup_result(id.lookup_id, result, "payload_download_response", cx); + self.on_lookup_result(id.lookup_id, result, None, "payload_download_response", cx); } /* Error responses */ @@ -539,7 +586,7 @@ impl BlockLookups { lookup.on_payload_processing_result(result, cx) } }; - self.on_lookup_result(lookup_id, lookup_result, "processing_result", cx); + self.on_lookup_result(lookup_id, lookup_result, None, "processing_result", cx); } pub fn has_any_awaiting_children(&self, block_root: Hash256) -> bool { @@ -572,7 +619,7 @@ impl BlockLookups { } for (id, result) in lookup_results { - self.on_lookup_result(id, result, "continue_child_lookups", cx); + self.on_lookup_result(id, result, None, "continue_child_lookups", cx); } } @@ -610,6 +657,7 @@ impl BlockLookups { &mut self, id: SingleLookupId, result: Result, + known_blocks: Option>, source: &str, cx: &mut SyncNetworkContext, ) -> bool { @@ -623,6 +671,10 @@ impl BlockLookups { }) => { if self.search_parent_of_child( parent_root, + known_blocks.and_then(|p| { + p.get(&parent_root) + .map(|b| BlockComponent::Block(b.clone())) + }), &PeerType::new(parent_block_hash), block_root, &peers, @@ -851,7 +903,7 @@ impl BlockLookups { // pruned with `drop_lookups_without_peers` because it has peers. This is rare corner // case, but it can result in stuck lookups. let result = lookup.continue_requests(cx); - self.on_lookup_result(lookup_id, result, "add_peers", cx); + self.on_lookup_result(lookup_id, result, None, "add_peers", cx); Ok(()) } else { Ok(()) 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 f03eed16384..6935ab8df63 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 @@ -44,6 +44,8 @@ pub enum LookupResult { pub enum LookupRequestError { /// Too many failed attempts TooManyAttempts, + /// This block is not descendant of finalized checkpoint + ConflictsWithFinality(Slot, Slot), /// Error sending event to network SendFailedNetwork(RpcRequestSendError), /// Error sending event to processor @@ -79,6 +81,13 @@ impl AwaitingParent { self.parent_root } + pub fn from_block(block: &SignedBeaconBlock) -> Self { + Self::new( + block.parent_root(), + block.payload_bid_parent_block_hash().ok(), + ) + } + pub fn into_peer_type(self) -> PeerType { PeerType::new(self.parent_block_hash) } @@ -379,10 +388,39 @@ impl SingleBlockLookup { cx.block_lookup_request(self.id, self.peers.clone(), self.block_root) })?; if self.awaiting_parent.is_none() - && let Some(data) = self.block_request.state.maybe_start_processing() + && let Some(data) = self.block_request.state.is_awaiting_process() { - cx.send_block_for_processing(self.id, self.block_root, data.value, data.seen_timestamp) + let DownloadResult { + value: block, + seen_timestamp, + .. + } = data; + let finalized_slot = cx.chain.head().finalized_slot(); + // Eagerly check if we can import without having to send the block for processing. This + // allows us to check many lookups in the same sync execution / loop. + if cx.chain.is_parent_imported(block) { + cx.send_block_for_processing( + self.id, + self.block_root, + block.clone(), + *seen_timestamp, + ) .map_err(LookupRequestError::SendFailedProcessor)?; + self.block_request.state.start_processing()?; + } else if block.slot() <= finalized_slot { + return Err(LookupRequestError::ConflictsWithFinality( + block.slot(), + finalized_slot, + )); + } else { + self.awaiting_parent = Some(AwaitingParent::from_block(block)); + return Ok(LookupResult::ParentUnknown { + parent_root: block.parent_root(), + parent_block_hash: block.payload_bid_parent_block_hash().ok(), + block_root: self.block_root, + peers: self.all_peers(), + }); + } } // === Data request === @@ -804,6 +842,16 @@ impl SingleLookupRequestState { } } + fn is_awaiting_process(&self) -> Option<&DownloadResult> { + match &self.state { + State::AwaitingDownload { .. } => None, + State::Downloading { .. } => None, + State::AwaitingProcess(result) => Some(result), + State::Processing(_) => None, + State::Processed(..) => None, + } + } + /// Drive download: check max attempts, issue request, handle result. fn maybe_start_downloading( &mut self, @@ -926,6 +974,21 @@ impl SingleLookupRequestState { } } + /// Switch to `Processing` if the request is in `AwaitingProcess` state, otherwise returns None. + pub fn start_processing(&mut self) -> Result<(), LookupRequestError> { + // For 2 lines replace state with placeholder to gain ownership of `result` + match &self.state { + State::AwaitingProcess(result) => { + let result = result.clone(); + self.state = State::Processing(result.clone()); + Ok(()) + } + other => Err(LookupRequestError::BadState(format!( + "Bad state start_processing expected AwaitingProcess got {other}" + ))), + } + } + /// Revert into `AwaitingProcessing`, if the payload if not invalid and can be submitted for /// processing latter. pub fn revert_to_awaiting_processing(&mut self) -> Result<(), LookupRequestError> { diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 8e7b8cd05ac..e12b3c07fdd 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -47,7 +47,7 @@ use crate::service::NetworkMessage; use crate::status::ToStatusMessage; use crate::sync::block_lookups::{BlockComponent, DownloadResult}; use crate::sync::custody_backfill_sync::CustodyBackFillSync; -use crate::sync::network_context::{PeerGroup, RpcResponseResult}; +use crate::sync::network_context::{AncestorBlocks, PeerGroup, RpcResponseResult}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::{BeaconChain, BeaconChainTypes, EngineState}; use futures::StreamExt; @@ -1123,7 +1123,11 @@ impl SyncManager { self.block_lookups.on_block_download_response( id, resp.map(|(value, seen_timestamp)| { - DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) + DownloadResult::new( + AncestorBlocks::from_single(value), + PeerGroup::from_single(peer_id), + seen_timestamp, + ) }), &mut self.network, ) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 8e8abd4fa63..7bb570b6f29 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -100,6 +100,21 @@ pub type RpcResponseResult = Result<(T, Duration), RpcResponseError>; pub type CustodyByRootResult = Result>, RpcResponseError>; +#[derive(Clone)] +pub struct AncestorBlocks { + pub first_block: Arc>, + pub ancestor_blocks: Vec>>, +} + +impl AncestorBlocks { + pub fn from_single(block: Arc>) -> Self { + Self { + first_block: block, + ancestor_blocks: vec![], + } + } +} + #[derive(Debug)] #[allow(private_interfaces)] pub enum RpcResponseError { From ebe6bab4068316561648da628bc91aa5d553e856 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 13:39:55 +0200 Subject: [PATCH 02/27] Implement beacon_blocks_by_head requester for block lookups Block lookups now prefer peers that advertise beacon_blocks_by_head and fetch the block (plus a run of its ancestors) via that protocol, falling back to beacon_blocks_by_root otherwise. on_block_download_response now receives a Vec of blocks; the ancestors beyond the requested root are not yet consumed (header-backfill step). --- .../src/peer_manager/mod.rs | 15 ++ .../src/peer_manager/peerdb/peer_info.rs | 19 ++ .../src/service/api_types.rs | 2 + beacon_node/network/src/router.rs | 30 ++- beacon_node/network/src/sync/manager.rs | 51 +++- .../network/src/sync/network_context.rs | 226 ++++++++++++------ .../src/sync/network_context/requests.rs | 2 + .../requests/blocks_by_head.rs | 61 +++++ 8 files changed, 329 insertions(+), 77 deletions(-) create mode 100644 beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 898b97a85f7..e2cec3d6648 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -32,6 +32,7 @@ use peerdb::score::{PeerAction, ReportSource}; pub use peerdb::sync_status::{SyncInfo, SyncStatus}; use std::collections::{HashMap, HashSet, hash_map::Entry}; use std::net::IpAddr; +use std::str::FromStr; use strum::IntoEnumIterator; use types::data::{CustodyIndex, compute_subnets_from_custody_group, get_custody_groups}; @@ -487,6 +488,20 @@ impl PeerManager { let previous_listening_addresses = peer_info.set_listening_addresses(info.listen_addrs.clone()); peer_info.set_client(peerdb::client::Client::from_identify_info(info)); + peer_info.set_supported_protocols( + info.protocols + .iter() + .filter_map(|protocol| { + // ReqResp protocol ids have the form + // `/eth2/beacon_chain/req///`. + let protocol_str: &str = protocol.as_ref(); + protocol_str + .strip_prefix("/eth2/beacon_chain/req/") + .and_then(|rest| rest.split('/').next()) + .and_then(|name| Protocol::from_str(name).ok()) + }) + .collect(), + ); if previous_kind != peer_info.client().kind || *peer_info.listening_addresses() != previous_listening_addresses diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs index 8ad7d10a882..c04ed802e9b 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs @@ -2,6 +2,7 @@ use super::client::Client; use super::score::{PeerAction, Score, ScoreState}; use super::sync_status::SyncStatus; use crate::discovery::Eth2Enr; +use crate::rpc::Protocol as RpcProtocol; use crate::{rpc::MetaData, types::Subnet}; use PeerConnectionStatus::*; use discv5::Enr; @@ -59,6 +60,9 @@ pub struct PeerInfo { connection_direction: Option, /// The enr of the peer, if known. enr: Option, + /// The set of ReqResp protocols the peer advertised via `identify`. Empty until identified. + #[serde(skip)] + supported_protocols: HashSet, } impl Default for PeerInfo { @@ -78,6 +82,7 @@ impl Default for PeerInfo { is_trusted: false, connection_direction: None, enr: None, + supported_protocols: HashSet::new(), } } } @@ -172,6 +177,11 @@ impl PeerInfo { self.enr.as_ref() } + /// Returns true if the peer advertised support for `protocol` via `identify`. + pub fn supports_protocol(&self, protocol: RpcProtocol) -> bool { + self.supported_protocols.contains(&protocol) + } + /// An iterator over all the subnets this peer is subscribed to. pub fn subnets(&self) -> impl Iterator { self.subnets.iter() @@ -392,6 +402,15 @@ impl PeerInfo { self.client = client } + /// Replaces the set of ReqResp protocols the peer advertised via `identify`. + // VISIBILITY: The peer manager is able to set the supported protocols + pub(in crate::peer_manager) fn set_supported_protocols( + &mut self, + supported_protocols: HashSet, + ) { + self.supported_protocols = supported_protocols; + } + /// Replaces the current listening addresses with those specified, returning the current /// listening addresses. // VISIBILITY: The peer manager is able to set the listening addresses diff --git a/beacon_node/lighthouse_network/src/service/api_types.rs b/beacon_node/lighthouse_network/src/service/api_types.rs index 4a8c6c55eb8..de55a696a9e 100644 --- a/beacon_node/lighthouse_network/src/service/api_types.rs +++ b/beacon_node/lighthouse_network/src/service/api_types.rs @@ -21,6 +21,8 @@ pub struct SingleLookupReqId { pub enum SyncRequestId { /// Request searching for a block given a hash. SingleBlock { id: SingleLookupReqId }, + /// Request walking the ancestors of a block given its root, via `beacon_blocks_by_head`. + BlocksByHead { id: SingleLookupReqId }, /// Request searching for a payload envelope given a hash. SinglePayloadEnvelope { id: SingleLookupReqId }, /// Request searching for a set of data columns given a hash and list of column indices. diff --git a/beacon_node/network/src/router.rs b/beacon_node/network/src/router.rs index 315ec9387dc..92b68b320d1 100644 --- a/beacon_node/network/src/router.rs +++ b/beacon_node/network/src/router.rs @@ -355,10 +355,8 @@ impl Router { Response::PayloadEnvelopesByRange(envelope) => { self.on_payload_envelopes_by_range_response(peer_id, app_request_id, envelope); } - // Lighthouse currently only serves BlocksByHead and does not issue it as a client, - // so receiving a response is unexpected. Drop it without crashing. - Response::BlocksByHead(_) => { - debug!("BlocksByHead response received but not requested by lighthouse"); + Response::BlocksByHead(beacon_block) => { + self.on_blocks_by_head_response(peer_id, app_request_id, beacon_block); } // Light client responses should not be received Response::LightClientBootstrap(_) @@ -720,6 +718,30 @@ impl Router { }); } + /// Handle a `BlocksByHead` response from the peer. + /// A `beacon_block` behaves as a stream which is terminated on a `None` response. + pub fn on_blocks_by_head_response( + &mut self, + peer_id: PeerId, + app_request_id: AppRequestId, + beacon_block: Option>>, + ) { + let sync_request_id = match app_request_id { + AppRequestId::Sync(id @ SyncRequestId::BlocksByHead { .. }) => id, + other => { + crit!(request = ?other, "BlocksByHead response on incorrect request"); + return; + } + }; + + self.send_to_sync(SyncMessage::RpcBlock { + peer_id, + sync_request_id, + beacon_block, + seen_timestamp: self.chain.slot_clock.now_duration().unwrap_or_default(), + }); + } + /// Handle a `DataColumnsByRoot` response from the peer. pub fn on_data_columns_by_root_response( &mut self, diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index e12b3c07fdd..cdad4e0d0ad 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -47,7 +47,9 @@ use crate::service::NetworkMessage; use crate::status::ToStatusMessage; use crate::sync::block_lookups::{BlockComponent, DownloadResult}; use crate::sync::custody_backfill_sync::CustodyBackFillSync; -use crate::sync::network_context::{AncestorBlocks, PeerGroup, RpcResponseResult}; +use crate::sync::network_context::{ + AncestorBlocks, LookupVerifyError, PeerGroup, RpcResponseError, RpcResponseResult, +}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::{BeaconChain, BeaconChainTypes, EngineState}; use futures::StreamExt; @@ -490,6 +492,9 @@ impl SyncManager { SyncRequestId::SingleBlock { id } => { self.on_single_block_response(id, peer_id, RpcEvent::RPCError(error)) } + SyncRequestId::BlocksByHead { id } => { + self.on_blocks_by_head_response(id, peer_id, RpcEvent::RPCError(error)) + } SyncRequestId::SinglePayloadEnvelope { id } => { self.on_single_payload_envelope_response(id, peer_id, RpcEvent::RPCError(error)) } @@ -1102,6 +1107,11 @@ impl SyncManager { peer_id, RpcEvent::from_chunk(block, seen_timestamp), ), + SyncRequestId::BlocksByHead { id } => self.on_blocks_by_head_response( + id, + peer_id, + RpcEvent::from_chunk(block, seen_timestamp), + ), SyncRequestId::BlocksByRange(id) => self.on_blocks_by_range_response( id, peer_id, @@ -1122,12 +1132,43 @@ 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( - AncestorBlocks::from_single(value), + resp.and_then(|(blocks, seen_timestamp)| { + let value = AncestorBlocks::from_vec(blocks).ok_or_else(|| { + RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { + actual: 0, + }) + })?; + Ok(DownloadResult::new( + value, + PeerGroup::from_single(peer_id), + seen_timestamp, + )) + }), + &mut self.network, + ) + } + } + + fn on_blocks_by_head_response( + &mut self, + id: SingleLookupReqId, + peer_id: PeerId, + block: RpcEvent>>, + ) { + if let Some(resp) = self.network.on_blocks_by_head_response(id, peer_id, block) { + self.block_lookups.on_block_download_response( + id, + resp.and_then(|(blocks, seen_timestamp)| { + let value = AncestorBlocks::from_vec(blocks).ok_or_else(|| { + RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { + actual: 0, + }) + })?; + Ok(DownloadResult::new( + value, PeerGroup::from_single(peer_id), seen_timestamp, - ) + )) }), &mut self.network, ) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 7bb570b6f29..7ad62c38e63 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -25,9 +25,12 @@ use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProcessStatus, EngineStat use custody::CustodyRequestResult; use fnv::FnvHashMap; use lighthouse_network::rpc::methods::{ - BlobsByRangeRequest, DataColumnsByRangeRequest, PayloadEnvelopesByRangeRequest, + BlobsByRangeRequest, BlocksByHeadRequest, DataColumnsByRangeRequest, + PayloadEnvelopesByRangeRequest, +}; +use lighthouse_network::rpc::{ + BlocksByRangeRequest, GoodbyeReason, Protocol, RPCError, RequestType, }; -use lighthouse_network::rpc::{BlocksByRangeRequest, GoodbyeReason, RPCError, RequestType}; pub use lighthouse_network::service::api_types::RangeRequestId; use lighthouse_network::service::api_types::{ AppRequestId, BlobsByRangeRequestId, BlocksByRangeRequestId, ComponentsByRangeRequestId, @@ -40,8 +43,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, + ActiveRequests, BlobsByRangeRequestItems, BlocksByHeadRequestItems, BlocksByRangeRequestItems, + BlocksByRootRequestItems, DataColumnsByRangeRequestItems, DataColumnsByRootRequestItems, PayloadEnvelopesByRangeRequestItems, PayloadEnvelopesByRootRequestItems, }; #[cfg(test)] @@ -78,6 +81,14 @@ macro_rules! new_range_request_span { /// Max retries for block components after which we fail the batch. pub const MAX_COLUMN_RETRIES: usize = 3; +/// Number of ancestors a block lookup requests when fetching a block via `beacon_blocks_by_head`. +/// The responder walks the block's parent chain and returns up to this many blocks (capped at +/// `MAX_REQUEST_BLOCKS_DENEB`), letting a single request fetch a run of ancestors instead of one +/// block per round-trip. +// TODO(tree-sync): the ancestors beyond the requested root are currently discarded by block +// lookups; they will be consumed to seed parent lookups in the header-backfill step. +pub const BLOCKS_BY_HEAD_REQUEST_COUNT: u64 = 32; + #[derive(Debug)] pub enum RpcEvent { StreamTermination, @@ -107,11 +118,17 @@ pub struct AncestorBlocks { } impl AncestorBlocks { - pub fn from_single(block: Arc>) -> Self { - Self { - first_block: block, - ancestor_blocks: vec![], + /// Splits a non-empty run of blocks (the requested root first, then its ancestors in descending + /// slot order) into the requested block and its ancestors. Returns `None` for an empty run. + pub fn from_vec(mut blocks: Vec>>) -> Option { + if blocks.is_empty() { + return None; } + let first_block = blocks.remove(0); + Some(Self { + first_block, + ancestor_blocks: blocks, + }) } } @@ -220,6 +237,9 @@ pub struct SyncNetworkContext { /// A mapping of active BlocksByRoot requests, including both current slot and parent lookups. blocks_by_root_requests: ActiveRequests>, + /// A mapping of active BlocksByHead requests, used by block lookups to walk a block's ancestors. + blocks_by_head_requests: + ActiveRequests>, /// A mapping of active PayloadEnvelopesByRoot requests payload_envelopes_by_root_requests: ActiveRequests>, @@ -324,6 +344,7 @@ impl SyncNetworkContext { execution_engine_state: EngineState::Online, // always assume `Online` at the start request_id: 1, blocks_by_root_requests: ActiveRequests::new("blocks_by_root"), + blocks_by_head_requests: ActiveRequests::new("blocks_by_head"), payload_envelopes_by_root_requests: ActiveRequests::new("payload_envelopes_by_root"), data_columns_by_root_requests: ActiveRequests::new("data_columns_by_root"), blocks_by_range_requests: ActiveRequests::new("blocks_by_range"), @@ -357,6 +378,7 @@ impl SyncNetworkContext { network_send: _, request_id: _, blocks_by_root_requests, + blocks_by_head_requests, payload_envelopes_by_root_requests, data_columns_by_root_requests, blocks_by_range_requests, @@ -378,6 +400,10 @@ impl SyncNetworkContext { .active_requests_of_peer(peer_id) .into_iter() .map(|id| SyncRequestId::SingleBlock { id: *id }); + let blocks_by_head_ids = blocks_by_head_requests + .active_requests_of_peer(peer_id) + .into_iter() + .map(|id| SyncRequestId::BlocksByHead { id: *id }); let payload_envelopes_by_root_ids = payload_envelopes_by_root_requests .active_requests_of_peer(peer_id) .into_iter() @@ -403,6 +429,7 @@ impl SyncNetworkContext { .into_iter() .map(|req_id| SyncRequestId::PayloadEnvelopesByRange(*req_id)); blocks_by_root_ids + .chain(blocks_by_head_ids) .chain(payload_envelopes_by_root_ids) .chain(data_column_by_root_ids) .chain(blocks_by_range_ids) @@ -421,6 +448,16 @@ impl SyncNetworkContext { &self.network_beacon_processor.network_globals } + /// Returns true if the peer advertised support for the `beacon_blocks_by_head` protocol. + pub fn peer_supports_blocks_by_head(&self, peer_id: &PeerId) -> bool { + self.network_globals() + .peers + .read() + .peer_info(peer_id) + .map(|info| info.supports_protocol(Protocol::BlocksByHead)) + .unwrap_or(false) + } + /// Returns the Client type of the peer if known pub fn client_type(&self, peer_id: &PeerId) -> Client { self.network_globals() @@ -460,6 +497,7 @@ impl SyncNetworkContext { network_send: _, request_id: _, blocks_by_root_requests, + blocks_by_head_requests, payload_envelopes_by_root_requests, data_columns_by_root_requests, blocks_by_range_requests, @@ -483,6 +521,7 @@ impl SyncNetworkContext { for peer_id in blocks_by_root_requests .iter_request_peers() + .chain(blocks_by_head_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()) @@ -897,20 +936,26 @@ impl SyncNetworkContext { block_root: Hash256, ) -> Result>>, RpcRequestSendError> { let active_request_count_by_peer = self.active_request_count_by_peer(); - let Some(peer_id) = lookup_peers + let Some((peer_id, supports_blocks_by_head)) = lookup_peers .read() .iter() .map(|peer| { + let supports_blocks_by_head = self.peer_supports_blocks_by_head(peer); ( - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), - // Random factor to break ties, otherwise the PeerID breaks ties - rand::random::(), - peer, + ( + // Prefer peers that support `beacon_blocks_by_head` + !supports_blocks_by_head, + // Prefer peers with less overall requests + active_request_count_by_peer.get(peer).copied().unwrap_or(0), + // Random factor to break ties, otherwise the PeerID breaks ties + rand::random::(), + ), + *peer, + supports_blocks_by_head, ) }) - .min() - .map(|(_, _, peer)| *peer) + .min_by_key(|(key, _, _)| *key) + .map(|(_, peer, supports_blocks_by_head)| (peer, supports_blocks_by_head)) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive @@ -948,49 +993,88 @@ impl SyncNetworkContext { req_id: self.next_id(), }; - let request = BlocksByRootSingleRequest(block_root); - // Lookup sync event safety: If network_send.send() returns Ok(_) we are guaranteed that - // eventually at least one this 3 events will be received: - // - StreamTermination(request_id): handled by `Self::on_single_block_response` - // - RPCError(request_id): handled by `Self::on_single_block_response` - // - Disconnect(peer_id) handled by `Self::peer_disconnected``which converts it to a - // ` RPCError(request_id)`event handled by the above method - let network_request = RequestType::BlocksByRoot( - request - .into_request(&self.fork_context) - .map_err(RpcRequestSendError::InternalError)?, - ); - self.network_send - .send(NetworkMessage::SendRequest { + // eventually at least one of these 3 events will be received: + // - StreamTermination(request_id): handled by `Self::on_{single_block,blocks_by_head}_response` + // - RPCError(request_id): handled by `Self::on_{single_block,blocks_by_head}_response` + // - Disconnect(peer_id) handled by `Self::peer_disconnected` which converts it to a + // `RPCError(request_id)` event handled by the above method + // + // If the peer supports `beacon_blocks_by_head`, fetch the block and a run of its ancestors + // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. + if supports_blocks_by_head { + let request = BlocksByHeadRequest { + beacon_root: block_root, + count: BLOCKS_BY_HEAD_REQUEST_COUNT, + }; + self.network_send + .send(NetworkMessage::SendRequest { + peer_id, + request: RequestType::BlocksByHead(request), + app_request_id: AppRequestId::Sync(SyncRequestId::BlocksByHead { id }), + }) + .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; + + debug!( + method = "BlocksByHead", + ?block_root, + peer = %peer_id, + %id, + "Sync RPC request sent" + ); + + let request_span = debug_span!( + parent: Span::current(), + "lh_outgoing_block_by_head_request", + %block_root, + ); + self.blocks_by_head_requests.insert( + id, peer_id, - request: network_request, - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), - }) - .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; + // false = the peer may return fewer blocks than requested (e.g. reached genesis or + // finalization), so the request completes on stream termination. + false, + BlocksByHeadRequestItems::new(block_root, BLOCKS_BY_HEAD_REQUEST_COUNT as usize), + request_span, + ); + } else { + let request = BlocksByRootSingleRequest(block_root); + let network_request = RequestType::BlocksByRoot( + request + .into_request(&self.fork_context) + .map_err(RpcRequestSendError::InternalError)?, + ); + self.network_send + .send(NetworkMessage::SendRequest { + peer_id, + request: network_request, + app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), + }) + .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; - debug!( - method = "BlocksByRoot", - ?block_root, - peer = %peer_id, - %id, - "Sync RPC request sent" - ); + debug!( + method = "BlocksByRoot", + ?block_root, + peer = %peer_id, + %id, + "Sync RPC request sent" + ); - let request_span = debug_span!( - parent: Span::current(), - "lh_outgoing_block_by_root_request", - %block_root, - ); - self.blocks_by_root_requests.insert( - id, - peer_id, - // true = enforce max_requests as returned for blocks_by_root. We always request a single - // block and the peer must have it. - true, - BlocksByRootRequestItems::new(request), - request_span, - ); + let request_span = debug_span!( + parent: Span::current(), + "lh_outgoing_block_by_root_request", + %block_root, + ); + self.blocks_by_root_requests.insert( + id, + peer_id, + // true = enforce max_requests as returned for blocks_by_root. We always request a + // single block and the peer must have it. + true, + BlocksByRootRequestItems::new(request), + request_span, + ); + } Ok(LookupRequestResult::RequestSent(id.req_id)) } @@ -1504,25 +1588,30 @@ impl SyncNetworkContext { // Request handlers + #[allow(clippy::type_complexity)] pub(crate) fn on_single_block_response( &mut self, id: SingleLookupReqId, peer_id: PeerId, rpc_event: RpcEvent>>, - ) -> Option>>> { + ) -> Option>>>> { + // `blocks_by_root_requests` enforces that exactly one chunk = one block is returned, so the + // returned vec always has length 1. Block lookups receive the response as a vec to share the + // handling with `beacon_blocks_by_head`, which may return a run of ancestors. let resp = self.blocks_by_root_requests.on_response(id, rpc_event); - let resp = resp.map(|res| { - res.and_then(|(mut blocks, seen_timestamp)| { - // 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)), - // Should never happen, `blocks_by_root_requests` enforces that we receive at least - // 1 chunk. - None => Err(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }.into()), - } - }) - }); + self.on_rpc_response_result(resp, peer_id) + } + + #[allow(clippy::type_complexity)] + pub(crate) fn on_blocks_by_head_response( + &mut self, + id: SingleLookupReqId, + peer_id: PeerId, + rpc_event: RpcEvent>>, + ) -> Option>>>> { + // The returned vec has the requested `beacon_root` first (the chain tip) followed by its + // ancestors in descending slot order. + let resp = self.blocks_by_head_requests.on_response(id, rpc_event); self.on_rpc_response_result(resp, peer_id) } @@ -1871,6 +1960,7 @@ impl SyncNetworkContext { pub(crate) fn register_metrics(&self) { for (id, count) in [ ("blocks_by_root", self.blocks_by_root_requests.len()), + ("blocks_by_head", self.blocks_by_head_requests.len()), ( "data_columns_by_root", self.data_columns_by_root_requests.len(), diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index cc747850987..ef8d7913b34 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -9,6 +9,7 @@ use tracing::{Span, debug}; use types::{Hash256, Slot}; pub use blobs_by_range::BlobsByRangeRequestItems; +pub use blocks_by_head::BlocksByHeadRequestItems; pub use blocks_by_range::BlocksByRangeRequestItems; pub use blocks_by_root::{BlocksByRootRequestItems, BlocksByRootSingleRequest}; pub use data_columns_by_range::DataColumnsByRangeRequestItems; @@ -25,6 +26,7 @@ use crate::metrics; use super::{RpcEvent, RpcResponseError, RpcResponseResult}; mod blobs_by_range; +mod blocks_by_head; mod blocks_by_range; mod blocks_by_root; mod data_columns_by_range; diff --git a/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs b/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs new file mode 100644 index 00000000000..5d989bd1565 --- /dev/null +++ b/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs @@ -0,0 +1,61 @@ +use super::{ActiveRequestItems, LookupVerifyError}; +use beacon_chain::get_block_root; +use std::sync::Arc; +use types::{EthSpec, Hash256, SignedBeaconBlock}; + +/// Accumulates the response of a `blocks_by_head` request. The responder walks the parent chain of +/// `beacon_root` (inclusive) and emits up to `count` blocks in descending slot order, so the first +/// chunk MUST be `beacon_root` and the rest are its ancestors. +pub struct BlocksByHeadRequestItems { + beacon_root: Hash256, + count: usize, + items: Vec>>, +} + +impl BlocksByHeadRequestItems { + pub fn new(beacon_root: Hash256, count: usize) -> Self { + Self { + beacon_root, + count, + items: vec![], + } + } +} + +impl ActiveRequestItems for BlocksByHeadRequestItems { + type Item = Arc>; + + /// Append a response chunk. The blocks must form a parent chain in strictly descending slot + /// order, with the chain tip (first chunk) equal to the requested `beacon_root`. + /// The active request SHOULD be dropped after `add` returns an error. + fn add(&mut self, block: Self::Item) -> Result { + let block_root = get_block_root(&block); + match self.items.last() { + // The first block returned is the chain tip and must match the requested root. + None => { + if self.beacon_root != block_root { + return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); + } + } + // Each subsequent block must be the parent of the previous one, with a strictly lower + // slot, so the response forms a contiguous descending chain. + Some(child) => { + if child.parent_root() != block_root { + return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); + } + if block.slot() >= child.slot() { + return Err(LookupVerifyError::UnrequestedSlot(block.slot())); + } + } + } + + self.items.push(block); + // The peer may return fewer blocks than requested (e.g. reached genesis or finalization), + // in which case the request completes on stream termination instead. + Ok(self.items.len() >= self.count) + } + + fn consume(&mut self) -> Vec { + std::mem::take(&mut self.items) + } +} From 32e9b2251824c24e0c7d737e937047be4f714288 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:14:08 +0200 Subject: [PATCH 03/27] Serialize peer supported_protocols instead of skipping Implement Serialize for the rpc Protocol enum (as its canonical name) so PeerInfo.supported_protocols is surfaced in the peers API, matching how subnet fields are serialized, rather than skipped. --- .../src/peer_manager/peerdb/peer_info.rs | 1 - beacon_node/lighthouse_network/src/rpc/protocol.rs | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs index c04ed802e9b..fee16bbf38e 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs @@ -61,7 +61,6 @@ pub struct PeerInfo { /// The enr of the peer, if known. enr: Option, /// The set of ReqResp protocols the peer advertised via `identify`. Empty until identified. - #[serde(skip)] supported_protocols: HashSet, } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index b444608468a..37f4c3b5459 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -302,6 +302,13 @@ pub enum Protocol { LightClientUpdatesByRange, } +impl serde::Serialize for Protocol { + /// Serialize as the canonical protocol name (matching `AsRef`/`Display`). + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(self.as_ref()) + } +} + impl Protocol { pub(crate) fn terminator(self) -> Option { match self { From ad0bcf953f121541d78465c563b087e1912dfbca Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:17:16 +0200 Subject: [PATCH 04/27] Extract rpc_protocol_from_id helper and unit test it Move the identify protocol-id parsing into a standalone rpc_protocol_from_id function and add a unit test covering ReqResp, non-ReqResp, and unknown protocol ids. --- .../src/peer_manager/mod.rs | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index e2cec3d6648..3dcca3aa931 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -491,15 +491,7 @@ impl PeerManager { peer_info.set_supported_protocols( info.protocols .iter() - .filter_map(|protocol| { - // ReqResp protocol ids have the form - // `/eth2/beacon_chain/req///`. - let protocol_str: &str = protocol.as_ref(); - protocol_str - .strip_prefix("/eth2/beacon_chain/req/") - .and_then(|rest| rest.split('/').next()) - .and_then(|name| Protocol::from_str(name).ok()) - }) + .filter_map(|protocol| rpc_protocol_from_id(protocol.as_ref())) .collect(), ); @@ -1721,6 +1713,15 @@ enum ConnectingType { }, } +/// Parses a libp2p protocol id of the form `/eth2/beacon_chain/req///` +/// into the corresponding ReqResp [`Protocol`], returning `None` for non-ReqResp protocols. +fn rpc_protocol_from_id(protocol_id: &str) -> Option { + protocol_id + .strip_prefix("/eth2/beacon_chain/req/") + .and_then(|rest| rest.split('/').next()) + .and_then(|name| Protocol::from_str(name).ok()) +} + #[cfg(test)] mod tests { use super::*; @@ -1728,6 +1729,30 @@ mod tests { use crate::rpc::MetaDataV3; use types::{ChainSpec, ForkName, MainnetEthSpec as E}; + #[test] + fn rpc_protocol_from_id_parses_reqresp_protocols() { + // Known ReqResp protocols are parsed from their name segment. + assert_eq!( + rpc_protocol_from_id("/eth2/beacon_chain/req/beacon_blocks_by_head/1/ssz_snappy"), + Some(Protocol::BlocksByHead) + ); + assert_eq!( + rpc_protocol_from_id("/eth2/beacon_chain/req/beacon_blocks_by_root/2/ssz_snappy"), + Some(Protocol::BlocksByRoot) + ); + assert_eq!( + rpc_protocol_from_id("/eth2/beacon_chain/req/status/1/ssz_snappy"), + Some(Protocol::Status) + ); + // Non-ReqResp protocols and unknown names are ignored. + assert_eq!(rpc_protocol_from_id("/meshsub/1.1.0"), None); + assert_eq!(rpc_protocol_from_id("/ipfs/id/1.0.0"), None); + assert_eq!( + rpc_protocol_from_id("/eth2/beacon_chain/req/not_a_real_protocol/1/ssz_snappy"), + None + ); + } + async fn build_peer_manager(target_peer_count: usize) -> PeerManager { build_peer_manager_with_trusted_peers(vec![], target_peer_count).await } From 0c4de40d19d65124287a001e21b163ea8043221d Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:19:55 +0200 Subject: [PATCH 05/27] Address review: iterator-based from_vec, if-let chain check, trim test - AncestorBlocks::from_vec consumes an iterator (no remove(0)) - BlocksByHeadRequestItems::add uses if-let instead of match - reduce rpc_protocol_from_id test to two cases --- .../src/peer_manager/mod.rs | 17 ++----------- .../network/src/sync/network_context.rs | 14 +++++------ .../requests/blocks_by_head.rs | 25 ++++++++----------- 3 files changed, 19 insertions(+), 37 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 3dcca3aa931..82fac58023d 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1731,26 +1731,13 @@ mod tests { #[test] fn rpc_protocol_from_id_parses_reqresp_protocols() { - // Known ReqResp protocols are parsed from their name segment. + // A known ReqResp protocol is parsed from its name segment. assert_eq!( rpc_protocol_from_id("/eth2/beacon_chain/req/beacon_blocks_by_head/1/ssz_snappy"), Some(Protocol::BlocksByHead) ); - assert_eq!( - rpc_protocol_from_id("/eth2/beacon_chain/req/beacon_blocks_by_root/2/ssz_snappy"), - Some(Protocol::BlocksByRoot) - ); - assert_eq!( - rpc_protocol_from_id("/eth2/beacon_chain/req/status/1/ssz_snappy"), - Some(Protocol::Status) - ); - // Non-ReqResp protocols and unknown names are ignored. + // Non-ReqResp protocols are ignored. assert_eq!(rpc_protocol_from_id("/meshsub/1.1.0"), None); - assert_eq!(rpc_protocol_from_id("/ipfs/id/1.0.0"), None); - assert_eq!( - rpc_protocol_from_id("/eth2/beacon_chain/req/not_a_real_protocol/1/ssz_snappy"), - None - ); } async fn build_peer_manager(target_peer_count: usize) -> PeerManager { diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 7ad62c38e63..6b50555383a 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -118,16 +118,14 @@ pub struct AncestorBlocks { } impl AncestorBlocks { - /// Splits a non-empty run of blocks (the requested root first, then its ancestors in descending - /// slot order) into the requested block and its ancestors. Returns `None` for an empty run. - pub fn from_vec(mut blocks: Vec>>) -> Option { - if blocks.is_empty() { - return None; - } - let first_block = blocks.remove(0); + /// Splits a run of blocks (the requested root first, then its ancestors in descending slot + /// order) into the requested block and its ancestors. Returns `None` for an empty run. + pub fn from_vec(blocks: Vec>>) -> Option { + let mut blocks = blocks.into_iter(); + let first_block = blocks.next()?; Some(Self { first_block, - ancestor_blocks: blocks, + ancestor_blocks: blocks.collect(), }) } } diff --git a/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs b/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs index 5d989bd1565..5258647bdf0 100644 --- a/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs +++ b/beacon_node/network/src/sync/network_context/requests/blocks_by_head.rs @@ -30,22 +30,19 @@ impl ActiveRequestItems for BlocksByHeadRequestItems { /// The active request SHOULD be dropped after `add` returns an error. fn add(&mut self, block: Self::Item) -> Result { let block_root = get_block_root(&block); - match self.items.last() { - // The first block returned is the chain tip and must match the requested root. - None => { - if self.beacon_root != block_root { - return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); - } - } + if let Some(child) = self.items.last() { // Each subsequent block must be the parent of the previous one, with a strictly lower // slot, so the response forms a contiguous descending chain. - Some(child) => { - if child.parent_root() != block_root { - return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); - } - if block.slot() >= child.slot() { - return Err(LookupVerifyError::UnrequestedSlot(block.slot())); - } + if child.parent_root() != block_root { + return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); + } + if block.slot() >= child.slot() { + return Err(LookupVerifyError::UnrequestedSlot(block.slot())); + } + } else { + // The first block returned is the chain tip and must match the requested root. + if self.beacon_root != block_root { + return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); } } From 26622952d3f5dea36c8bb6c84bb9b7ea73c7cf35 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:27:14 +0200 Subject: [PATCH 06/27] Address review: split send fns, build AncestorBlocks in network_context - Split block_lookup_request send paths into send_block_by_root_request and send_blocks_by_head, both taking the selected PeerId - on_{single_block,blocks_by_head}_response return AncestorBlocks so the sync manager does no transformation - query peer_supports_blocks_by_head after selection instead of carrying it --- beacon_node/network/src/sync/manager.rs | 30 +-- .../network/src/sync/network_context.rs | 231 ++++++++++-------- 2 files changed, 136 insertions(+), 125 deletions(-) diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index cdad4e0d0ad..e4e626093f7 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -47,9 +47,7 @@ use crate::service::NetworkMessage; use crate::status::ToStatusMessage; use crate::sync::block_lookups::{BlockComponent, DownloadResult}; use crate::sync::custody_backfill_sync::CustodyBackFillSync; -use crate::sync::network_context::{ - AncestorBlocks, LookupVerifyError, PeerGroup, RpcResponseError, RpcResponseResult, -}; +use crate::sync::network_context::{PeerGroup, RpcResponseResult}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::{BeaconChain, BeaconChainTypes, EngineState}; use futures::StreamExt; @@ -1132,17 +1130,8 @@ 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.and_then(|(blocks, seen_timestamp)| { - let value = AncestorBlocks::from_vec(blocks).ok_or_else(|| { - RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { - actual: 0, - }) - })?; - Ok(DownloadResult::new( - value, - PeerGroup::from_single(peer_id), - seen_timestamp, - )) + resp.map(|(value, seen_timestamp)| { + DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) }), &mut self.network, ) @@ -1158,17 +1147,8 @@ impl SyncManager { if let Some(resp) = self.network.on_blocks_by_head_response(id, peer_id, block) { self.block_lookups.on_block_download_response( id, - resp.and_then(|(blocks, seen_timestamp)| { - let value = AncestorBlocks::from_vec(blocks).ok_or_else(|| { - RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { - actual: 0, - }) - })?; - Ok(DownloadResult::new( - value, - PeerGroup::from_single(peer_id), - seen_timestamp, - )) + resp.map(|(value, seen_timestamp)| { + DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) }), &mut self.network, ) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 6b50555383a..8fefc53f0d9 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -117,17 +117,27 @@ pub struct AncestorBlocks { pub ancestor_blocks: Vec>>, } -impl AncestorBlocks { - /// Splits a run of blocks (the requested root first, then its ancestors in descending slot - /// order) into the requested block and its ancestors. Returns `None` for an empty run. - pub fn from_vec(blocks: Vec>>) -> Option { - let mut blocks = blocks.into_iter(); - let first_block = blocks.next()?; - Some(Self { - first_block, - ancestor_blocks: blocks.collect(), +/// Converts a block download response into [`AncestorBlocks`]: the requested root first (becoming +/// `first_block`), then its ancestors in descending slot order. Errors on an empty run so block +/// lookups always receive at least the requested block. +fn into_ancestor_blocks( + resp: Option>>>>, +) -> Option>> { + resp.map(|res| { + res.and_then(|(blocks, seen_timestamp)| { + let mut blocks = blocks.into_iter(); + let first_block = blocks.next().ok_or_else(|| { + RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }) + })?; + Ok(( + AncestorBlocks { + first_block, + ancestor_blocks: blocks.collect(), + }, + seen_timestamp, + )) }) - } + }) } #[derive(Debug)] @@ -934,26 +944,24 @@ impl SyncNetworkContext { block_root: Hash256, ) -> Result>>, RpcRequestSendError> { let active_request_count_by_peer = self.active_request_count_by_peer(); - let Some((peer_id, supports_blocks_by_head)) = lookup_peers + let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { - let supports_blocks_by_head = self.peer_supports_blocks_by_head(peer); ( ( // Prefer peers that support `beacon_blocks_by_head` - !supports_blocks_by_head, + !self.peer_supports_blocks_by_head(peer), // Prefer peers with less overall requests active_request_count_by_peer.get(peer).copied().unwrap_or(0), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), ), *peer, - supports_blocks_by_head, ) }) - .min_by_key(|(key, _, _)| *key) - .map(|(_, peer, supports_blocks_by_head)| (peer, supports_blocks_by_head)) + .min_by_key(|(key, _)| *key) + .map(|(_, peer)| peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive @@ -991,90 +999,116 @@ impl SyncNetworkContext { req_id: self.next_id(), }; + // If the peer supports `beacon_blocks_by_head`, fetch the block and a run of its ancestors + // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. + if self.peer_supports_blocks_by_head(&peer_id) { + self.send_blocks_by_head(peer_id, id, block_root)?; + } else { + self.send_block_by_root_request(peer_id, id, block_root)?; + } + + Ok(LookupRequestResult::RequestSent(id.req_id)) + } + + /// Sends a `beacon_blocks_by_root` request to `peer_id` for the single block `block_root`. + fn send_block_by_root_request( + &mut self, + peer_id: PeerId, + id: SingleLookupReqId, + block_root: Hash256, + ) -> Result<(), RpcRequestSendError> { // Lookup sync event safety: If network_send.send() returns Ok(_) we are guaranteed that // eventually at least one of these 3 events will be received: - // - StreamTermination(request_id): handled by `Self::on_{single_block,blocks_by_head}_response` - // - RPCError(request_id): handled by `Self::on_{single_block,blocks_by_head}_response` + // - StreamTermination(request_id): handled by `Self::on_single_block_response` + // - RPCError(request_id): handled by `Self::on_single_block_response` // - Disconnect(peer_id) handled by `Self::peer_disconnected` which converts it to a // `RPCError(request_id)` event handled by the above method - // - // If the peer supports `beacon_blocks_by_head`, fetch the block and a run of its ancestors - // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. - if supports_blocks_by_head { - let request = BlocksByHeadRequest { - beacon_root: block_root, - count: BLOCKS_BY_HEAD_REQUEST_COUNT, - }; - self.network_send - .send(NetworkMessage::SendRequest { - peer_id, - request: RequestType::BlocksByHead(request), - app_request_id: AppRequestId::Sync(SyncRequestId::BlocksByHead { id }), - }) - .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; + let request = BlocksByRootSingleRequest(block_root); + let network_request = RequestType::BlocksByRoot( + request + .into_request(&self.fork_context) + .map_err(RpcRequestSendError::InternalError)?, + ); + self.network_send + .send(NetworkMessage::SendRequest { + peer_id, + request: network_request, + app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), + }) + .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; - debug!( - method = "BlocksByHead", - ?block_root, - peer = %peer_id, - %id, - "Sync RPC request sent" - ); + debug!( + method = "BlocksByRoot", + ?block_root, + peer = %peer_id, + %id, + "Sync RPC request sent" + ); - let request_span = debug_span!( - parent: Span::current(), - "lh_outgoing_block_by_head_request", - %block_root, - ); - self.blocks_by_head_requests.insert( - id, - peer_id, - // false = the peer may return fewer blocks than requested (e.g. reached genesis or - // finalization), so the request completes on stream termination. - false, - BlocksByHeadRequestItems::new(block_root, BLOCKS_BY_HEAD_REQUEST_COUNT as usize), - request_span, - ); - } else { - let request = BlocksByRootSingleRequest(block_root); - let network_request = RequestType::BlocksByRoot( - request - .into_request(&self.fork_context) - .map_err(RpcRequestSendError::InternalError)?, - ); - self.network_send - .send(NetworkMessage::SendRequest { - peer_id, - request: network_request, - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), - }) - .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; + let request_span = debug_span!( + parent: Span::current(), + "lh_outgoing_block_by_root_request", + %block_root, + ); + self.blocks_by_root_requests.insert( + id, + peer_id, + // true = enforce max_requests as returned for blocks_by_root. We always request a + // single block and the peer must have it. + true, + BlocksByRootRequestItems::new(request), + request_span, + ); - debug!( - method = "BlocksByRoot", - ?block_root, - peer = %peer_id, - %id, - "Sync RPC request sent" - ); + Ok(()) + } - let request_span = debug_span!( - parent: Span::current(), - "lh_outgoing_block_by_root_request", - %block_root, - ); - self.blocks_by_root_requests.insert( - id, + /// Sends a `beacon_blocks_by_head` request to `peer_id`, fetching `block_root` and a run of its + /// ancestors in a single request. + fn send_blocks_by_head( + &mut self, + peer_id: PeerId, + id: SingleLookupReqId, + block_root: Hash256, + ) -> Result<(), RpcRequestSendError> { + // Lookup sync event safety: see `send_block_by_root_request`. The same guarantees apply, + // with the events handled by `Self::on_blocks_by_head_response`. + let request = BlocksByHeadRequest { + beacon_root: block_root, + count: BLOCKS_BY_HEAD_REQUEST_COUNT, + }; + self.network_send + .send(NetworkMessage::SendRequest { peer_id, - // true = enforce max_requests as returned for blocks_by_root. We always request a - // single block and the peer must have it. - true, - BlocksByRootRequestItems::new(request), - request_span, - ); - } + request: RequestType::BlocksByHead(request), + app_request_id: AppRequestId::Sync(SyncRequestId::BlocksByHead { id }), + }) + .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; - Ok(LookupRequestResult::RequestSent(id.req_id)) + debug!( + method = "BlocksByHead", + ?block_root, + peer = %peer_id, + %id, + "Sync RPC request sent" + ); + + let request_span = debug_span!( + parent: Span::current(), + "lh_outgoing_block_by_head_request", + %block_root, + ); + self.blocks_by_head_requests.insert( + id, + peer_id, + // false = the peer may return fewer blocks than requested (e.g. reached genesis or + // finalization), so the request completes on stream termination. + false, + BlocksByHeadRequestItems::new(block_root, BLOCKS_BY_HEAD_REQUEST_COUNT as usize), + request_span, + ); + + Ok(()) } /// Request a payload envelope for a block root via PayloadEnvelopesByRoot RPC. @@ -1586,30 +1620,27 @@ impl SyncNetworkContext { // Request handlers - #[allow(clippy::type_complexity)] pub(crate) fn on_single_block_response( &mut self, id: SingleLookupReqId, peer_id: PeerId, rpc_event: RpcEvent>>, - ) -> Option>>>> { + ) -> Option>> { // `blocks_by_root_requests` enforces that exactly one chunk = one block is returned, so the - // returned vec always has length 1. Block lookups receive the response as a vec to share the - // handling with `beacon_blocks_by_head`, which may return a run of ancestors. - let resp = self.blocks_by_root_requests.on_response(id, rpc_event); + // `AncestorBlocks` always has a single block and no ancestors. + let resp = into_ancestor_blocks(self.blocks_by_root_requests.on_response(id, rpc_event)); self.on_rpc_response_result(resp, peer_id) } - #[allow(clippy::type_complexity)] pub(crate) fn on_blocks_by_head_response( &mut self, id: SingleLookupReqId, peer_id: PeerId, rpc_event: RpcEvent>>, - ) -> Option>>>> { - // The returned vec has the requested `beacon_root` first (the chain tip) followed by its - // ancestors in descending slot order. - let resp = self.blocks_by_head_requests.on_response(id, rpc_event); + ) -> Option>> { + // The response has the requested `beacon_root` first (the chain tip, becoming `first_block`) + // followed by its ancestors in descending slot order. + let resp = into_ancestor_blocks(self.blocks_by_head_requests.on_response(id, rpc_event)); self.on_rpc_response_result(resp, peer_id) } From 77f660a6ee6f12a0ed98f60746a015ec133b6a64 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:36:06 +0200 Subject: [PATCH 07/27] Minimize peer-selection diff: keep flat tuple + min() Add the beacon_blocks_by_head support flag as the first tuple element and keep unstable's .min()/peer-in-tuple shape instead of restructuring. --- .../network/src/sync/network_context.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 8fefc53f0d9..52b27a6d51e 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -949,19 +949,17 @@ impl SyncNetworkContext { .iter() .map(|peer| { ( - ( - // Prefer peers that support `beacon_blocks_by_head` - !self.peer_supports_blocks_by_head(peer), - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), - // Random factor to break ties, otherwise the PeerID breaks ties - rand::random::(), - ), - *peer, + // Prefer peers that support `beacon_blocks_by_head` + !self.peer_supports_blocks_by_head(peer), + // Prefer peers with less overall requests + active_request_count_by_peer.get(peer).copied().unwrap_or(0), + // Random factor to break ties, otherwise the PeerID breaks ties + rand::random::(), + peer, ) }) - .min_by_key(|(key, _)| *key) - .map(|(_, peer)| peer) + .min() + .map(|(_, _, _, peer)| *peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive From 21fec2e1493ae3ddcb54b54ea40702d9b9af3680 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:48:06 +0200 Subject: [PATCH 08/27] Gate lookup peer selection on per-protocol concurrent request limit Strictly de-prioritize peers already at MAX_CONCURRENT_REQUESTS for the protocol we would use (by_head if supported, else by_root), counting only that protocol's in-flight requests per the spec's per-protocol-ID limit. Expose MAX_CONCURRENT_REQUESTS and add ActiveRequests::active_request_count_of_peer. --- beacon_node/lighthouse_network/src/rpc/mod.rs | 2 +- .../network/src/sync/network_context.rs | 24 ++++++++++++++----- .../src/sync/network_context/requests.rs | 8 +++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/mod.rs b/beacon_node/lighthouse_network/src/rpc/mod.rs index 7c43018af83..e9177586cbd 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 52b27a6d51e..549e5ccd06b 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -29,7 +29,7 @@ use lighthouse_network::rpc::methods::{ PayloadEnvelopesByRangeRequest, }; use lighthouse_network::rpc::{ - BlocksByRangeRequest, GoodbyeReason, Protocol, RPCError, RequestType, + BlocksByRangeRequest, GoodbyeReason, MAX_CONCURRENT_REQUESTS, Protocol, RPCError, RequestType, }; pub use lighthouse_network::service::api_types::RangeRequestId; use lighthouse_network::service::api_types::{ @@ -943,23 +943,35 @@ impl SyncNetworkContext { lookup_peers: Arc>>, block_root: Hash256, ) -> Result>>, RpcRequestSendError> { - let active_request_count_by_peer = self.active_request_count_by_peer(); let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { + let supports_blocks_by_head = self.peer_supports_blocks_by_head(peer); + // Count concurrent requests to this peer on the protocol we would use. The spec + // limits concurrent requests per protocol ID, so only the relevant protocol counts. + let active_request_count = if supports_blocks_by_head { + self.blocks_by_head_requests + .active_request_count_of_peer(peer) + } else { + self.blocks_by_root_requests + .active_request_count_of_peer(peer) + }; ( + // Strictly de-prioritize peers already at the concurrent-request limit, so we + // don't exceed `MAX_CONCURRENT_REQUESTS` per protocol ID. + active_request_count >= MAX_CONCURRENT_REQUESTS, // Prefer peers that support `beacon_blocks_by_head` - !self.peer_supports_blocks_by_head(peer), - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), + !supports_blocks_by_head, + // Prefer peers with fewer concurrent requests on that protocol + active_request_count, // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, ) }) .min() - .map(|(_, _, _, peer)| *peer) + .map(|(_, _, _, _, peer)| *peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index ef8d7913b34..e04e62d2ac8 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -246,6 +246,14 @@ impl ActiveReque .collect() } + /// Number of active requests to `peer_id` on this protocol. + pub fn active_request_count_of_peer(&self, peer_id: &PeerId) -> usize { + self.requests + .values() + .filter(|request| &request.peer_id == peer_id) + .count() + } + pub fn iter_request_peers(&self) -> impl Iterator + '_ { self.requests.values().map(|request| request.peer_id) } From 41dfc46e6afb3724f62ca61ede1f51c8129e7f0d Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:54:33 +0200 Subject: [PATCH 09/27] Use precomputed per-protocol request-count maps for peer selection Build per-peer blocks_by_root / blocks_by_head count maps once instead of scanning per candidate peer; gate selection on the relevant protocol's count >= MAX_CONCURRENT_REQUESTS. --- .../network/src/sync/network_context.rs | 50 ++++++++++++++----- .../src/sync/network_context/requests.rs | 8 --- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 549e5ccd06b..9de100bcddf 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -543,6 +543,24 @@ impl SyncNetworkContext { active_request_count_by_peer } + /// Number of active `blocks_by_root` requests per peer. + fn active_blocks_by_root_requests_by_peer(&self) -> HashMap { + let mut count_by_peer = HashMap::new(); + for peer_id in self.blocks_by_root_requests.iter_request_peers() { + *count_by_peer.entry(peer_id).or_default() += 1; + } + count_by_peer + } + + /// Number of active `blocks_by_head` requests per peer. + fn active_blocks_by_head_requests_by_peer(&self) -> HashMap { + let mut count_by_peer = HashMap::new(); + for peer_id in self.blocks_by_head_requests.iter_request_peers() { + *count_by_peer.entry(peer_id).or_default() += 1; + } + 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 @@ -943,28 +961,34 @@ impl SyncNetworkContext { lookup_peers: Arc>>, block_root: Hash256, ) -> Result>>, RpcRequestSendError> { + let active_request_count_by_peer = self.active_request_count_by_peer(); + let active_blocks_by_root_requests = self.active_blocks_by_root_requests_by_peer(); + let active_blocks_by_head_requests = self.active_blocks_by_head_requests_by_peer(); let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { let supports_blocks_by_head = self.peer_supports_blocks_by_head(peer); - // Count concurrent requests to this peer on the protocol we would use. The spec - // limits concurrent requests per protocol ID, so only the relevant protocol counts. - let active_request_count = if supports_blocks_by_head { - self.blocks_by_head_requests - .active_request_count_of_peer(peer) + // The spec limits concurrent requests per protocol ID, so gate on the in-flight + // count for the protocol we would actually use. + let at_concurrency_limit = (if supports_blocks_by_head { + active_blocks_by_head_requests + .get(peer) + .copied() + .unwrap_or(0) } else { - self.blocks_by_root_requests - .active_request_count_of_peer(peer) - }; + active_blocks_by_root_requests + .get(peer) + .copied() + .unwrap_or(0) + }) >= MAX_CONCURRENT_REQUESTS; ( - // Strictly de-prioritize peers already at the concurrent-request limit, so we - // don't exceed `MAX_CONCURRENT_REQUESTS` per protocol ID. - active_request_count >= MAX_CONCURRENT_REQUESTS, + // Strictly de-prioritize peers already at the concurrent-request limit + at_concurrency_limit, // Prefer peers that support `beacon_blocks_by_head` !supports_blocks_by_head, - // Prefer peers with fewer concurrent requests on that protocol - active_request_count, + // Prefer peers with less overall requests + active_request_count_by_peer.get(peer).copied().unwrap_or(0), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index e04e62d2ac8..ef8d7913b34 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -246,14 +246,6 @@ impl ActiveReque .collect() } - /// Number of active requests to `peer_id` on this protocol. - pub fn active_request_count_of_peer(&self, peer_id: &PeerId) -> usize { - self.requests - .values() - .filter(|request| &request.peer_id == peer_id) - .count() - } - pub fn iter_request_peers(&self) -> impl Iterator + '_ { self.requests.values().map(|request| request.peer_id) } From 00f662da1682cf86f6c0ef3de135e587a5edc49e Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:05:21 +0200 Subject: [PATCH 10/27] Refactor: next_id in send fns, ActiveRequestsPerPeer, AncestorBlocks ctors - send_block_by_root_request/send_blocks_by_head take lookup_id, allocate their own req_id, and return it - replace the two per-peer count helpers with an ActiveRequestsPerPeer struct exposing at_concurrency_limit(peer) - give AncestorBlocks from_single/from_vec constructors and inline into_ancestor_blocks into the response handlers --- .../network/src/sync/network_context.rs | 164 ++++++++++-------- 1 file changed, 91 insertions(+), 73 deletions(-) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 9de100bcddf..796986f983e 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -43,9 +43,10 @@ use lighthouse_network::{Client, NetworkGlobals, PeerAction, PeerId, ReportSourc use parking_lot::RwLock; pub use requests::LookupVerifyError; use requests::{ - ActiveRequests, BlobsByRangeRequestItems, BlocksByHeadRequestItems, BlocksByRangeRequestItems, - BlocksByRootRequestItems, DataColumnsByRangeRequestItems, DataColumnsByRootRequestItems, - PayloadEnvelopesByRangeRequestItems, PayloadEnvelopesByRootRequestItems, + ActiveRequestItems, ActiveRequests, BlobsByRangeRequestItems, BlocksByHeadRequestItems, + BlocksByRangeRequestItems, BlocksByRootRequestItems, DataColumnsByRangeRequestItems, + DataColumnsByRootRequestItems, PayloadEnvelopesByRangeRequestItems, + PayloadEnvelopesByRootRequestItems, }; #[cfg(test)] use slot_clock::SlotClock; @@ -117,27 +118,49 @@ pub struct AncestorBlocks { pub ancestor_blocks: Vec>>, } -/// Converts a block download response into [`AncestorBlocks`]: the requested root first (becoming -/// `first_block`), then its ancestors in descending slot order. Errors on an empty run so block -/// lookups always receive at least the requested block. -fn into_ancestor_blocks( - resp: Option>>>>, -) -> Option>> { - resp.map(|res| { - res.and_then(|(blocks, seen_timestamp)| { - let mut blocks = blocks.into_iter(); - let first_block = blocks.next().ok_or_else(|| { - RpcResponseError::from(LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }) - })?; - Ok(( - AncestorBlocks { - first_block, - ancestor_blocks: blocks.collect(), - }, - seen_timestamp, - )) +impl AncestorBlocks { + /// A single block with no ancestors. + pub fn from_single(block: Arc>) -> Self { + Self { + first_block: block, + ancestor_blocks: vec![], + } + } + + /// The requested root first (becoming `first_block`), then its ancestors in descending slot + /// order. Returns `None` for an empty run. + pub fn from_vec(blocks: Vec>>) -> Option { + let mut blocks = blocks.into_iter(); + let first_block = blocks.next()?; + Some(Self { + first_block, + ancestor_blocks: blocks.collect(), }) - }) + } +} + +/// 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)] @@ -543,24 +566,6 @@ impl SyncNetworkContext { active_request_count_by_peer } - /// Number of active `blocks_by_root` requests per peer. - fn active_blocks_by_root_requests_by_peer(&self) -> HashMap { - let mut count_by_peer = HashMap::new(); - for peer_id in self.blocks_by_root_requests.iter_request_peers() { - *count_by_peer.entry(peer_id).or_default() += 1; - } - count_by_peer - } - - /// Number of active `blocks_by_head` requests per peer. - fn active_blocks_by_head_requests_by_peer(&self) -> HashMap { - let mut count_by_peer = HashMap::new(); - for peer_id in self.blocks_by_head_requests.iter_request_peers() { - *count_by_peer.entry(peer_id).or_default() += 1; - } - 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 @@ -962,26 +967,20 @@ impl SyncNetworkContext { block_root: Hash256, ) -> Result>>, RpcRequestSendError> { let active_request_count_by_peer = self.active_request_count_by_peer(); - let active_blocks_by_root_requests = self.active_blocks_by_root_requests_by_peer(); - let active_blocks_by_head_requests = self.active_blocks_by_head_requests_by_peer(); + let blocks_by_root_per_peer = ActiveRequestsPerPeer::new(&self.blocks_by_root_requests); + let blocks_by_head_per_peer = ActiveRequestsPerPeer::new(&self.blocks_by_head_requests); let Some(peer_id) = lookup_peers .read() .iter() .map(|peer| { let supports_blocks_by_head = self.peer_supports_blocks_by_head(peer); - // The spec limits concurrent requests per protocol ID, so gate on the in-flight - // count for the protocol we would actually use. - let at_concurrency_limit = (if supports_blocks_by_head { - active_blocks_by_head_requests - .get(peer) - .copied() - .unwrap_or(0) + // The spec limits concurrent requests per protocol ID, so gate on the protocol we + // would actually use. + let at_concurrency_limit = if supports_blocks_by_head { + blocks_by_head_per_peer.at_concurrency_limit(peer) } else { - active_blocks_by_root_requests - .get(peer) - .copied() - .unwrap_or(0) - }) >= MAX_CONCURRENT_REQUESTS; + blocks_by_root_per_peer.at_concurrency_limit(peer) + }; ( // Strictly de-prioritize peers already at the concurrent-request limit at_concurrency_limit, @@ -1028,29 +1027,28 @@ impl SyncNetworkContext { } } - let id = SingleLookupReqId { - lookup_id, - req_id: self.next_id(), - }; - // If the peer supports `beacon_blocks_by_head`, fetch the block and a run of its ancestors // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. - if self.peer_supports_blocks_by_head(&peer_id) { - self.send_blocks_by_head(peer_id, id, block_root)?; + let req_id = if self.peer_supports_blocks_by_head(&peer_id) { + self.send_blocks_by_head(peer_id, lookup_id, block_root)? } else { - self.send_block_by_root_request(peer_id, id, block_root)?; - } + self.send_block_by_root_request(peer_id, lookup_id, block_root)? + }; - Ok(LookupRequestResult::RequestSent(id.req_id)) + Ok(LookupRequestResult::RequestSent(req_id)) } /// Sends a `beacon_blocks_by_root` request to `peer_id` for the single block `block_root`. fn send_block_by_root_request( &mut self, peer_id: PeerId, - id: SingleLookupReqId, + lookup_id: SingleLookupId, block_root: Hash256, - ) -> Result<(), RpcRequestSendError> { + ) -> Result { + let id = SingleLookupReqId { + lookup_id, + req_id: self.next_id(), + }; // Lookup sync event safety: If network_send.send() returns Ok(_) we are guaranteed that // eventually at least one of these 3 events will be received: // - StreamTermination(request_id): handled by `Self::on_single_block_response` @@ -1094,7 +1092,7 @@ impl SyncNetworkContext { request_span, ); - Ok(()) + Ok(id.req_id) } /// Sends a `beacon_blocks_by_head` request to `peer_id`, fetching `block_root` and a run of its @@ -1102,9 +1100,13 @@ impl SyncNetworkContext { fn send_blocks_by_head( &mut self, peer_id: PeerId, - id: SingleLookupReqId, + lookup_id: SingleLookupId, block_root: Hash256, - ) -> Result<(), RpcRequestSendError> { + ) -> Result { + let id = SingleLookupReqId { + lookup_id, + req_id: self.next_id(), + }; // Lookup sync event safety: see `send_block_by_root_request`. The same guarantees apply, // with the events handled by `Self::on_blocks_by_head_response`. let request = BlocksByHeadRequest { @@ -1142,7 +1144,7 @@ impl SyncNetworkContext { request_span, ); - Ok(()) + Ok(id.req_id) } /// Request a payload envelope for a block root via PayloadEnvelopesByRoot RPC. @@ -1662,7 +1664,15 @@ impl SyncNetworkContext { ) -> Option>> { // `blocks_by_root_requests` enforces that exactly one chunk = one block is returned, so the // `AncestorBlocks` always has a single block and no ancestors. - let resp = into_ancestor_blocks(self.blocks_by_root_requests.on_response(id, rpc_event)); + let resp = self.blocks_by_root_requests.on_response(id, rpc_event); + let resp = resp.map(|res| { + res.and_then(|(mut blocks, seen_timestamp)| { + let block = blocks.pop().ok_or(RpcResponseError::from( + LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }, + ))?; + Ok((AncestorBlocks::from_single(block), seen_timestamp)) + }) + }); self.on_rpc_response_result(resp, peer_id) } @@ -1674,7 +1684,15 @@ impl SyncNetworkContext { ) -> Option>> { // The response has the requested `beacon_root` first (the chain tip, becoming `first_block`) // followed by its ancestors in descending slot order. - let resp = into_ancestor_blocks(self.blocks_by_head_requests.on_response(id, rpc_event)); + let resp = self.blocks_by_head_requests.on_response(id, rpc_event); + let resp = resp.map(|res| { + res.and_then(|(blocks, seen_timestamp)| { + let value = AncestorBlocks::from_vec(blocks).ok_or(RpcResponseError::from( + LookupVerifyError::NotEnoughResponsesReturned { actual: 0 }, + ))?; + Ok((value, seen_timestamp)) + }) + }); self.on_rpc_response_result(resp, peer_id) } From 06b5897b64ac388f5960c01d3f09dc89bbe9f907 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:13:28 +0200 Subject: [PATCH 11/27] Restore unstable comments in send_block_by_root_request; inline req_id --- .../network/src/sync/network_context.rs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 796986f983e..02a01486b07 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -1029,13 +1029,13 @@ impl SyncNetworkContext { // If the peer supports `beacon_blocks_by_head`, fetch the block and a run of its ancestors // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. - let req_id = if self.peer_supports_blocks_by_head(&peer_id) { - self.send_blocks_by_head(peer_id, lookup_id, block_root)? - } else { - self.send_block_by_root_request(peer_id, lookup_id, block_root)? - }; - - Ok(LookupRequestResult::RequestSent(req_id)) + Ok(LookupRequestResult::RequestSent( + if self.peer_supports_blocks_by_head(&peer_id) { + self.send_blocks_by_head(peer_id, lookup_id, block_root)? + } else { + self.send_block_by_root_request(peer_id, lookup_id, block_root)? + }, + )) } /// Sends a `beacon_blocks_by_root` request to `peer_id` for the single block `block_root`. @@ -1049,13 +1049,15 @@ impl SyncNetworkContext { lookup_id, req_id: self.next_id(), }; + + let request = BlocksByRootSingleRequest(block_root); + // Lookup sync event safety: If network_send.send() returns Ok(_) we are guaranteed that - // eventually at least one of these 3 events will be received: + // eventually at least one this 3 events will be received: // - StreamTermination(request_id): handled by `Self::on_single_block_response` // - RPCError(request_id): handled by `Self::on_single_block_response` - // - Disconnect(peer_id) handled by `Self::peer_disconnected` which converts it to a - // `RPCError(request_id)` event handled by the above method - let request = BlocksByRootSingleRequest(block_root); + // - Disconnect(peer_id) handled by `Self::peer_disconnected``which converts it to a + // ` RPCError(request_id)`event handled by the above method let network_request = RequestType::BlocksByRoot( request .into_request(&self.fork_context) @@ -1085,8 +1087,8 @@ impl SyncNetworkContext { self.blocks_by_root_requests.insert( id, peer_id, - // true = enforce max_requests as returned for blocks_by_root. We always request a - // single block and the peer must have it. + // true = enforce max_requests as returned for blocks_by_root. We always request a single + // block and the peer must have it. true, BlocksByRootRequestItems::new(request), request_span, From 5bd5df0c6677e7384ae38d4fa88d3a6c66ab32dc Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:49:37 +0200 Subject: [PATCH 12/27] Treat already-imported blocks as parent-imported; finality conflict helper get_parent_import_status now reports an already-imported block (genesis, or a finalized block whose parent was pruned) as having an imported parent, so a lookup for such an anchor is processed rather than awaiting a missing parent. Block lookups use cx.conflicts_with_finality (returning a reason String) to drop blocks at/below finalization that are off the finalized chain. --- .../src/sync/block_lookups/single_block_lookup.rs | 12 ++++-------- beacon_node/network/src/sync/network_context.rs | 13 +++++++++++++ consensus/fork_choice/src/fork_choice.rs | 4 ++++ 3 files changed, 21 insertions(+), 8 deletions(-) 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 6935ab8df63..ee97d88ce64 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 @@ -44,8 +44,8 @@ pub enum LookupResult { pub enum LookupRequestError { /// Too many failed attempts TooManyAttempts, - /// This block is not descendant of finalized checkpoint - ConflictsWithFinality(Slot, Slot), + /// This block is not a descendant of the finalized checkpoint + ConflictsWithFinality(String), /// Error sending event to network SendFailedNetwork(RpcRequestSendError), /// Error sending event to processor @@ -395,7 +395,6 @@ impl SingleBlockLookup { seen_timestamp, .. } = data; - let finalized_slot = cx.chain.head().finalized_slot(); // Eagerly check if we can import without having to send the block for processing. This // allows us to check many lookups in the same sync execution / loop. if cx.chain.is_parent_imported(block) { @@ -407,11 +406,8 @@ impl SingleBlockLookup { ) .map_err(LookupRequestError::SendFailedProcessor)?; self.block_request.state.start_processing()?; - } else if block.slot() <= finalized_slot { - return Err(LookupRequestError::ConflictsWithFinality( - block.slot(), - finalized_slot, - )); + } else if let Some(reason) = cx.conflicts_with_finality(block) { + return Err(LookupRequestError::ConflictsWithFinality(reason)); } else { self.awaiting_parent = Some(AwaitingParent::from_block(block)); return Ok(LookupResult::ParentUnknown { diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 02a01486b07..651571dd97c 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -489,6 +489,19 @@ impl SyncNetworkContext { .unwrap_or(false) } + /// Returns `Some(reason)` if `block` is at or below finalization but off the finalized chain. + pub fn conflicts_with_finality(&self, block: &SignedBeaconBlock) -> Option { + let finalized_slot = self.chain.head().finalized_slot(); + if block.slot() > finalized_slot { + None + } else { + Some(format!( + "block slot {} <= finalized slot {finalized_slot}", + block.slot() + )) + } + } + /// Returns the Client type of the peer if known pub fn client_type(&self, peer_id: &PeerId) -> Client { self.network_globals() diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index edced9b2467..2e5bf2fe5da 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1575,6 +1575,10 @@ where } else { ParentImportStatus::Imported(parent_block) } + } else if let Some(block) = self.get_block(&block.canonical_root()) { + // The block is itself imported (e.g. genesis, or a finalized block whose parent has been + // pruned), so its parent was imported too. + ParentImportStatus::Imported(block) } else { ParentImportStatus::UnknownBlock } From c44b5f855abb2a778715cf043abf90b1773e7c00 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:11:56 +0200 Subject: [PATCH 13/27] Thread known ancestor blocks through parent-lookup creation Pass known_blocks down search_parent_of_child -> new_current_lookup -> on_lookup_result so every ancestor seeded from a single beacon_blocks_by_head response is served from the cache, rather than only the first parent level. --- .../network/src/sync/block_lookups/mod.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index d587d065e8b..eb586b56fc9 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -191,6 +191,7 @@ impl BlockLookups { &PeerType::new(parent_block_hash), block_root, &[peer_id], + None, cx, ); // Only create the child lookup if the parent exists @@ -205,6 +206,7 @@ impl BlockLookups { // peers to house the block components. &[], &PeerType::Block, + None, cx, ) } else { @@ -222,7 +224,15 @@ impl BlockLookups { peer_source: &[PeerId], cx: &mut SyncNetworkContext, ) -> bool { - self.new_current_lookup(block_root, None, None, peer_source, &PeerType::Block, cx) + self.new_current_lookup( + block_root, + None, + None, + peer_source, + &PeerType::Block, + None, + cx, + ) } /// A block or blob triggers the search of a parent. @@ -232,6 +242,7 @@ impl BlockLookups { /// /// Returns true if the lookup is created or already exists #[must_use = "only reference the new lookup if returns true"] + #[allow(clippy::too_many_arguments)] pub fn search_parent_of_child( &mut self, block_root_to_search: Hash256, @@ -239,6 +250,7 @@ impl BlockLookups { peer_type: &PeerType, child_block_root_trigger: Hash256, peers: &[PeerId], + known_blocks: Option>, cx: &mut SyncNetworkContext, ) -> bool { let parent_chains = self.active_parent_lookups(); @@ -335,6 +347,7 @@ impl BlockLookups { None, peers, peer_type, + known_blocks, cx, ) } @@ -343,6 +356,7 @@ impl BlockLookups { /// constructed. /// Returns true if the lookup is created or already exists #[must_use = "only reference the new lookup if returns true"] + #[allow(clippy::too_many_arguments)] fn new_current_lookup( &mut self, block_root: Hash256, @@ -350,6 +364,7 @@ impl BlockLookups { awaiting_parent: Option, peers: &[PeerId], peer_type: &PeerType, + known_blocks: Option>, cx: &mut SyncNetworkContext, ) -> bool { // If this block or it's parent is part of a known ignored chain, ignore it. @@ -429,7 +444,7 @@ impl BlockLookups { self.metrics.created_lookups += 1; let result = lookup.continue_requests(cx); - if self.on_lookup_result(id, result, None, "new_current_lookup", cx) { + if self.on_lookup_result(id, result, known_blocks, "new_current_lookup", cx) { self.update_metrics(); true } else { @@ -669,15 +684,17 @@ impl BlockLookups { block_root, peers, }) => { + let block_component = known_blocks.as_ref().and_then(|p| { + p.get(&parent_root) + .map(|b| BlockComponent::Block(b.clone())) + }); if self.search_parent_of_child( parent_root, - known_blocks.and_then(|p| { - p.get(&parent_root) - .map(|b| BlockComponent::Block(b.clone())) - }), + block_component, &PeerType::new(parent_block_hash), block_root, &peers, + known_blocks, cx, ) { true From 307ed67afef3aee70b41fd1481f873afd77952f6 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:26:46 +0200 Subject: [PATCH 14/27] Test: one beacon_blocks_by_head request fetches a whole ancestor chain - size PARENT_DEPTH_TOLERANCE to BLOCKS_BY_HEAD_REQUEST_COUNT + 1 so one by-head batch can seed a full chain without tripping the depth limit - rig: serve BlocksByHead (walk parent chain) and a peerdb test setter + rig helper to mark a peer as advertising beacon_blocks_by_head - add a test asserting a 32-block chain resolves with 1 by-head request and 0 blocks-by-root, all blocks cached --- .../src/peer_manager/peerdb.rs | 11 ++++ .../network/src/sync/block_lookups/mod.rs | 17 ++++--- beacon_node/network/src/sync/tests/lookups.rs | 50 ++++++++++++++++++- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs index 0a338bb0119..e5cecd6d8b5 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs @@ -882,6 +882,17 @@ impl PeerDB { .ok_or_else(|| "Cannot set custody subnets, peer not found".to_string()) } + pub fn __set_supported_protocols( + &mut self, + peer_id: &PeerId, + supported_protocols: HashSet, + ) -> Result<(), String> { + self.peers + .get_mut(peer_id) + .map(|info| info.set_supported_protocols(supported_protocols)) + .ok_or_else(|| "Cannot set supported protocols, peer not found".to_string()) + } + /// The connection state of the peer has been changed. Modify the peer in the db to ensure all /// variables are in sync with libp2p. /// Updating the state can lead to a `BanOperation` which needs to be processed via the peer diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index eb586b56fc9..7caaa363f71 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -23,8 +23,10 @@ use self::parent_chain::{NodeChain, compute_parent_chains}; pub use self::single_block_lookup::DownloadResult; use self::single_block_lookup::{LookupRequestError, PeerType, SingleBlockLookup}; -use super::manager::{BlockProcessType, SLOT_IMPORT_TOLERANCE}; -use super::network_context::{AncestorBlocks, RpcResponseError, SyncNetworkContext}; +use super::manager::BlockProcessType; +use super::network_context::{ + AncestorBlocks, BLOCKS_BY_HEAD_REQUEST_COUNT, RpcResponseError, SyncNetworkContext, +}; use crate::metrics; use crate::network_beacon_processor::BlockProcessingResult; use crate::sync::SyncMessage; @@ -51,13 +53,12 @@ use types::{ pub mod parent_chain; mod single_block_lookup; -/// The maximum depth we will search for a parent block. In principle we should have sync'd any -/// canonical chain to its head once the peer connects. A chain should not appear where it's depth -/// is further back than the most recent head slot. +/// The maximum depth we will search for a parent block. Once a lookup chain reaches this depth it +/// is dropped and the head is force-transitioned to range sync. /// -/// Have the same value as range's sync tolerance to consider a peer synced. Once sync lookup -/// reaches the maximum depth it will force trigger range sync. -pub(crate) const PARENT_DEPTH_TOLERANCE: usize = SLOT_IMPORT_TOLERANCE; +/// Allow one full `beacon_blocks_by_head` batch (plus its triggering child) so a single response +/// can always seed an entire ancestor chain without tripping the limit. +pub(crate) const PARENT_DEPTH_TOLERANCE: usize = BLOCKS_BY_HEAD_REQUEST_COUNT as usize + 1; const IGNORED_CHAINS_CACHE_EXPIRY_SECONDS: u64 = 60; pub const SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS: u8 = 4; diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 13eeaee9aac..be914670123 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -6,6 +6,7 @@ use crate::network_beacon_processor::{ ChainSegmentProcessId, InvalidBlockStorage, NetworkBeaconProcessor, }; use crate::sync::block_lookups::{BlockLookupSummary, PARENT_DEPTH_TOLERANCE}; +use crate::sync::network_context::BLOCKS_BY_HEAD_REQUEST_COUNT; use crate::sync::{ SyncMessage, manager::{BatchProcessResult, BlockProcessType, SyncManager}, @@ -27,7 +28,7 @@ use itertools::Itertools; use lighthouse_network::discovery::CombinedKey; use lighthouse_network::{ NetworkConfig, NetworkGlobals, PeerAction, PeerId, - rpc::{RPCError, RequestType}, + rpc::{Protocol, RPCError, RequestType}, service::api_types::{AppRequestId, SyncRequestId}, types::SyncState, }; @@ -849,6 +850,22 @@ impl TestRig { self.send_rpc_envelopes_response(req_id, peer_id, &envelopes); } + (RequestType::BlocksByHead(req), AppRequestId::Sync(req_id)) => { + // Walk the parent chain from `beacon_root`, returning up to `count` blocks in + // descending slot order (the same shape the responder produces). + let mut blocks = vec![]; + let mut block_root = req.beacon_root; + while (blocks.len() as u64) < req.count { + let Some(block) = self.network_blocks_by_root.get(&block_root) else { + break; + }; + let block = block.block_cloned(); + block_root = block.parent_root(); + blocks.push(block); + } + self.send_rpc_blocks_response(req_id, peer_id, &blocks); + } + (RequestType::Status(_req), AppRequestId::Router) => { // Ignore Status requests for now } @@ -1805,6 +1822,15 @@ impl TestRig { peer_id } + /// Mark a peer as advertising the `beacon_blocks_by_head` protocol so block lookups will use it. + fn set_peer_supports_by_head(&mut self, peer_id: PeerId) { + self.network_globals + .peers + .write() + .__set_supported_protocols(&peer_id, HashSet::from([Protocol::BlocksByHead])) + .unwrap(); + } + pub fn new_connected_supernode_peer(&mut self) -> PeerId { let key = self.determinstic_key(); let peer_id = self @@ -2099,6 +2125,28 @@ macro_rules! run_lookups_tests_for_depths { run_lookups_tests_for_depths!(1, 2); +/// A peer that supports `beacon_blocks_by_head` should fetch a whole unknown ancestor chain with a +/// single request, caching every block, so no `beacon_blocks_by_root` requests are needed. +#[tokio::test] +async fn blocks_by_head_fetches_whole_chain_in_one_request() { + let mut r = TestRig::default(); + // A chain that fits in a single by-head response and stays under `PARENT_DEPTH_TOLERANCE`. + r.build_chain(BLOCKS_BY_HEAD_REQUEST_COUNT as usize).await; + // A peer that advertises `beacon_blocks_by_head`. + let peer_id = r.new_connected_supernode_peer(); + r.set_peer_supports_by_head(peer_id); + // Trigger an unknown-parent lookup for the chain tip from that peer. + let tip = r.get_last_block().block_cloned(); + r.trigger_unknown_parent_block(peer_id, tip); + r.simulate(SimulateConfig::happy_path()).await; + + r.assert_successful_lookup_sync(); + // One by-head request fetched and cached the whole ancestor chain, so no by-root was needed. + let counts = r.requests_count(); + assert_eq!(counts.get("BlocksByHead").copied().unwrap_or(0), 1); + assert_eq!(counts.get("BlocksByRoot").copied().unwrap_or(0), 0); +} + /// Assert that lookup sync succeeds with the happy case async fn happy_path_unknown_attestation(depth: usize) { let mut r = TestRig::default(); From 6062cde557c5bb68ebc8b316991b8772afe96fc8 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:40:33 +0200 Subject: [PATCH 15/27] Treat genesis/anchor blocks as parent-imported is_parent_imported now also returns true when the block is itself in fork choice (contains_block), covering genesis and finalized blocks whose anchor proto-node isn't queryable via get_block. This lets a lookup for such a block be processed instead of dropped as conflicting with finality. --- consensus/fork_choice/src/fork_choice.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 2e5bf2fe5da..8053303a86b 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1552,10 +1552,13 @@ where /// Returns `true` if the block's parent is imported (and, for a post-Gloas FULL child, its /// parent's payload is imported too). See [`Self::get_parent_import_status`]. pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { + // A block that is itself in fork choice (e.g. genesis, or a finalized block whose parent has + // been pruned) had an imported parent. `contains_block` is used rather than `get_block` + // because the anchor block is present in the index but has no queryable proto-node. matches!( self.get_parent_import_status(block), ParentImportStatus::Imported(_) - ) + ) || self.contains_block(&block.canonical_root()) } /// Returns the import status of the parent of `block`. @@ -1575,10 +1578,6 @@ where } else { ParentImportStatus::Imported(parent_block) } - } else if let Some(block) = self.get_block(&block.canonical_root()) { - // The block is itself imported (e.g. genesis, or a finalized block whose parent has been - // pruned), so its parent was imported too. - ParentImportStatus::Imported(block) } else { ParentImportStatus::UnknownBlock } From 48b967f1f1fd4720d8adc4bd32fc911d634d61d5 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:40:35 +0200 Subject: [PATCH 16/27] Run the lookup test suite against both by_root and by_head peers Serve beacon_blocks_by_head in the test rig (honouring the empty/wrong-block failure strategies), add a PeerSupport enum threaded into TestRig::default / new_after_fulu, and nest the depth macro so every lookup test runs as a by_root and a by_head variant. --- beacon_node/network/src/sync/tests/lookups.rs | 223 +++++++++++------- beacon_node/network/src/sync/tests/mod.rs | 11 + beacon_node/network/src/sync/tests/range.rs | 42 ++-- 3 files changed, 167 insertions(+), 109 deletions(-) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index be914670123..b483bc8f06f 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -229,6 +229,8 @@ pub(crate) struct TestRigConfig { fulu_test_type: FuluTestType, /// Override the node custody type derived from `fulu_test_type` node_custody_type_override: Option, + /// Which block-request protocol the rig's peers advertise. + peer_support: PeerSupport, } struct FullEmptyFork { @@ -339,14 +341,16 @@ impl TestRig { complete_strategy: <_>::default(), initial_block_lookups_metrics: <_>::default(), fulu_test_type: test_rig_config.fulu_test_type, + peer_support: test_rig_config.peer_support, } } - pub fn default() -> Self { + pub fn default(peer_support: PeerSupport) -> Self { // Before Fulu, FuluTestType is irrelevant Self::new(TestRigConfig { fulu_test_type: FuluTestType::WeFullnodeThemSupernode, node_custody_type_override: None, + peer_support, }) } @@ -355,6 +359,7 @@ impl TestRig { Self::new(TestRigConfig { fulu_test_type: FuluTestType::WeFullnodeThemSupernode, node_custody_type_override: Some(node_custody_type), + peer_support: PeerSupport::default(), }) } @@ -851,18 +856,27 @@ impl TestRig { } (RequestType::BlocksByHead(req), AppRequestId::Sync(req_id)) => { - // Walk the parent chain from `beacon_root`, returning up to `count` blocks in - // descending slot order (the same shape the responder produces). - let mut blocks = vec![]; - let mut block_root = req.beacon_root; - while (blocks.len() as u64) < req.count { - let Some(block) = self.network_blocks_by_root.get(&block_root) else { - break; - }; - let block = block.block_cloned(); - block_root = block.parent_root(); - blocks.push(block); - } + let blocks = if self.complete_strategy.return_no_blocks_n_times > 0 { + self.complete_strategy.return_no_blocks_n_times -= 1; + vec![] + } else if self.complete_strategy.return_wrong_blocks_n_times > 0 { + self.complete_strategy.return_wrong_blocks_n_times -= 1; + vec![Arc::new(self.rand_block())] + } else { + // Walk the parent chain from `beacon_root`, returning up to `count` blocks in + // descending slot order (the same shape the responder produces). + let mut blocks = vec![]; + let mut block_root = req.beacon_root; + while (blocks.len() as u64) < req.count { + let Some(block) = self.network_blocks_by_root.get(&block_root) else { + break; + }; + let block = block.block_cloned(); + block_root = block.parent_root(); + blocks.push(block); + } + blocks + }; self.send_rpc_blocks_response(req_id, peer_id, &blocks); } @@ -1636,8 +1650,10 @@ impl TestRig { // Test setup - fn new_after_fulu() -> Option { - genesis_fork().fulu_enabled().then(Self::default) + fn new_after_fulu(peer_support: PeerSupport) -> Option { + genesis_fork() + .fulu_enabled() + .then(|| Self::default(peer_support)) } pub fn new_fulu_peer_test(fulu_test_type: FuluTestType) -> Option { @@ -1645,6 +1661,7 @@ impl TestRig { Self::new(TestRigConfig { fulu_test_type, node_custody_type_override: None, + peer_support: PeerSupport::default(), }) }) } @@ -1819,6 +1836,9 @@ impl TestRig { self.log(&format!( "Added new peer for testing {peer_id:?}, custody: {peer_custody_str}" )); + if self.peer_support == PeerSupport::SupportsByHead { + self.set_peer_supports_by_head(peer_id); + } peer_id } @@ -1841,6 +1861,9 @@ impl TestRig { self.log(&format!( "Added new peer for testing {peer_id:?}, custody: supernode" )); + if self.peer_support == PeerSupport::SupportsByHead { + self.set_peer_supports_by_head(peer_id); + } peer_id } @@ -1849,10 +1872,15 @@ impl TestRig { /// a connection is established. pub fn new_connected_supernode_peer_no_metadata_custody_subnet(&mut self) -> PeerId { let key = self.determinstic_key(); - self.network_globals - .peers - .write() - .__add_connected_peer(true, key, &self.harness.spec) + let peer_id = + self.network_globals + .peers + .write() + .__add_connected_peer(true, key, &self.harness.spec); + if self.peer_support == PeerSupport::SupportsByHead { + self.set_peer_supports_by_head(peer_id); + } + peer_id } /// Update the peer's custody subnet in PeerDB and send a `UpdatedPeerCgc` message to sync. @@ -2050,91 +2078,110 @@ fn stable_arbitrary() { ); } +/// Generates the whole lookup test suite for a single peer kind across the given depths. macro_rules! run_lookups_tests_for_depths { - ($($depth:literal),+ $(,)?) => { + ($peer_kind:ident, $protocol:expr, depths: [$($depth:literal),+ $(,)?]) => { paste::paste! { $( #[tokio::test] - async fn []() { - happy_path_unknown_attestation($depth).await; + async fn []() { + happy_path_unknown_attestation($depth, $protocol).await; } #[tokio::test] - async fn []() { - happy_path_unknown_block_parent($depth).await; + async fn []() { + happy_path_unknown_block_parent($depth, $protocol).await; } #[tokio::test] - async fn []() { - happy_path_unknown_data_parent($depth).await; + async fn []() { + happy_path_unknown_data_parent($depth, $protocol).await; } #[tokio::test] - async fn []() { - happy_path_multiple_triggers($depth).await; + async fn []() { + happy_path_multiple_triggers($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_empty_block_response($depth).await; + async fn []() { + bad_peer_empty_block_response($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_empty_data_response($depth).await; + async fn []() { + bad_peer_empty_data_response($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_too_few_data_response($depth).await; + async fn []() { + bad_peer_too_few_data_response($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_wrong_block_response($depth).await; + async fn []() { + bad_peer_wrong_block_response($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_wrong_data_response($depth).await; + async fn []() { + bad_peer_wrong_data_response($depth, $protocol).await; } #[tokio::test] - async fn []() { - bad_peer_rpc_failure($depth).await; + async fn []() { + bad_peer_rpc_failure($depth, $protocol).await; } #[tokio::test] - async fn []() { - too_many_download_failures($depth).await; + async fn []() { + too_many_download_failures($depth, $protocol).await; } #[tokio::test] - async fn []() { - too_many_processing_failures($depth).await; + async fn []() { + too_many_processing_failures($depth, $protocol).await; } #[tokio::test] - async fn []() { - peer_disconnected_then_rpc_error($depth).await; + async fn []() { + peer_disconnected_then_rpc_error($depth, $protocol).await; } )+ } }; } -run_lookups_tests_for_depths!(1, 2); +/// Runs the whole lookup suite against each peer kind (by-root only vs. by-head capable) for all +/// depths, by invoking [`run_lookups_tests_for_depths`] once per peer kind. +macro_rules! run_lookups_tests { + ( + peers: [$($peer_kind:ident => $protocol:expr),+ $(,)?], + depths: $depths:tt $(,)? + ) => { + $( + run_lookups_tests_for_depths!($peer_kind, $protocol, depths: $depths); + )+ + }; +} + +run_lookups_tests!( + peers: [ + by_root => PeerSupport::DoesNotSupport, + by_head => PeerSupport::SupportsByHead, + ], + depths: [1, 2], +); /// A peer that supports `beacon_blocks_by_head` should fetch a whole unknown ancestor chain with a /// single request, caching every block, so no `beacon_blocks_by_root` requests are needed. #[tokio::test] async fn blocks_by_head_fetches_whole_chain_in_one_request() { - let mut r = TestRig::default(); + // Peers advertise `beacon_blocks_by_head`, so lookups fetch via it. + let mut r = TestRig::default(PeerSupport::SupportsByHead); // A chain that fits in a single by-head response and stays under `PARENT_DEPTH_TOLERANCE`. r.build_chain(BLOCKS_BY_HEAD_REQUEST_COUNT as usize).await; - // A peer that advertises `beacon_blocks_by_head`. let peer_id = r.new_connected_supernode_peer(); - r.set_peer_supports_by_head(peer_id); // Trigger an unknown-parent lookup for the chain tip from that peer. let tip = r.get_last_block().block_cloned(); r.trigger_unknown_parent_block(peer_id, tip); @@ -2148,8 +2195,8 @@ async fn blocks_by_head_fetches_whole_chain_in_one_request() { } /// Assert that lookup sync succeeds with the happy case -async fn happy_path_unknown_attestation(depth: usize) { - let mut r = TestRig::default(); +async fn happy_path_unknown_attestation(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); // We get attestation for a block descendant (depth) blocks of current head r.build_chain_and_trigger_last_block(depth).await; // Complete the request with good peer behaviour @@ -2157,8 +2204,8 @@ async fn happy_path_unknown_attestation(depth: usize) { r.assert_successful_lookup_sync(); } -async fn happy_path_unknown_block_parent(depth: usize) { - let mut r = TestRig::default(); +async fn happy_path_unknown_block_parent(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain(depth).await; r.trigger_with_last_unknown_block_parent(); r.simulate(SimulateConfig::happy_path()).await; @@ -2173,8 +2220,8 @@ async fn happy_path_unknown_block_parent(depth: usize) { } /// Assert that sync completes from an UnknownDataColumnParent -async fn happy_path_unknown_data_parent(depth: usize) { - let Some(mut r) = TestRig::new_after_fulu() else { +async fn happy_path_unknown_data_parent(depth: usize, peer_support: PeerSupport) { + let Some(mut r) = TestRig::new_after_fulu(peer_support) else { return; }; // No unknown-parent data-column trigger post-Gloas. @@ -2188,8 +2235,8 @@ async fn happy_path_unknown_data_parent(depth: usize) { } /// Assert that multiple trigger types don't create extra lookups -async fn happy_path_multiple_triggers(depth: usize) { - let Some(mut r) = TestRig::new_after_fulu() else { +async fn happy_path_multiple_triggers(depth: usize, peer_support: PeerSupport) { + let Some(mut r) = TestRig::new_after_fulu(peer_support) else { return; }; // + 1, because the unknown parent trigger needs two new blocks @@ -2209,8 +2256,8 @@ async fn happy_path_multiple_triggers(depth: usize) { // Test bad behaviour of peers /// Assert that if peer responds with no blocks, we downscore, and retry the same lookup -async fn bad_peer_empty_block_response(depth: usize) { - let mut r = TestRig::default(); +async fn bad_peer_empty_block_response(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that peer returns empty response once, then good behaviour r.simulate(SimulateConfig::new().return_no_blocks_once()) @@ -2224,8 +2271,8 @@ async fn bad_peer_empty_block_response(depth: usize) { } /// Assert that if peer responds with no columns, we downscore, and retry the same lookup. -async fn bad_peer_empty_data_response(depth: usize) { - let Some(mut r) = TestRig::new_after_fulu() else { +async fn bad_peer_empty_data_response(depth: usize, peer_support: PeerSupport) { + let Some(mut r) = TestRig::new_after_fulu(peer_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2242,8 +2289,8 @@ async fn bad_peer_empty_data_response(depth: usize) { /// Assert that if peer responds with not enough columns, we downscore, and retry the same /// lookup. -async fn bad_peer_too_few_data_response(depth: usize) { - let Some(mut r) = TestRig::new_after_fulu() else { +async fn bad_peer_too_few_data_response(depth: usize, peer_support: PeerSupport) { + let Some(mut r) = TestRig::new_after_fulu(peer_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2259,8 +2306,8 @@ async fn bad_peer_too_few_data_response(depth: usize) { } /// Assert that if peer responds with bad blocks, we downscore, and retry the same lookup -async fn bad_peer_wrong_block_response(depth: usize) { - let mut r = TestRig::default(); +async fn bad_peer_wrong_block_response(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; r.simulate(SimulateConfig::new().return_wrong_blocks_once()) .await; @@ -2271,8 +2318,8 @@ async fn bad_peer_wrong_block_response(depth: usize) { } /// Assert that if peer responds with bad columns, we downscore, and retry the same lookup. -async fn bad_peer_wrong_data_response(depth: usize) { - let Some(mut r) = TestRig::new_after_fulu() else { +async fn bad_peer_wrong_data_response(depth: usize, peer_support: PeerSupport) { + let Some(mut r) = TestRig::new_after_fulu(peer_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2290,8 +2337,8 @@ async fn bad_peer_wrong_data_response(depth: usize) { } /// Assert that on network error, we DON'T downscore, and retry the same lookup -async fn bad_peer_rpc_failure(depth: usize) { - let mut r = TestRig::default(); +async fn bad_peer_rpc_failure(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; r.simulate(SimulateConfig::new().return_rpc_error(RPCError::UnsupportedProtocol)) .await; @@ -2302,8 +2349,8 @@ async fn bad_peer_rpc_failure(depth: usize) { // Test retry logic /// Assert that on too many download failures the lookup fails, but we can still sync -async fn too_many_download_failures(depth: usize) { - let mut r = TestRig::default(); +async fn too_many_download_failures(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that a peer always returns empty r.simulate(SimulateConfig::new().return_no_blocks_always()) @@ -2321,8 +2368,8 @@ async fn too_many_download_failures(depth: usize) { } /// Assert that on too many processing failures the lookup fails, but we can still sync -async fn too_many_processing_failures(depth: usize) { - let mut r = TestRig::default(); +async fn too_many_processing_failures(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that a peer always returns empty r.simulate( @@ -2351,7 +2398,7 @@ async fn too_many_processing_failures(depth: usize) { #[tokio::test] /// Assert that multiple trigger types don't create extra lookups async fn unknown_parent_does_not_add_peers_to_itself() { - let Some(mut r) = TestRig::new_after_fulu() else { + let Some(mut r) = TestRig::new_after_fulu(PeerSupport::DoesNotSupport) else { return; }; // 2, because the unknown parent trigger needs two new blocks @@ -2386,7 +2433,7 @@ async fn unknown_parent_does_not_add_peers_to_itself() { /// Assert that a non-attributable processing error (e.g. processor overloaded) is retried up to /// `SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS`, no peer is penalized, and the lookup is then dropped. async fn test_single_block_lookup_ignored_response() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain_and_trigger_last_block(1).await; r.simulate( SimulateConfig::new().with_process_result(|| BlockProcessingResult::Error { @@ -2406,7 +2453,7 @@ async fn test_single_block_lookup_ignored_response() { #[tokio::test] /// 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(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); // The mock only covers block processing; Gloas also needs real envelope/column results. if r.is_after_gloas() { return; @@ -2424,8 +2471,8 @@ async fn test_single_block_lookup_duplicate_response() { } /// Assert that when peers disconnect the lookups are not dropped (kept with zero peers) -async fn peer_disconnected_then_rpc_error(depth: usize) { - let mut r = TestRig::default(); +async fn peer_disconnected_then_rpc_error(depth: usize, peer_support: PeerSupport) { + let mut r = TestRig::default(peer_support); r.build_chain_and_trigger_last_block(depth).await; r.assert_single_lookups_count(1); // The peer disconnect event reaches sync before the rpc error. @@ -2449,7 +2496,7 @@ async fn peer_disconnected_then_rpc_error(depth: usize) { /// peers recursively from child to parent. async fn lookups_form_chain() { let depth = 5; - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(depth).await; for slot in (1..=depth).rev() { r.trigger_with_block_at_slot(slot as u64); @@ -2474,7 +2521,7 @@ async fn lookups_form_chain() { #[tokio::test] /// 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(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); // TODO(gloas): range sync does not fetch payload envelopes yet. if r.is_after_gloas() { return; @@ -2505,7 +2552,7 @@ async fn test_parent_lookup_too_deep_grow_ancestor_one() { #[tokio::test] async fn test_parent_lookup_too_deep_grow_ancestor_zero() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(PARENT_DEPTH_TOLERANCE).await; r.trigger_with_last_block(); r.simulate(SimulateConfig::happy_path()).await; @@ -2525,7 +2572,7 @@ async fn test_parent_lookup_too_deep_grow_ancestor_zero() { // ignored chains cache. The regression test still applies as the child lookup is not created #[tokio::test] async fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); let depth = PARENT_DEPTH_TOLERANCE + 1; r.build_chain(depth + 1).await; r.trigger_with_block_at_slot(depth as u64); @@ -2553,7 +2600,7 @@ async fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing /// Assert that if a lookup chain (by appending tips) is too long we drop it async fn test_parent_lookup_too_deep_grow_tip() { let depth = PARENT_DEPTH_TOLERANCE + 1; - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(depth).await; for slot in (1..=depth).rev() { r.trigger_with_block_at_slot(slot as u64); @@ -2577,7 +2624,7 @@ async fn test_parent_lookup_too_deep_grow_tip() { #[tokio::test] async fn test_skip_creating_ignored_parent_lookup() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(2).await; r.insert_ignored_chain(r.block_root_at_slot(1)); r.trigger_with_last_block(); @@ -2600,7 +2647,7 @@ async fn test_skip_creating_ignored_parent_lookup() { /// - Block 2: Import ok (parent block 1 is available) /// - Block 3: Import ok (parent block 2 is available) async fn test_same_chain_race_condition() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(3).await; let block_1_root = r.block_root_at_slot(1); @@ -2626,7 +2673,7 @@ async fn test_same_chain_race_condition() { /// 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 - let Some(mut r) = TestRig::new_after_fulu() else { + let Some(mut r) = TestRig::new_after_fulu(PeerSupport::DoesNotSupport) else { return; }; // TODO(gloas): the helper does not populate the envelope missing-component path yet. @@ -2744,7 +2791,7 @@ async fn custody_lookup_permanent_custody_failures(test_type: FuluTestType) { // assert that signatures and kzg proofs are checked #[tokio::test] async fn crypto_on_fail_with_invalid_block_signature() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.build_chain(1).await; r.corrupt_last_block_signature(); r.trigger_with_last_block(); diff --git a/beacon_node/network/src/sync/tests/mod.rs b/beacon_node/network/src/sync/tests/mod.rs index 2f318bfb9a0..48e2fafc1d0 100644 --- a/beacon_node/network/src/sync/tests/mod.rs +++ b/beacon_node/network/src/sync/tests/mod.rs @@ -53,6 +53,15 @@ type T = Witness; /// +-----------------+ /// | - BlockLookups | /// +-----------------+ +/// Whether the rig's peers advertise `beacon_blocks_by_head` (so block lookups fetch via it) or +/// only `beacon_blocks_by_root`. +#[derive(Clone, Copy, Default, PartialEq)] +enum PeerSupport { + SupportsByHead, + #[default] + DoesNotSupport, +} + struct TestRig { /// Receiver for `BeaconProcessor` events (e.g. block processing results). beacon_processor_rx: mpsc::Receiver>, @@ -92,6 +101,8 @@ struct TestRig { initial_block_lookups_metrics: BlockLookupsMetrics, /// Fulu test type fulu_test_type: FuluTestType, + /// Whether peers added to the rig advertise `beacon_blocks_by_head` so lookups fetch via it. + peer_support: PeerSupport, } enum FuluTestType { diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index e6890cf2425..124bc930ffe 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -3,7 +3,7 @@ //! Tests follow the pattern from `lookups.rs`: //! ```ignore //! async fn test_name() { -//! let mut r = TestRig::default(); +//! let mut r = TestRig::default(PeerSupport::DoesNotSupport); //! r.setup_xyz().await; //! r.simulate(SimulateConfig::happy_path()).await; //! r.assert_range_sync_completed(); @@ -266,7 +266,7 @@ impl TestRig { /// Head sync: single peer slightly ahead → download batches → all blocks ingested. #[tokio::test] async fn head_sync_completes() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -280,7 +280,7 @@ async fn head_sync_completes() { /// then head chains are created from awaiting_head_peers to sync the remaining gap. #[tokio::test] async fn finalized_to_head_transition() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -294,7 +294,7 @@ async fn finalized_to_head_transition() { /// finalized epoch advances past genesis. #[tokio::test] async fn finalized_sync_completes() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -308,7 +308,7 @@ async fn finalized_sync_completes() { /// sync completes with no penalties (RPC errors are not penalized). #[tokio::test] async fn batch_rpc_error_retries() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -321,7 +321,7 @@ async fn batch_rpc_error_retries() { /// Peer returns zero blocks for a BlocksByRange request. Batch retries, sync completes. #[tokio::test] async fn batch_peer_returns_empty_then_succeeds() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_no_range_blocks_n_times(1)) .await; @@ -332,7 +332,7 @@ async fn batch_peer_returns_empty_then_succeeds() { /// Only exercises column logic on fulu+. #[tokio::test] async fn batch_peer_returns_no_columns_then_succeeds() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_no_range_columns_n_times(1)) .await; @@ -343,7 +343,7 @@ async fn batch_peer_returns_no_columns_then_succeeds() { /// Batch retries from another peer, sync completes. #[tokio::test] async fn batch_peer_returns_wrong_column_indices_then_succeeds() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_wrong_range_column_indices_n_times(1)) .await; @@ -354,7 +354,7 @@ async fn batch_peer_returns_wrong_column_indices_then_succeeds() { /// Batch retries from another peer, sync completes. #[tokio::test] async fn batch_peer_returns_wrong_column_slots_then_succeeds() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_wrong_range_column_slots_n_times(1)) .await; @@ -365,7 +365,7 @@ async fn batch_peer_returns_wrong_column_slots_then_succeeds() { /// missing columns → CouplingError::DataColumnPeerFailure → retry_partial_batch from other peers. #[tokio::test] async fn batch_peer_returns_partial_columns_then_succeeds() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if !r.fork_name.fulu_enabled() { return; } @@ -379,7 +379,7 @@ async fn batch_peer_returns_partial_columns_then_succeeds() { /// AwaitingDownload, retries without penalty, sync completes. #[tokio::test] async fn batch_non_faulty_failure_retries() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -393,7 +393,7 @@ async fn batch_non_faulty_failure_retries() { /// batch redownloaded from a different peer, sync completes. #[tokio::test] async fn batch_faulty_failure_redownloads() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -408,7 +408,7 @@ async fn batch_faulty_failure_redownloads() { /// Chain removed, all peers penalized with "faulty_chain". #[tokio::test] async fn batch_max_failures_removes_chain() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_faulty_failures(3)) .await; @@ -419,7 +419,7 @@ async fn batch_max_failures_removes_chain() { /// A new peer advertising the same finalized root gets disconnected with GoodbyeReason. #[tokio::test] async fn failed_chain_blacklisted() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); let remote_info = r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_faulty_failures(3)) .await; @@ -430,7 +430,7 @@ async fn failed_chain_blacklisted() { /// All peers disconnect before any request is fulfilled → chain removed (EmptyPeerPool). #[tokio::test] async fn all_peers_disconnect_removes_chain() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_disconnect_after_range_requests(0)) .await; @@ -441,7 +441,7 @@ async fn all_peers_disconnect_removes_chain() { /// for a chain that no longer exists — verified as a no-op (no crash). #[tokio::test] async fn late_response_for_removed_chain() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_disconnect_after_range_requests(1)) .await; @@ -452,7 +452,7 @@ async fn late_response_for_removed_chain() { /// is paused. After 2 responses, EE comes back online, queued batches process, sync completes. #[tokio::test] async fn ee_offline_then_online_resumes_sync() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -467,7 +467,7 @@ async fn ee_offline_then_online_resumes_sync() { /// with sequential processing from the start. All blocks ingested. #[tokio::test] async fn finalized_sync_with_local_head_partial() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -480,7 +480,7 @@ async fn finalized_sync_with_local_head_partial() { /// final gap. Tests optimistic start where local head is near the target. #[tokio::test] async fn finalized_sync_with_local_head_near_target() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if r.skip_range_sync_under_gloas() { return; } @@ -501,7 +501,7 @@ async fn finalized_sync_with_local_head_near_target() { /// Once enough fullnodes + a supernode arrive, sync proceeds and completes. #[tokio::test] async fn not_enough_custody_peers_then_peers_arrive() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { return; } @@ -528,7 +528,7 @@ async fn not_enough_custody_peers_then_peers_arrive() { /// 5. finalized sync should resume as soon as CGC updates are received from peer 2 or 3. #[tokio::test] async fn finalized_sync_not_enough_custody_peers_resume_after_peer_cgc_update() { - let mut r = TestRig::default(); + let mut r = TestRig::default(PeerSupport::DoesNotSupport); if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { return; } From 3f743c2fbde8006656fde8978c7da51e651a8965 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 22:40:10 +0200 Subject: [PATCH 17/27] Dynamic beacon_blocks_by_head count: lone lookups fetch a few, chains fetch a batch A lookup that nothing is waiting on is a lone island and requests a small count (4); once a child is awaiting it the lookup is part of a chain proven deep and requests a large batch (32). Derived from the lookup graph at creation, no escalation state. Tune the counts against mainnet metrics. --- .../network/src/sync/block_lookups/mod.rs | 43 ++++++++++++++++--- .../sync/block_lookups/single_block_lookup.rs | 12 +++++- .../network/src/sync/network_context.rs | 15 +++---- beacon_node/network/src/sync/tests/lookups.rs | 37 ++++++++++++---- 4 files changed, 82 insertions(+), 25 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 7caaa363f71..1a35eafc855 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -24,9 +24,7 @@ use self::parent_chain::{NodeChain, compute_parent_chains}; pub use self::single_block_lookup::DownloadResult; use self::single_block_lookup::{LookupRequestError, PeerType, SingleBlockLookup}; use super::manager::BlockProcessType; -use super::network_context::{ - AncestorBlocks, BLOCKS_BY_HEAD_REQUEST_COUNT, RpcResponseError, SyncNetworkContext, -}; +use super::network_context::{AncestorBlocks, RpcResponseError, SyncNetworkContext}; use crate::metrics; use crate::network_beacon_processor::BlockProcessingResult; use crate::sync::SyncMessage; @@ -53,12 +51,19 @@ use types::{ pub mod parent_chain; mod single_block_lookup; +/// `beacon_blocks_by_head` count for a lone lookup (nothing awaits it yet): fetch only a few +/// ancestors so the common shallow case doesn't over-fetch. +pub(crate) const BLOCKS_BY_HEAD_LONE_REQUEST_COUNT: u64 = 4; +/// `beacon_blocks_by_head` count for a lookup that's part of a known parent chain: fetch a large +/// batch to resolve a deep chain in few round-trips. Tune against mainnet metrics. +pub(crate) const BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT: u64 = 32; + /// The maximum depth we will search for a parent block. Once a lookup chain reaches this depth it /// is dropped and the head is force-transitioned to range sync. /// /// Allow one full `beacon_blocks_by_head` batch (plus its triggering child) so a single response /// can always seed an entire ancestor chain without tripping the limit. -pub(crate) const PARENT_DEPTH_TOLERANCE: usize = BLOCKS_BY_HEAD_REQUEST_COUNT as usize + 1; +pub(crate) const PARENT_DEPTH_TOLERANCE: usize = BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT as usize + 1; const IGNORED_CHAINS_CACHE_EXPIRY_SECONDS: u64 = 60; pub const SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS: u8 = 4; @@ -170,6 +175,21 @@ impl BlockLookups { ) } + /// The `beacon_blocks_by_head` count to request when fetching `block_root`. A lookup that + /// nothing is waiting on is a lone island and fetches only a few ancestors; a lookup a child is + /// awaiting is part of a chain already proven to be deep, so it fetches a large batch. + fn by_head_request_count(&self, block_root: Hash256) -> u64 { + if self + .single_block_lookups + .values() + .any(|lookup| lookup.is_awaiting_block(block_root)) + { + BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT + } else { + BLOCKS_BY_HEAD_LONE_REQUEST_COUNT + } + } + /* Lookup requests */ /// Creates a parent lookup for the block with the given `block_root` and immediately triggers it. @@ -413,10 +433,21 @@ impl BlockLookups { return false; } + // Read the by-head count before inserting this lookup below: a lookup nothing yet awaits is + // a lone island and fetches a few ancestors; one a child already awaits is part of a deep + // chain and fetches a large batch. + let by_head_count = self.by_head_request_count(block_root); + // If we know that this lookup has unknown parent (is awaiting a parent lookup to resolve), // signal here to hold processing downloaded data. - let mut lookup = - SingleBlockLookup::new(block_root, peers, peer_type, cx.next_id(), awaiting_parent); + let mut lookup = SingleBlockLookup::new( + block_root, + peers, + peer_type, + cx.next_id(), + awaiting_parent, + by_head_count, + ); let _guard = lookup.span.clone().entered(); // Add block components to the new request 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 ee97d88ce64..bb6684fcb3a 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 @@ -212,6 +212,9 @@ pub struct SingleBlockLookup { #[educe(Debug(method(fmt_peer_map_as_len)))] gloas_child_peers: HashMap, awaiting_parent: Option, + /// Count to request over `beacon_blocks_by_head`: small for a lone lookup, large once it's known + /// to be part of a deep chain. Fixed at creation from the lookup graph. + by_head_count: u64, created: Instant, pub(crate) span: Span, } @@ -223,6 +226,7 @@ impl SingleBlockLookup { peer_type: &PeerType, id: Id, awaiting_parent: Option, + by_head_count: u64, ) -> Self { let lookup_span = debug_span!( "lh_single_block_lookup", @@ -248,6 +252,7 @@ impl SingleBlockLookup { peers: block_peers, gloas_child_peers, awaiting_parent, + by_head_count, created: Instant::now(), span: lookup_span, } @@ -385,7 +390,12 @@ impl SingleBlockLookup { // === Block request === self.block_request.state.maybe_start_downloading(|| { - cx.block_lookup_request(self.id, self.peers.clone(), self.block_root) + cx.block_lookup_request( + self.id, + self.peers.clone(), + self.block_root, + self.by_head_count, + ) })?; if self.awaiting_parent.is_none() && let Some(data) = self.block_request.state.is_awaiting_process() diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 651571dd97c..ad64d151131 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -83,13 +83,6 @@ macro_rules! new_range_request_span { pub const MAX_COLUMN_RETRIES: usize = 3; /// Number of ancestors a block lookup requests when fetching a block via `beacon_blocks_by_head`. -/// The responder walks the block's parent chain and returns up to this many blocks (capped at -/// `MAX_REQUEST_BLOCKS_DENEB`), letting a single request fetch a run of ancestors instead of one -/// block per round-trip. -// TODO(tree-sync): the ancestors beyond the requested root are currently discarded by block -// lookups; they will be consumed to seed parent lookups in the header-backfill step. -pub const BLOCKS_BY_HEAD_REQUEST_COUNT: u64 = 32; - #[derive(Debug)] pub enum RpcEvent { StreamTermination, @@ -978,6 +971,7 @@ impl SyncNetworkContext { lookup_id: SingleLookupId, lookup_peers: Arc>>, block_root: Hash256, + by_head_count: u64, ) -> 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); @@ -1044,7 +1038,7 @@ impl SyncNetworkContext { // in a single request; otherwise fall back to `beacon_blocks_by_root` for the single block. Ok(LookupRequestResult::RequestSent( if self.peer_supports_blocks_by_head(&peer_id) { - self.send_blocks_by_head(peer_id, lookup_id, block_root)? + self.send_blocks_by_head(peer_id, lookup_id, block_root, by_head_count)? } else { self.send_block_by_root_request(peer_id, lookup_id, block_root)? }, @@ -1117,6 +1111,7 @@ impl SyncNetworkContext { peer_id: PeerId, lookup_id: SingleLookupId, block_root: Hash256, + count: u64, ) -> Result { let id = SingleLookupReqId { lookup_id, @@ -1126,7 +1121,7 @@ impl SyncNetworkContext { // with the events handled by `Self::on_blocks_by_head_response`. let request = BlocksByHeadRequest { beacon_root: block_root, - count: BLOCKS_BY_HEAD_REQUEST_COUNT, + count, }; self.network_send .send(NetworkMessage::SendRequest { @@ -1155,7 +1150,7 @@ impl SyncNetworkContext { // false = the peer may return fewer blocks than requested (e.g. reached genesis or // finalization), so the request completes on stream termination. false, - BlocksByHeadRequestItems::new(block_root, BLOCKS_BY_HEAD_REQUEST_COUNT as usize), + BlocksByHeadRequestItems::new(block_root, count as usize), request_span, ); diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index b483bc8f06f..4f44c249072 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -5,8 +5,10 @@ use crate::network_beacon_processor::sync_methods::WhichPeerToPenalize; use crate::network_beacon_processor::{ ChainSegmentProcessId, InvalidBlockStorage, NetworkBeaconProcessor, }; +use crate::sync::block_lookups::{ + BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT, BLOCKS_BY_HEAD_LONE_REQUEST_COUNT, +}; use crate::sync::block_lookups::{BlockLookupSummary, PARENT_DEPTH_TOLERANCE}; -use crate::sync::network_context::BLOCKS_BY_HEAD_REQUEST_COUNT; use crate::sync::{ SyncMessage, manager::{BatchProcessResult, BlockProcessType, SyncManager}, @@ -2176,11 +2178,13 @@ run_lookups_tests!( /// A peer that supports `beacon_blocks_by_head` should fetch a whole unknown ancestor chain with a /// single request, caching every block, so no `beacon_blocks_by_root` requests are needed. #[tokio::test] -async fn blocks_by_head_fetches_whole_chain_in_one_request() { +async fn blocks_by_head_request_count_grows_for_a_chain() { // Peers advertise `beacon_blocks_by_head`, so lookups fetch via it. let mut r = TestRig::default(PeerSupport::SupportsByHead); - // A chain that fits in a single by-head response and stays under `PARENT_DEPTH_TOLERANCE`. - r.build_chain(BLOCKS_BY_HEAD_REQUEST_COUNT as usize).await; + // Deep enough that the lone count doesn't resolve it, forcing a second (chain) request, while + // staying under `PARENT_DEPTH_TOLERANCE`. + let depth = BLOCKS_BY_HEAD_LONE_REQUEST_COUNT as usize + 4; + r.build_chain(depth).await; let peer_id = r.new_connected_supernode_peer(); // Trigger an unknown-parent lookup for the chain tip from that peer. let tip = r.get_last_block().block_cloned(); @@ -2188,10 +2192,27 @@ async fn blocks_by_head_fetches_whole_chain_in_one_request() { r.simulate(SimulateConfig::happy_path()).await; r.assert_successful_lookup_sync(); - // One by-head request fetched and cached the whole ancestor chain, so no by-root was needed. - let counts = r.requests_count(); - assert_eq!(counts.get("BlocksByHead").copied().unwrap_or(0), 1); - assert_eq!(counts.get("BlocksByRoot").copied().unwrap_or(0), 0); + // The lone first lookup fetches only a few ancestors; once the chain is proven deep the next + // request fetches a large batch. No by-root is used. + let by_head_counts = r + .requests + .iter() + .filter_map(|(req, _)| match req { + RequestType::BlocksByHead(req) => Some(req.count), + _ => None, + }) + .collect::>(); + assert_eq!( + by_head_counts, + vec![ + BLOCKS_BY_HEAD_LONE_REQUEST_COUNT, + BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT + ] + ); + assert_eq!( + r.requests_count().get("BlocksByRoot").copied().unwrap_or(0), + 0 + ); } /// Assert that lookup sync succeeds with the happy case From 5ddee3a822e660d63a26b0b9dd757224511605c6 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:11:35 +0200 Subject: [PATCH 18/27] Use the chain count for the by-head escalation test depth --- beacon_node/network/src/sync/tests/lookups.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 4f44c249072..b4bf5ee7464 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -2175,16 +2175,17 @@ run_lookups_tests!( depths: [1, 2], ); -/// A peer that supports `beacon_blocks_by_head` should fetch a whole unknown ancestor chain with a -/// single request, caching every block, so no `beacon_blocks_by_root` requests are needed. +/// A lone lookup over `beacon_blocks_by_head` fetches only a few ancestors; once the chain proves +/// deep the next request fetches a large batch, caching every block, so no `beacon_blocks_by_root` +/// requests are needed. #[tokio::test] async fn blocks_by_head_request_count_grows_for_a_chain() { // Peers advertise `beacon_blocks_by_head`, so lookups fetch via it. let mut r = TestRig::default(PeerSupport::SupportsByHead); // Deep enough that the lone count doesn't resolve it, forcing a second (chain) request, while // staying under `PARENT_DEPTH_TOLERANCE`. - let depth = BLOCKS_BY_HEAD_LONE_REQUEST_COUNT as usize + 4; - r.build_chain(depth).await; + r.build_chain(BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT as usize) + .await; let peer_id = r.new_connected_supernode_peer(); // Trigger an unknown-parent lookup for the chain tip from that peer. let tip = r.get_last_block().block_cloned(); From 9b7498a7238514f68890ea1297e62b2cf50d087d Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:17:40 +0200 Subject: [PATCH 19/27] Rename PeerSupport enum to ByHeadSupport { Supported, Unsupported } The subject (beacon_blocks_by_head) now lives in the type name, so the variants read clearly on their own instead of the ambiguous DoesNotSupport. --- beacon_node/network/src/sync/tests/lookups.rs | 112 +++++++++--------- beacon_node/network/src/sync/tests/mod.rs | 8 +- beacon_node/network/src/sync/tests/range.rs | 42 +++---- 3 files changed, 81 insertions(+), 81 deletions(-) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index b4bf5ee7464..10579892f62 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -232,7 +232,7 @@ pub(crate) struct TestRigConfig { /// Override the node custody type derived from `fulu_test_type` node_custody_type_override: Option, /// Which block-request protocol the rig's peers advertise. - peer_support: PeerSupport, + by_head_support: ByHeadSupport, } struct FullEmptyFork { @@ -343,16 +343,16 @@ impl TestRig { complete_strategy: <_>::default(), initial_block_lookups_metrics: <_>::default(), fulu_test_type: test_rig_config.fulu_test_type, - peer_support: test_rig_config.peer_support, + by_head_support: test_rig_config.by_head_support, } } - pub fn default(peer_support: PeerSupport) -> Self { + pub fn default(by_head_support: ByHeadSupport) -> Self { // Before Fulu, FuluTestType is irrelevant Self::new(TestRigConfig { fulu_test_type: FuluTestType::WeFullnodeThemSupernode, node_custody_type_override: None, - peer_support, + by_head_support, }) } @@ -361,7 +361,7 @@ impl TestRig { Self::new(TestRigConfig { fulu_test_type: FuluTestType::WeFullnodeThemSupernode, node_custody_type_override: Some(node_custody_type), - peer_support: PeerSupport::default(), + by_head_support: ByHeadSupport::default(), }) } @@ -1652,10 +1652,10 @@ impl TestRig { // Test setup - fn new_after_fulu(peer_support: PeerSupport) -> Option { + fn new_after_fulu(by_head_support: ByHeadSupport) -> Option { genesis_fork() .fulu_enabled() - .then(|| Self::default(peer_support)) + .then(|| Self::default(by_head_support)) } pub fn new_fulu_peer_test(fulu_test_type: FuluTestType) -> Option { @@ -1663,7 +1663,7 @@ impl TestRig { Self::new(TestRigConfig { fulu_test_type, node_custody_type_override: None, - peer_support: PeerSupport::default(), + by_head_support: ByHeadSupport::default(), }) }) } @@ -1838,14 +1838,14 @@ impl TestRig { self.log(&format!( "Added new peer for testing {peer_id:?}, custody: {peer_custody_str}" )); - if self.peer_support == PeerSupport::SupportsByHead { - self.set_peer_supports_by_head(peer_id); + if self.by_head_support == ByHeadSupport::Supported { + self.set_by_head_supports_by_head(peer_id); } peer_id } /// Mark a peer as advertising the `beacon_blocks_by_head` protocol so block lookups will use it. - fn set_peer_supports_by_head(&mut self, peer_id: PeerId) { + fn set_by_head_supports_by_head(&mut self, peer_id: PeerId) { self.network_globals .peers .write() @@ -1863,8 +1863,8 @@ impl TestRig { self.log(&format!( "Added new peer for testing {peer_id:?}, custody: supernode" )); - if self.peer_support == PeerSupport::SupportsByHead { - self.set_peer_supports_by_head(peer_id); + if self.by_head_support == ByHeadSupport::Supported { + self.set_by_head_supports_by_head(peer_id); } peer_id } @@ -1879,8 +1879,8 @@ impl TestRig { .peers .write() .__add_connected_peer(true, key, &self.harness.spec); - if self.peer_support == PeerSupport::SupportsByHead { - self.set_peer_supports_by_head(peer_id); + if self.by_head_support == ByHeadSupport::Supported { + self.set_by_head_supports_by_head(peer_id); } peer_id } @@ -2169,8 +2169,8 @@ macro_rules! run_lookups_tests { run_lookups_tests!( peers: [ - by_root => PeerSupport::DoesNotSupport, - by_head => PeerSupport::SupportsByHead, + by_root => ByHeadSupport::Unsupported, + by_head => ByHeadSupport::Supported, ], depths: [1, 2], ); @@ -2181,7 +2181,7 @@ run_lookups_tests!( #[tokio::test] async fn blocks_by_head_request_count_grows_for_a_chain() { // Peers advertise `beacon_blocks_by_head`, so lookups fetch via it. - let mut r = TestRig::default(PeerSupport::SupportsByHead); + let mut r = TestRig::default(ByHeadSupport::Supported); // Deep enough that the lone count doesn't resolve it, forcing a second (chain) request, while // staying under `PARENT_DEPTH_TOLERANCE`. r.build_chain(BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT as usize) @@ -2217,8 +2217,8 @@ async fn blocks_by_head_request_count_grows_for_a_chain() { } /// Assert that lookup sync succeeds with the happy case -async fn happy_path_unknown_attestation(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn happy_path_unknown_attestation(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); // We get attestation for a block descendant (depth) blocks of current head r.build_chain_and_trigger_last_block(depth).await; // Complete the request with good peer behaviour @@ -2226,8 +2226,8 @@ async fn happy_path_unknown_attestation(depth: usize, peer_support: PeerSupport) r.assert_successful_lookup_sync(); } -async fn happy_path_unknown_block_parent(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn happy_path_unknown_block_parent(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain(depth).await; r.trigger_with_last_unknown_block_parent(); r.simulate(SimulateConfig::happy_path()).await; @@ -2242,8 +2242,8 @@ async fn happy_path_unknown_block_parent(depth: usize, peer_support: PeerSupport } /// Assert that sync completes from an UnknownDataColumnParent -async fn happy_path_unknown_data_parent(depth: usize, peer_support: PeerSupport) { - let Some(mut r) = TestRig::new_after_fulu(peer_support) else { +async fn happy_path_unknown_data_parent(depth: usize, by_head_support: ByHeadSupport) { + let Some(mut r) = TestRig::new_after_fulu(by_head_support) else { return; }; // No unknown-parent data-column trigger post-Gloas. @@ -2257,8 +2257,8 @@ async fn happy_path_unknown_data_parent(depth: usize, peer_support: PeerSupport) } /// Assert that multiple trigger types don't create extra lookups -async fn happy_path_multiple_triggers(depth: usize, peer_support: PeerSupport) { - let Some(mut r) = TestRig::new_after_fulu(peer_support) else { +async fn happy_path_multiple_triggers(depth: usize, by_head_support: ByHeadSupport) { + let Some(mut r) = TestRig::new_after_fulu(by_head_support) else { return; }; // + 1, because the unknown parent trigger needs two new blocks @@ -2278,8 +2278,8 @@ async fn happy_path_multiple_triggers(depth: usize, peer_support: PeerSupport) { // Test bad behaviour of peers /// Assert that if peer responds with no blocks, we downscore, and retry the same lookup -async fn bad_peer_empty_block_response(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn bad_peer_empty_block_response(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that peer returns empty response once, then good behaviour r.simulate(SimulateConfig::new().return_no_blocks_once()) @@ -2293,8 +2293,8 @@ async fn bad_peer_empty_block_response(depth: usize, peer_support: PeerSupport) } /// Assert that if peer responds with no columns, we downscore, and retry the same lookup. -async fn bad_peer_empty_data_response(depth: usize, peer_support: PeerSupport) { - let Some(mut r) = TestRig::new_after_fulu(peer_support) else { +async fn bad_peer_empty_data_response(depth: usize, by_head_support: ByHeadSupport) { + let Some(mut r) = TestRig::new_after_fulu(by_head_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2311,8 +2311,8 @@ async fn bad_peer_empty_data_response(depth: usize, peer_support: PeerSupport) { /// Assert that if peer responds with not enough columns, we downscore, and retry the same /// lookup. -async fn bad_peer_too_few_data_response(depth: usize, peer_support: PeerSupport) { - let Some(mut r) = TestRig::new_after_fulu(peer_support) else { +async fn bad_peer_too_few_data_response(depth: usize, by_head_support: ByHeadSupport) { + let Some(mut r) = TestRig::new_after_fulu(by_head_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2328,8 +2328,8 @@ async fn bad_peer_too_few_data_response(depth: usize, peer_support: PeerSupport) } /// Assert that if peer responds with bad blocks, we downscore, and retry the same lookup -async fn bad_peer_wrong_block_response(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn bad_peer_wrong_block_response(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; r.simulate(SimulateConfig::new().return_wrong_blocks_once()) .await; @@ -2340,8 +2340,8 @@ async fn bad_peer_wrong_block_response(depth: usize, peer_support: PeerSupport) } /// Assert that if peer responds with bad columns, we downscore, and retry the same lookup. -async fn bad_peer_wrong_data_response(depth: usize, peer_support: PeerSupport) { - let Some(mut r) = TestRig::new_after_fulu(peer_support) else { +async fn bad_peer_wrong_data_response(depth: usize, by_head_support: ByHeadSupport) { + let Some(mut r) = TestRig::new_after_fulu(by_head_support) else { return; }; r.build_chain_and_trigger_last_block(depth).await; @@ -2359,8 +2359,8 @@ async fn bad_peer_wrong_data_response(depth: usize, peer_support: PeerSupport) { } /// Assert that on network error, we DON'T downscore, and retry the same lookup -async fn bad_peer_rpc_failure(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn bad_peer_rpc_failure(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; r.simulate(SimulateConfig::new().return_rpc_error(RPCError::UnsupportedProtocol)) .await; @@ -2371,8 +2371,8 @@ async fn bad_peer_rpc_failure(depth: usize, peer_support: PeerSupport) { // Test retry logic /// Assert that on too many download failures the lookup fails, but we can still sync -async fn too_many_download_failures(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn too_many_download_failures(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that a peer always returns empty r.simulate(SimulateConfig::new().return_no_blocks_always()) @@ -2390,8 +2390,8 @@ async fn too_many_download_failures(depth: usize, peer_support: PeerSupport) { } /// Assert that on too many processing failures the lookup fails, but we can still sync -async fn too_many_processing_failures(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn too_many_processing_failures(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; // Simulate that a peer always returns empty r.simulate( @@ -2420,7 +2420,7 @@ async fn too_many_processing_failures(depth: usize, peer_support: PeerSupport) { #[tokio::test] /// Assert that multiple trigger types don't create extra lookups async fn unknown_parent_does_not_add_peers_to_itself() { - let Some(mut r) = TestRig::new_after_fulu(PeerSupport::DoesNotSupport) else { + let Some(mut r) = TestRig::new_after_fulu(ByHeadSupport::Unsupported) else { return; }; // 2, because the unknown parent trigger needs two new blocks @@ -2455,7 +2455,7 @@ async fn unknown_parent_does_not_add_peers_to_itself() { /// Assert that a non-attributable processing error (e.g. processor overloaded) is retried up to /// `SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS`, no peer is penalized, and the lookup is then dropped. async fn test_single_block_lookup_ignored_response() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain_and_trigger_last_block(1).await; r.simulate( SimulateConfig::new().with_process_result(|| BlockProcessingResult::Error { @@ -2475,7 +2475,7 @@ async fn test_single_block_lookup_ignored_response() { #[tokio::test] /// 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(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); // The mock only covers block processing; Gloas also needs real envelope/column results. if r.is_after_gloas() { return; @@ -2493,8 +2493,8 @@ async fn test_single_block_lookup_duplicate_response() { } /// Assert that when peers disconnect the lookups are not dropped (kept with zero peers) -async fn peer_disconnected_then_rpc_error(depth: usize, peer_support: PeerSupport) { - let mut r = TestRig::default(peer_support); +async fn peer_disconnected_then_rpc_error(depth: usize, by_head_support: ByHeadSupport) { + let mut r = TestRig::default(by_head_support); r.build_chain_and_trigger_last_block(depth).await; r.assert_single_lookups_count(1); // The peer disconnect event reaches sync before the rpc error. @@ -2518,7 +2518,7 @@ async fn peer_disconnected_then_rpc_error(depth: usize, peer_support: PeerSuppor /// peers recursively from child to parent. async fn lookups_form_chain() { let depth = 5; - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(depth).await; for slot in (1..=depth).rev() { r.trigger_with_block_at_slot(slot as u64); @@ -2543,7 +2543,7 @@ async fn lookups_form_chain() { #[tokio::test] /// 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(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); // TODO(gloas): range sync does not fetch payload envelopes yet. if r.is_after_gloas() { return; @@ -2574,7 +2574,7 @@ async fn test_parent_lookup_too_deep_grow_ancestor_one() { #[tokio::test] async fn test_parent_lookup_too_deep_grow_ancestor_zero() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(PARENT_DEPTH_TOLERANCE).await; r.trigger_with_last_block(); r.simulate(SimulateConfig::happy_path()).await; @@ -2594,7 +2594,7 @@ async fn test_parent_lookup_too_deep_grow_ancestor_zero() { // ignored chains cache. The regression test still applies as the child lookup is not created #[tokio::test] async fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); let depth = PARENT_DEPTH_TOLERANCE + 1; r.build_chain(depth + 1).await; r.trigger_with_block_at_slot(depth as u64); @@ -2622,7 +2622,7 @@ async fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing /// Assert that if a lookup chain (by appending tips) is too long we drop it async fn test_parent_lookup_too_deep_grow_tip() { let depth = PARENT_DEPTH_TOLERANCE + 1; - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(depth).await; for slot in (1..=depth).rev() { r.trigger_with_block_at_slot(slot as u64); @@ -2646,7 +2646,7 @@ async fn test_parent_lookup_too_deep_grow_tip() { #[tokio::test] async fn test_skip_creating_ignored_parent_lookup() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(2).await; r.insert_ignored_chain(r.block_root_at_slot(1)); r.trigger_with_last_block(); @@ -2669,7 +2669,7 @@ async fn test_skip_creating_ignored_parent_lookup() { /// - Block 2: Import ok (parent block 1 is available) /// - Block 3: Import ok (parent block 2 is available) async fn test_same_chain_race_condition() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(3).await; let block_1_root = r.block_root_at_slot(1); @@ -2695,7 +2695,7 @@ async fn test_same_chain_race_condition() { /// 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 - let Some(mut r) = TestRig::new_after_fulu(PeerSupport::DoesNotSupport) else { + let Some(mut r) = TestRig::new_after_fulu(ByHeadSupport::Unsupported) else { return; }; // TODO(gloas): the helper does not populate the envelope missing-component path yet. @@ -2813,7 +2813,7 @@ async fn custody_lookup_permanent_custody_failures(test_type: FuluTestType) { // assert that signatures and kzg proofs are checked #[tokio::test] async fn crypto_on_fail_with_invalid_block_signature() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.build_chain(1).await; r.corrupt_last_block_signature(); r.trigger_with_last_block(); diff --git a/beacon_node/network/src/sync/tests/mod.rs b/beacon_node/network/src/sync/tests/mod.rs index 48e2fafc1d0..ad59df4e22f 100644 --- a/beacon_node/network/src/sync/tests/mod.rs +++ b/beacon_node/network/src/sync/tests/mod.rs @@ -56,10 +56,10 @@ type T = Witness; /// Whether the rig's peers advertise `beacon_blocks_by_head` (so block lookups fetch via it) or /// only `beacon_blocks_by_root`. #[derive(Clone, Copy, Default, PartialEq)] -enum PeerSupport { - SupportsByHead, +enum ByHeadSupport { + Supported, #[default] - DoesNotSupport, + Unsupported, } struct TestRig { @@ -102,7 +102,7 @@ struct TestRig { /// Fulu test type fulu_test_type: FuluTestType, /// Whether peers added to the rig advertise `beacon_blocks_by_head` so lookups fetch via it. - peer_support: PeerSupport, + by_head_support: ByHeadSupport, } enum FuluTestType { diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index 124bc930ffe..293f9ffc5a9 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -3,7 +3,7 @@ //! Tests follow the pattern from `lookups.rs`: //! ```ignore //! async fn test_name() { -//! let mut r = TestRig::default(PeerSupport::DoesNotSupport); +//! let mut r = TestRig::default(ByHeadSupport::Unsupported); //! r.setup_xyz().await; //! r.simulate(SimulateConfig::happy_path()).await; //! r.assert_range_sync_completed(); @@ -266,7 +266,7 @@ impl TestRig { /// Head sync: single peer slightly ahead → download batches → all blocks ingested. #[tokio::test] async fn head_sync_completes() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -280,7 +280,7 @@ async fn head_sync_completes() { /// then head chains are created from awaiting_head_peers to sync the remaining gap. #[tokio::test] async fn finalized_to_head_transition() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -294,7 +294,7 @@ async fn finalized_to_head_transition() { /// finalized epoch advances past genesis. #[tokio::test] async fn finalized_sync_completes() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -308,7 +308,7 @@ async fn finalized_sync_completes() { /// sync completes with no penalties (RPC errors are not penalized). #[tokio::test] async fn batch_rpc_error_retries() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -321,7 +321,7 @@ async fn batch_rpc_error_retries() { /// Peer returns zero blocks for a BlocksByRange request. Batch retries, sync completes. #[tokio::test] async fn batch_peer_returns_empty_then_succeeds() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_no_range_blocks_n_times(1)) .await; @@ -332,7 +332,7 @@ async fn batch_peer_returns_empty_then_succeeds() { /// Only exercises column logic on fulu+. #[tokio::test] async fn batch_peer_returns_no_columns_then_succeeds() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_no_range_columns_n_times(1)) .await; @@ -343,7 +343,7 @@ async fn batch_peer_returns_no_columns_then_succeeds() { /// Batch retries from another peer, sync completes. #[tokio::test] async fn batch_peer_returns_wrong_column_indices_then_succeeds() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_wrong_range_column_indices_n_times(1)) .await; @@ -354,7 +354,7 @@ async fn batch_peer_returns_wrong_column_indices_then_succeeds() { /// Batch retries from another peer, sync completes. #[tokio::test] async fn batch_peer_returns_wrong_column_slots_then_succeeds() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_wrong_range_column_slots_n_times(1)) .await; @@ -365,7 +365,7 @@ async fn batch_peer_returns_wrong_column_slots_then_succeeds() { /// missing columns → CouplingError::DataColumnPeerFailure → retry_partial_batch from other peers. #[tokio::test] async fn batch_peer_returns_partial_columns_then_succeeds() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if !r.fork_name.fulu_enabled() { return; } @@ -379,7 +379,7 @@ async fn batch_peer_returns_partial_columns_then_succeeds() { /// AwaitingDownload, retries without penalty, sync completes. #[tokio::test] async fn batch_non_faulty_failure_retries() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -393,7 +393,7 @@ async fn batch_non_faulty_failure_retries() { /// batch redownloaded from a different peer, sync completes. #[tokio::test] async fn batch_faulty_failure_redownloads() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -408,7 +408,7 @@ async fn batch_faulty_failure_redownloads() { /// Chain removed, all peers penalized with "faulty_chain". #[tokio::test] async fn batch_max_failures_removes_chain() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_faulty_failures(3)) .await; @@ -419,7 +419,7 @@ async fn batch_max_failures_removes_chain() { /// A new peer advertising the same finalized root gets disconnected with GoodbyeReason. #[tokio::test] async fn failed_chain_blacklisted() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); let remote_info = r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_range_faulty_failures(3)) .await; @@ -430,7 +430,7 @@ async fn failed_chain_blacklisted() { /// All peers disconnect before any request is fulfilled → chain removed (EmptyPeerPool). #[tokio::test] async fn all_peers_disconnect_removes_chain() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_disconnect_after_range_requests(0)) .await; @@ -441,7 +441,7 @@ async fn all_peers_disconnect_removes_chain() { /// for a chain that no longer exists — verified as a no-op (no crash). #[tokio::test] async fn late_response_for_removed_chain() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); r.setup_finalized_sync().await; r.simulate(SimulateConfig::happy_path().with_disconnect_after_range_requests(1)) .await; @@ -452,7 +452,7 @@ async fn late_response_for_removed_chain() { /// is paused. After 2 responses, EE comes back online, queued batches process, sync completes. #[tokio::test] async fn ee_offline_then_online_resumes_sync() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -467,7 +467,7 @@ async fn ee_offline_then_online_resumes_sync() { /// with sequential processing from the start. All blocks ingested. #[tokio::test] async fn finalized_sync_with_local_head_partial() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -480,7 +480,7 @@ async fn finalized_sync_with_local_head_partial() { /// final gap. Tests optimistic start where local head is near the target. #[tokio::test] async fn finalized_sync_with_local_head_near_target() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if r.skip_range_sync_under_gloas() { return; } @@ -501,7 +501,7 @@ async fn finalized_sync_with_local_head_near_target() { /// Once enough fullnodes + a supernode arrive, sync proceeds and completes. #[tokio::test] async fn not_enough_custody_peers_then_peers_arrive() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { return; } @@ -528,7 +528,7 @@ async fn not_enough_custody_peers_then_peers_arrive() { /// 5. finalized sync should resume as soon as CGC updates are received from peer 2 or 3. #[tokio::test] async fn finalized_sync_not_enough_custody_peers_resume_after_peer_cgc_update() { - let mut r = TestRig::default(PeerSupport::DoesNotSupport); + let mut r = TestRig::default(ByHeadSupport::Unsupported); if !r.fork_name.fulu_enabled() || r.skip_range_sync_under_gloas() { return; } From d8f5dc6a6567925a3efe91045715503caf729105 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:52:58 +0200 Subject: [PATCH 20/27] Fall back to other peers after a failed lookup download SingleLookupRequestState now records the peers that failed a download for the request, and block_lookup_request de-prioritizes them in peer selection (just below the concurrency-limit check). This stops a faulty by_head peer from being re-picked on every retry in a mixed peer set, letting the lookup fall back to a by_root peer instead of exhausting its attempts and dropping. Found in review. --- .../network/src/sync/block_lookups/mod.rs | 2 ++ .../sync/block_lookups/single_block_lookup.rs | 21 +++++++++++++++++++ beacon_node/network/src/sync/manager.rs | 2 ++ .../network/src/sync/network_context.rs | 6 +++++- 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 1a35eafc855..837fd015420 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -490,6 +490,7 @@ impl BlockLookups { pub fn on_block_download_response( &mut self, id: SingleLookupReqId, + peer_id: PeerId, response: BlocksDownloadResponse, cx: &mut SyncNetworkContext, ) { @@ -519,6 +520,7 @@ impl BlockLookups { let result = lookup.on_block_download_response( id.req_id, + peer_id, response.map(|d| DownloadResult { value: d.value.first_block, seen_timestamp: d.seen_timestamp, 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 bb6684fcb3a..87e36096b76 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 @@ -389,12 +389,15 @@ impl SingleBlockLookup { let _guard = self.span.clone().entered(); // === Block request === + // Snapshot before borrowing `block_request.state` mutably in `maybe_start_downloading`. + let failed_peers = self.block_request.state.failed_peers().clone(); self.block_request.state.maybe_start_downloading(|| { cx.block_lookup_request( self.id, self.peers.clone(), self.block_root, self.by_head_count, + &failed_peers, ) })?; if self.awaiting_parent.is_none() @@ -653,9 +656,13 @@ impl SingleBlockLookup { pub fn on_block_download_response( &mut self, req_id: ReqId, + peer_id: PeerId, result: BlockDownloadResponse, cx: &mut SyncNetworkContext, ) -> Result { + if result.is_err() { + self.block_request.state.record_failed_peer(peer_id); + } self.block_request .state .on_download_response(req_id, result)?; @@ -789,6 +796,9 @@ pub struct SingleLookupRequestState { failed_processing: u8, /// How many times have we attempted to download this block or blob. failed_downloading: u8, + /// Peers that failed a download for this request. De-prioritized when picking a peer so retries + /// fall back to another peer (e.g. a `by_root` peer when a `by_head` peer is faulty). + failed_peers: HashSet, } impl SingleLookupRequestState { @@ -797,9 +807,20 @@ impl SingleLookupRequestState { state: State::AwaitingDownload("not started"), failed_processing: 0, failed_downloading: 0, + failed_peers: HashSet::new(), } } + /// Peers that failed a download for this request. + pub fn failed_peers(&self) -> &HashSet { + &self.failed_peers + } + + /// Record that `peer_id` failed a download for this request. + pub fn record_failed_peer(&mut self, peer_id: PeerId) { + self.failed_peers.insert(peer_id); + } + pub fn is_awaiting_download(&self) -> bool { match self.state { State::AwaitingDownload { .. } => true, diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index e4e626093f7..ff94e878586 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -1130,6 +1130,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, + peer_id, resp.map(|(value, seen_timestamp)| { DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) }), @@ -1147,6 +1148,7 @@ impl SyncManager { if let Some(resp) = self.network.on_blocks_by_head_response(id, peer_id, block) { self.block_lookups.on_block_download_response( id, + peer_id, resp.map(|(value, seen_timestamp)| { DownloadResult::new(value, PeerGroup::from_single(peer_id), seen_timestamp) }), diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index ad64d151131..88f6badb8cb 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -972,6 +972,7 @@ impl SyncNetworkContext { lookup_peers: Arc>>, block_root: Hash256, by_head_count: u64, + failed_peers: &HashSet, ) -> 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); @@ -991,6 +992,9 @@ impl SyncNetworkContext { ( // Strictly de-prioritize peers already at the concurrent-request limit at_concurrency_limit, + // De-prioritize peers that already failed a download for this lookup, so a retry + // falls back to a fresh peer (e.g. a by_root peer when a by_head peer is faulty) + failed_peers.contains(peer), // Prefer peers that support `beacon_blocks_by_head` !supports_blocks_by_head, // Prefer peers with less overall requests @@ -1001,7 +1005,7 @@ impl SyncNetworkContext { ) }) .min() - .map(|(_, _, _, _, peer)| *peer) + .map(|(_, _, _, _, _, peer)| *peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive From 768ef00197ac7a8b131995184ac768285b84a774 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:40:41 +0200 Subject: [PATCH 21/27] Test peer fallback after a failed lookup download Add a rig test asserting a faulty by_head peer's failed download forces the retry to fall back to a by_root peer (1 BlocksByHead + 1 BlocksByRoot, lookup succeeds). Also restore the set_peer_supports_by_head helper name, mangled by the earlier ByHeadSupport rename. --- beacon_node/network/src/sync/tests/lookups.rs | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 10579892f62..ce2f637b98b 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1839,13 +1839,13 @@ impl TestRig { "Added new peer for testing {peer_id:?}, custody: {peer_custody_str}" )); if self.by_head_support == ByHeadSupport::Supported { - self.set_by_head_supports_by_head(peer_id); + self.set_peer_supports_by_head(peer_id); } peer_id } /// Mark a peer as advertising the `beacon_blocks_by_head` protocol so block lookups will use it. - fn set_by_head_supports_by_head(&mut self, peer_id: PeerId) { + fn set_peer_supports_by_head(&mut self, peer_id: PeerId) { self.network_globals .peers .write() @@ -1864,7 +1864,7 @@ impl TestRig { "Added new peer for testing {peer_id:?}, custody: supernode" )); if self.by_head_support == ByHeadSupport::Supported { - self.set_by_head_supports_by_head(peer_id); + self.set_peer_supports_by_head(peer_id); } peer_id } @@ -1880,7 +1880,7 @@ impl TestRig { .write() .__add_connected_peer(true, key, &self.harness.spec); if self.by_head_support == ByHeadSupport::Supported { - self.set_by_head_supports_by_head(peer_id); + self.set_peer_supports_by_head(peer_id); } peer_id } @@ -2216,6 +2216,34 @@ async fn blocks_by_head_request_count_grows_for_a_chain() { ); } +/// A peer that fails a lookup download is de-prioritized for the retry, so the lookup falls back to +/// another peer (here a `by_root` peer when the `by_head` peer is faulty) instead of re-picking the +/// failing peer until the lookup is dropped. +#[tokio::test] +async fn lookup_falls_back_to_another_peer_after_a_failed_download() { + // Peers are `by_root` by default; one peer additionally supports `by_head`. Supernode peers so + // the block's data columns (post-Fulu) are served and the lookup can complete. + let mut r = TestRig::default(ByHeadSupport::Unsupported); + let block_root = r.build_chain(1).await; + let by_head_peer = r.new_connected_supernode_peer(); + r.set_peer_supports_by_head(by_head_peer); + let by_root_peer = r.new_connected_supernode_peer(); + // Trigger from the by-head peer first so the lookup's first download goes to it, then add the + // by-root peer to the same lookup. + r.trigger_unknown_block_from_attestation(block_root, by_head_peer); + r.trigger_unknown_block_from_attestation(block_root, by_root_peer); + r.assert_single_lookups_count(1); + // The first (by-head) download returns empty and fails; the retry must fall back to the by-root + // peer rather than re-picking the failed by-head peer. + r.simulate(SimulateConfig::new().return_no_blocks_once()) + .await; + + r.assert_successful_lookup_sync(); + let counts = r.requests_count(); + assert_eq!(counts.get("BlocksByHead").copied().unwrap_or(0), 1); + assert_eq!(counts.get("BlocksByRoot").copied().unwrap_or(0), 1); +} + /// Assert that lookup sync succeeds with the happy case async fn happy_path_unknown_attestation(depth: usize, by_head_support: ByHeadSupport) { let mut r = TestRig::default(by_head_support); From 25ef766b0a4fd1d2983d0c69842b441dcedae928 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 25 Jun 2026 18:19:29 +0200 Subject: [PATCH 22/27] Address review: drop peer-fallback, guard future-slot blocks, fold anchor handling into get_parent_import_status - Remove the out-of-scope failed_peers peer-fallback (block lookups no longer de-prioritize peers that failed a download); it can be a separate PR. - Reject a downloaded block more than SLOT_IMPORT_TOLERANCE ahead of the current slot before creating a parent lookup, so a peer can't make us walk ancestors or force range sync to a bogus future head (cx.block_too_far_in_future). - Move the genesis/anchor 'block is itself in fork choice' case into get_parent_import_status via a proto-less ParentImportStatus::AlreadyImported, so is_parent_imported is a plain matches!. --- .../beacon_chain/src/block_verification.rs | 15 +++++--- .../network/src/sync/block_lookups/mod.rs | 2 - .../sync/block_lookups/single_block_lookup.rs | 25 ++----------- beacon_node/network/src/sync/manager.rs | 2 - .../network/src/sync/network_context.rs | 23 +++++++++--- beacon_node/network/src/sync/tests/lookups.rs | 37 +++++++------------ consensus/fork_choice/src/fork_choice.rs | 16 +++++--- 7 files changed, 54 insertions(+), 66 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 0de9a5cdb18..891d351f81f 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1455,6 +1455,8 @@ impl ExecutionPendingBlock { }); } } + // The block is itself already in fork choice; its parent was validated at import. + ParentImportStatus::AlreadyImported => {} ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), @@ -1926,12 +1928,13 @@ fn verify_parent_block_and_envelope_are_known( ) -> Result<(ProtoBlock, Arc>), BlockError> { match fork_choice_read_lock.get_parent_import_status(&block) { ParentImportStatus::Imported(parent) => Ok((parent, block)), - ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { - Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - parent_block_hash: block.payload_bid_parent_block_hash().ok(), - }) - } + // Unreachable for the not-yet-imported blocks this verifies; treat defensively as unknown. + ParentImportStatus::UnknownBlock + | ParentImportStatus::UnknownPayload + | ParentImportStatus::AlreadyImported => Err(BlockError::ParentUnknown { + parent_root: block.parent_root(), + parent_block_hash: block.payload_bid_parent_block_hash().ok(), + }), } } diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 309bfa70fa8..67d51ec5ae1 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -493,7 +493,6 @@ impl BlockLookups { pub fn on_block_download_response( &mut self, id: SingleLookupReqId, - peer_id: PeerId, response: BlocksDownloadResponse, cx: &mut SyncNetworkContext, ) { @@ -522,7 +521,6 @@ impl BlockLookups { let result = lookup.on_block_download_response( id.req_id, - peer_id, response.map(|d| DownloadResult { value: d.value.first_block, peer_group: d.peer_group, 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 352757c0879..c6912ca71a9 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 @@ -393,15 +393,12 @@ impl SingleBlockLookup { let _guard = self.span.clone().entered(); // === Block request === - // Snapshot before borrowing `block_request.state` mutably in `maybe_start_downloading`. - let failed_peers = self.block_request.state.failed_peers().clone(); self.block_request.state.maybe_start_downloading(|| { cx.block_lookup_request( self.id, self.peers.clone(), self.block_root, self.by_head_count, - &failed_peers, ) })?; if self.awaiting_parent.is_none() @@ -416,6 +413,10 @@ impl SingleBlockLookup { self.block_request.state.start_processing()?; } else if let Some(reason) = cx.conflicts_with_finality(block) { return Err(LookupRequestError::ConflictsWithFinality(reason)); + } else if let Some(reason) = cx.block_too_far_in_future(block) { + // A peer served a block too far ahead of our head for an unknown root. Drop it + // instead of walking ancestors / forcing range sync to a bogus future head. + return Err(LookupRequestError::Failed(reason)); } else { self.awaiting_parent = Some(AwaitingParent::from_block(block)); return Ok(LookupResult::ParentUnknown { @@ -648,13 +649,9 @@ impl SingleBlockLookup { pub fn on_block_download_response( &mut self, req_id: ReqId, - peer_id: PeerId, result: BlockDownloadResponse, cx: &mut SyncNetworkContext, ) -> Result { - if result.is_err() { - self.block_request.state.record_failed_peer(peer_id); - } self.block_request .state .on_download_response(req_id, result)?; @@ -783,9 +780,6 @@ pub struct SingleLookupRequestState { failed_processing: u8, /// How many times have we attempted to download this block or blob. failed_downloading: u8, - /// Peers that failed a download for this request. De-prioritized when picking a peer so retries - /// fall back to another peer (e.g. a `by_root` peer when a `by_head` peer is faulty). - failed_peers: HashSet, } impl SingleLookupRequestState { @@ -794,20 +788,9 @@ impl SingleLookupRequestState { state: State::AwaitingDownload("not started"), failed_processing: 0, failed_downloading: 0, - failed_peers: HashSet::new(), } } - /// Peers that failed a download for this request. - pub fn failed_peers(&self) -> &HashSet { - &self.failed_peers - } - - /// Record that `peer_id` failed a download for this request. - pub fn record_failed_peer(&mut self, peer_id: PeerId) { - self.failed_peers.insert(peer_id); - } - pub fn is_awaiting_download(&self) -> bool { match self.state { State::AwaitingDownload { .. } => true, diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 974140eefcf..3b73779ecf8 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -1135,7 +1135,6 @@ impl SyncManager { if let Some(resp) = self.network.on_single_block_response(id, peer_id, block) { self.block_lookups.on_block_download_response( id, - peer_id, resp.map(|value| DownloadResult::new(value, PeerGroup::from_single(peer_id))), &mut self.network, ) @@ -1151,7 +1150,6 @@ impl SyncManager { if let Some(resp) = self.network.on_blocks_by_head_response(id, peer_id, block) { self.block_lookups.on_block_download_response( id, - peer_id, 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 5081437c3f1..77e0546e6f4 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -8,7 +8,7 @@ pub use self::requests::{ }; use super::SyncMessage; use super::block_sidecar_coupling::RangeBlockComponentsRequest; -use super::manager::BlockProcessType; +use super::manager::{BlockProcessType, SLOT_IMPORT_TOLERANCE}; use crate::metrics; use crate::network_beacon_processor::NetworkBeaconProcessor; #[cfg(test)] @@ -493,6 +493,21 @@ impl SyncNetworkContext { } } + /// Returns `Some(reason)` if `block`'s slot is implausibly far ahead of the current wall-clock + /// slot — a peer shouldn't be serving it, so it must not trigger parent lookups or forced range + /// sync. Compared against the clock (not our head) so normal deep sync isn't rejected. + pub fn block_too_far_in_future(&self, block: &SignedBeaconBlock) -> Option { + let current_slot = self.chain.slot().ok()?; + if block.slot().as_u64() > current_slot.as_u64() + SLOT_IMPORT_TOLERANCE as u64 { + Some(format!( + "block slot {} more than {SLOT_IMPORT_TOLERANCE} ahead of current slot {current_slot}", + block.slot() + )) + } else { + None + } + } + /// Returns the Client type of the peer if known pub fn client_type(&self, peer_id: &PeerId) -> Client { self.network_globals() @@ -965,7 +980,6 @@ impl SyncNetworkContext { lookup_peers: Arc>>, block_root: Hash256, by_head_count: u64, - failed_peers: &HashSet, ) -> 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); @@ -985,9 +999,6 @@ impl SyncNetworkContext { ( // Strictly de-prioritize peers already at the concurrent-request limit at_concurrency_limit, - // De-prioritize peers that already failed a download for this lookup, so a retry - // falls back to a fresh peer (e.g. a by_root peer when a by_head peer is faulty) - failed_peers.contains(peer), // Prefer peers that support `beacon_blocks_by_head` !supports_blocks_by_head, // Prefer peers with less overall requests @@ -998,7 +1009,7 @@ impl SyncNetworkContext { ) }) .min() - .map(|(_, _, _, _, _, peer)| *peer) + .map(|(_, _, _, _, peer)| *peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 7864d8a36ee..2ae0bae9010 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -11,7 +11,7 @@ use crate::sync::block_lookups::{ use crate::sync::block_lookups::{BlockLookupSummary, PARENT_DEPTH_TOLERANCE}; use crate::sync::{ SyncMessage, - manager::{BatchProcessResult, BlockProcessType, SyncManager}, + manager::{BatchProcessResult, BlockProcessType, SLOT_IMPORT_TOLERANCE, SyncManager}, }; use beacon_chain::block_verification_types::LookupBlock; use beacon_chain::custody_context::NodeCustodyType; @@ -2532,32 +2532,21 @@ async fn blocks_by_head_request_count_grows_for_a_chain() { ); } -/// A peer that fails a lookup download is de-prioritized for the retry, so the lookup falls back to -/// another peer (here a `by_root` peer when the `by_head` peer is faulty) instead of re-picking the -/// failing peer until the lookup is dropped. +/// A peer serves a block whose slot is far ahead of the wall-clock slot. It must be dropped, not +/// turned into a parent lookup / forced range sync to a bogus future head. #[tokio::test] -async fn lookup_falls_back_to_another_peer_after_a_failed_download() { - // Peers are `by_root` by default; one peer additionally supports `by_head`. Supernode peers so - // the block's data columns (post-Fulu) are served and the lookup can complete. +async fn lookup_drops_block_too_far_in_future() { let mut r = TestRig::default(); - let block_root = r.build_chain(1).await; - let by_head_peer = r.new_connected_supernode_peer(); - r.set_peer_supports_by_head(by_head_peer); - let by_root_peer = r.new_connected_supernode_peer(); - // Trigger from the by-head peer first so the lookup's first download goes to it, then add the - // by-root peer to the same lookup. - r.trigger_unknown_block_from_attestation(block_root, by_head_peer); - r.trigger_unknown_block_from_attestation(block_root, by_root_peer); - r.assert_single_lookups_count(1); - // The first (by-head) download returns empty and fails; the retry must fall back to the by-root - // peer rather than re-picking the failed by-head peer. - r.simulate(SimulateConfig::new().return_no_blocks_once()) - .await; + // Tip is `> SLOT_IMPORT_TOLERANCE` slots ahead of where we hold the clock, so the served block + // looks far in the future. Set the clock directly (back) to dodge the harness epoch guard. + let block_root = r.build_chain(SLOT_IMPORT_TOLERANCE + 2).await; + r.harness.chain.slot_clock.set_slot(1); + let peer = r.new_connected_supernode_peer(); + r.trigger_unknown_block_from_attestation(block_root, peer); + r.simulate(SimulateConfig::happy_path()).await; - r.assert_successful_lookup_sync(); - let counts = r.requests_count(); - assert_eq!(counts.get("BlocksByHead").copied().unwrap_or(0), 1); - assert_eq!(counts.get("BlocksByRoot").copied().unwrap_or(0), 1); + // The lookup is dropped on download; no parent lookup and no range sync are spawned. + r.assert_failed_lookup_sync(); } #[tokio::test] diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index f56d7e0e629..66bba5d15d9 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -217,6 +217,10 @@ pub enum ParentImportStatus { UnknownBlock, /// The parent block is known, but the child's bid commits to a payload not known to fork choice. UnknownPayload, + /// The block is itself in fork choice (e.g. genesis/anchor, or a finalized block whose parent + /// was pruned), so its parent was imported. No parent `ProtoBlock` is carried because the anchor + /// has no queryable proto-node (`contains_block` is true but `get_block` returns `None`). + AlreadyImported, } impl From for Error { @@ -1591,13 +1595,10 @@ where /// Returns `true` if the block's parent is imported (and, for a post-Gloas FULL child, its /// parent's payload is imported too). See [`Self::get_parent_import_status`]. pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { - // A block that is itself in fork choice (e.g. genesis, or a finalized block whose parent has - // been pruned) had an imported parent. `contains_block` is used rather than `get_block` - // because the anchor block is present in the index but has no queryable proto-node. matches!( self.get_parent_import_status(block), - ParentImportStatus::Imported(_) - ) || self.contains_block(&block.canonical_root()) + ParentImportStatus::Imported(_) | ParentImportStatus::AlreadyImported + ) } /// Returns the import status of the parent of `block`. @@ -1617,6 +1618,11 @@ where } else { ParentImportStatus::Imported(parent_block) } + } else if self.contains_block(&block.canonical_root()) { + // The block is itself in fork choice (e.g. genesis/anchor, or a finalized block whose + // parent was pruned), so its parent was imported. `contains_block` is used rather than + // `get_block` because the anchor is in the index but has no queryable proto-node. + ParentImportStatus::AlreadyImported } else { ParentImportStatus::UnknownBlock } From b46eefb570791f824c064480770f6e3702e00141 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 26 Jun 2026 11:51:06 +0200 Subject: [PATCH 23/27] Keep anchor special-case out of consensus get_parent_import_status Revert the consensus-side move from the previous commit: get_parent_import_status returns to pure parent-only semantics (no AlreadyImported variant) and block_verification.rs is restored byte-identical to upstream. The genesis/anchor 'block is itself imported' case lives only in the sync-facing is_parent_imported via '|| contains_block', so consensus block-import paths are untouched. --- .../beacon_chain/src/block_verification.rs | 15 ++++++--------- consensus/fork_choice/src/fork_choice.rs | 16 +++++----------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 891d351f81f..0de9a5cdb18 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1455,8 +1455,6 @@ impl ExecutionPendingBlock { }); } } - // The block is itself already in fork choice; its parent was validated at import. - ParentImportStatus::AlreadyImported => {} ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), @@ -1928,13 +1926,12 @@ fn verify_parent_block_and_envelope_are_known( ) -> Result<(ProtoBlock, Arc>), BlockError> { match fork_choice_read_lock.get_parent_import_status(&block) { ParentImportStatus::Imported(parent) => Ok((parent, block)), - // Unreachable for the not-yet-imported blocks this verifies; treat defensively as unknown. - ParentImportStatus::UnknownBlock - | ParentImportStatus::UnknownPayload - | ParentImportStatus::AlreadyImported => Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - parent_block_hash: block.payload_bid_parent_block_hash().ok(), - }), + ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { + Err(BlockError::ParentUnknown { + parent_root: block.parent_root(), + parent_block_hash: block.payload_bid_parent_block_hash().ok(), + }) + } } } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 66bba5d15d9..f56d7e0e629 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -217,10 +217,6 @@ pub enum ParentImportStatus { UnknownBlock, /// The parent block is known, but the child's bid commits to a payload not known to fork choice. UnknownPayload, - /// The block is itself in fork choice (e.g. genesis/anchor, or a finalized block whose parent - /// was pruned), so its parent was imported. No parent `ProtoBlock` is carried because the anchor - /// has no queryable proto-node (`contains_block` is true but `get_block` returns `None`). - AlreadyImported, } impl From for Error { @@ -1595,10 +1591,13 @@ where /// Returns `true` if the block's parent is imported (and, for a post-Gloas FULL child, its /// parent's payload is imported too). See [`Self::get_parent_import_status`]. pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { + // A block that is itself in fork choice (e.g. genesis, or a finalized block whose parent has + // been pruned) had an imported parent. `contains_block` is used rather than `get_block` + // because the anchor block is present in the index but has no queryable proto-node. matches!( self.get_parent_import_status(block), - ParentImportStatus::Imported(_) | ParentImportStatus::AlreadyImported - ) + ParentImportStatus::Imported(_) + ) || self.contains_block(&block.canonical_root()) } /// Returns the import status of the parent of `block`. @@ -1618,11 +1617,6 @@ where } else { ParentImportStatus::Imported(parent_block) } - } else if self.contains_block(&block.canonical_root()) { - // The block is itself in fork choice (e.g. genesis/anchor, or a finalized block whose - // parent was pruned), so its parent was imported. `contains_block` is used rather than - // `get_block` because the anchor is in the index but has no queryable proto-node. - ParentImportStatus::AlreadyImported } else { ParentImportStatus::UnknownBlock } From 9a95c8c68aca34343aeba2dd2ab3c2534f1819b0 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:04:20 +0200 Subject: [PATCH 24/27] Address review: BlocksByHead tuple variant, drop request-count tiebreaker, finality uses Failed, lookup tidy-ups - SyncRequestId::BlocksByHead uses a tuple variant. - Drop the per-peer request-count tiebreaker from lookup peer selection (and the now-unused active_request_count_by_peer helper). - Reuse LookupRequestError::Failed for finality-conflicting blocks; remove the unused ConflictsWithFinality variant. - Collect known ancestors via iterator; tidy two comments. --- .../src/service/api_types.rs | 2 +- .../network/src/sync/block_lookups/mod.rs | 30 ++++++----- .../sync/block_lookups/single_block_lookup.rs | 4 +- beacon_node/network/src/sync/manager.rs | 4 +- .../network/src/sync/network_context.rs | 52 ++----------------- 5 files changed, 23 insertions(+), 69 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/api_types.rs b/beacon_node/lighthouse_network/src/service/api_types.rs index de55a696a9e..489ea2794d6 100644 --- a/beacon_node/lighthouse_network/src/service/api_types.rs +++ b/beacon_node/lighthouse_network/src/service/api_types.rs @@ -22,7 +22,7 @@ pub enum SyncRequestId { /// Request searching for a block given a hash. SingleBlock { id: SingleLookupReqId }, /// Request walking the ancestors of a block given its root, via `beacon_blocks_by_head`. - BlocksByHead { id: SingleLookupReqId }, + BlocksByHead(SingleLookupReqId), /// Request searching for a payload envelope given a hash. SinglePayloadEnvelope { id: SingleLookupReqId }, /// Request searching for a set of data columns given a hash and list of column indices. diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 67d51ec5ae1..f45a48f59ec 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -436,9 +436,8 @@ impl BlockLookups { return false; } - // Read the by-head count before inserting this lookup below: a lookup nothing yet awaits is - // a lone island and fetches a few ancestors; one a child already awaits is part of a deep - // chain and fetches a large batch. + // Compute before inserting below: a lookup nothing awaits is a lone island (fetch a few + // ancestors); one a child already awaits is part of a deep chain (fetch a large batch). let by_head_count = self.by_head_request_count(block_root); // If we know that this lookup has unknown parent (is awaiting a parent lookup to resolve), @@ -506,17 +505,19 @@ impl BlockLookups { // - If yes: sends block for processing // - If not: marks the lookup as awaiting parent let known_ancestors = response.as_ref().ok().map(|d| { - let mut known_ancestors = HashMap::new(); - for block in &d.value.ancestor_blocks { - known_ancestors.insert( - block.canonical_root(), - DownloadResult { - value: block.clone(), - peer_group: d.peer_group.clone(), - }, - ); - } - known_ancestors + d.value + .ancestor_blocks + .iter() + .map(|block| { + ( + block.canonical_root(), + DownloadResult { + value: block.clone(), + peer_group: d.peer_group.clone(), + }, + ) + }) + .collect::>() }); let result = lookup.on_block_download_response( @@ -717,6 +718,7 @@ impl BlockLookups { block_root, peers, }) => { + // If a prior by-head response already fetched the parent, seed its lookup with it. let block_component = known_blocks.as_ref().and_then(|p| { p.get(&parent_root) .map(|b| BlockComponent::Block(b.clone())) 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 c6912ca71a9..049f768e5dd 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 @@ -44,8 +44,6 @@ pub enum LookupResult { pub enum LookupRequestError { /// Too many failed attempts TooManyAttempts, - /// This block is not a descendant of the finalized checkpoint - ConflictsWithFinality(String), /// Error sending event to network SendFailedNetwork(RpcRequestSendError), /// Error sending event to processor @@ -412,7 +410,7 @@ impl SingleBlockLookup { .map_err(LookupRequestError::SendFailedProcessor)?; self.block_request.state.start_processing()?; } else if let Some(reason) = cx.conflicts_with_finality(block) { - return Err(LookupRequestError::ConflictsWithFinality(reason)); + return Err(LookupRequestError::Failed(reason)); } else if let Some(reason) = cx.block_too_far_in_future(block) { // A peer served a block too far ahead of our head for an unknown root. Drop it // instead of walking ancestors / forcing range sync to a bogus future head. diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 3b73779ecf8..ea92af7653f 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -498,7 +498,7 @@ impl SyncManager { SyncRequestId::SingleBlock { id } => { self.on_single_block_response(id, peer_id, RpcEvent::RPCError(error)) } - SyncRequestId::BlocksByHead { id } => { + SyncRequestId::BlocksByHead(id) => { self.on_blocks_by_head_response(id, peer_id, RpcEvent::RPCError(error)) } SyncRequestId::SinglePayloadEnvelope { id } => { @@ -1114,7 +1114,7 @@ impl SyncManager { SyncRequestId::SingleBlock { id } => { self.on_single_block_response(id, peer_id, RpcEvent::from_chunk(block)) } - SyncRequestId::BlocksByHead { id } => { + SyncRequestId::BlocksByHead(id) => { self.on_blocks_by_head_response(id, peer_id, RpcEvent::from_chunk(block)) } SyncRequestId::BlocksByRange(id) => { diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 77e0546e6f4..d1e562743ad 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -425,7 +425,7 @@ impl SyncNetworkContext { let blocks_by_head_ids = blocks_by_head_requests .active_requests_of_peer(peer_id) .into_iter() - .map(|id| SyncRequestId::BlocksByHead { id: *id }); + .map(|id| SyncRequestId::BlocksByHead(*id)); let payload_envelopes_by_root_ids = payload_envelopes_by_root_requests .active_requests_of_peer(peer_id) .into_iter() @@ -542,49 +542,6 @@ impl SyncNetworkContext { } } - fn active_request_count_by_peer(&self) -> HashMap { - let Self { - network_send: _, - request_id: _, - blocks_by_root_requests, - blocks_by_head_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(blocks_by_head_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 @@ -981,7 +938,6 @@ impl SyncNetworkContext { block_root: Hash256, by_head_count: u64, ) -> 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 blocks_by_head_per_peer = ActiveRequestsPerPeer::new(&self.blocks_by_head_requests); let Some(peer_id) = lookup_peers @@ -1001,15 +957,13 @@ impl SyncNetworkContext { at_concurrency_limit, // Prefer peers that support `beacon_blocks_by_head` !supports_blocks_by_head, - // Prefer peers with less overall requests - active_request_count_by_peer.get(peer).copied().unwrap_or(0), // Random factor to break ties, otherwise the PeerID breaks ties rand::random::(), peer, ) }) .min() - .map(|(_, _, _, _, peer)| *peer) + .map(|(_, _, _, peer)| *peer) else { // Allow lookup to not have any peers and do nothing. This is an optimization to not // lose progress of lookups created from a block with unknown parent before we receive @@ -1135,7 +1089,7 @@ impl SyncNetworkContext { .send(NetworkMessage::SendRequest { peer_id, request: RequestType::BlocksByHead(request), - app_request_id: AppRequestId::Sync(SyncRequestId::BlocksByHead { id }), + app_request_id: AppRequestId::Sync(SyncRequestId::BlocksByHead(id)), }) .map_err(|_| RpcRequestSendError::InternalError("network send error".to_owned()))?; From 8a045fde50696a85d560d8a43549976af9f94f5d Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:17:43 +0200 Subject: [PATCH 25/27] Use build_chain_and_trigger_last_block helper in by-head count test --- beacon_node/network/src/sync/tests/lookups.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 2ae0bae9010..af3ca214864 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -2500,12 +2500,8 @@ async fn blocks_by_head_request_count_grows_for_a_chain() { let mut r = TestRig::new(ByHeadSupport::Supported); // Deep enough that the lone count doesn't resolve it, forcing a second (chain) request, while // staying under `PARENT_DEPTH_TOLERANCE`. - r.build_chain(BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT as usize) + r.build_chain_and_trigger_last_block(BLOCKS_BY_HEAD_CHAIN_REQUEST_COUNT as usize) .await; - let peer_id = r.new_connected_supernode_peer(); - // Trigger an unknown-parent lookup for the chain tip from that peer. - let tip = r.get_last_block().block_cloned(); - r.trigger_unknown_parent_block(peer_id, tip); r.simulate(SimulateConfig::happy_path()).await; r.assert_successful_lookup_sync(); From 2ac6141da8374b228127028c4bd76ed981260b5d Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:33:14 +0200 Subject: [PATCH 26/27] Handle anchor/self-imported block in sync, restore fork_choice to upstream is_parent_imported goes back to the upstream-pure wrapper of get_parent_import_status (no contains_block, no block hashing on the consensus block-verification path). The genesis/anchor case (a looked-up block already in fork choice, or whose parent was pruned by finalization) is handled in the lookup eager path via block_is_known_to_fork_choice, keyed on the root the lookup already holds. --- .../network/src/sync/block_lookups/single_block_lookup.rs | 8 ++++++-- consensus/fork_choice/src/fork_choice.rs | 5 +---- 2 files changed, 7 insertions(+), 6 deletions(-) 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 049f768e5dd..58dd1da1cbc 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 @@ -404,8 +404,12 @@ impl SingleBlockLookup { { let block = &data.value; // Eagerly check if we can import without having to send the block for processing. This - // allows us to check many lookups in the same sync execution / loop. - if cx.chain.is_parent_imported(block) { + // allows us to check many lookups in the same sync execution / loop. The block may also + // already be in fork choice itself (e.g. genesis/anchor, or one whose parent has been + // pruned by finalization), in which case it's importable rather than parent-unknown. + if cx.chain.is_parent_imported(block) + || cx.chain.block_is_known_to_fork_choice(&self.block_root) + { cx.send_block_for_processing(self.id, self.block_root, block.clone()) .map_err(LookupRequestError::SendFailedProcessor)?; self.block_request.state.start_processing()?; diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index f56d7e0e629..95c8f70a044 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1591,13 +1591,10 @@ where /// Returns `true` if the block's parent is imported (and, for a post-Gloas FULL child, its /// parent's payload is imported too). See [`Self::get_parent_import_status`]. pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { - // A block that is itself in fork choice (e.g. genesis, or a finalized block whose parent has - // been pruned) had an imported parent. `contains_block` is used rather than `get_block` - // because the anchor block is present in the index but has no queryable proto-node. matches!( self.get_parent_import_status(block), ParentImportStatus::Imported(_) - ) || self.contains_block(&block.canonical_root()) + ) } /// Returns the import status of the parent of `block`. From 59d231ec165f39b5b16f86178920f0004a651450 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:59:42 +0200 Subject: [PATCH 27/27] Add beacon_blocks_by_head lookup-sync metrics - libp2p_peers_supporting_blocks_by_head: connected peers advertising the protocol, counted alongside the other per-peer gauges. - sync_blocks_by_head_requested_ancestors: histogram of the requested-ancestor count. - sync_lookups_known_parent_reused_total: blocks reused from a prior by-head response to seed a parent lookup instead of re-downloading (KnownParents optimization). (by-head request timings already exist via sync_rpc_request_duration_sec{protocol}.) --- beacon_node/lighthouse_network/src/metrics.rs | 7 +++++++ .../lighthouse_network/src/peer_manager/mod.rs | 11 +++++++++++ beacon_node/network/src/metrics.rs | 16 ++++++++++++++++ .../network/src/sync/block_lookups/mod.rs | 3 +++ beacon_node/network/src/sync/network_context.rs | 4 ++++ 5 files changed, 41 insertions(+) diff --git a/beacon_node/lighthouse_network/src/metrics.rs b/beacon_node/lighthouse_network/src/metrics.rs index d5d1ed50534..714e967d17f 100644 --- a/beacon_node/lighthouse_network/src/metrics.rs +++ b/beacon_node/lighthouse_network/src/metrics.rs @@ -11,6 +11,13 @@ pub static PEERS_CONNECTED: LazyLock> = LazyLock::new(|| { try_create_int_gauge("libp2p_peers", "Count of libp2p peers currently connected") }); +pub static PEERS_SUPPORTING_BLOCKS_BY_HEAD: LazyLock> = LazyLock::new(|| { + try_create_int_gauge( + "libp2p_peers_supporting_blocks_by_head", + "Count of connected peers advertising the beacon_blocks_by_head protocol", + ) +}); + pub static PEERS_CONNECTED_MULTI: LazyLock> = LazyLock::new(|| { try_create_int_gauge_vec( "libp2p_peers_multi", diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 82fac58023d..0442daec1bb 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1555,6 +1555,7 @@ impl PeerManager { // Update peer count related metrics. fn update_peer_count_metrics(&self) { let mut peers_connected = 0; + let mut peers_supporting_blocks_by_head: i64 = 0; let mut clients_per_peer = HashMap::new(); let mut inbound_ipv4_peers_connected: usize = 0; let mut inbound_ipv6_peers_connected: usize = 0; @@ -1564,6 +1565,10 @@ impl PeerManager { for (_, peer_info) in self.network_globals.peers.read().connected_peers() { peers_connected += 1; + if peer_info.supports_protocol(Protocol::BlocksByHead) { + peers_supporting_blocks_by_head += 1; + } + *clients_per_peer .entry(peer_info.client().kind.to_string()) .or_default() += 1; @@ -1624,6 +1629,12 @@ impl PeerManager { // PEERS_CONNECTED metrics::set_gauge(&metrics::PEERS_CONNECTED, peers_connected); + // PEERS_SUPPORTING_BLOCKS_BY_HEAD + metrics::set_gauge( + &metrics::PEERS_SUPPORTING_BLOCKS_BY_HEAD, + peers_supporting_blocks_by_head, + ); + // CUSTODY_GROUP_COUNT for (custody_group_count, peer_count) in peers_per_custody_group_count.into_iter() { metrics::set_gauge_vec( diff --git a/beacon_node/network/src/metrics.rs b/beacon_node/network/src/metrics.rs index 07e6d7fdb24..da9694137db 100644 --- a/beacon_node/network/src/metrics.rs +++ b/beacon_node/network/src/metrics.rs @@ -516,6 +516,22 @@ pub static SYNC_LOOKUPS_STUCK: LazyLock> = LazyLock::new(|| { "Total count of sync lookups that are stuck and dropped", ) }); +pub static SYNC_BLOCKS_BY_HEAD_REQUESTED_ANCESTORS: LazyLock> = LazyLock::new( + || { + try_create_histogram_with_buckets( + "sync_blocks_by_head_requested_ancestors", + "Distribution of the count parameter (ancestors requested) in beacon_blocks_by_head requests", + Ok(vec![1.0, 2.0, 4.0, 8.0, 16.0, 32.0, 48.0, 64.0]), + ) + }, +); +pub static SYNC_LOOKUP_KNOWN_PARENT_REUSED: LazyLock> = LazyLock::new(|| { + try_create_int_counter( + "sync_lookups_known_parent_reused_total", + "Total blocks reused from a prior beacon_blocks_by_head response to seed a parent lookup, \ + instead of being re-downloaded (KnownParents optimization)", + ) +}); pub static SYNC_ACTIVE_NETWORK_REQUESTS: LazyLock> = LazyLock::new(|| { try_create_int_gauge_vec( "sync_active_network_requests", diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index f45a48f59ec..b1285cfc071 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -723,6 +723,9 @@ impl BlockLookups { p.get(&parent_root) .map(|b| BlockComponent::Block(b.clone())) }); + if block_component.is_some() { + metrics::inc_counter(&metrics::SYNC_LOOKUP_KNOWN_PARENT_REUSED); + } if self.search_parent_of_child( parent_root, block_component, diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index d1e562743ad..a82b60f9121 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -1081,6 +1081,10 @@ impl SyncNetworkContext { }; // Lookup sync event safety: see `send_block_by_root_request`. The same guarantees apply, // with the events handled by `Self::on_blocks_by_head_response`. + metrics::observe( + &metrics::SYNC_BLOCKS_BY_HEAD_REQUESTED_ANCESTORS, + count as f64, + ); let request = BlocksByHeadRequest { beacon_root: block_root, count,