De-duplicate code

sigp · Feb 5, 2025 · cbc378a · cbc378a
1 parent 4583116
commit cbc378a
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 55 deletions.
diff --git a/beacon_node/lighthouse_network/src/service/api_types.rs b/beacon_node/lighthouse_network/src/service/api_types.rs
@@ -234,45 +234,27 @@ impl slog::Value for RequestId {
     }
 }
 
+macro_rules! impl_display {
+    ($structname: ty, $format: literal, $($field:ident),*) => {
+        impl Display for $structname {
+            fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+                write!(f, $format, $(self.$field,)*)
+            }
+        }
+    };
+}
+
 // Since each request Id is deeply nested with various types, if rendered with Debug on logs they
 // take too much visual space. This custom Display implementations make the overall Id short while
 // not losing information
-impl Display for BlocksByRangeRequestId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.id, self.parent_request_id)
-    }
-}
-
-impl Display for BlobsByRangeRequestId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.id, self.parent_request_id)
-    }
-}
-
-impl Display for DataColumnsByRangeRequestId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.id, self.parent_request_id)
-    }
-}
-
-impl Display for ComponentsByRangeRequestId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.id, self.requester)
-    }
-}
-
-impl Display for SingleLookupReqId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/Lookup/{}", self.req_id, self.lookup_id)
-    }
-}
-
-// This custom impl reduces log boilerplate not printing `DataColumnsByRootRequestId` on each id log
-impl Display for DataColumnsByRootRequestId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.id, self.requester)
-    }
-}
+impl_display!(BlocksByRangeRequestId, "{}/{}", id, parent_request_id);
+impl_display!(BlobsByRangeRequestId, "{}/{}", id, parent_request_id);
+impl_display!(DataColumnsByRangeRequestId, "{}/{}", id, parent_request_id);
+impl_display!(ComponentsByRangeRequestId, "{}/{}", id, requester);
+impl_display!(DataColumnsByRootRequestId, "{}/{}", id, requester);
+impl_display!(SingleLookupReqId, "{}/Lookup/{}", req_id, lookup_id);
+impl_display!(CustodyId, "{}", requester);
+impl_display!(SamplingId, "{}/{}", sampling_request_id, id);
 
 impl Display for DataColumnsByRootRequester {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
@@ -283,12 +265,6 @@ impl Display for DataColumnsByRootRequester {
     }
 }
 
-impl Display for CustodyId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.requester)
-    }
-}
-
 impl Display for CustodyRequester {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.0)
@@ -304,12 +280,6 @@ impl Display for RangeRequestId {
     }
 }
 
-impl Display for SamplingId {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}/{}", self.sampling_request_id, self.id)
-    }
-}
-
 impl Display for SamplingRequestId {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.0)
@@ -361,11 +331,11 @@ mod tests {
             parent_request_id: ComponentsByRangeRequestId {
                 id: 122,
                 requester: RangeRequestId::RangeSync {
-                    chain_id: 5492900659401505034,
+                    chain_id: 54,
                     batch_id: Epoch::new(0),
                 },
             },
         };
-        assert_eq!(format!("{id}"), "123/122/RangeSync/0/5492900659401505034");
+        assert_eq!(format!("{id}"), "123/122/RangeSync/0/54");
     }
 }
diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs
@@ -612,13 +612,13 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         process_id: ChainSegmentProcessId,
         blocks: Vec<RpcBlock<T::EthSpec>>,
     ) -> Result<(), Error<T::EthSpec>> {
+        let is_backfill = matches!(&process_id, ChainSegmentProcessId::BackSyncBatchId { .. });
         debug!(self.log, "Batch sending for process";
             "blocks" => blocks.len(),
             "id" => ?process_id,
         );
 
         let processor = self.clone();
-        let id = process_id.clone();
         let process_fn = async move {
             let notify_execution_layer = if processor
                 .network_globals
@@ -631,16 +631,17 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 NotifyExecutionLayer::Yes
             };
             processor
-                .process_chain_segment(id, blocks, notify_execution_layer)
+                .process_chain_segment(process_id, blocks, notify_execution_layer)
                 .await;
         };
         let process_fn = Box::pin(process_fn);
 
         // Back-sync batches are dispatched with a different `Work` variant so
         // they can be rate-limited.
-        let work = match process_id {
-            ChainSegmentProcessId::RangeBatchId { .. } => Work::ChainSegment(process_fn),
-            ChainSegmentProcessId::BackSyncBatchId { .. } => Work::ChainSegmentBackfill(process_fn),
+        let work = if is_backfill {
+            Work::ChainSegmentBackfill(process_fn)
+        } else {
+            Work::ChainSegment(process_fn)
         };
 
         self.try_send(BeaconWorkEvent {