From e5c92084bae4ddaebe12ff3fdebb0eac6046d7ac Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Fri, 17 Jun 2022 09:12:35 +1000
Subject: [PATCH] Group PeerSet fields into themes (#4618)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 zebra-network/src/peer_set/set.rs            | 59 +++++++++++++-------
 zebra-network/src/peer_set/set/tests.rs      |  2 +
 zebra-network/src/peer_set/set/tests/prop.rs |  2 +
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/zebra-network/src/peer_set/set.rs b/zebra-network/src/peer_set/set.rs
index d12b60b9..62ba4217 100644
--- a/zebra-network/src/peer_set/set.rs
+++ b/zebra-network/src/peer_set/set.rs
@@ -170,14 +170,23 @@ where
     D::Error: Into<BoxError>,
     C: ChainTip,
 {
+    // Peer Tracking: New Peers
+    //
     /// Provides new and deleted peer [`Change`]s to the peer set,
     /// via the [`Discover`] trait implementation.
     discover: D,
 
+    /// A channel that asks the peer crawler task to connect to more peers.
+    demand_signal: mpsc::Sender<MorePeers>,
+
+    // Peer Tracking: Ready Peers
+    //
     /// Connected peers that are ready to receive requests from Zebra,
     /// or send requests to Zebra.
     ready_services: HashMap<D::Key, D::Service>,
 
+    // Request Routing
+    //
     /// A preselected ready service.
     ///
     /// # Correctness
@@ -194,6 +203,8 @@ where
     /// Used to route inventory requests to peers that are likely to have it.
     inventory_registry: InventoryRegistry,
 
+    // Peer Tracking: Busy Peers
+    //
     /// Connected peers that are handling a Zebra request,
     /// or Zebra is handling one of their requests.
     unready_services: FuturesUnordered<UnreadyService<D::Key, D::Service, Request>>,
@@ -201,9 +212,22 @@ where
     /// Channels used to cancel the request that an unready service is doing.
     cancel_handles: HashMap<D::Key, oneshot::Sender<CancelClientWork>>,
 
-    /// A channel that asks the peer crawler task to connect to more peers.
-    demand_signal: mpsc::Sender<MorePeers>,
+    // Peer Validation
+    //
+    /// An endpoint to see the minimum peer protocol version in real time.
+    ///
+    /// The minimum version depends on the block height, and [`MinimumPeerVersion`] listens for
+    /// height changes and determines the correct minimum version.
+    minimum_peer_version: MinimumPeerVersion<C>,
 
+    /// The configured limit for inbound and outbound connections.
+    ///
+    /// The peer set panics if this size is exceeded.
+    /// If that happens, our connection limit code has a bug.
+    peerset_total_connection_limit: usize,
+
+    // Background Tasks
+    //
     /// Channel for passing ownership of tokio JoinHandles from PeerSet's background tasks
     ///
     /// The join handles passed into the PeerSet are used populate the `guards` member
@@ -215,6 +239,8 @@ where
     /// the `PeerSet` propagate errors from background tasks back to the user
     guards: futures::stream::FuturesUnordered<JoinHandle<Result<(), BoxError>>>,
 
+    // Metrics and Logging
+    //
     /// Address book metrics watch channel.
     ///
     /// Used for logging diagnostics.
@@ -222,18 +248,6 @@ where
 
     /// The last time we logged a message about the peer set size
     last_peer_log: Option<Instant>,
-
-    /// The configured limit for inbound and outbound connections.
-    ///
-    /// The peer set panics if this size is exceeded.
-    /// If that happens, our connection limit code has a bug.
-    peerset_total_connection_limit: usize,
-
-    /// An endpoint to see the minimum peer protocol version in real time.
-    ///
-    /// The minimum version depends on the block height, and [`MinimumPeerVersion`] listens for
-    /// height changes and determines the correct minimum version.
-    minimum_peer_version: MinimumPeerVersion<C>,
 }
 
 impl<D, C> Drop for PeerSet<D, C>
@@ -275,16 +289,23 @@ where
         minimum_peer_version: MinimumPeerVersion<C>,
     ) -> Self {
         Self {
-            // Ready peers
+            // New peers
             discover,
+            demand_signal,
+
+            // Ready peers
             ready_services: HashMap::new(),
+            // Request Routing
             preselected_p2c_peer: None,
             inventory_registry: InventoryRegistry::new(inv_stream),
 
-            // Unready peers
+            // Busy peers
             unready_services: FuturesUnordered::new(),
             cancel_handles: HashMap::new(),
-            demand_signal,
+
+            // Peer validation
+            minimum_peer_version,
+            peerset_total_connection_limit: config.peerset_total_connection_limit(),
 
             // Background tasks
             handle_rx,
@@ -293,10 +314,6 @@ where
             // Metrics
             last_peer_log: None,
             address_metrics,
-            peerset_total_connection_limit: config.peerset_total_connection_limit(),
-
-            // Real-time parameters
-            minimum_peer_version,
         }
     }
 
diff --git a/zebra-network/src/peer_set/set/tests.rs b/zebra-network/src/peer_set/set/tests.rs
index b00da501..bc01d978 100644
--- a/zebra-network/src/peer_set/set/tests.rs
+++ b/zebra-network/src/peer_set/set/tests.rs
@@ -1,3 +1,5 @@
+//! Peer set unit tests, and test setup code.
+
 use std::{net::SocketAddr, sync::Arc};
 
 use futures::{channel::mpsc, stream, Stream, StreamExt};
diff --git a/zebra-network/src/peer_set/set/tests/prop.rs b/zebra-network/src/peer_set/set/tests/prop.rs
index aa48a457..2cc65436 100644
--- a/zebra-network/src/peer_set/set/tests/prop.rs
+++ b/zebra-network/src/peer_set/set/tests/prop.rs
@@ -1,3 +1,5 @@
+//! Randomised property tests for the peer set.
+
 use std::net::SocketAddr;
 
 use futures::FutureExt;