From a99fbde676c51cfea4425677aca42b09c3056e0c Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 4 Jun 2026 22:42:59 +0200 Subject: [PATCH] Derive lookup's bid parent_block_hash from its block; drop ParentUnknown field Remove SingleBlockLookup::awaiting_parent_bid_hash (duplicated awaiting_parent state) and derive the bid parent_block_hash from the lookup's own downloaded block. This removes the parent_block_hash field from BlockError::ParentUnknown / BlockProcessingResult::ParentUnknown, re-aligning them with unstable. --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +-- .../beacon_chain/src/block_verification.rs | 10 +---- .../beacon_chain/tests/block_verification.rs | 2 +- .../network_beacon_processor/sync_methods.rs | 9 +--- .../sync/block_lookups/single_block_lookup.rs | 42 ++++++++----------- 5 files changed, 22 insertions(+), 47 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 67deb88f6f..d826895a25 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3394,7 +3394,6 @@ impl BeaconChain { { return Err(BlockError::ParentUnknown { parent_root: blob.block_parent_root(), - parent_block_hash: None, }); } } @@ -3521,10 +3520,7 @@ impl BeaconChain { .fork_choice_read_lock() .contains_block(&parent_root) { - return Err(BlockError::ParentUnknown { - parent_root, - parent_block_hash: None, - }); + return Err(BlockError::ParentUnknown { parent_root }); } self.emit_sse_data_column_sidecar_events( diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index a160938750..de592e8dae 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -95,7 +95,6 @@ use store::{Error as DBError, KeyValueStore}; use strum::{AsRefStr, IntoStaticStr}; use task_executor::JoinHandle; use tracing::{Instrument, Span, debug, debug_span, error, info_span, instrument}; -use types::ExecutionBlockHash; use types::{ BeaconBlockRef, BeaconState, BeaconStateError, BlobsList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, FullPayload, Hash256, InconsistentFork, KzgProofs, RelativeEpoch, @@ -123,10 +122,7 @@ pub enum BlockError { /// /// It's unclear if this block is valid, but it cannot be processed without already knowing /// its parent. - ParentUnknown { - parent_root: Hash256, - parent_block_hash: Option, - }, + ParentUnknown { parent_root: Hash256 }, /// The block slot is greater than the present slot. /// /// ## Peer scoring @@ -1393,7 +1389,6 @@ impl ExecutionPendingBlock { ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), - parent_block_hash: block.as_block().parent_block_hash(), }); } } @@ -1769,7 +1764,6 @@ pub fn check_block_is_finalized_checkpoint_or_descendant< } else { Err(BlockError::ParentUnknown { parent_root: block.parent_root(), - parent_block_hash: block.as_block().parent_block_hash(), }) } } @@ -1864,7 +1858,6 @@ fn verify_parent_block_and_envelope_are_known( ParentImportStatus::UnknownBlock | ParentImportStatus::UnknownPayload => { Err(BlockError::ParentUnknown { parent_root: block.parent_root(), - parent_block_hash: block.parent_block_hash(), }) } } @@ -1897,7 +1890,6 @@ fn load_parent>( { return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), - parent_block_hash: block.as_block().parent_block_hash(), }); } diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 6da9bf8ebe..deadafac36 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1341,7 +1341,7 @@ async fn block_gossip_verification() { assert!( matches!( unwrap_err(harness.chain.verify_block_for_gossip(Arc::new(SignedBeaconBlock::from_block(block, signature))).await), - BlockError::ParentUnknown {parent_root: p, ..} + BlockError::ParentUnknown {parent_root: p} if p == parent_root ), "should not import a block for an unknown parent" diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index 289a893176..f6396e7e06 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -28,7 +28,7 @@ use logging::crit; use std::sync::Arc; use std::time::Duration; use tracing::{debug, debug_span, error, info, instrument, warn}; -use types::{BlockImportSource, DataColumnSidecarList, Epoch, ExecutionBlockHash, Hash256}; +use types::{BlockImportSource, DataColumnSidecarList, Epoch, Hash256}; /// Id associated to a batch processing request, either a sync batch or a parent lookup. #[derive(Clone, Debug, PartialEq)] @@ -969,7 +969,6 @@ pub enum BlockProcessingResult { Imported(bool, &'static str), ParentUnknown { parent_root: Hash256, - parent_block_hash: Option, }, /// Processing failed. `penalty` is `Some` when an attributable peer should be downscored; /// the third tuple element is the `report_peer` telemetry msg. `reason` is for logs only. @@ -1001,13 +1000,9 @@ impl From> for BlockProcessingR return Self::Imported(true, "duplicate"); } BlockError::GenesisBlock => return Self::Imported(true, "genesis"), - BlockError::ParentUnknown { - parent_root, - parent_block_hash, - } => { + BlockError::ParentUnknown { parent_root } => { return Self::ParentUnknown { parent_root: *parent_root, - parent_block_hash: *parent_block_hash, }; } BlockError::BeaconChainError(_) | BlockError::InternalError(_) => None, 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 163b798af7..fef6d6b2b2 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 @@ -177,9 +177,6 @@ pub struct SingleBlockLookup { #[educe(Debug(method(fmt_peer_map_as_len)))] gloas_child_peers: GloasChildPeers, awaiting_parent: Option, - /// Post-Gloas only: this block's bid `parent_block_hash` (the parent's execution hash). Used to - /// derive the `PeerType` when propagating peers up to the parent lookup. - awaiting_parent_bid_hash: Option, created: Instant, pub(crate) span: Span, } @@ -216,7 +213,6 @@ impl SingleBlockLookup { peers: block_peers, gloas_child_peers: Arc::new(RwLock::new(gloas_child_peers)), awaiting_parent, - awaiting_parent_bid_hash: None, created: Instant::now(), span: lookup_span, } @@ -247,27 +243,29 @@ impl SingleBlockLookup { } /// Mark this lookup as awaiting a parent lookup from being processed. Meanwhile don't send - /// components for processing. `parent_block_hash` is the block's bid `parent_block_hash` - /// (post-Gloas only), used to partition the parent lookup's peers. - pub fn set_awaiting_parent( - &mut self, - parent_root: Hash256, - parent_block_hash: Option, - ) { + /// components for processing. + pub fn set_awaiting_parent(&mut self, parent_root: Hash256) { self.awaiting_parent = Some(parent_root); - self.awaiting_parent_bid_hash = parent_block_hash; } /// Mark this lookup as no longer awaiting a parent lookup. Components can be sent for /// processing. pub fn resolve_awaiting_parent(&mut self) { self.awaiting_parent = None; - self.awaiting_parent_bid_hash = None; + } + + /// This block's bid `parent_block_hash` (the parent's execution hash), derived from the + /// downloaded block. Post-Gloas only; `None` pre-Gloas or before the block is downloaded. + fn bid_parent_block_hash(&self) -> Option { + self.block_request + .state + .peek_downloaded_data() + .and_then(|block| block.parent_block_hash()) } /// Returns the `PeerType` to use when propagating this lookup's peers up to its parent lookup. pub fn awaiting_parent_peer_type(&self) -> PeerType { - match self.awaiting_parent_bid_hash { + match self.bid_parent_block_hash() { Some(execution_hash) => PeerType::PostGloas(execution_hash), None => PeerType::PreGloas, } @@ -343,13 +341,9 @@ impl SingleBlockLookup { self.data_request = if block.num_expected_blobs() == 0 { DataRequest::NoData } else if cx.chain.should_fetch_custody_columns(block_epoch) { - let slot = block.slot(); - // Post-Gloas data columns are served by the FULL children's peers, not - // by `self.peers`. Pre-Gloas this returns `self.peers` unchanged. - let peers = self.get_data_peers(block); DataRequest::Request { - slot, - peers, + slot: block.slot(), + peers: self.get_data_peers(block), state: SingleLookupRequestState::new(), } } else { @@ -466,16 +460,14 @@ impl SingleBlockLookup { BlockProcessingResult::Imported(_fully_imported, _info) => { self.block_request.state.on_processing_success()?; } - BlockProcessingResult::ParentUnknown { - parent_root, - parent_block_hash, - } => { + BlockProcessingResult::ParentUnknown { parent_root } => { // `BlockError::ParentUnknown` is only returned when processing blocks. Revert the // block request to `Downloaded` and park this lookup until the parent resolves; a // future call to `continue_requests` will re-submit the block for processing once // the parent lookup completes. + let parent_block_hash = self.bid_parent_block_hash(); self.block_request.state.revert_to_awaiting_processing()?; - self.set_awaiting_parent(parent_root, parent_block_hash); + self.set_awaiting_parent(parent_root); return Ok(LookupResult::ParentUnknown { parent_root, parent_block_hash,