diff --git a/crates/tor-circmgr/src/hspool.rs b/crates/tor-circmgr/src/hspool.rs index f788bc99f..57458d5ea 100644 --- a/crates/tor-circmgr/src/hspool.rs +++ b/crates/tor-circmgr/src/hspool.rs @@ -279,7 +279,7 @@ impl HsCircPool { let mut inner = self.inner.lock().expect("lock poisoned"); inner .pool - .retain(|circ| all_circ_relays_are_listed_in(circ, netdir)); + .retain(|circ| circuit_still_useable(netdir, circ, |_relay| true)); } } @@ -331,6 +331,21 @@ fn circuit_compatible_with_target( circ: &ClientCirc, target: Option<&TargetInfo<'_>>, ) -> bool { + circuit_still_useable(netdir, circ, |relay| match target { + Some(t) => t.may_share_circuit_with(relay, subnet_config), + None => true, + }) +} + +/// Return true if we can still use a given pre-build circuit. +/// +/// We require that the circuit is open, that every hop in the circuit is +/// listed in `netdir`, and that `relay_okay` returns true for every hop on the +/// circuit. +fn circuit_still_useable(netdir: &NetDir, circ: &ClientCirc, relay_okay: F) -> bool +where + F: Fn(&Relay<'_>) -> bool, +{ if circ.is_closing() { return false; } @@ -342,38 +357,17 @@ fn circuit_compatible_with_target( // This is a virtual hop; it's necessarily compatible with everything. return true; }; - match (target, netdir.by_ids(c)) { + let Some(relay) = netdir.by_ids(c) else { // We require that every relay in this circuit is still listed; an // unlisted relay means "reject". - (_, None) => false, - // If we have a target, the relay must be compatible with it. - (Some(t), Some(r)) => t.may_share_circuit_with(&r, subnet_config), - // If we have no target, any listed relay is okay. - (None, Some(_)) => true, - } + return false; + }; + // Now it's all down to the predicate. + relay_okay(&relay) }); all_compatible } -/// Return true if every relay in `circ` is listed in `netdir`. -fn all_circ_relays_are_listed_in(circ: &ClientCirc, netdir: &NetDir) -> bool { - let path = circ.path_ref(); - - // TODO HS: There is some duplicate logic here and in - // circuit_compatible_with_target. I think that's acceptable for now, but - // we should consider refactoring if these functions grow. - - // (We have to use a binding here to appease borrowck.) - let all_listed = path.iter().all(|ent: &circuit::PathEntry| { - if let Some(c) = ent.as_chan_target() { - netdir.by_ids(c).is_some() - } else { - true - } - }); - all_listed -} - /// Background task to launch onion circuits as needed. async fn launch_hs_circuits_as_needed( pool: Weak>,