From 6c5dd0569873bc29671a3debdf8080febfa11c65 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 17 Nov 2022 12:17:42 -0500
Subject: [PATCH] GuardMgr: split Reachable::Unknown into Untried and Retriable

There are two cases here, and we will want to log them
differently.

(By removing the "Unknown" variant entirely, we ensure that we
didn't miss any code that formerly checked for Unknown.)
---
 crates/tor-guardmgr/src/guard.rs  | 33 +++++++++++++++----------------
 crates/tor-guardmgr/src/sample.rs |  8 +++++---
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/crates/tor-guardmgr/src/guard.rs b/crates/tor-guardmgr/src/guard.rs
index 9eeb3bec0..98669a600 100644
--- a/crates/tor-guardmgr/src/guard.rs
+++ b/crates/tor-guardmgr/src/guard.rs
@@ -29,16 +29,15 @@ pub(crate) enum Reachable {
     /// used it more recently than we've failed.
     Reachable,
     /// A guard is believed to be unreachable, since recent attempts
-    /// to use it have failed.
+    /// to use it have failed, and not enough time has elapsed since then.
     Unreachable,
-    /// A guard's reachability status is unknown.
-    ///
-    /// The status might be unknown for a variety of reasons, including:
-    ///   * We haven't tried to use the guard.
-    ///   * Attempts to use it have failed, but those attempts are far
-    ///     enough in the past that we're willing to retry them.
+    /// We have never (during the lifetime of the current guard manager)
+    /// tried to connect to this guard.
     #[educe(Default)]
-    Unknown,
+    Untried,
+    /// The last time that we tried to connect to this guard, it failed,
+    /// but enough time has elapsed that we think it is worth trying again.
+    Retriable,
 }
 
 /// The name and version of the crate that first picked a potential
@@ -298,7 +297,7 @@ impl Guard {
             unlisted_since: None,
             dir_info_missing: false,
             last_tried_to_connect_at: None,
-            reachable: Reachable::Unknown,
+            reachable: Reachable::Untried,
             retry_at: None,
             dir_status: guard_dirstatus(),
             retry_schedule: None,
@@ -453,10 +452,10 @@ impl Guard {
     }
 
     /// If this guard is marked Unreachable, clear its unreachability status
-    /// and mark it as Unknown.
+    /// and mark it as Retriable.
     pub(crate) fn mark_retriable(&mut self) {
-        if self.reachable != Reachable::Reachable {
-            self.set_reachable(Reachable::Unknown);
+        if self.reachable == Reachable::Unreachable {
+            self.set_reachable(Reachable::Retriable);
             self.retry_at = None;
             self.retry_schedule = None;
         }
@@ -919,7 +918,7 @@ mod test {
         assert_eq!(g.guard_id(), &id);
         assert!(g.same_relay_ids(&FirstHopId::in_sample(GuardSetSelector::Default, id)));
         assert_eq!(g.addrs(), &["127.0.0.7:7777".parse().unwrap()]);
-        assert_eq!(g.reachable(), Reachable::Unknown);
+        assert_eq!(g.reachable(), Reachable::Untried);
         assert_eq!(g.reachable(), Reachable::default());
 
         use crate::GuardUsageBuilder;
@@ -1077,7 +1076,7 @@ mod test {
         // Retriable right after the retry time.
         g.consider_retry(g.retry_at.unwrap() + Duration::from_secs(1));
         assert!(g.retry_at.is_none());
-        assert_eq!(g.reachable(), Reachable::Unknown);
+        assert_eq!(g.reachable(), Reachable::Retriable);
     }
 
     #[test]
@@ -1290,11 +1289,11 @@ mod test {
         let mut g = basic_guard();
         use super::Reachable::*;
 
-        assert_eq!(g.reachable(), Unknown);
+        assert_eq!(g.reachable(), Untried);
 
         for (pre, post) in &[
-            (Unknown, Unknown),
-            (Unreachable, Unknown),
+            (Untried, Untried),
+            (Unreachable, Retriable),
             (Reachable, Reachable),
         ] {
             g.reachable = *pre;
diff --git a/crates/tor-guardmgr/src/sample.rs b/crates/tor-guardmgr/src/sample.rs
index 5ddf0f4a6..bb2da8242 100644
--- a/crates/tor-guardmgr/src/sample.rs
+++ b/crates/tor-guardmgr/src/sample.rs
@@ -825,8 +825,10 @@ impl GuardSet {
                 match (src, guard.reachable()) {
                     (_, Reachable::Reachable) => return Some(false),
                     (_, Reachable::Unreachable) => (),
-                    (ListKind::Primary, Reachable::Unknown) => return Some(false),
-                    (_, Reachable::Unknown) => {
+                    (ListKind::Primary, Reachable::Untried | Reachable::Retriable) => {
+                        return Some(false)
+                    }
+                    (_, Reachable::Untried | Reachable::Retriable) => {
                         if guard.exploratory_attempt_after(cutoff) {
                             return None;
                         }
@@ -1501,7 +1503,7 @@ mod test {
             let g2 = guards1.get(&id2).unwrap();
             assert!(g1.confirmed());
             assert!(!g2.confirmed());
-            assert_eq!(g1.reachable(), Reachable::Unknown);
+            assert_eq!(g1.reachable(), Reachable::Untried);
             assert_eq!(g2.reachable(), Reachable::Unreachable);
         }