From 9a7c22536b1a381b68acad2975fc136460fc3d21 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Fri, 15 Aug 2025 22:10:00 +0200 Subject: [PATCH] radicle/config/node: Use newtypes --- crates/radicle-cli/tests/util/environment.rs | 6 +- crates/radicle-node/src/main.rs | 4 +- crates/radicle-node/src/runtime.rs | 2 +- crates/radicle-node/src/tests.rs | 16 +- crates/radicle-node/src/tests/e2e.rs | 2 +- crates/radicle-protocol/src/service.rs | 30 +- .../radicle-protocol/src/service/session.rs | 2 +- crates/radicle/src/node/config.rs | 309 +++++++++--------- 8 files changed, 179 insertions(+), 192 deletions(-) diff --git a/crates/radicle-cli/tests/util/environment.rs b/crates/radicle-cli/tests/util/environment.rs index dfa91148..d6305ef6 100644 --- a/crates/radicle-cli/tests/util/environment.rs +++ b/crates/radicle-cli/tests/util/environment.rs @@ -36,11 +36,13 @@ pub(crate) mod config { inbound: RateLimit { fill_rate: 1.0, capacity: usize::MAX, - }, + } + .into(), outbound: RateLimit { fill_rate: 1.0, capacity: usize::MAX, - }, + } + .into(), }, ..Limits::default() }, diff --git a/crates/radicle-node/src/main.rs b/crates/radicle-node/src/main.rs index e1a557ec..f94dfb56 100644 --- a/crates/radicle-node/src/main.rs +++ b/crates/radicle-node/src/main.rs @@ -90,7 +90,7 @@ fn execute() -> anyhow::Result<()> { let config = options.config.unwrap_or_else(|| home.config()); let mut config = profile::Config::load(&config)?; - let level = options.log.unwrap_or(config.node.log); + let level = options.log.unwrap_or_else(|| config.node.log.into()); let logger = { let journal = { @@ -135,7 +135,7 @@ fn execute() -> anyhow::Result<()> { config.node.listen.clone() }; - if let Err(e) = radicle::io::set_file_limit(config.node.limits.max_open_files) { + if let Err(e) = radicle::io::set_file_limit::(config.node.limits.max_open_files.into()) { log::warn!(target: "node", "Unable to set process open file limit: {e}"); } diff --git a/crates/radicle-node/src/runtime.rs b/crates/radicle-node/src/runtime.rs index 45e471ec..b0de2905 100644 --- a/crates/radicle-node/src/runtime.rs +++ b/crates/radicle-node/src/runtime.rs @@ -245,7 +245,7 @@ impl Runtime { cobs_cache, db, worker::Config { - capacity: config.workers, + capacity: config.workers.into(), storage: storage.clone(), fetch, policy, diff --git a/crates/radicle-node/src/tests.rs b/crates/radicle-node/src/tests.rs index 16650dbb..faf4aa4f 100644 --- a/crates/radicle-node/src/tests.rs +++ b/crates/radicle-node/src/tests.rs @@ -308,8 +308,8 @@ fn test_inventory_pruning() { // All zero Test { limits: Limits { - routing_max_size: 0, - routing_max_age: LocalDuration::from_secs(0), + routing_max_size: 0.into(), + routing_max_age: LocalDuration::from_secs(0).into(), ..Limits::default() }, peer_projects: vec![10; 5], @@ -319,8 +319,8 @@ fn test_inventory_pruning() { // All entries are too young to expire. Test { limits: Limits { - routing_max_size: 0, - routing_max_age: LocalDuration::from_mins(7 * 24 * 60), + routing_max_size: 0.into(), + routing_max_age: LimitRoutingMaxAge::from(LocalDuration::from_mins(7 * 24 * 60)), ..Limits::default() }, peer_projects: vec![10; 5], @@ -330,8 +330,8 @@ fn test_inventory_pruning() { // All entries remain because the table is unconstrained. Test { limits: Limits { - routing_max_size: 50, - routing_max_age: LocalDuration::from_mins(0), + routing_max_size: 50.into(), + routing_max_age: LocalDuration::from_mins(0).into(), ..Limits::default() }, peer_projects: vec![10; 5], @@ -341,8 +341,8 @@ fn test_inventory_pruning() { // Some entries are pruned because the table is constrained. Test { limits: Limits { - routing_max_size: 25, - routing_max_age: LocalDuration::from_mins(7 * 24 * 60), + routing_max_size: 25.into(), + routing_max_age: LocalDuration::from_mins(7 * 24 * 60).into(), ..Limits::default() }, peer_projects: vec![10; 5], diff --git a/crates/radicle-node/src/tests/e2e.rs b/crates/radicle-node/src/tests/e2e.rs index e568e393..0f42260d 100644 --- a/crates/radicle-node/src/tests/e2e.rs +++ b/crates/radicle-node/src/tests/e2e.rs @@ -679,7 +679,7 @@ fn test_concurrent_fetches() { let repos = scale.max(4); let limits = Limits { // Have one fetch be queued. - fetch_concurrency: repos - 1, + fetch_concurrency: (repos - 1).into(), ..Limits::default() }; let mut bob_repos = HashSet::new(); diff --git a/crates/radicle-protocol/src/service.rs b/crates/radicle-protocol/src/service.rs index 75c0f5d0..10d013e2 100644 --- a/crates/radicle-protocol/src/service.rs +++ b/crates/radicle-protocol/src/service.rs @@ -27,7 +27,7 @@ use radicle::node; use radicle::node::address; use radicle::node::address::Store as _; use radicle::node::address::{AddressBook, AddressType, KnownAddress}; -use radicle::node::config::PeerConfig; +use radicle::node::config::{PeerConfig, RateLimit}; use radicle::node::device::Device; use radicle::node::refs::Store as _; use radicle::node::routing::Store as _; @@ -845,7 +845,7 @@ where if let Err(err) = self .db .gossip_mut() - .prune((now - self.config.limits.gossip_max_age).into()) + .prune((now - LocalDuration::from(self.config.limits.gossip_max_age)).into()) { error!(target: "service", "Error pruning gossip entries: {err}"); } @@ -1302,7 +1302,7 @@ where return true; } // Check for inbound connection limit. - if self.sessions.inbound().count() >= self.config.limits.connection.inbound { + if self.sessions.inbound().count() >= self.config.limits.connection.inbound.into() { return false; } match self.db.addresses().is_ip_banned(ip) { @@ -1315,13 +1315,9 @@ where Err(e) => error!(target: "service", "Error querying ban status for {ip}: {e}"), } let host: HostName = ip.into(); + let tokens = RateLimit::from(self.config.limits.rate.inbound.clone()); - if self.limiter.limit( - host.clone(), - None, - &self.config.limits.rate.inbound, - self.clock, - ) { + if self.limiter.limit(host.clone(), None, &tokens, self.clock) { trace!(target: "service", "Rate limiting inbound connection from {host}.."); return false; } @@ -1824,13 +1820,13 @@ where }; peer.last_active = self.clock; - let limit = match peer.link { - Link::Outbound => &self.config.limits.rate.outbound, - Link::Inbound => &self.config.limits.rate.inbound, + let limit: RateLimit = match peer.link { + Link::Outbound => self.config.limits.rate.outbound.clone().into(), + Link::Inbound => self.config.limits.rate.inbound.clone().into(), }; if self .limiter - .limit(peer.addr.clone().into(), Some(remote), limit, self.clock) + .limit(peer.addr.clone().into(), Some(remote), &limit, self.clock) { debug!(target: "service", "Rate limiting message from {remote} ({})", peer.addr); return Ok(()); @@ -2260,7 +2256,7 @@ where if self.sessions.contains_key(&nid) { return Err(ConnectError::SessionExists { nid }); } - if self.sessions.outbound().count() >= self.config.limits.connection.outbound { + if self.sessions.outbound().count() >= self.config.limits.connection.outbound.into() { return Err(ConnectError::LimitReached { nid, addr }); } let persistent = self.config.is_persistent(&nid); @@ -2432,14 +2428,14 @@ where fn prune_routing_entries(&mut self, now: &LocalTime) -> Result<(), routing::Error> { let count = self.db.routing().len()?; - if count <= self.config.limits.routing_max_size { + if count <= self.config.limits.routing_max_size.into() { return Ok(()); } - let delta = count - self.config.limits.routing_max_size; + let delta = count - usize::from(self.config.limits.routing_max_size); let nid = self.node_id(); self.db.routing_mut().prune( - (*now - self.config.limits.routing_max_age).into(), + (*now - LocalDuration::from(self.config.limits.routing_max_age)).into(), Some(delta), &nid, )?; diff --git a/crates/radicle-protocol/src/service/session.rs b/crates/radicle-protocol/src/service/session.rs index 07103eb1..024c42ed 100644 --- a/crates/radicle-protocol/src/service/session.rs +++ b/crates/radicle-protocol/src/service/session.rs @@ -226,7 +226,7 @@ impl Session { pub fn is_at_capacity(&self) -> bool { if let State::Connected { fetching, .. } = &self.state { - if fetching.len() >= self.limits.fetch_concurrency { + if fetching.len() >= self.limits.fetch_concurrency.into() { return true; } } diff --git a/crates/radicle/src/node/config.rs b/crates/radicle/src/node/config.rs index 1260cb0b..40327225 100644 --- a/crates/radicle/src/node/config.rs +++ b/crates/radicle/src/node/config.rs @@ -120,72 +120,35 @@ impl Network { } /// Configuration parameters defining attributes of minima and maxima. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[serde(default, rename_all = "camelCase")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct Limits { /// Number of routing table entries before we start pruning. - #[serde(default = "defaults::limit_routing_max_size")] - pub routing_max_size: usize, + pub routing_max_size: LimitRoutingMaxSize, /// How long to keep a routing table entry before being pruned. - #[serde( - default = "defaults::limit_routing_max_age", - with = "crate::serde_ext::localtime::duration" - )] - #[cfg_attr( - feature = "schemars", - schemars(with = "crate::schemars_ext::localtime::LocalDuration") - )] - pub routing_max_age: LocalDuration, + pub routing_max_age: LimitRoutingMaxAge, /// How long to keep a gossip message entry before pruning it. - #[serde( - default = "defaults::limit_gossip_max_age", - with = "crate::serde_ext::localtime::duration" - )] - #[cfg_attr( - feature = "schemars", - schemars(with = "crate::schemars_ext::localtime::LocalDuration") - )] - pub gossip_max_age: LocalDuration, + pub gossip_max_age: LimitGossipMaxAge, /// Maximum number of concurrent fetches per peer connection. - #[serde(default = "defaults::limit_fetch_concurrency")] - pub fetch_concurrency: usize, + pub fetch_concurrency: LimitFetchConcurrency, /// Maximum number of open files. - #[serde(default = "defaults::limit_max_open_files")] - pub max_open_files: usize, + pub max_open_files: LimitMaxOpenFiles, /// Rate limitter settings. - #[serde(default)] pub rate: RateLimits, /// Connection limits. - #[serde(default)] pub connection: ConnectionLimits, /// Channel limits. - #[serde(default)] pub fetch_pack_receive: FetchPackSizeLimit, } -impl Default for Limits { - fn default() -> Self { - Self { - routing_max_size: defaults::limit_routing_max_size(), - routing_max_age: defaults::limit_routing_max_age(), - gossip_max_age: defaults::limit_gossip_max_age(), - fetch_concurrency: defaults::limit_fetch_concurrency(), - max_open_files: defaults::limit_max_open_files(), - rate: RateLimits::default(), - connection: ConnectionLimits::default(), - fetch_pack_receive: FetchPackSizeLimit::default(), - } - } -} - /// Limiter for byte streams. /// /// Default: 500MiB @@ -277,30 +240,20 @@ impl Default for FetchPackSizeLimit { } /// Connection limits. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[serde(default, rename_all = "camelCase")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct ConnectionLimits { /// Max inbound connections. - #[serde(default = "defaults::limit_connections_inbound")] - pub inbound: usize, + pub inbound: LimitConnectionsInbound, /// Max outbound connections. Note that this can be higher than the *target* number. - #[serde(default = "defaults::limit_connections_outbound")] - pub outbound: usize, -} - -impl Default for ConnectionLimits { - fn default() -> Self { - Self { - inbound: defaults::limit_connections_inbound(), - outbound: defaults::limit_connections_outbound(), - } - } + pub outbound: LimitConnectionsOutbound, } /// Rate limts for a single connection. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Display)] +#[display("RateLimit(fill_rate={fill_rate}, capacity={capacity})")] #[serde(rename_all = "camelCase")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct RateLimit { @@ -309,24 +262,13 @@ pub struct RateLimit { } /// Rate limits for inbound and outbound connections. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] #[serde(rename_all = "camelCase")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct RateLimits { - #[serde(default = "defaults::limit_rate_inbound")] - pub inbound: RateLimit, + pub inbound: LimitRateInbound, - #[serde(default = "defaults::limit_rate_outbound")] - pub outbound: RateLimit, -} - -impl Default for RateLimits { - fn default() -> Self { - Self { - inbound: defaults::limit_rate_inbound(), - outbound: defaults::limit_rate_outbound(), - } - } + pub outbound: LimitRateOutbound, } /// Full address used to connect to a remote node. @@ -388,22 +330,17 @@ impl Deref for ConnectAddress { } /// Peer configuration. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] #[serde(rename_all = "camelCase", tag = "type")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum PeerConfig { /// Static peer set. Connect to the configured peers and maintain the connections. Static, /// Dynamic peer set. + #[default] Dynamic, } -impl Default for PeerConfig { - fn default() -> Self { - Self::Dynamic - } -} - /// Relay configuration. #[derive(Debug, Copy, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -505,13 +442,8 @@ pub struct Config { #[serde(default)] pub network: Network, /// Log level. - #[serde(default = "defaults::log")] - #[serde(with = "crate::serde_ext::string")] - #[cfg_attr( - feature = "schemars", - schemars(with = "crate::schemars_ext::log::Level") - )] - pub log: log::Level, + #[serde(default)] + pub log: LogLevel, /// Whether or not our node should relay messages. #[serde(default, deserialize_with = "crate::serde_ext::ok_or_default")] pub relay: Relay, @@ -519,8 +451,8 @@ pub struct Config { #[serde(default)] pub limits: Limits, /// Number of worker threads to spawn. - #[serde(default = "defaults::workers")] - pub workers: usize, + #[serde(default)] + pub workers: Workers, /// Default seeding policy. #[serde(default)] pub seeding_policy: DefaultSeedingPolicy, @@ -549,8 +481,8 @@ impl Config { onion: None, relay: Relay::default(), limits: Limits::default(), - workers: defaults::workers(), - log: defaults::log(), + workers: Workers::default(), + log: LogLevel::default(), seeding_policy: DefaultSeedingPolicy::default(), extra: json::Map::default(), } @@ -587,72 +519,129 @@ impl Config { } } -/// Defaults as functions, for serde. -mod defaults { - /// Default number of workers to spawn. - #[inline] - pub const fn workers() -> usize { - 8 - } +#[derive(Clone, Copy, Debug, Display, Deserialize, Serialize, From)] +#[serde(transparent)] +#[display("{0}")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct LogLevel( + #[serde(with = "crate::serde_ext::string")] + #[cfg_attr( + feature = "schemars", + schemars(with = "crate::schemars_ext::log::Level") + )] + log::Level, +); - /// Log level. - #[inline] - pub const fn log() -> log::Level { - log::Level::Info - } - - #[inline] - pub const fn limit_connections_inbound() -> usize { - 128 - } - - #[inline] - pub const fn limit_connections_outbound() -> usize { - 16 - } - - #[inline] - pub const fn limit_routing_max_size() -> usize { - 1000 - } - - #[inline] - pub const fn limit_routing_max_age() -> localtime::LocalDuration { - localtime::LocalDuration::from_mins(7 * 24 * 60) // One week - } - - #[inline] - pub const fn limit_gossip_max_age() -> localtime::LocalDuration { - localtime::LocalDuration::from_mins(2 * 7 * 24 * 60) // Two weeks - } - - #[inline] - pub const fn limit_fetch_concurrency() -> usize { - 1 - } - - #[inline] - pub const fn limit_max_open_files() -> usize { - 4096 - } - - #[inline] - pub const fn limit_rate_inbound() -> super::RateLimit { - super::RateLimit { - fill_rate: 5.0, - capacity: 1024, - } - } - - #[inline] - pub const fn limit_rate_outbound() -> super::RateLimit { - super::RateLimit { - fill_rate: 10.0, - capacity: 2048, - } +impl Default for LogLevel { + fn default() -> Self { + Self(log::Level::Info) } } +impl From for log::Level { + fn from(value: LogLevel) -> Self { + value.0 + } +} + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[serde(transparent)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct LimitRoutingMaxAge( + #[serde(with = "crate::serde_ext::localtime::duration")] + #[cfg_attr( + feature = "schemars", + schemars(with = "crate::schemars_ext::localtime::LocalDuration") + )] + localtime::LocalDuration, +); + +impl Default for LimitRoutingMaxAge { + fn default() -> Self { + Self(localtime::LocalDuration::from_mins(7 * 24 * 60)) // One week + } +} + +impl From for LocalDuration { + fn from(value: LimitRoutingMaxAge) -> Self { + value.0 + } +} + +impl From for LimitRoutingMaxAge { + fn from(value: LocalDuration) -> Self { + Self(value) + } +} + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[serde(transparent)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct LimitGossipMaxAge( + #[serde(with = "crate::serde_ext::localtime::duration")] + #[cfg_attr( + feature = "schemars", + schemars(with = "crate::schemars_ext::localtime::LocalDuration") + )] + localtime::LocalDuration, +); + +impl Default for LimitGossipMaxAge { + fn default() -> Self { + Self(localtime::LocalDuration::from_mins(2 * 7 * 24 * 60)) // Two weeks + } +} + +impl From for LocalDuration { + fn from(value: LimitGossipMaxAge) -> Self { + value.0 + } +} + +macro_rules! wrapper { + ($name:ident, $type:ty, $default:expr $(, $derive:ty)*) => { + #[derive(Clone, Debug, Deserialize, Display, Serialize, From $(, $derive)*)] + #[display("{0}")] + #[serde(transparent)] + #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] + pub struct $name($type); + + impl Default for $name { + fn default() -> Self { + Self($default) + } + } + + impl From<$name> for $type { + fn from(value: $name) -> Self { + value.0 + } + } + }; +} +wrapper!(Workers, usize, 8, Copy); +wrapper!(LimitConnectionsInbound, usize, 128, Copy); +wrapper!(LimitConnectionsOutbound, usize, 16, Copy); +wrapper!(LimitRoutingMaxSize, usize, 1000, Copy); +wrapper!(LimitFetchConcurrency, usize, 1, Copy); +wrapper!( + LimitRateInbound, + RateLimit, + RateLimit { + fill_rate: 5.0, + capacity: 1024, + } +); +wrapper!(LimitMaxOpenFiles, usize, 4096, Copy); +wrapper!( + LimitRateOutbound, + RateLimit, + RateLimit { + fill_rate: 10.0, + capacity: 2048, + } +); + #[cfg(test)] #[allow(clippy::unwrap_used)] mod test { @@ -671,10 +660,10 @@ mod test { } )) .unwrap(); - assert_eq!(config.limits.connection.inbound, 1337); + assert_eq!(config.limits.connection.inbound.0, 1337); assert_eq!( - config.limits.connection.outbound, - super::defaults::limit_connections_outbound() + config.limits.connection.outbound.0, + super::LimitConnectionsOutbound::default().0, ); let config: Config = serde_json::from_value(json!({ @@ -688,9 +677,9 @@ mod test { )) .unwrap(); assert_eq!( - config.limits.connection.inbound, - super::defaults::limit_connections_inbound() + config.limits.connection.inbound.0, + super::LimitConnectionsInbound::default().0, ); - assert_eq!(config.limits.connection.outbound, 1337); + assert_eq!(config.limits.connection.outbound.0, 1337); } }