From e098a5a303fc50ea344b5a9b3fd573d5b181192e Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 8 Jun 2022 09:27:48 -0400
Subject: [PATCH] Move set_filter into GuardMgrInner.

Convert its argument type to Option<&NetDir> to better match the
rest of the API.
---
 crates/tor-guardmgr/semver.md     |  1 +
 crates/tor-guardmgr/src/filter.rs |  1 -
 crates/tor-guardmgr/src/lib.rs    | 73 ++++++++++++++-----------------
 3 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/crates/tor-guardmgr/semver.md b/crates/tor-guardmgr/semver.md
index 2586dcf93..d4a5b3125 100644
--- a/crates/tor-guardmgr/semver.md
+++ b/crates/tor-guardmgr/semver.md
@@ -1 +1,2 @@
 BREAKING: GuardFilter is no longer an enum.
+BREAKING: set_filter now takes an Option<&NetDir>
diff --git a/crates/tor-guardmgr/src/filter.rs b/crates/tor-guardmgr/src/filter.rs
index 680b5a624..1ff9d7b4d 100644
--- a/crates/tor-guardmgr/src/filter.rs
+++ b/crates/tor-guardmgr/src/filter.rs
@@ -82,7 +82,6 @@ impl GuardFilter {
 
     /// Return a fraction between 0.0 and 1.0 describing what fraction of the
     /// guard bandwidth this filter permits.
-    #[allow(dead_code)]
     pub(crate) fn frac_bw_permitted(&self, netdir: &tor_netdir::NetDir) -> f64 {
         use tor_netdir::{RelayWeight, WeightRole};
         let mut guard_bw: RelayWeight = 0.into();
diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs
index f209d36fe..09c7a5d59 100644
--- a/crates/tor-guardmgr/src/lib.rs
+++ b/crates/tor-guardmgr/src/lib.rs
@@ -453,47 +453,10 @@ impl<R: Runtime> GuardMgr<R> {
     ///
     /// (Since there is only one kind of filter right now, there's no
     /// real reason to call this function, but at least it should work.
-    pub fn set_filter(&self, filter: GuardFilter, netdir: &NetDir) {
-        // First we have to see how much of the possible guard space
-        // this new filter allows.  (We don't use this info yet, but we will
-        // one we have nontrivial filters.)
-        let n_guards = netdir.relays().filter(|r| r.is_flagged_guard()).count();
-        let n_permitted = netdir
-            .relays()
-            .filter(|r| r.is_flagged_guard() && filter.permits(r))
-            .count();
-        let frac_permitted = if n_guards > 0 {
-            n_permitted as f64 / (n_guards as f64)
-        } else {
-            1.0
-        };
-
+    pub fn set_filter(&self, filter: GuardFilter, netdir: Option<&NetDir>) {
         let now = self.runtime.wallclock();
         let mut inner = self.inner.lock().expect("Poisoned lock");
-
-        let restrictive_filter = frac_permitted < inner.params.filter_threshold;
-
-        // TODO: Once we support nontrivial filters, we might have to
-        // swap out "active_guards" depending on which set it is.
-
-        if frac_permitted < inner.params.extreme_threshold {
-            warn!(
-                "The number of guards permitted is smaller than the guard param minimum of {}%.",
-                inner.params.extreme_threshold * 100.0,
-            );
-        }
-
-        info!(
-            ?filter,
-            restrictive = restrictive_filter,
-            "Guard filter replaced."
-        );
-
-        inner
-            .guards
-            .active_guards_mut()
-            .set_filter(filter, restrictive_filter);
-        inner.update(now, Some(netdir));
+        inner.set_filter(filter, netdir, now);
     }
 
     /// Select a guard for a given [`GuardUsage`].
@@ -805,6 +768,36 @@ impl GuardMgrInner {
         }
     }
 
+    /// Replace the current GuardFilter with `filter`.
+    fn set_filter(&mut self, filter: GuardFilter, netdir: Option<&NetDir>, now: SystemTime) {
+        self.with_opt_netdir(netdir, |this, netdir| {
+            let frac_permitted = netdir.map(|nd| filter.frac_bw_permitted(nd)).unwrap_or(1.0);
+
+            let restrictive_filter = frac_permitted < this.params.filter_threshold;
+
+            // TODO: Once we support nontrivial filters, we might have to
+            // swap out "active_guards" depending on which set it is.
+
+            if frac_permitted < this.params.extreme_threshold {
+                warn!(
+                "The number of guards permitted is smaller than the guard param minimum of {}%.",
+                this.params.extreme_threshold * 100.0,
+            );
+            }
+
+            info!(
+                ?filter,
+                restrictive = restrictive_filter,
+                "Guard filter replaced."
+            );
+
+            this.guards
+                .active_guards_mut()
+                .set_filter(filter, restrictive_filter);
+            this.update_internal(now, netdir);
+        });
+    }
+
     /// Called when the circuit manager reports (via [`GuardMonitor`]) that
     /// a guard succeeded or failed.
     ///
@@ -1494,7 +1487,7 @@ mod test {
                 f.push_reachable_addresses(vec!["2.0.0.0/7:9001".parse().unwrap()]);
                 f
             };
-            guardmgr.set_filter(filter, &netdir);
+            guardmgr.set_filter(filter, Some(&netdir));
             let (guard, _mon, _usable) = guardmgr.select_guard(u, Some(&netdir)).unwrap();
             // Make sure that the filter worked.
             let addr = guard.addrs()[0];