From 16942963274d08873dfab3a138cd4fa8a9d79ccd Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 27 Jul 2022 21:03:16 +0100
Subject: [PATCH] chanmgr configuration: Hadle lack of a NetDir

Now that the code that actually handles the netdir information can
cope with its lack, we can change the types of the various netdir
parameters and get rid of the foolish Bugs.
---
 crates/tor-chanmgr/src/lib.rs     |  1 -
 crates/tor-chanmgr/src/mgr.rs     | 10 +---------
 crates/tor-chanmgr/src/mgr/map.rs | 18 ++++++++++--------
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/crates/tor-chanmgr/src/lib.rs b/crates/tor-chanmgr/src/lib.rs
index a721a9b71..93e6cb3b9 100644
--- a/crates/tor-chanmgr/src/lib.rs
+++ b/crates/tor-chanmgr/src/lib.rs
@@ -264,7 +264,6 @@ impl<R: Runtime> ChanMgr<R> {
                         let self_ = self_.upgrade().ok_or("channel manager gone away")?;
                         let netdir = netdir.upgrade().ok_or("netdir gone away")?;
                         let netdir = netdir.timely_netdir();
-                        let netdir = if let Ok(nd) = netdir { nd } else { continue };
                         self_.mgr.update_netdir(netdir).map_err(|e| {
                             error!("continually_update_channels_config: failed to process! {} {:?}",
                                    &e, &e);
diff --git a/crates/tor-chanmgr/src/mgr.rs b/crates/tor-chanmgr/src/mgr.rs
index f595de200..f733dffc5 100644
--- a/crates/tor-chanmgr/src/mgr.rs
+++ b/crates/tor-chanmgr/src/mgr.rs
@@ -264,11 +264,9 @@ impl<CF: ChannelFactory> AbstractChanMgr<CF> {
     }
 
     /// Update the netdir
-    ///
-    /// TODO: Handle lack of a NetDir
     pub(crate) fn update_netdir(
         &self,
-        netdir: Arc<NetDir>
+        netdir: tor_netdir::Result<Arc<NetDir>>,
     ) -> StdResult<(), tor_error::Bug> {
         self.channels.reconfigure_general(None, None, netdir)
     }
@@ -279,9 +277,6 @@ impl<CF: ChannelFactory> AbstractChanMgr<CF> {
         dormancy: Dormancy,
         netdir: tor_netdir::Result<Arc<NetDir>>,
     ) -> StdResult<(), tor_error::Bug> {
-        let netdir = netdir.map_err(
-            |_| internal!("should handle lack of netdir!"), // TODO this needs to go away
-        )?;
         self.channels
             .reconfigure_general(None, Some(dormancy), netdir)
     }
@@ -292,9 +287,6 @@ impl<CF: ChannelFactory> AbstractChanMgr<CF> {
         config: &ChannelConfig,
         netdir: tor_netdir::Result<Arc<NetDir>>,
     ) -> StdResult<(), tor_error::Bug> {
-        let netdir = netdir.map_err(
-            |_| internal!("should handle lack of netdir!"), // TODO this needs to go away
-        )?;
         self.channels
             .reconfigure_general(Some(config), None, netdir)
     }
diff --git a/crates/tor-chanmgr/src/mgr/map.rs b/crates/tor-chanmgr/src/mgr/map.rs
index ab36dbc3d..826dec7fd 100644
--- a/crates/tor-chanmgr/src/mgr/map.rs
+++ b/crates/tor-chanmgr/src/mgr/map.rs
@@ -323,13 +323,11 @@ impl<C: AbstractChannel> ChannelMap<C> {
     ///   - dormancy (TODO, this doesn't do anything yet)
     ///
     /// For `new_config` and `new_dormancy`, `None` means "no change to previous info".
-    ///
-    /// TODO: Make this function be able to cope with netdir not being unavailable.
     pub(super) fn reconfigure_general(
         &self,
         new_config: Option<&ChannelConfig>,
         new_dormancy: Option<Dormancy>,
-        netdir: Arc<NetDir>,
+        netdir: tor_netdir::Result<Arc<NetDir>>,
     ) -> StdResult<(), tor_error::Bug> {
         use ChannelState as CS;
 
@@ -340,11 +338,14 @@ impl<C: AbstractChannel> ChannelMap<C> {
         // TODO when we support operation as a relay, inter-relay channels ought
         // not to get padding.
         let netdir = {
-            let nde = NetDirExtract::from(&*netdir);
+            let extract = netdir
+                .as_ref()
+                .map(|n| NetDirExtract::from(&**n))
+                .map_err(|_| ());
             // Drop the `Arc<NetDir>` as soon as we have got what we need from it,
             // before we take the channel map lock.
             drop(netdir);
-            nde
+            extract
         };
 
         let mut inner = self
@@ -359,7 +360,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
             inner.dormancy = new_dormancy;
         }
 
-        let padding_parameters = padding_parameters(inner.config.padding, Ok(&netdir))?;
+        let padding_parameters = padding_parameters(inner.config.padding, netdir.as_ref())?;
         // TODO if this is equal to all_zeroes(), do not enable padding
         // (when we enable padding at all, which we do not do yet...)
 
@@ -672,7 +673,8 @@ mod test {
         };
 
         eprintln!("-- process a default netdir, which should send an update --");
-        map.reconfigure_general(None, None, netdir.clone()).unwrap();
+        map.reconfigure_general(None, None, Ok(netdir.clone()))
+            .unwrap();
         with_ch(&|ch| {
             assert_eq!(
                 format!("{:?}", ch.params_update.take().unwrap()),
@@ -686,7 +688,7 @@ mod test {
         eprintln!();
 
         eprintln!("-- process a default netdir again, which should *not* send an update --");
-        map.reconfigure_general(None, None, netdir).unwrap();
+        map.reconfigure_general(None, None, Ok(netdir)).unwrap();
         with_ch(&|ch| assert_eq!(ch.params_update, None));
     }