Comment fixes and ticket TODO references

ZcashFoundation · Jun 18, 2021 · 1313f10 · 1313f10
1 parent 03c5fb0
commit 1313f10
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 12 deletions.
diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs
@@ -148,7 +148,7 @@ pub struct MetaAddr {
     /// records, older peer versions, or buggy or malicious peers.
     //
     // TODO: make services private and optional
-    //       split gossiped and handshake services?
+    //       split gossiped and handshake services? (#2234)
     pub services: PeerServices,
 
     /// The unverified "last seen time" gossiped by the remote peer that sent us
@@ -549,9 +549,10 @@ impl MetaAddrChange {
             NewAlternate {
                 untrusted_services, ..
             } => Some(*untrusted_services),
-            // TODO: create a "services implemented by Zebra" constant
+            // TODO: create a "services implemented by Zebra" constant (#2234)
             NewLocal { .. } => Some(PeerServices::NODE_NETWORK),
             UpdateAttempt { .. } => None,
+            // TODO: split untrusted and direct services (#2234)
             UpdateResponded { services, .. } => Some(*services),
             UpdateFailed { services, .. } => *services,
         }
@@ -639,7 +640,7 @@ impl MetaAddrChange {
         match self {
             NewGossiped { .. } | NewAlternate { .. } | NewLocal { .. } => Some(MetaAddr {
                 addr: self.addr(),
-                // TODO: make services optional when we add a DNS seeder change/state
+                // TODO: make services optional when we add a DNS seeder change and state
                 services: self
                     .untrusted_services()
                     .expect("unexpected missing services"),
@@ -688,7 +689,7 @@ impl MetaAddrChange {
                     // so malicious peers can't keep changing our peer connection order.
                     Some(MetaAddr {
                         addr: self.addr(),
-                        // TODO: or(self.untrusted_services()) when services become optional
+                        // TODO: or(self.untrusted_services()) when services become optional (#2234)
                         services: previous.services,
                         untrusted_last_seen: previous
                             .untrusted_last_seen
@@ -711,8 +712,10 @@ impl MetaAddrChange {
                 // connection timeout, even if changes are applied out of order.
                 Some(MetaAddr {
                     addr: self.addr(),
+                    // We want up-to-date services, even if they have fewer bits,
+                    // or they are applied out of order.
                     services: self.untrusted_services().unwrap_or(previous.services),
-                    // only NeverAttempted changes can modify the last seen field
+                    // Only NeverAttempted changes can modify the last seen field
                     untrusted_last_seen: previous.untrusted_last_seen,
                     // Since Some(time) is always greater than None, `max` prefers:
                     // - the latest time if both are Some
@@ -790,7 +793,7 @@ impl Ord for MetaAddr {
         // So this comparison will have no impact until Zebra implements
         // more service features.
         //
-        // TODO: order services by usefulness, not bit pattern values
+        // TODO: order services by usefulness, not bit pattern values (#2234)
         //       Security: split gossiped and direct services
         let larger_services = self.services.cmp(&other.services);
 

diff --git a/zebra-network/src/peer_set/candidate_set.rs b/zebra-network/src/peer_set/candidate_set.rs
@@ -393,7 +393,7 @@ fn limit_last_seen_times(addrs: &mut Vec<MetaAddr>, last_seen_limit: DateTime32)
             .checked_duration_since(last_seen_limit)
             .expect("unexpected underflow: just checked newest_seen is greater");
 
-        // Apply offset to oldest timestamp to check for underflow
+        // Check for underflow
         if oldest_seen.checked_sub(offset).is_some() {
             // No underflow is possible, so apply offset to all addresses
             for addr in addrs {

diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs
@@ -201,12 +201,16 @@ where
     S::Future: Send + 'static,
 {
     info!(?initial_peers, "connecting to initial peer set");
-    // ## Correctness:
+    // # Security
     //
-    // Each `CallAll` can hold one `Buffer` or `Batch` reservation for
-    // an indefinite period. We can use `CallAllUnordered` without filling
-    // the underlying `Inbound` buffer, because we immediately drive this
-    // single `CallAll` to completion, and handshakes have a short timeout.
+    // TODO: rate-limit initial seed peer connections (#2326)
+    //
+    // # Correctness
+    //
+    // Each `FuturesUnordered` can hold one `Buffer` or `Batch` reservation for
+    // an indefinite period. We can use `FuturesUnordered` without filling
+    // the underlying network buffers, because we immediately drive this
+    // single `FuturesUnordered` to completion, and handshakes have a short timeout.
     let mut handshakes: FuturesUnordered<_> = initial_peers
         .into_iter()
         .map(|addr| {