diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index 1dfe4310324..dc82000ef1a 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -28,15 +28,10 @@ pub enum ResponseType { /// is further back than the most recent head slot. pub(crate) const PARENT_DEPTH_TOLERANCE: usize = SLOT_IMPORT_TOLERANCE * 2; -pub enum AwaitingParent { - True, - False, -} - -pub enum BlockIsProcessed { - True, - False, -} +/// Wrapper around bool to prevent mixing this argument with `BlockIsProcessed` +pub(crate) struct AwaitingParent(pub bool); +/// Wrapper around bool to prevent mixing this argument with `AwaitingParent` +pub(crate) struct BlockIsProcessed(pub bool); /// This trait unifies common single block lookup functionality across blocks and blobs. This /// includes making requests, verifying responses, and handling processing results. A @@ -84,9 +79,8 @@ pub trait RequestState { // Otherwise, attempt to progress awaiting processing // If this request is awaiting a parent lookup to be processed, do not send for processing. // The request will be rejected with unknown parent error. - } else if matches!(awaiting_parent, AwaitingParent::False) - && (matches!(block_is_processed, BlockIsProcessed::True) - || matches!(Self::response_type(), ResponseType::Block)) + } else if !awaiting_parent.0 + && (block_is_processed.0 || matches!(Self::response_type(), ResponseType::Block)) { // maybe_start_processing returns Some if state == AwaitingProcess. This pattern is // useful to conditionally access the result data. diff --git a/beacon_node/network/src/sync/block_lookups/parent_chain.rs b/beacon_node/network/src/sync/block_lookups/parent_chain.rs index 01a39a69713..55f2cfe1292 100644 --- a/beacon_node/network/src/sync/block_lookups/parent_chain.rs +++ b/beacon_node/network/src/sync/block_lookups/parent_chain.rs @@ -1,10 +1,9 @@ -use std::collections::{HashMap, HashSet}; - +use super::single_block_lookup::SingleBlockLookup; use beacon_chain::BeaconChainTypes; +use std::collections::{HashMap, HashSet}; use types::Hash256; -use super::single_block_lookup::SingleBlockLookup; - +/// Summary of a lookup of which we may not know it's parent_root yet pub(crate) struct Node { block_root: Hash256, parent_root: Option, @@ -19,6 +18,7 @@ impl From<&SingleBlockLookup> for Node { } } +/// Wrapper around a chain of block roots that have a least one element (tip) pub(crate) struct NodeChain { // Parent chain blocks in descending slot order pub(crate) chain: Vec, @@ -26,6 +26,7 @@ pub(crate) struct NodeChain { } impl NodeChain { + /// Returns the block_root of the oldest ancestor (min slot) of this chain pub(crate) fn ancestor(&self) -> Hash256 { self.chain.last().copied().unwrap_or(self.tip) } @@ -58,16 +59,14 @@ pub(crate) fn compute_parent_chains(nodes: &[Node]) -> Vec { let mut chain = vec![]; // Resolve chain of blocks - 'inner: loop { - if let Some(parent_root) = child_to_parent.get(&block_root) { - // block_root is a known block that may or may not have a parent root - chain.push(block_root); - if let Some(parent_root) = parent_root { - block_root = *parent_root; - continue 'inner; - } + while let Some(parent_root) = child_to_parent.get(&block_root) { + // block_root is a known block that may or may not have a parent root + chain.push(block_root); + if let Some(parent_root) = parent_root { + block_root = *parent_root; + } else { + break; } - break 'inner; } if chain.len() > 1 { 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 19b3f5326d0..a08a6e9d2b4 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 @@ -126,26 +126,18 @@ impl SingleBlockLookup { cx: &mut SyncNetworkContext, ) -> Result<(), LookupRequestError> { let id = self.id; - let awaiting_parent = if self.awaiting_parent.is_some() { - AwaitingParent::True - } else { - AwaitingParent::False - }; + let awaiting_parent = self.awaiting_parent.is_some(); let downloaded_block_expected_blobs = self .block_request_state .state .peek_downloaded_data() .map(|block| block.num_expected_blobs()); - let block_is_processed = if self.block_request_state.state.is_processed() { - BlockIsProcessed::True - } else { - BlockIsProcessed::False - }; + let block_is_processed = self.block_request_state.state.is_processed(); R::request_state_mut(self).continue_request( id, - awaiting_parent, + AwaitingParent(awaiting_parent), downloaded_block_expected_blobs, - block_is_processed, + BlockIsProcessed(block_is_processed), cx, ) }