cleanup(net): Deleted unused NeverAttemptedAlternate peer state

zebra-network/src/address_book.rs

@@ -112,9 +112,6 @@ pub struct AddressMetrics {
     /// The number of addresses in the `NeverAttemptedGossiped` state.
     pub never_attempted_gossiped: usize,
 
-    /// The number of addresses in the `NeverAttemptedAlternate` state.
-    pub never_attempted_alternate: usize,
-
     /// The number of addresses in the `Failed` state.
     pub failed: usize,
 
@@ -667,9 +664,6 @@ impl AddressBook {
         let never_attempted_gossiped = self
             .state_peers(PeerAddrState::NeverAttemptedGossiped)
             .count();
-        let never_attempted_alternate = self
-            .state_peers(PeerAddrState::NeverAttemptedAlternate)
-            .count();
         let failed = self.state_peers(PeerAddrState::Failed).count();
         let attempt_pending = self.state_peers(PeerAddrState::AttemptPending).count();
 
@@ -683,7 +677,6 @@ impl AddressBook {
         AddressMetrics {
             responded,
             never_attempted_gossiped,
-            never_attempted_alternate,
             failed,
             attempt_pending,
             recently_live,
@@ -705,10 +698,6 @@ impl AddressBook {
         // TODO: rename to address_book.[state_name]
         metrics::gauge!("candidate_set.responded", m.responded as f64);
         metrics::gauge!("candidate_set.gossiped", m.never_attempted_gossiped as f64);
-        metrics::gauge!(
-            "candidate_set.alternate",
-            m.never_attempted_alternate as f64,
-        );
         metrics::gauge!("candidate_set.failed", m.failed as f64);
         metrics::gauge!("candidate_set.pending", m.attempt_pending as f64);
 
@@ -754,12 +743,7 @@ impl AddressBook {
 
         self.last_address_log = Some(now);
         // if all peers have failed
-        if m.responded
-            + m.attempt_pending
-            + m.never_attempted_gossiped
-            + m.never_attempted_alternate
-            == 0
-        {
+        if m.responded + m.attempt_pending + m.never_attempted_gossiped == 0 {
             warn!(
                 address_metrics = ?m,
                 "all peer addresses have failed. Hint: check your network connection"

zebra-network/src/address_book_updater.rs

@@ -106,10 +106,9 @@ impl AddressBookUpdater {
                         .set_pos(u64::try_from(address_info.num_addresses).expect("fits in u64"));
                     // .set_len(u64::try_from(address_info.address_limit).expect("fits in u64"));
 
-                    let never_attempted = address_info.never_attempted_alternate
-                        + address_info.never_attempted_gossiped;
-
-                    never_bar.set_pos(u64::try_from(never_attempted).expect("fits in u64"));
+                    never_bar.set_pos(
+                        u64::try_from(address_info.never_attempted_gossiped).expect("fits in u64"),
+                    );
                     // .set_len(u64::try_from(address_info.address_limit).expect("fits in u64"));
 
                     failed_bar.set_pos(u64::try_from(address_info.failed).expect("fits in u64"));

zebra-network/src/meta_addr.rs

@@ -54,15 +54,10 @@ pub enum PeerAddrState {
     Responded,
 
     /// The peer's address has just been fetched from a DNS seeder, or via peer
-    /// gossip, but we haven't attempted to connect to it yet.
+    /// gossip, or as part of a `Version` message, or guessed from an inbound remote IP,
+    /// but we haven't attempted to connect to it yet.
     NeverAttemptedGossiped,
 
-    /// The peer's address has just been received as part of a `Version` message,
-    /// so we might already be connected to this peer.
-    ///
-    /// Alternate addresses are attempted after gossiped addresses.
-    NeverAttemptedAlternate,
-
     /// The peer's TCP connection failed, or the peer sent us an unexpected
     /// Zcash protocol message, so we failed the connection.
     Failed,
@@ -75,7 +70,7 @@ impl PeerAddrState {
     /// Return true if this state is a "never attempted" state.
     pub fn is_never_attempted(&self) -> bool {
         match self {
-            NeverAttemptedGossiped | NeverAttemptedAlternate => true,
+            NeverAttemptedGossiped => true,
             AttemptPending | Responded | Failed => false,
         }
     }
@@ -88,17 +83,8 @@ impl PeerAddrState {
         use Ordering::*;
         match (self, other) {
             _ if self == other => Equal,
-            // Peers start in one of the "never attempted" states,
+            // Peers start in the "never attempted" state,
             // then typically progress towards a "responded" or "failed" state.
-            //
-            // # Security
-            //
-            // Prefer gossiped addresses to alternate addresses,
-            // so that peers can't replace the addresses of other peers.
-            // (This is currently checked explicitly by the address update code,
-            // but we respect the same order here as a precaution.)
-            (NeverAttemptedAlternate, _) => Less,
-            (_, NeverAttemptedAlternate) => Greater,
             (NeverAttemptedGossiped, _) => Less,
             (_, NeverAttemptedGossiped) => Greater,
             (AttemptPending, _) => Less,
@@ -139,8 +125,6 @@ impl Ord for PeerAddrState {
             (_, Responded) => Greater,
             (NeverAttemptedGossiped, _) => Less,
             (_, NeverAttemptedGossiped) => Greater,
-            (NeverAttemptedAlternate, _) => Less,
-            (_, NeverAttemptedAlternate) => Greater,
             (Failed, _) => Less,
             (_, Failed) => Greater,
             // These patterns are redundant, but Rust doesn't assume that `==` is reflexive,
@@ -249,18 +233,6 @@ pub enum MetaAddrChange {
         untrusted_last_seen: DateTime32,
     },
 
-    /// Creates new alternate `MetaAddr`.
-    ///
-    /// Based on the canonical peer address in `Version` messages.
-    NewAlternate {
-        #[cfg_attr(
-            any(test, feature = "proptest-impl"),
-            proptest(strategy = "canonical_peer_addr_strategy()")
-        )]
-        addr: PeerSocketAddr,
-        untrusted_services: PeerServices,
-    },
-
     /// Creates new local listener `MetaAddr`.
     NewLocal {
         #[cfg_attr(
@@ -398,18 +370,6 @@ impl MetaAddr {
         }
     }
 
-    /// Returns a [`MetaAddrChange::NewAlternate`] for a peer's alternate address,
-    /// received via a `Version` message.
-    pub fn new_alternate(
-        addr: PeerSocketAddr,
-        untrusted_services: &PeerServices,
-    ) -> MetaAddrChange {
-        NewAlternate {
-            addr: canonical_peer_addr(*addr),
-            untrusted_services: *untrusted_services,
-        }
-    }
-
     /// Returns a [`MetaAddrChange::NewLocal`] for our own listener address.
     pub fn new_local_listener_change(addr: impl Into<PeerSocketAddr>) -> MetaAddrChange {
         NewLocal {
@@ -715,7 +675,6 @@ impl MetaAddrChange {
         match self {
             NewInitial { addr }
             | NewGossiped { addr, .. }
-            | NewAlternate { addr, .. }
             | NewLocal { addr, .. }
             | UpdateAttempt { addr }
             | UpdateConnected { addr, .. }
@@ -732,7 +691,6 @@ impl MetaAddrChange {
         match self {
             NewInitial { addr }
             | NewGossiped { addr, .. }
-            | NewAlternate { addr, .. }
             | NewLocal { addr, .. }
             | UpdateAttempt { addr }
             | UpdateConnected { addr, .. }
@@ -748,9 +706,6 @@ impl MetaAddrChange {
             // TODO: split untrusted and direct services (#2324)
             NewGossiped {
                 untrusted_services, ..
-            }
-            | NewAlternate {
-                untrusted_services, ..
             } => Some(*untrusted_services),
             // TODO: create a "services implemented by Zebra" constant (#2324)
             NewLocal { .. } => Some(PeerServices::NODE_NETWORK),
@@ -769,7 +724,6 @@ impl MetaAddrChange {
                 untrusted_last_seen,
                 ..
             } => Some(*untrusted_last_seen),
-            NewAlternate { .. } => None,
             // We know that our local listener is available
             NewLocal { .. } => Some(now),
             UpdateAttempt { .. }
@@ -801,7 +755,7 @@ impl MetaAddrChange {
     /// Return the last attempt for this change, if available.
     pub fn last_attempt(&self, now: Instant) -> Option<Instant> {
         match self {
-            NewInitial { .. } | NewGossiped { .. } | NewAlternate { .. } | NewLocal { .. } => None,
+            NewInitial { .. } | NewGossiped { .. } | NewLocal { .. } => None,
             // Attempt changes are applied before we start the handshake to the
             // peer address. So the attempt time is a lower bound for the actual
             // handshake time.
@@ -813,11 +767,7 @@ impl MetaAddrChange {
     /// Return the last response for this change, if available.
     pub fn last_response(&self, now: DateTime32) -> Option<DateTime32> {
         match self {
-            NewInitial { .. }
-            | NewGossiped { .. }
-            | NewAlternate { .. }
-            | NewLocal { .. }
-            | UpdateAttempt { .. } => None,
+            NewInitial { .. } | NewGossiped { .. } | NewLocal { .. } | UpdateAttempt { .. } => None,
             // If there is a large delay applying this change, then:
             // - the peer might stay in the `AttemptPending` state for longer,
             // - we might send outdated last seen times to our peers, and
@@ -833,7 +783,6 @@ impl MetaAddrChange {
         match self {
             NewInitial { .. }
             | NewGossiped { .. }
-            | NewAlternate { .. }
             | NewLocal { .. }
             | UpdateAttempt { .. }
             | UpdateConnected { .. }
@@ -852,7 +801,6 @@ impl MetaAddrChange {
         match self {
             NewInitial { .. } => NeverAttemptedGossiped,
             NewGossiped { .. } => NeverAttemptedGossiped,
-            NewAlternate { .. } => NeverAttemptedAlternate,
             // local listeners get sanitized, so the state doesn't matter here
             NewLocal { .. } => NeverAttemptedGossiped,
             UpdateAttempt { .. } => AttemptPending,

zebra-network/src/meta_addr/arbitrary.rs

@@ -1,6 +1,6 @@
 //! Randomised test data generation for MetaAddr.
 
-use std::{net::IpAddr, time::Instant};
+use std::net::IpAddr;
 
 use proptest::{arbitrary::any, collection::vec, prelude::*};
 
@@ -45,28 +45,6 @@ impl MetaAddr {
             })
             .boxed()
     }
-
-    /// Create a strategy that generates [`MetaAddr`]s in the
-    /// [`NeverAttemptedAlternate`][1] state.
-    ///
-    /// [1]: super::PeerAddrState::NeverAttemptedAlternate
-    pub fn alternate_strategy() -> BoxedStrategy<Self> {
-        (
-            canonical_peer_addr_strategy(),
-            any::<PeerServices>(),
-            any::<Instant>(),
-            any::<DateTime32>(),
-        )
-            .prop_map(
-                |(socket_addr, untrusted_services, instant_now, local_now)| {
-                    // instant_now is not actually used for this variant,
-                    // so we could just provide a default value
-                    MetaAddr::new_alternate(socket_addr, &untrusted_services)
-                        .into_new_meta_addr(instant_now, local_now)
-                },
-            )
-            .boxed()
-    }
 }
 
 impl MetaAddrChange {
@@ -109,33 +87,27 @@ impl MetaAddrChange {
             .boxed()
     }
 
-    /// Create a strategy that generates port numbers for [`MetaAddrChange`]s which are ready for
+    /// Create a strategy that generates port numbers for [`MetaAddr`]s which are ready for
     /// outbound connections.
     ///
-    /// Currently, all generated changes are the [`NewAlternate`][1] variant.
-    /// TODO: Generate all [`MetaAddrChange`] variants, and give them ready
-    /// fields. (After PR #2276 merges.)
+    /// Currently, all generated [`MetaAddr`]s are the [`NeverAttemptedGossiped`][1] variant.
+    ///
+    /// TODO: Generate all [`MetaAddr`] variants, and give them ready fields.
     ///
-    /// [1]: super::NewAlternate
+    /// [1]: super::NeverAttemptedGossiped
     pub fn ready_outbound_strategy_port() -> BoxedStrategy<u16> {
         (
             canonical_peer_addr_strategy(),
-            any::<Instant>(),
+            any::<PeerServices>(),
             any::<DateTime32>(),
         )
             .prop_filter_map(
                 "failed MetaAddr::is_valid_for_outbound",
-                |(addr, instant_now, local_now)| {
-                    // Alternate nodes use the current time, so they're always ready
-                    //
-                    // TODO: create a "Zebra supported services" constant
+                |(addr, services, local_now)| {
+                    let addr = MetaAddr::new_gossiped_meta_addr(addr, services, local_now);
 
-                    let change = MetaAddr::new_alternate(addr, &PeerServices::NODE_NETWORK);
-                    if change
-                        .into_new_meta_addr(instant_now, local_now)
-                        .last_known_info_is_valid_for_outbound(Mainnet)
-                    {
-                        Some(addr.port())
+                    if addr.last_known_info_is_valid_for_outbound(Mainnet) {
+                        Some(addr.addr.port())
                     } else {
                         None
                     }