Drop lookup type trait for a simple arg

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

@@ -29,34 +29,12 @@ pub enum LookupType {
     Parent,
 }
 
-/// This trait helps differentiate `SingleBlockLookup`s from `ParentLookup`s .This is useful in
-/// ensuring requests and responses are handled separately and enables us to use different failure
-/// tolerances for each, while re-using the same basic request and retry logic.
-pub trait Lookup {
-    const MAX_ATTEMPTS: u8;
-    fn lookup_type() -> LookupType;
-    fn max_attempts() -> u8 {
-        Self::MAX_ATTEMPTS
-    }
-}
-
-/// A `Lookup` that is a part of a `ParentLookup`.
-pub struct Parent;
-
-impl Lookup for Parent {
-    const MAX_ATTEMPTS: u8 = PARENT_FAIL_TOLERANCE;
-    fn lookup_type() -> LookupType {
-        LookupType::Parent
-    }
-}
-
-/// A `Lookup` that part of a single block lookup.
-pub struct Current;
-
-impl Lookup for Current {
-    const MAX_ATTEMPTS: u8 = SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS;
-    fn lookup_type() -> LookupType {
-        LookupType::Current
+impl LookupType {
+    fn max_attempts(&self) -> u8 {
+        match self {
+            LookupType::Current => SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS,
+            LookupType::Parent => PARENT_FAIL_TOLERANCE,
+        }
     }
 }
 
@@ -68,7 +46,7 @@ impl Lookup for Current {
 /// The use of the `ResponseType` associated type gives us a degree of type
 /// safety when handling a block/blob response ensuring we only mutate the correct corresponding
 /// state.
-pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
+pub trait RequestState<T: BeaconChainTypes> {
     /// The type of the request .
     type RequestType;
 
@@ -81,9 +59,12 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
     /* Request building methods */
 
     /// Construct a new request.
-    fn build_request(&mut self) -> Result<(PeerId, Self::RequestType), LookupRequestError> {
+    fn build_request(
+        &mut self,
+        lookup_type: LookupType,
+    ) -> Result<(PeerId, Self::RequestType), LookupRequestError> {
         // Verify and construct request.
-        self.too_many_attempts()?;
+        self.too_many_attempts(lookup_type)?;
         let peer = self.get_peer()?;
         let request = self.new_request();
         Ok((peer, request))
@@ -93,6 +74,7 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
     fn build_request_and_send(
         &mut self,
         id: Id,
+        lookup_type: LookupType,
         cx: &mut SyncNetworkContext<T>,
     ) -> Result<(), LookupRequestError> {
         // Check if request is necessary.
@@ -101,7 +83,7 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
         }
 
         // Construct request.
-        let (peer_id, request) = self.build_request()?;
+        let (peer_id, request) = self.build_request(lookup_type)?;
 
         // Update request state.
         let req_counter = self.get_state_mut().on_download_start(peer_id);
@@ -110,17 +92,16 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
         let id = SingleLookupReqId {
             id,
             req_counter,
-            lookup_type: L::lookup_type(),
+            lookup_type,
         };
         Self::make_request(id, peer_id, request, cx)
     }
 
     /// Verify the current request has not exceeded the maximum number of attempts.
-    fn too_many_attempts(&self) -> Result<(), LookupRequestError> {
-        let max_attempts = L::max_attempts();
+    fn too_many_attempts(&self, lookup_type: LookupType) -> Result<(), LookupRequestError> {
         let request_state = self.get_state();
 
-        if request_state.failed_attempts() >= max_attempts {
+        if request_state.failed_attempts() >= lookup_type.max_attempts() {
             let cannot_process = request_state.more_failed_processing_attempts();
             Err(LookupRequestError::TooManyAttempts { cannot_process })
         } else {
@@ -187,7 +168,7 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
     fn response_type() -> ResponseType;
 
     /// A getter for the `BlockRequestState` or `BlobRequestState` associated with this trait.
-    fn request_state_mut(request: &mut SingleBlockLookup<L, T>) -> &mut Self;
+    fn request_state_mut(request: &mut SingleBlockLookup<T>) -> &mut Self;
 
     /// A getter for a reference to the `SingleLookupRequestState` associated with this trait.
     fn get_state(&self) -> &SingleLookupRequestState;
@@ -196,7 +177,7 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
     fn get_state_mut(&mut self) -> &mut SingleLookupRequestState;
 }
 
-impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlockRequestState<L> {
+impl<T: BeaconChainTypes> RequestState<T> for BlockRequestState {
     type RequestType = BlocksByRootSingleRequest;
     type VerifiedResponseType = Arc<SignedBeaconBlock<T::EthSpec>>;
     type ReconstructedResponseType = RpcBlock<T::EthSpec>;
@@ -253,7 +234,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlockRequestState<L>
     fn response_type() -> ResponseType {
         ResponseType::Block
     }
-    fn request_state_mut(request: &mut SingleBlockLookup<L, T>) -> &mut Self {
+    fn request_state_mut(request: &mut SingleBlockLookup<T>) -> &mut Self {
         &mut request.block_request_state
     }
     fn get_state(&self) -> &SingleLookupRequestState {
@@ -264,7 +245,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlockRequestState<L>
     }
 }
 
-impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L, T::EthSpec> {
+impl<T: BeaconChainTypes> RequestState<T> for BlobRequestState<T::EthSpec> {
     type RequestType = BlobsByRootSingleBlockRequest;
     type VerifiedResponseType = FixedBlobSidecarList<T::EthSpec>;
     type ReconstructedResponseType = FixedBlobSidecarList<T::EthSpec>;
@@ -328,7 +309,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
     fn response_type() -> ResponseType {
         ResponseType::Blob
     }
-    fn request_state_mut(request: &mut SingleBlockLookup<L, T>) -> &mut Self {
+    fn request_state_mut(request: &mut SingleBlockLookup<T>) -> &mut Self {
         &mut request.blob_request_state
     }
     fn get_state(&self) -> &SingleLookupRequestState {

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

@@ -16,9 +16,6 @@ use beacon_chain::data_availability_checker::{
 };
 use beacon_chain::validator_monitor::timestamp_now;
 use beacon_chain::{AvailabilityProcessingStatus, BeaconChainTypes, BlockError};
-pub use common::Current;
-pub use common::Lookup;
-pub use common::Parent;
 pub use common::RequestState;
 use fnv::FnvHashMap;
 use lighthouse_network::{PeerAction, PeerId};
@@ -55,12 +52,12 @@ pub struct BlockLookups<T: BeaconChainTypes> {
     /// Parent chain lookups being downloaded.
     parent_lookups: SmallVec<[ParentLookup<T>; 3]>,
 
-    processing_parent_lookups: HashMap<Hash256, (Vec<Hash256>, SingleBlockLookup<Parent, T>)>,
+    processing_parent_lookups: HashMap<Hash256, (Vec<Hash256>, SingleBlockLookup<T>)>,
 
     /// A cache of failed chain lookups to prevent duplicate searches.
     failed_chains: LRUTimeCache<Hash256>,
 
-    single_block_lookups: FnvHashMap<Id, SingleBlockLookup<Current, T>>,
+    single_block_lookups: FnvHashMap<Id, SingleBlockLookup<T>>,
 
     pub(crate) da_checker: Arc<DataAvailabilityChecker<T>>,
 
@@ -131,7 +128,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     /// Attempts to trigger the request matching the given `block_root`.
     pub fn trigger_single_lookup(
         &mut self,
-        mut single_block_lookup: SingleBlockLookup<Current, T>,
+        mut single_block_lookup: SingleBlockLookup<T>,
         cx: &mut SyncNetworkContext<T>,
     ) {
         let block_root = single_block_lookup.block_root();
@@ -147,7 +144,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     }
 
     /// Adds a lookup to the `single_block_lookups` map.
-    pub fn add_single_lookup(&mut self, single_block_lookup: SingleBlockLookup<Current, T>) {
+    pub fn add_single_lookup(&mut self, single_block_lookup: SingleBlockLookup<T>) {
         self.single_block_lookups
             .insert(single_block_lookup.id, single_block_lookup);
 
@@ -212,6 +209,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
             peers,
             self.da_checker.clone(),
             cx.next_id(),
+            LookupType::Current,
         );
 
         debug!(
@@ -284,10 +282,10 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     /// Get a single block lookup by its ID. This method additionally ensures the `req_counter`
     /// matches the current `req_counter` for the lookup. This ensures any stale responses from requests
     /// that have been retried are ignored.
-    fn get_single_lookup<R: RequestState<Current, T>>(
+    fn get_single_lookup<R: RequestState<T>>(
         &mut self,
         id: SingleLookupReqId,
-    ) -> Option<SingleBlockLookup<Current, T>> {
+    ) -> Option<SingleBlockLookup<T>> {
         let mut lookup = self.single_block_lookups.remove(&id.id)?;
 
         let request_state = R::request_state_mut(&mut lookup);
@@ -314,7 +312,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     }
 
     /// Process a block or blob response received from a single lookup request.
-    pub fn single_lookup_response<R: RequestState<Current, T>>(
+    pub fn single_lookup_response<R: RequestState<T>>(
         &mut self,
         lookup_id: SingleLookupReqId,
         peer_id: PeerId,
@@ -345,7 +343,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
             "response_type" => ?response_type,
         );
 
-        match self.handle_verified_response::<Current, R>(
+        match self.handle_verified_response::<R>(
             seen_timestamp,
             cx,
             BlockProcessType::SingleBlock { id: lookup.id },
@@ -372,13 +370,13 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
 
     /// Consolidates error handling for `single_lookup_response`. An `Err` here should always mean
     /// the lookup is dropped.
-    fn handle_verified_response<L: Lookup, R: RequestState<L, T>>(
+    fn handle_verified_response<R: RequestState<T>>(
         &self,
         seen_timestamp: Duration,
         cx: &mut SyncNetworkContext<T>,
         process_type: BlockProcessType,
         verified_response: R::VerifiedResponseType,
-        lookup: &mut SingleBlockLookup<L, T>,
+        lookup: &mut SingleBlockLookup<T>,
     ) -> Result<(), LookupRequestError> {
         let id = lookup.id;
         let block_root = lookup.block_root();
@@ -389,7 +387,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 // If we have an outstanding parent request for this block, delay sending the response until
                 // all parent blocks have been processed, otherwise we will fail validation with an
                 // `UnknownParent`.
-                let delay_send = match L::lookup_type() {
+                let delay_send = match lookup.lookup_type {
                     LookupType::Parent => false,
                     LookupType::Current => self.has_pending_parent_request(lookup.block_root()),
                 };
@@ -453,7 +451,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     /// Get a parent block lookup by its ID. This method additionally ensures the `req_counter`
     /// matches the current `req_counter` for the lookup. This any stale responses from requests
     /// that have been retried are ignored.
-    fn get_parent_lookup<R: RequestState<Parent, T>>(
+    fn get_parent_lookup<R: RequestState<T>>(
         &mut self,
         id: SingleLookupReqId,
     ) -> Option<ParentLookup<T>> {
@@ -479,7 +477,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     }
 
     /// Process a response received from a parent lookup request.
-    pub fn parent_lookup_response<R: RequestState<Parent, T>>(
+    pub fn parent_lookup_response<R: RequestState<T>>(
         &mut self,
         id: SingleLookupReqId,
         peer_id: PeerId,
@@ -523,7 +521,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
 
     /// Consolidates error handling for `parent_lookup_response`. An `Err` here should always mean
     /// the lookup is dropped.
-    fn parent_lookup_response_inner<R: RequestState<Parent, T>>(
+    fn parent_lookup_response_inner<R: RequestState<T>>(
         &mut self,
         peer_id: PeerId,
         response: R::VerifiedResponseType,
@@ -554,7 +552,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
             }
         }
 
-        self.handle_verified_response::<Parent, R>(
+        self.handle_verified_response::<R>(
             seen_timestamp,
             cx,
             BlockProcessType::ParentLookup {
@@ -633,7 +631,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     }
 
     /// An RPC error has occurred during a parent lookup. This function handles this case.
-    pub fn parent_lookup_failed<R: RequestState<Parent, T>>(
+    pub fn parent_lookup_failed<R: RequestState<T>>(
         &mut self,
         id: SingleLookupReqId,
         peer_id: &PeerId,
@@ -669,7 +667,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
     }
 
     /// An RPC error has occurred during a single lookup. This function handles this case.\
-    pub fn single_block_lookup_failed<R: RequestState<Current, T>>(
+    pub fn single_block_lookup_failed<R: RequestState<T>>(
         &mut self,
         id: SingleLookupReqId,
         peer_id: &PeerId,
@@ -717,7 +715,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
 
     /* Processing responses */
 
-    pub fn single_block_component_processed<R: RequestState<Current, T>>(
+    pub fn single_block_component_processed<R: RequestState<T>>(
         &mut self,
         target_id: Id,
         result: BlockProcessingResult<T::EthSpec>,

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

@@ -1,6 +1,6 @@
+use super::common::LookupType;
 use super::single_block_lookup::{LookupRequestError, SingleBlockLookup};
 use super::{DownloadedBlock, PeerId};
-use crate::sync::block_lookups::common::Parent;
 use crate::sync::{manager::SLOT_IMPORT_TOLERANCE, network_context::SyncNetworkContext};
 use beacon_chain::block_verification_types::AsBlock;
 use beacon_chain::block_verification_types::RpcBlock;
@@ -24,7 +24,7 @@ pub(crate) struct ParentLookup<T: BeaconChainTypes> {
     /// The blocks that have currently been downloaded.
     downloaded_blocks: Vec<DownloadedBlock<T::EthSpec>>,
     /// Request of the last parent.
-    pub current_parent_request: SingleBlockLookup<Parent, T>,
+    pub current_parent_request: SingleBlockLookup<T>,
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -55,6 +55,7 @@ impl<T: BeaconChainTypes> ParentLookup<T> {
             &[peer_id],
             da_checker,
             cx.next_id(),
+            LookupType::Parent,
         );
 
         Self {
@@ -132,7 +133,7 @@ impl<T: BeaconChainTypes> ParentLookup<T> {
         Hash256,
         VecDeque<RpcBlock<T::EthSpec>>,
         Vec<Hash256>,
-        SingleBlockLookup<Parent, T>,
+        SingleBlockLookup<T>,
     ) {
         let ParentLookup {
             chain_hash,