From 4c80d82948bb1de69bccf8e0ea236b417e09db9b Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 27 May 2026 21:59:06 -0600 Subject: [PATCH] Fix tests --- .../network/src/sync/block_lookups/mod.rs | 49 +++++++++++++++++-- .../sync/block_lookups/single_block_lookup.rs | 35 +++++++------ beacon_node/network/src/sync/manager.rs | 4 +- beacon_node/network/src/sync/tests/lookups.rs | 2 + 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index d73dd83248..86ee42c32c 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -87,6 +87,12 @@ pub enum BlockComponent { pub type SingleLookupId = u32; +#[derive(Debug, Copy, Clone)] +pub enum NewLookupTrigger { + ParentUnknown(Hash256), + NetworkMessage, +} + pub struct BlockLookups { /// A cache of block roots that must be ignored for some time to prevent useless searches. For /// example if a chain is too long, its lookup chain is dropped, and range sync is expected to @@ -176,12 +182,18 @@ impl BlockLookups { block_component: BlockComponent, awaiting_parent: AwaitingParent, peer_id: PeerId, + new_lookup_trigger: NewLookupTrigger, cx: &mut SyncNetworkContext, ) -> bool { // We don't know the child's fork yet (no block downloaded), use PreGloas conservatively. // The correct AwaitingParent will be set when the child's block downloads. - let parent_lookup_exists = - self.search_parent_of_child(awaiting_parent, block_root, &[peer_id], cx); + let parent_lookup_exists = self.search_parent_of_child( + awaiting_parent, + block_root, + &[peer_id], + new_lookup_trigger, + cx, + ); // Only create the child lookup if the parent exists if parent_lookup_exists { // `search_parent_of_child` ensures that the parent lookup exists so we can safely wait for it @@ -194,6 +206,7 @@ impl BlockLookups { // the lookup with zero peers to house the block components. &[], &PeerType::PreGloas, + new_lookup_trigger, cx, ) } else { @@ -209,9 +222,18 @@ impl BlockLookups { &mut self, block_root: Hash256, peer_source: &[PeerId], + new_lookup_trigger: NewLookupTrigger, cx: &mut SyncNetworkContext, ) -> bool { - self.new_current_lookup(block_root, None, None, peer_source, &PeerType::PreGloas, cx) + self.new_current_lookup( + block_root, + None, + None, + peer_source, + &PeerType::PreGloas, + new_lookup_trigger, + cx, + ) } /// A block or blob triggers the search of a parent. @@ -226,6 +248,7 @@ impl BlockLookups { awaiting_parent: AwaitingParent, child_block_root_trigger: Hash256, peers: &[PeerId], + new_lookup_trigger: NewLookupTrigger, cx: &mut SyncNetworkContext, ) -> bool { let block_root_to_search = awaiting_parent.parent_root(); @@ -331,7 +354,15 @@ impl BlockLookups { None => PeerType::PreGloas, }; // `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, + None, + None, + peers, + &peer_type, + new_lookup_trigger, + cx, + ) } /// Searches for a single block hash. If the blocks parent is unknown, a chain of blocks is @@ -345,6 +376,7 @@ impl BlockLookups { awaiting_parent: Option, peers: &[PeerId], peer_type: &PeerType, + new_lookup_trigger: NewLookupTrigger, cx: &mut SyncNetworkContext, ) -> bool { // If this block or it's parent is part of a known ignored chain, ignore it. @@ -420,6 +452,7 @@ impl BlockLookups { .map(|root| root.to_string()) .unwrap_or("none".to_owned()), id = lookup.id, + ?new_lookup_trigger, "Created block lookup" ); metrics::inc_counter(&metrics::SYNC_LOOKUP_CREATED); @@ -688,7 +721,13 @@ impl BlockLookups { peers, .. }) => { - if self.search_parent_of_child(awaiting_parent, block_root, &peers, cx) { + if self.search_parent_of_child( + awaiting_parent, + block_root, + &peers, + NewLookupTrigger::ParentUnknown(awaiting_parent.parent_root()), + cx, + ) { true } else { self.drop_lookup_and_children(id, "Failed"); 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 c73933def1..3fa79bc174 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 @@ -53,6 +53,11 @@ impl AwaitingParent { .fork_choice_read_lock() .get_block(&self.parent_root) { + if parent_block.slot == cx.spec().genesis_slot { + // The genesis block is always imported by definition + return true; + } + if let Some(gloas_bid_parent_hash) = self.gloas_bid_parent_hash { // Post-gloas block, check if it's FULL or EMPTY let parent_hash = match parent_block.execution_status { @@ -60,8 +65,12 @@ impl AwaitingParent { ExecutionStatus::Invalid(hash) => hash, ExecutionStatus::Optimistic(hash) => hash, ExecutionStatus::Irrelevant(_) => { - // This should never happen! - return false; + if let Some(hash) = parent_block.execution_payload_block_hash { + hash + } else { + // This should never happen! + return false; + } } }; let is_full = gloas_bid_parent_hash == parent_hash; @@ -716,24 +725,22 @@ impl SingleBlockLookup { state.make_request(|| cx.block_lookup_request(id, peers, block_root))?; if state.is_completed() { - // Block is fully execution-validated and cached in the availability - // checker (NoRequestNeeded). Pull it from the processing-status cache - // so the data/payload streams can continue, and mark the block stream - // complete without re-processing. - match cx.chain.get_block_process_status(&block_root) { + // Block is fully execution-validated and cached in the da_checker or fully + // imported. + // The block MUST be somewhere... and the code below needs to block to know + // if it should fetch data + let block = match cx.chain.get_block_process_status(&block_root) { BlockProcessStatus::NotValidated(block, _) - | BlockProcessStatus::ExecutionValidated(block) => { - // No peer to attribute against on a cache hit. - self.block_request = BlockRequest::Complete { block, peer: None }; - continue; - } + | BlockProcessStatus::ExecutionValidated(block) => block, BlockProcessStatus::Unknown => { // Race: the block was imported into fork-choice between // `block_lookup_request` and this check. All components must // have landed with it, so the lookup has nothing left to do. - return Ok(LookupResult::Completed); + panic!("We have to find the block somewhere"); } - } + }; + // No peer to attribute against on a cache hit. + self.block_request = BlockRequest::Complete { block, peer: None }; } else if let Some(result) = state.take_download_result() { // Block download requests are sent to a single peer, so the returned // PeerGroup contains exactly one entry. Take the first and only. diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index b3deffc346..90d76689d9 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -34,7 +34,7 @@ //! search for the block and subsequently search for parents if needed. use super::backfill_sync::{BackFillSync, ProcessResult, SyncStart}; -use super::block_lookups::BlockLookups; +use super::block_lookups::{BlockLookups, NewLookupTrigger}; use super::network_context::{ CustodyByRootResult, RangeBlockComponent, RangeRequestId, RpcEvent, SyncNetworkContext, }; @@ -1038,6 +1038,7 @@ impl SyncManager { block_component, awaiting_parent, peer_id, + NewLookupTrigger::NetworkMessage, &mut self.network, ) { // Lookup created. No need to log here it's logged in `new_current_lookup` @@ -1066,6 +1067,7 @@ impl SyncManager { if self.block_lookups.search_unknown_block( block_root, &[peer_id], + NewLookupTrigger::NetworkMessage, &mut self.network, ) { // Lookup created. No need to log here it's logged in `new_current_lookup` diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index aa8334e4eb..141ea37bc2 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1004,6 +1004,7 @@ impl TestRig { // Add genesis block for completeness let genesis_block = external_harness.get_head_block(); + let genesis_block_root = genesis_block.canonical_root(); self.network_blocks_by_root .insert(genesis_block.canonical_root(), genesis_block.clone()); self.network_blocks_by_slot @@ -1038,6 +1039,7 @@ impl TestRig { } // Re-log to have a nice list of block roots at the end + self.log(&format!("Build chain (Slot(0), {genesis_block_root})")); for block in &blocks { self.log(&format!("Build chain {block:?}")); }