circmgr: Use guard-manager's view of the fallbacks when possible.

If we're building a path with the guard manager involved, we now ask
the guard manager to pick our first hop no matter what.  We only
pick from the fallback list ourselves if we're using the API with no
guard manager.

This causes some follow-on changes where we have to remember an
OwnedChanTarget object in a TorPath we've built, and where we gain
the ability to say we're building a path "from nothing extra at
all."  Those are all internal to the crate, though.

Closes #220, by making sure that we use our guards to get a fresh
netdir (if we can) before falling back to any fallbacks, even if our
consensus is old.

Compilation should be fixed in the next commit.

crates/tor-circmgr/src/lib.rs

@@ -114,6 +114,9 @@ pub enum DirInfo<'a> {
     Fallbacks(&'a FallbackList),
     /// A complete network directory
     Directory(&'a NetDir),
+    /// No information: we can only build one-hop paths: and that, only if the
+    /// guard manager knows some guards or fallbacks.
+    Nothing,
 }
 
 impl<'a> From<&'a FallbackList> for DirInfo<'a> {
@@ -144,8 +147,8 @@ impl<'a> DirInfo<'a> {
         }
 
         match self {
-            DirInfo::Fallbacks(_) => from_netparams(&NetParameters::default()),
             DirInfo::Directory(d) => from_netparams(d.params()),
+            _ => from_netparams(&NetParameters::default()),
         }
     }
 }

crates/tor-circmgr/src/path.rs

@@ -32,6 +32,8 @@ enum TorPathInner<'a> {
     /// A single-hop path for use with a directory cache, when we don't have
     /// a consensus.
     FallbackOneHop(&'a FallbackDir),
+    /// A single-hop path taken from an OwnedChanTarget.
+    OwnedOneHop(OwnedChanTarget),
     /// A multi-hop path, containing one or more relays.
     Path(Vec<Relay<'a>>),
 }
@@ -53,6 +55,14 @@ impl<'a> TorPath<'a> {
         }
     }
 
+    /// Construct a new one-hop path for directory use from an arbitrarily
+    /// chosen channel target.
+    pub fn new_one_hop_owned<T: tor_linkspec::ChanTarget>(target: &T) -> Self {
+        Self {
+            inner: TorPathInner::OwnedOneHop(OwnedChanTarget::from_chan_target(target)),
+        }
+    }
+
     /// Create a new multi-hop path with a given number of ordered relays.
     pub fn new_multihop(relays: impl IntoIterator<Item = Relay<'a>>) -> Self {
         Self {
@@ -82,6 +92,7 @@ impl<'a> TorPath<'a> {
         match &self.inner {
             OneHop(_) => 1,
             FallbackOneHop(_) => 1,
+            OwnedOneHop(_) => 1,
             Path(p) => p.len(),
         }
     }
@@ -104,6 +115,7 @@ impl<'a> TryFrom<&TorPath<'a>> for OwnedPath {
         Ok(match &p.inner {
             FallbackOneHop(h) => OwnedPath::ChannelOnly(OwnedChanTarget::from_chan_target(*h)),
             OneHop(h) => OwnedPath::Normal(vec![OwnedCircTarget::from_circ_target(h)]),
+            OwnedOneHop(owned) => OwnedPath::ChannelOnly(owned.clone()),
             Path(p) if !p.is_empty() => {
                 OwnedPath::Normal(p.iter().map(OwnedCircTarget::from_circ_target).collect())
             }

crates/tor-circmgr/src/path/dirpath.rs

 //! Code to construct paths to a directory for non-anonymous downloads
 use super::TorPath;
 use crate::{DirInfo, Error, Result};
+use tor_error::bad_api_usage;
 use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable};
 use tor_netdir::{Relay, WeightRole};
 use tor_rtcompat::Runtime;
@@ -42,18 +43,31 @@ impl DirPathBuilder {
                     return Ok((TorPath::new_one_hop(r), None, None));
                 }
             }
-            (DirInfo::Directory(netdir), Some(guardmgr)) => {
-                // TODO: We might want to use the guardmgr even if
-                // we don't have a netdir.  See arti#220.
-                guardmgr.update_network(netdir); // possibly unnecessary.
+            (DirInfo::Nothing, None) => {
+                return Err(bad_api_usage!(
+                    "Tried to build a one hop path with no directory, fallbacks, or guard manager"
+                )
+                .into());
+            }
+            (dirinfo, Some(guardmgr)) => {
+                // We use a guardmgr whenever we have one, regardless of whether
+                // there's a netdir.
+                //
+                // That way, we prefer our guards (if they're up) before we default to the fallback directories.
+                let netdir = match dirinfo {
+                    DirInfo::Directory(netdir) => {
+                        guardmgr.update_network(netdir); // possibly unnecessary.
+                        Some(netdir)
+                    }
+                    _ => None,
+                };
+
                 let guard_usage = tor_guardmgr::GuardUsageBuilder::default()
                     .kind(tor_guardmgr::GuardUsageKind::OneHopDirectory)
                     .build()
                     .expect("Unable to build directory guard usage");
-                let (guard, mon, usable) = guardmgr.select_guard(guard_usage, Some(netdir))?;
-                if let Some(r) = guard.get_relay(netdir) {
-                    return Ok((TorPath::new_one_hop(r), Some(mon), Some(usable)));
-                }
+                let (guard, mon, usable) = guardmgr.select_guard(guard_usage, netdir)?;
+                return Ok((TorPath::new_one_hop_owned(&guard), Some(mon), Some(usable)));
             }
         }
         Err(Error::NoPath(
@@ -142,7 +156,13 @@ mod test {
         let guards: OptDummyGuardMgr<'_> = None;
 
         let err = DirPathBuilder::default().pick_path(&mut rng, dirinfo, guards);
-        assert!(matches!(err, Err(Error::NoPath(_))));
+        dbg!(err.as_ref().err());
+        assert!(matches!(
+            err,
+            Err(Error::Guard(
+                tor_guardmgr::PickGuardError::AllFallbacksDown { .. }
+            ))
+        ));
     }
 
     #[test]
@@ -166,7 +186,7 @@ mod test {
                 let (path, mon, usable) = DirPathBuilder::new()
                     .pick_path(&mut rng, dirinfo, Some(&guards))
                     .unwrap();
-                if let crate::path::TorPathInner::OneHop(relay) = path.inner {
+                if let crate::path::TorPathInner::OwnedOneHop(relay) = path.inner {
                     distinct_guards.insert(relay.ed_identity().clone());
                     mon.unwrap().succeeded();
                     assert!(usable.unwrap().await.unwrap());

crates/tor-circmgr/src/path/exitpath.rs

@@ -125,13 +125,13 @@ impl<'a> ExitPathBuilder<'a> {
         now: SystemTime,
     ) -> Result<(TorPath<'a>, Option<GuardMonitor>, Option<GuardUsable>)> {
         let netdir = match netdir {
-            DirInfo::Fallbacks(_) => {
+            DirInfo::Directory(d) => d,
+            _ => {
                 return Err(bad_api_usage!(
-                    "Tried to build a multihop path using only a list of fallback caches"
+                    "Tried to build a multihop path without a network directory"
                 )
                 .into())
             }
-            DirInfo::Directory(d) => d,
         };
         let subnet_config = config.subnet_config();
         let lifetime = netdir.lifetime();