move transport config checks to respective modules

6 files changed, 204 insertions, 59 deletions

diff --git a/src/config/mod.rs b/src/config/mod.rs
index 22cbef2..b82ad9b 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -577,7 +577,8 @@ impl Settings {
 
     /// The options only a supervised Mycelium daemon takes, and whether each is set: the
     /// other side of `CONFLICTS`' `mycelium-rest-url` (a test keeps them the same).
-    fn supervised_only(&self) -> [(&'static str, bool); 4] {
+    #[cfg_attr(not(any(feature = "mycelium", test)), allow(dead_code))]
+    pub(crate) fn supervised_only(&self) -> [(&'static str, bool); 4] {
         [
             ("mycelium-key-file", self.mycelium_key_file.is_some()),
             (
@@ -603,20 +604,6 @@ impl Settings {
                 crate::logging::LogFormat::NAMES.join(", ")
             );
         }
-        if let Some(url) = &self.mycelium_rest_url {
-            url::Url::parse(url).with_context(|| format!("mycelium-rest-url {url:?}"))?;
-        }
-        if self.mycelium_rest_url.is_some()
-            && let Some((name, _)) = self.supervised_only().iter().find(|(_, set)| *set)
-        {
-            bail!(
-                "{name} is for a supervised Mycelium daemon and can't be combined with mycelium-rest-url"
-            );
-        }
-        // A private network has both; one alone would be ignored or invented.
-        if self.mycelium_network_key_file.is_some() != self.mycelium_network_name.is_some() {
-            bail!("mycelium-network-key-file and mycelium-network-name go together");
-        }
         crate::force::Force::parse(self.force.iter().map(String::as_str), FORCE)
             .map_err(|reason| anyhow!("force: {reason}"))?;
         // Contradictions, not overrides: an error wherever the options come from.
@@ -654,11 +641,6 @@ impl Settings {
                     .join(", ")
             );
         }
-        if self.transports.iter().any(|name| name == "tor") && self.tor_state_dir.is_none() {
-            bail!(
-                "tor needs tor-state-dir: where arti keeps its state, the onion service's key among it"
-            );
-        }
         for (index, name) in self.transports.iter().enumerate() {
             match TRANSPORTS.iter().find(|(known, _)| known == name) {
                 None => bail!(
@@ -678,6 +660,8 @@ impl Settings {
                 Some(_) => {}
             }
         }
+        // Each transport's own options, now that the names are good.
+        crate::transport::check(self)?;
         if self.debug_epoch.is_some() && !self.debug_api {
             bail!("debug-epoch needs debug-api");
         }
@@ -713,14 +697,7 @@ impl Settings {
         if self.transports != ["libp2p"] {
             bail!("relay-only relays over libp2p alone: set transports to libp2p");
         }
-        if self.libp2p_listen.is_empty() {
-            bail!("relay-only needs libp2p-listen: where nodes reach the relay");
-        }
-        if self.libp2p_key_file.is_none() {
-            bail!(
-                "relay-only needs libp2p-key-file: without it, the relay's peer ID changes with every start and nodes lose it"
-            );
-        }
+        // Its listen address and key: libp2p's checks (`transport::libp2p::check`).
         let swarm_options = [
             ("node-key-file", self.node_key_file.is_some()),
             ("create-node-key", self.create_node_key),
@@ -852,7 +829,7 @@ fn unset_entry(provenance: Provenance) -> Entry {
 /// Options that can't be combined. A source that sets one side drops the other side's
 /// values from earlier sources (lower precedence), so e.g. `--mycelium-rest-url` on the
 /// command line overrides `mycelium-key-file` from a file. Both set in one source is an
-/// error (`Settings::check`).
+/// error (`transport::mycelium::check`, where Mycelium is built).
 const CONFLICTS: &[(&[&str], &[&str])] = &[(
     &["mycelium-rest-url"],
     &[
@@ -1222,6 +1199,29 @@ pub(crate) struct Loaded {
     pub(crate) print_only: bool,
 }
 
+impl Loaded {
+    /// The options given for transports this node doesn't use (`transports`), which it
+    /// ignores: per transport, whether this build has it, and the options. A shared
+    /// configuration may well carry them; still worth a warning, as it may as well be a
+    /// mistake (a build without that transport, or a transport left out of the list).
+    pub(crate) fn unused_transport_options(&self) -> Vec<(&'static str, bool, Vec<String>)> {
+        TRANSPORTS
+            .iter()
+            .filter(|(name, _)| !self.settings.transports.iter().any(|used| used == name))
+            .filter_map(|(name, built)| {
+                let prefix = format!("{name}-");
+                let options: Vec<String> = self
+                    .origins
+                    .iter()
+                    .filter(|(option, origin)| option.starts_with(&prefix) && !origin.unset)
+                    .map(|(option, _)| option.clone())
+                    .collect();
+                (!options.is_empty()).then_some((*name, *built, options))
+            })
+            .collect()
+    }
+}
+
 /// How `GET /admin/config` shows text with placeholders (`?values=`).
 #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -1426,10 +1426,22 @@ mod tests {
             .iter()
             .map(|(name, value)| (name.clone(), entry(value.clone())))
             .collect();
+        with_built_transport(&mut layer);
         layer
-            .entry("transports".to_owned())
-            .or_insert_with(|| entry(json!([built_transport()])));
-        layer
+    }
+
+    /// Unless `layer` names transports: this build's first, with what it needs to start
+    /// (Tor its state directory).
+    fn with_built_transport(layer: &mut Layer) {
+        if layer.contains_key("transports") {
+            return;
+        }
+        layer.insert("transports".to_owned(), entry(json!([built_transport()])));
+        if built_transport() == "tor" {
+            layer
+                .entry("tor-state-dir".to_owned())
+                .or_insert_with(|| entry(json!("tor")));
+        }
     }
 
     #[test]
@@ -1526,12 +1538,7 @@ mod tests {
             file(json!("a")),
             Overlay::FileOverFile,
         );
-        overlay(
-            &mut merged,
-            "transports".into(),
-            file(json!([built_transport()])),
-            Overlay::FileOverFile,
-        );
+        with_built_transport(&mut merged);
 
         // Both in one source: an error.
         let mut same = merged.clone();
@@ -1541,7 +1548,11 @@ mod tests {
             file(url.clone()),
             Overlay::FileOverFile,
         );
-        assert!(resolve(same, Vec::new()).is_err());
+        // (Mycelium's check, where it's built.)
+        assert_eq!(
+            resolve(same, Vec::new()).is_err(),
+            cfg!(feature = "mycelium")
+        );
 
         // The command line wins over the conflicting file value, and unsets.
         overlay(
@@ -1602,6 +1613,52 @@ mod tests {
         assert_eq!(from_text(Kind::Flag, "yes").unwrap(), json!(true));
     }
 
+    /// Options of transports not in `transports` are named, per transport; those of
+    /// the transports used aren't, nor unset ones.
+    #[test]
+    fn unused_transport_options_are_named() {
+        let mut values = layer(json!({
+            "htpasswd": "u",
+            "node-key-file": "node.key",
+            "libp2p-listen": ["/ip4/0.0.0.0/udp/0/quic-v1"],
+            "libp2p-mdns": true,
+            "tor-port": 9000,
+            "mycelium-peers": ["tcp://a:9651"],
+        }));
+        values.get_mut("tor-port").unwrap().origin.unset = true;
+        let loaded = resolve(values, Vec::new()).unwrap();
+        let unused = loaded.unused_transport_options();
+        for (name, built, options) in &unused {
+            assert_ne!(*name, built_transport());
+            assert_eq!(
+                *built,
+                TRANSPORTS.iter().any(|(known, b)| known == name && *b)
+            );
+            assert!(
+                options
+                    .iter()
+                    .all(|option| option.starts_with(&format!("{name}-")))
+            );
+        }
+        let named = |name: &str| {
+            unused
+                .iter()
+                .find(|(n, _, _)| *n == name)
+                .map(|(_, _, o)| o.clone())
+        };
+        if built_transport() != "libp2p" {
+            assert_eq!(
+                named("libp2p"),
+                Some(vec!["libp2p-listen".to_owned(), "libp2p-mdns".to_owned()])
+            );
+        }
+        if built_transport() != "mycelium" {
+            assert_eq!(named("mycelium"), Some(vec!["mycelium-peers".to_owned()]));
+        }
+        // Unset: not given.
+        assert_eq!(named("tor"), None);
+    }
+
     /// `Settings::check` and the override rule name the same supervised-only options.
     #[test]
     fn supervised_only_options_are_the_conflicts() {
@@ -1638,7 +1695,7 @@ mod tests {
             )
             .is_err()
         );
-        assert!(
+        assert_eq!(
             resolve(
                 layer(json!({
                     "htpasswd": "u",
@@ -1648,7 +1705,8 @@ mod tests {
                 })),
                 Vec::new()
             )
-            .is_err()
+            .is_err(),
+            cfg!(feature = "mycelium")
         );
         assert!(
             resolve(
@@ -1757,9 +1815,7 @@ mod tests {
         let mut layer = arguments(&matches).unwrap();
         assert!(!layer["alias"].origin.is_imported());
         assert_eq!(layer["alias"].shown("alias"), "\"node-berlin\"");
-        layer
-            .entry("transports".to_owned())
-            .or_insert_with(|| entry(json!([built_transport()])));
+        with_built_transport(&mut layer);
         let loaded = resolve(layer, Vec::new()).unwrap();
 
         let report = loaded.report(Values::Interpolated);
diff --git a/src/main.rs b/src/main.rs
index 1908775..da79668 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -54,6 +54,14 @@ async fn main() -> Result<()> {
     if let Some(format) = logging::LogFormat::parse(&args.log_format) {
         logging::set_format(format);
     }
+    for (transport, built, options) in config.unused_transport_options() {
+        warn!(
+            transport,
+            built,
+            options = ?options,
+            "Options of a transport this node doesn't use are ignored"
+        );
+    }
     if args.relay_only {
         return relay_only(args).await;
     }
diff --git a/src/transport/libp2p.rs b/src/transport/libp2p.rs
index 2d67d2c..d023079 100644
--- a/src/transport/libp2p.rs
+++ b/src/transport/libp2p.rs
@@ -204,6 +204,48 @@ pub(crate) struct Libp2pConfig {
     pub(crate) memory: bool,
 }
 
+/// The multiaddrs of option `option`.
+fn parse_addresses(option: &str, addresses: &[String]) -> Result<Vec<Multiaddr>> {
+    addresses
+        .iter()
+        .map(|address| {
+            address
+                .parse::<Multiaddr>()
+                .with_context(|| format!("{option} {address:?}"))
+        })
+        .collect()
+}
+
+/// The ranges of `libp2p-ignore`.
+fn parse_ranges(ranges: &[String]) -> Result<Vec<IpRange>> {
+    ranges
+        .iter()
+        .map(|range| range.parse().context("libp2p-ignore"))
+        .collect()
+}
+
+/// This transport's options, checked when the configuration is loaded
+/// (`config::Settings::check`), so `--print-config` tells a wrong one too: the
+/// multiaddrs and ranges parse, and a relay that is no node (`--relay-only`) can be
+/// dialled and keeps its peer ID.
+pub(crate) fn check(settings: &crate::config::Settings) -> Result<()> {
+    parse_addresses("libp2p-listen", &settings.libp2p_listen)?;
+    parse_addresses("libp2p-announce", &settings.libp2p_announce)?;
+    parse_addresses("libp2p-relays", &settings.libp2p_relays)?;
+    parse_ranges(&settings.libp2p_ignore)?;
+    if settings.relay_only {
+        if settings.libp2p_listen.is_empty() {
+            bail!("relay-only needs libp2p-listen: where nodes reach the relay");
+        }
+        if settings.libp2p_key_file.is_none() {
+            bail!(
+                "relay-only needs libp2p-key-file: without it, the relay's peer ID changes with every start and nodes lose it"
+            );
+        }
+    }
+    Ok(())
+}
+
 impl Libp2pConfig {
     /// The configuration `settings` give; `relay_only`: a relay that is no node
     /// (`--relay-only`).
@@ -211,16 +253,7 @@ impl Libp2pConfig {
         settings: &crate::config::Settings,
         relay_only: bool,
     ) -> Result<Self> {
-        let parse = |option: &str, addresses: &[String]| {
-            addresses
-                .iter()
-                .map(|address| {
-                    address
-                        .parse::<Multiaddr>()
-                        .with_context(|| format!("{option} {address:?}"))
-                })
-                .collect::<Result<Vec<_>>>()
-        };
+        let parse = parse_addresses;
         Ok(Self {
             key: Self::load_key(settings.libp2p_key_file.as_deref())?,
             listen: parse("libp2p-listen", &settings.libp2p_listen)?,
@@ -230,11 +263,7 @@ impl Libp2pConfig {
             continuum: !relay_only,
             mdns: settings.libp2p_mdns,
             upnp: settings.libp2p_upnp,
-            ignore: settings
-                .libp2p_ignore
-                .iter()
-                .map(|range| range.parse().context("libp2p-ignore"))
-                .collect::<Result<Vec<_>>>()?,
+            ignore: parse_ranges(&settings.libp2p_ignore)?,
             public: settings.libp2p_public,
             memory: false,
         })
diff --git a/src/transport/mod.rs b/src/transport/mod.rs
index d8c6e2b..fd3651b 100644
--- a/src/transport/mod.rs
+++ b/src/transport/mod.rs
@@ -34,6 +34,27 @@ compile_error!(
     "continuum needs a transport: build with at least one of the mycelium, libp2p and tor features, e.g. `cargo build --features mycelium`"
 );
 
+/// The options of the transports this build has, each checked by its transport when the
+/// configuration is loaded (`config::Settings::check`, after the names in `transports`):
+/// Mycelium's always (its conflicts are errors wherever they come from, as before);
+/// libp2p's and Tor's if the node uses them. Options of transports not built aren't
+/// checked: they're ignored, and said so (`config::Loaded::unused_transport_options`).
+pub(crate) fn check(settings: &crate::config::Settings) -> Result<()> {
+    let uses = |name: &str| settings.transports.iter().any(|used| used == name);
+    let _ = uses;
+    #[cfg(feature = "mycelium")]
+    mycelium::check(settings)?;
+    #[cfg(feature = "libp2p")]
+    if uses("libp2p") {
+        libp2p::check(settings)?;
+    }
+    #[cfg(feature = "tor")]
+    if uses("tor") {
+        tor::TorTransport::check(settings)?;
+    }
+    Ok(())
+}
+
 /// Starts the transport `name` (one of `config::TRANSPORTS`) as `settings` configure
 /// it: its scheme and the transport, or None if `stop` came first (some take a while to
 /// start: a supervised Mycelium daemon, arti). `key`: the node key, which some prove
diff --git a/src/transport/mycelium/mod.rs b/src/transport/mycelium/mod.rs
index 7abcced..20abee8 100644
--- a/src/transport/mycelium/mod.rs
+++ b/src/transport/mycelium/mod.rs
@@ -91,6 +91,26 @@ pub(crate) struct MyceliumTransport {
     local_id: String,
 }
 
+/// This transport's options, checked when the configuration is loaded
+/// (`config::Settings::check`): a daemon elsewhere (`mycelium-rest-url`) by a URL, and
+/// none of the options only a supervised one takes with it; a private network with
+/// both its key and its name.
+pub(crate) fn check(settings: &crate::config::Settings) -> Result<()> {
+    if let Some(url) = &settings.mycelium_rest_url {
+        Url::parse(url).with_context(|| format!("mycelium-rest-url {url:?}"))?;
+        if let Some((name, _)) = settings.supervised_only().iter().find(|(_, set)| *set) {
+            anyhow::bail!(
+                "{name} is for a supervised Mycelium daemon and can't be combined with mycelium-rest-url"
+            );
+        }
+    }
+    // A private network has both; one alone would be ignored or invented.
+    if settings.mycelium_network_key_file.is_some() != settings.mycelium_network_name.is_some() {
+        anyhow::bail!("mycelium-network-key-file and mycelium-network-name go together");
+    }
+    Ok(())
+}
+
 /// The transport as `settings` configure it (`transport::start`): its daemon supervised
 /// (started, which can take a while) or one running elsewhere. None if `stop` came
 /// first.
diff --git a/src/transport/tor.rs b/src/transport/tor.rs
index 8eb1d02..f2e0199 100644
--- a/src/transport/tor.rs
+++ b/src/transport/tor.rs
@@ -198,6 +198,17 @@ impl TorTransport {
     /// while: it fetches what it needs to know of the Tor network). Until it is done,
     /// dials wait for it (arti bootstraps on demand), and the onion service is published
     /// once it is: the node starts without waiting for Tor.
+    /// This transport's options, checked when the configuration is loaded
+    /// (`config::Settings::check`), if the node uses it: where arti keeps its state.
+    pub(crate) fn check(settings: &crate::config::Settings) -> Result<()> {
+        if settings.tor_state_dir.is_none() {
+            bail!(
+                "tor needs tor-state-dir: where arti keeps its state, the onion service's key among it"
+            );
+        }
+        Ok(())
+    }
+
     /// The transport as `settings` configure it (`transport::start`), proving `key`.
     /// None if `stop` came first (bootstrapping arti can take a while).
     pub(crate) async fn from_settings(