diff --git a/CHANGELOG.md b/CHANGELOG.md index d7060c2..e5e04df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 pre-`0.3.0` so a tagged release supersedes any prior dev .deb. Tagged release builds (no `-dev` in Cargo.toml) keep the clean `-1` form. Operator override via `--version` still wins +- One-shot startup advert sweep for Nostr open-discovery. On daemon + startup under `node.discovery.nostr.policy: open`, after a short + settle delay (`startup_sweep_delay_secs`, default 5s) the cached + overlay-advert table is iterated once and recent adverts (newer + than `startup_sweep_max_age_secs`, default 3600s) are queued for + outbound retry, modulo the same skip-filters as the per-tick sweep + (configured peer, already connected, retry-pending, connecting). + Closes the gap where peers learned only through relay backlog at + startup were not dialed until they republished. +- Diagnostic logging on the open-discovery sweep. Each `queued retry` + now logs at info-level with the peer short-npub and advert age, + and a one-line summary (cached count, queued count, per-reason + skip counts) is emitted on every startup sweep and on any per-tick + sweep that queues at least one retry. Operator-facing visibility + into what the auto-dial path is doing. ### Changed diff --git a/docs/design/fips-configuration.md b/docs/design/fips-configuration.md index 228a9fa..686dc82 100644 --- a/docs/design/fips-configuration.md +++ b/docs/design/fips-configuration.md @@ -205,6 +205,8 @@ without that feature ignore `udp:nat` bootstrap configuration. | `node.discovery.nostr.punch_duration_ms` | u64 | `10000` | How long to keep punching before failure | | `node.discovery.nostr.advert_ttl_secs` | u64 | `3600` | Advert TTL in seconds | | `node.discovery.nostr.advert_refresh_secs` | u64 | `1800` | How often adverts are refreshed in seconds | +| `node.discovery.nostr.startup_sweep_delay_secs` | u64 | `5` | Settle delay after Nostr discovery starts before the one-shot startup advert sweep runs (only used under `policy: open`). Allows the relay subscription backlog to populate the in-memory advert cache before the sweep fires | +| `node.discovery.nostr.startup_sweep_max_age_secs` | u64 | `3600` | Maximum advert age (`now - created_at`) considered by the one-shot startup sweep (only used under `policy: open`). Adverts older than this are skipped on startup; the per-tick sweep still considers them up to `valid_until_ms` | If `stun_servers` is omitted, the built-in default list above is used. If it is specified in YAML, the configured list fully overrides the defaults. diff --git a/src/config/node.rs b/src/config/node.rs index 619e098..93c7c40 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -353,6 +353,18 @@ pub struct NostrDiscoveryConfig { /// How often adverts are refreshed in seconds. #[serde(default = "NostrDiscoveryConfig::default_advert_refresh_secs")] pub advert_refresh_secs: u64, + /// Settle delay in seconds after Nostr discovery starts before the + /// one-shot startup sweep of cached adverts runs. Allows the relay + /// subscription backlog to populate the in-memory advert cache. + /// Only used under `policy: open`. Default: 5. + #[serde(default = "NostrDiscoveryConfig::default_startup_sweep_delay_secs")] + pub startup_sweep_delay_secs: u64, + /// Maximum age in seconds for cached adverts considered by the + /// one-shot startup sweep. Adverts whose `created_at` is older than + /// `now - startup_sweep_max_age_secs` are skipped. Only used under + /// `policy: open`. Default: 3600 (1 hour). + #[serde(default = "NostrDiscoveryConfig::default_startup_sweep_max_age_secs")] + pub startup_sweep_max_age_secs: u64, } impl Default for NostrDiscoveryConfig { @@ -378,6 +390,8 @@ impl Default for NostrDiscoveryConfig { punch_duration_ms: Self::default_punch_duration_ms(), advert_ttl_secs: Self::default_advert_ttl_secs(), advert_refresh_secs: Self::default_advert_refresh_secs(), + startup_sweep_delay_secs: Self::default_startup_sweep_delay_secs(), + startup_sweep_max_age_secs: Self::default_startup_sweep_max_age_secs(), } } } @@ -462,6 +476,14 @@ impl NostrDiscoveryConfig { fn default_advert_refresh_secs() -> u64 { 1_800 } + + fn default_startup_sweep_delay_secs() -> u64 { + 5 + } + + fn default_startup_sweep_max_age_secs() -> u64 { + 3_600 + } } /// Spanning tree (`node.tree.*`). @@ -1036,6 +1058,32 @@ mod tests { assert!((c.etx_threshold - 3.0).abs() < 1e-9); // default } + #[test] + fn test_nostr_discovery_startup_sweep_defaults() { + let c = NostrDiscoveryConfig::default(); + assert_eq!(c.startup_sweep_delay_secs, 5); + assert_eq!(c.startup_sweep_max_age_secs, 3_600); + } + + #[test] + fn test_nostr_discovery_startup_sweep_yaml_override() { + let yaml = "enabled: true\npolicy: open\nstartup_sweep_delay_secs: 10\nstartup_sweep_max_age_secs: 1800\n"; + let c: NostrDiscoveryConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(c.enabled); + assert_eq!(c.policy, NostrDiscoveryPolicy::Open); + assert_eq!(c.startup_sweep_delay_secs, 10); + assert_eq!(c.startup_sweep_max_age_secs, 1_800); + } + + #[test] + fn test_nostr_discovery_startup_sweep_partial_yaml_uses_defaults() { + // Only override delay; max_age should fall back to default. + let yaml = "enabled: true\nstartup_sweep_delay_secs: 30\n"; + let c: NostrDiscoveryConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(c.startup_sweep_delay_secs, 30); + assert_eq!(c.startup_sweep_max_age_secs, 3_600); + } + #[cfg(windows)] #[test] fn test_default_socket_path_windows() { diff --git a/src/discovery/nostr/runtime.rs b/src/discovery/nostr/runtime.rs index 7aa1245..cba7b83 100644 --- a/src/discovery/nostr/runtime.rs +++ b/src/discovery/nostr/runtime.rs @@ -198,7 +198,7 @@ impl NostrDiscovery { pub async fn cached_open_discovery_candidates( &self, max: usize, - ) -> Vec<(String, Vec)> { + ) -> Vec<(String, Vec, u64)> { self.prune_advert_cache().await; let now = now_ms(); let cache = self.advert_cache.read().await; @@ -206,7 +206,13 @@ impl NostrDiscovery { .values() .filter(|entry| entry.author_npub != self.npub) .filter(|entry| entry.valid_until_ms > now) - .map(|entry| (entry.author_npub.clone(), entry.advert.endpoints.clone())) + .map(|entry| { + ( + entry.author_npub.clone(), + entry.advert.endpoints.clone(), + entry.created_at, + ) + }) .take(max) .collect() } diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index 70f1e3d..df1ba0d 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -426,6 +426,8 @@ impl Node { } } + self.maybe_run_startup_open_discovery_sweep(&bootstrap) + .await; self.queue_open_discovery_retries(&bootstrap).await; } @@ -574,6 +576,7 @@ impl Node { warn!(error = %err, "Failed to publish initial Nostr overlay advert"); } self.nostr_discovery = Some(runtime); + self.nostr_discovery_started_at_ms = Some(Self::now_ms()); info!("Nostr overlay discovery enabled"); } Err(err) => { @@ -1141,6 +1144,26 @@ impl Node { } async fn queue_open_discovery_retries(&mut self, bootstrap: &std::sync::Arc) { + self.run_open_discovery_sweep(bootstrap, None, "per-tick") + .await; + } + + /// Open-discovery cache sweep. Iterates the cached overlay adverts and + /// queues retries for non-configured, not-yet-connected peers. + /// + /// `max_age_secs`, if set, filters out adverts whose `created_at` is + /// older than `now - max_age_secs`. The per-tick sweep passes `None` + /// (relies on the cache's own `valid_until_ms` filter); the one-shot + /// startup sweep passes `Some(startup_sweep_max_age_secs)`. + /// + /// `caller` is a short label included in log lines so per-tick and + /// startup sweeps are distinguishable in operator-facing logs. + async fn run_open_discovery_sweep( + &mut self, + bootstrap: &std::sync::Arc, + max_age_secs: Option, + caller: &'static str, + ) { if !self.config.node.discovery.nostr.enabled || self.config.node.discovery.nostr.policy != crate::config::NostrDiscoveryPolicy::Open { @@ -1154,28 +1177,63 @@ impl Node { .map(|peer| peer.npub.clone()) .collect::>(); let now_ms = Self::now_ms(); + let now_secs = now_ms / 1000; let mut enqueue_budget = self.open_discovery_enqueue_budget(&configured_npubs); if enqueue_budget == 0 { + debug!( + caller = %caller, + "open-discovery sweep: enqueue budget is 0, skipping" + ); return; } - for (npub, endpoints) in bootstrap.cached_open_discovery_candidates(64).await { + let candidates = bootstrap.cached_open_discovery_candidates(64).await; + let cached_count = candidates.len(); + let mut enqueued = 0usize; + let mut skipped_age = 0usize; + let mut skipped_configured = 0usize; + let mut skipped_self = 0usize; + let mut skipped_connected = 0usize; + let mut skipped_retry_pending = 0usize; + let mut skipped_connecting = 0usize; + let mut skipped_no_endpoints = 0usize; + let mut skipped_invalid_npub = 0usize; + + for (npub, endpoints, created_at_secs) in candidates { if enqueue_budget == 0 { break; } + + if let Some(max_age) = max_age_secs + && now_secs.saturating_sub(created_at_secs) > max_age + { + skipped_age = skipped_age.saturating_add(1); + continue; + } + if configured_npubs.contains(&npub) { + skipped_configured = skipped_configured.saturating_add(1); continue; } let peer_identity = match PeerIdentity::from_npub(&npub) { Ok(identity) => identity, - Err(_) => continue, + Err(_) => { + skipped_invalid_npub = skipped_invalid_npub.saturating_add(1); + continue; + } }; let node_addr = *peer_identity.node_addr(); - if node_addr == *self.identity.node_addr() || self.peers.contains_key(&node_addr) { + if node_addr == *self.identity.node_addr() { + skipped_self = skipped_self.saturating_add(1); + continue; + } + if self.peers.contains_key(&node_addr) { + skipped_connected = skipped_connected.saturating_add(1); continue; } if self.retry_pending.contains_key(&node_addr) { + skipped_retry_pending = skipped_retry_pending.saturating_add(1); continue; } let connecting = self.connections.values().any(|conn| { @@ -1184,6 +1242,7 @@ impl Node { .unwrap_or(false) }); if connecting { + skipped_connecting = skipped_connecting.saturating_add(1); continue; } @@ -1203,6 +1262,7 @@ impl Node { priority = priority.saturating_add(1); } if addresses.is_empty() { + skipped_no_endpoints = skipped_no_endpoints.saturating_add(1); continue; } @@ -1223,8 +1283,87 @@ impl Node { state.retry_after_ms = now_ms; state.expires_at_ms = Some(self.open_discovery_retry_expires_at_ms(now_ms)); self.retry_pending.insert(node_addr, state); + info!( + caller = %caller, + peer = %peer_identity.short_npub(), + advert_age_secs = now_secs.saturating_sub(created_at_secs), + "open-discovery sweep: queued retry for cached advert" + ); enqueue_budget = enqueue_budget.saturating_sub(1); + enqueued = enqueued.saturating_add(1); } + + // Always log a one-line summary on the startup sweep so operators + // can verify it ran. Per-tick sweeps are noisier; only summarize + // when something happened. + let total_skipped = skipped_age + + skipped_configured + + skipped_self + + skipped_connected + + skipped_retry_pending + + skipped_connecting + + skipped_no_endpoints + + skipped_invalid_npub; + let should_summarize = caller == "startup" || enqueued > 0; + if should_summarize { + info!( + caller = %caller, + cached = cached_count, + queued = enqueued, + skipped_age = skipped_age, + skipped_configured = skipped_configured, + skipped_self = skipped_self, + skipped_connected = skipped_connected, + skipped_retry_pending = skipped_retry_pending, + skipped_connecting = skipped_connecting, + skipped_no_endpoints = skipped_no_endpoints, + skipped_invalid_npub = skipped_invalid_npub, + skipped_total = total_skipped, + "open-discovery sweep complete" + ); + } + } + + /// One-shot startup sweep: runs once after the configured settle + /// delay, iterating the cached overlay adverts and queueing retries + /// for any peer with a recent enough advert that we haven't already + /// configured statically or established a link to. + /// + /// Gated identically to [`run_open_discovery_sweep`]: requires + /// `node.discovery.nostr.enabled` and `policy == open`. + async fn maybe_run_startup_open_discovery_sweep( + &mut self, + bootstrap: &std::sync::Arc, + ) { + if self.startup_open_discovery_sweep_done { + return; + } + if !self.config.node.discovery.nostr.enabled + || self.config.node.discovery.nostr.policy != crate::config::NostrDiscoveryPolicy::Open + { + // Mark done so we don't keep re-checking on every tick. + self.startup_open_discovery_sweep_done = true; + return; + } + let Some(started_at_ms) = self.nostr_discovery_started_at_ms else { + return; + }; + let now_ms = Self::now_ms(); + let delay_ms = self + .config + .node + .discovery + .nostr + .startup_sweep_delay_secs + .saturating_mul(1000); + if now_ms < started_at_ms.saturating_add(delay_ms) { + return; + } + + let max_age_secs = self.config.node.discovery.nostr.startup_sweep_max_age_secs; + self.run_open_discovery_sweep(bootstrap, Some(max_age_secs), "startup") + .await; + self.startup_open_discovery_sweep_done = true; } fn available_outbound_slots(&self) -> usize { diff --git a/src/node/mod.rs b/src/node/mod.rs index e20f0f0..4da248d 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -432,6 +432,15 @@ pub struct Node { /// Optional Nostr/STUN overlay discovery coordinator for `udp:nat` peers. nostr_discovery: Option>, + /// Wall-clock ms when Nostr discovery successfully started, used to + /// schedule the one-shot startup advert sweep after a settle delay. + /// `None` until discovery comes up; remains `None` if discovery is + /// disabled or failed to start. + nostr_discovery_started_at_ms: Option, + /// Whether the one-shot startup advert sweep has run. Set to true + /// after the first sweep fires (under `policy: open`); thereafter + /// only the per-tick `queue_open_discovery_retries` continues. + startup_open_discovery_sweep_done: bool, /// Per-peer UDP transports adopted from NAT traversal handoff. bootstrap_transports: HashSet, @@ -596,6 +605,8 @@ impl Node { pending_connects: Vec::new(), retry_pending: HashMap::new(), nostr_discovery: None, + nostr_discovery_started_at_ms: None, + startup_open_discovery_sweep_done: false, bootstrap_transports: HashSet::new(), last_parent_reeval: None, last_congestion_log: None, @@ -723,6 +734,8 @@ impl Node { pending_connects: Vec::new(), retry_pending: HashMap::new(), nostr_discovery: None, + nostr_discovery_started_at_ms: None, + startup_open_discovery_sweep_done: false, bootstrap_transports: HashSet::new(), last_parent_reeval: None, last_congestion_log: None,