From f091fb2a9188f686e5b1f9012c510a1b3c8ad840 Mon Sep 17 00:00:00 2001 From: tv Date: Fri, 30 Oct 2026 16:32:23 +0100 Subject: move transport config checks to respective modules --- src/config/mod.rs | 144 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 100 insertions(+), 44 deletions(-) (limited to 'src/config/mod.rs') 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)> { + TRANSPORTS + .iter() + .filter(|(name, _)| !self.settings.transports.iter().any(|used| used == name)) + .filter_map(|(name, built)| { + let prefix = format!("{name}-"); + let options: Vec = 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); -- cgit v1.3.1