Various fixes

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<T: BeaconChainTypes> {
         // 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.

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<Hash256>,
@@ -19,13 +18,15 @@ impl<T: BeaconChainTypes> From<&SingleBlockLookup<T>> 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<Hash256>,
     pub(crate) tip: Hash256,
 }
 
 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<NodeChain> {
             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 {

beacon_node/network/src/sync/block_lookups/single_block_lookup.rs

@@ -126,26 +126,18 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
         cx: &mut SyncNetworkContext<T>,
     ) -> 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,
         )
     }