diff --git a/src/config.rs b/src/config.rs index 44001d8..74327c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -156,6 +156,12 @@ pub struct Config { default_value_t = 604800 )] pub rejected_cold_index_expiry_secs: u64, + + /// Hours before removing relay from naughty list (default: 12) + /// Relays with persistent infrastructure issues (DNS, TLS, protocol errors) are + /// tracked separately and retried after this expiration period. + #[arg(long, env = "NGIT_NAUGHTY_LIST_EXPIRATION_HOURS", default_value_t = 12)] + pub naughty_list_expiration_hours: u64, } impl Config { @@ -281,6 +287,7 @@ impl Config { sync_disable_negentropy: false, rejected_hot_cache_duration_secs: 120, rejected_cold_index_expiry_secs: 604800, + naughty_list_expiration_hours: 12, } } } diff --git a/src/sync/health.rs b/src/sync/health.rs index 2948707..833918b 100644 --- a/src/sync/health.rs +++ b/src/sync/health.rs @@ -5,6 +5,7 @@ //! - Exponential backoff with configurable max delay //! - Dead relay detection after 24h of continuous failures //! - Rate limit detection and fixed cooldown period +//! - Naughty list for persistent infrastructure issues (DNS, TLS, protocol errors) //! //! ## Health States //! @@ -18,6 +19,7 @@ use std::time::{Duration, Instant}; use dashmap::DashMap; +use super::naughty_list::NaughtyListTracker; use crate::config::Config; /// Duration threshold before a relay is considered dead (24 hours) @@ -213,15 +215,21 @@ pub struct RelayHealthTracker { health: DashMap, max_backoff_secs: u64, base_backoff_secs: u64, + naughty_list: Option>, } impl RelayHealthTracker { /// Create a new RelayHealthTracker pub fn new(config: &Config) -> Self { + let naughty_list = Some(Arc::new(NaughtyListTracker::new( + config.naughty_list_expiration_hours, + ))); + Self { health: DashMap::new(), max_backoff_secs: config.sync_max_backoff_secs, base_backoff_secs: config.sync_base_backoff_secs, + naughty_list, } } @@ -231,6 +239,7 @@ impl RelayHealthTracker { health: DashMap::new(), max_backoff_secs: DEFAULT_MAX_BACKOFF_SECS, base_backoff_secs: DEFAULT_BASE_BACKOFF_SECS, + naughty_list: Some(Arc::new(NaughtyListTracker::with_defaults())), } } @@ -240,6 +249,7 @@ impl RelayHealthTracker { health: DashMap::new(), max_backoff_secs, base_backoff_secs: DEFAULT_BASE_BACKOFF_SECS, + naughty_list: Some(Arc::new(NaughtyListTracker::with_defaults())), } } @@ -549,6 +559,11 @@ impl RelayHealthTracker { .get(relay_url) .map(|entry| entry.value().clone()) } + + /// Get a reference to the naughty list tracker + pub fn naughty_list(&self) -> Option> { + self.naughty_list.clone() + } } /// Create a shared RelayHealthTracker wrapped in Arc diff --git a/src/sync/metrics.rs b/src/sync/metrics.rs index 13211b9..8a05f57 100644 --- a/src/sync/metrics.rs +++ b/src/sync/metrics.rs @@ -56,6 +56,12 @@ pub struct SyncMetrics { rejected_cold_index_expired_total: IntCounterVec, /// Total invalidations (by event_type: announcement, state) rejected_invalidated_total: IntCounterVec, + + // === Naughty List Metrics === + /// Number of relays on naughty list by category + naughty_relays_total: IntGaugeVec, + /// Detailed info about naughty relays (relay, category, reason) + naughty_relay_info: IntGaugeVec, } impl SyncMetrics { @@ -193,6 +199,25 @@ impl SyncMetrics { )?; registry.register(Box::new(rejected_invalidated_total.clone()))?; + // Naughty list metrics + let naughty_relays_total = IntGaugeVec::new( + Opts::new( + "ngit_sync_naughty_relays_total", + "Number of relays on naughty list by category", + ), + &["category"], + )?; + registry.register(Box::new(naughty_relays_total.clone()))?; + + let naughty_relay_info = IntGaugeVec::new( + Opts::new( + "ngit_sync_naughty_relay_info", + "Detailed info about naughty relays (occurrence count)", + ), + &["relay", "category", "reason"], + )?; + registry.register(Box::new(naughty_relay_info.clone()))?; + Ok(Self { relay_connected, connection_attempts_total, @@ -209,6 +234,8 @@ impl SyncMetrics { rejected_cold_index_current, rejected_cold_index_expired_total, rejected_invalidated_total, + naughty_relays_total, + naughty_relay_info, }) } @@ -465,6 +492,56 @@ impl SyncMetrics { .with_label_values(&[event_type]) .inc_by(count as u64); } + + // === Naughty List Recording Methods === + + /// Update naughty list metrics from current naughty list state + /// + /// This method resets and rebuilds all naughty list metrics based on + /// the provided entries. Should be called periodically to keep metrics + /// in sync with the naughty list state. + /// + /// # Arguments + /// + /// * `entries` - Vector of (relay_url, naughty_entry) tuples from NaughtyListTracker::get_all() + pub fn update_naughty_list(&self, entries: Vec<(String, super::naughty_list::NaughtyEntry)>) { + use super::naughty_list::NaughtyCategory; + + // Reset all naughty list metrics + self.naughty_relays_total.reset(); + self.naughty_relay_info.reset(); + + // Count by category + let mut dns_count = 0; + let mut tls_count = 0; + let mut protocol_count = 0; + + // Update metrics for each naughty relay + for (url, entry) in entries { + // Update category counts + match entry.category { + NaughtyCategory::DnsLookupFailed => dns_count += 1, + NaughtyCategory::TlsCertificateInvalid => tls_count += 1, + NaughtyCategory::ProtocolError => protocol_count += 1, + } + + // Update detailed info (occurrence count) + self.naughty_relay_info + .with_label_values(&[&url, entry.category.as_str(), &entry.reason]) + .set(entry.occurrence_count as i64); + } + + // Set category totals + self.naughty_relays_total + .with_label_values(&["dns_lookup_failed"]) + .set(dns_count); + self.naughty_relays_total + .with_label_values(&["tls_certificate_invalid"]) + .set(tls_count); + self.naughty_relays_total + .with_label_values(&["protocol_error"]) + .set(protocol_count); + } } #[cfg(test)] diff --git a/src/sync/mod.rs b/src/sync/mod.rs index 412cd16..8b51fac 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -16,6 +16,7 @@ pub mod algorithms; pub mod filters; pub mod health; pub mod metrics; +pub mod naughty_list; pub mod rejected_index; pub mod relay_connection; pub mod self_subscriber; @@ -483,7 +484,18 @@ async fn run_health_and_metrics_checker( // 2. Check for rate limit recovery manager.check_rate_limit_recovery().await; - // 3. Update metrics with current health states + // 3. Check for naughty list expiration + if let Some(naughty_list) = manager.health_tracker.naughty_list() { + let recovered = naughty_list.expire_old_entries(); + for url in recovered { + tracing::info!( + relay = %url, + "Relay removed from naughty list after expiration, will retry" + ); + } + } + + // 4. Update metrics with current health states and naughty list if let Some(ref metrics) = manager.metrics { // Get all tracked relay URLs let relay_urls: Vec = { @@ -496,6 +508,12 @@ async fn run_health_and_metrics_checker( let state = manager.health_tracker.get_state(&relay_url); metrics.record_health_state(&relay_url, state); } + + // Update naughty list metrics + if let Some(naughty_list) = manager.health_tracker.naughty_list() { + let entries = naughty_list.get_all(); + metrics.update_naughty_list(entries); + } } } _ = shutdown_rx.recv() => { @@ -2018,7 +2036,38 @@ impl SyncManager { } } Err(e) => { - tracing::error!(relay = %relay_url, error = %e, "Connection failed"); + // Classify error to determine if it's a naughty relay or transient issue + let error_str = e.to_string(); + + if let Some(category) = naughty_list::NaughtyListTracker::classify_error(&error_str) + { + // Persistent infrastructure issue - use naughty list + if let Some(ref naughty_list) = self.health_tracker.naughty_list() { + let is_new = naughty_list.record(relay_url, category, error_str.clone()); + + if is_new { + tracing::warn!( + relay = %relay_url, + category = ?category, + error = %e, + "Relay has persistent configuration issue, added to naughty list" + ); + } else { + tracing::debug!( + relay = %relay_url, + category = ?category, + "Naughty relay failure (already tracked)" + ); + } + } + } else { + // Transient network issue - use existing backoff flow + tracing::debug!( + relay = %relay_url, + error = %e, + "Connection failed (transient issue, backoff active)" + ); + } // 4. Update state back to Disconnected on failure { diff --git a/src/sync/naughty_list.rs b/src/sync/naughty_list.rs new file mode 100644 index 0000000..311b9bb --- /dev/null +++ b/src/sync/naughty_list.rs @@ -0,0 +1,546 @@ +//! Naughty List Tracker for Relays with Persistent Infrastructure Issues +//! +//! This module tracks relays with persistent configuration/infrastructure problems +//! (DNS failures, TLS certificate errors, protocol violations) separately from +//! transient network issues (timeouts, connection refused). +//! +//! ## Failure Classification +//! +//! **Naughty List (12-hour expiration, log WARN on first occurrence, DEBUG on repeat):** +//! - `DnsLookupFailed`: Domain doesn't resolve or DNS errors +//! - `TlsCertificateInvalid`: Certificate errors (expired, mismatch, self-signed) +//! - `ProtocolError`: WebSocket/Nostr protocol violations +//! +//! **NOT Naughty (use existing HealthTracker backoff):** +//! - Connection timeouts (could be network congestion) +//! - Connection refused (could be temporary maintenance) +//! +//! ## Automatic Expiration +//! +//! Entries expire after 12 hours (configurable) to allow relays to recover from +//! infrastructure issues. After expiration, the relay is automatically retried. + +use dashmap::DashMap; +use std::time::Instant; + +/// Category of persistent relay failure that qualifies for the naughty list +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum NaughtyCategory { + /// DNS lookup failures (domain doesn't resolve) + DnsLookupFailed, + /// TLS certificate errors (expired, invalid, mismatch) + TlsCertificateInvalid, + /// WebSocket or Nostr protocol violations + ProtocolError, +} + +impl NaughtyCategory { + /// Get string representation for metrics labels + pub fn as_str(&self) -> &'static str { + match self { + NaughtyCategory::DnsLookupFailed => "dns_lookup_failed", + NaughtyCategory::TlsCertificateInvalid => "tls_certificate_invalid", + NaughtyCategory::ProtocolError => "protocol_error", + } + } +} + +impl std::fmt::Display for NaughtyCategory { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.as_str()) + } +} + +/// Naughty list entry for a relay with persistent issues +#[derive(Debug, Clone)] +pub struct NaughtyEntry { + /// Category of the persistent failure + pub category: NaughtyCategory, + /// Full error message + pub reason: String, + /// When this relay was first added to the naughty list + pub first_seen: Instant, + /// Most recent occurrence of the issue + pub last_seen: Instant, + /// Number of times we've seen this issue + pub occurrence_count: u32, +} + +/// Tracks relays with persistent infrastructure/configuration issues +/// +/// Separate from HealthTracker's backoff logic - this is specifically for +/// relays with configuration problems that are unlikely to be fixed quickly. +#[derive(Debug)] +pub struct NaughtyListTracker { + /// Map of relay URL to naughty entry + entries: DashMap, + /// How many hours before removing a relay from the naughty list + expiration_hours: u64, +} + +impl NaughtyListTracker { + /// Create a new NaughtyListTracker with the specified expiration time + /// + /// # Arguments + /// + /// * `expiration_hours` - Hours before a naughty entry expires (default: 12) + pub fn new(expiration_hours: u64) -> Self { + Self { + entries: DashMap::new(), + expiration_hours, + } + } + + /// Create a new NaughtyListTracker with default 12-hour expiration + pub fn with_defaults() -> Self { + Self::new(12) + } + + /// Classify an error string into a naughty category or return None for transient errors + /// + /// # Arguments + /// + /// * `error` - The error message string to classify + /// + /// # Returns + /// + /// - `Some(NaughtyCategory)` if the error indicates a persistent infrastructure issue + /// - `None` if the error is a transient network issue (use HealthTracker backoff) + pub fn classify_error(error: &str) -> Option { + let error_lower = error.to_lowercase(); + + // DNS lookup failures + if error_lower.contains("failed to lookup address") + || error_lower.contains("name or service not known") + || error_lower.contains("nodename nor servname provided") + || (error_lower.contains("dns") && !error_lower.contains("timeout")) + { + return Some(NaughtyCategory::DnsLookupFailed); + } + + // TLS certificate errors + if error_lower.contains("certificate") + || error_lower.contains("ssl") + || error_lower.contains("tls") + { + // Exclude timeout errors that mention TLS + if !error_lower.contains("timeout") && !error_lower.contains("timed out") { + return Some(NaughtyCategory::TlsCertificateInvalid); + } + } + + // Protocol errors + if error_lower.contains("websocket") + || error_lower.contains("protocol") + || error_lower.contains("invalid frame") + { + // Exclude connection errors + if !error_lower.contains("connection") + && !error_lower.contains("timeout") + && !error_lower.contains("refused") + { + return Some(NaughtyCategory::ProtocolError); + } + } + + // Everything else is transient (timeouts, refused, etc.) + None + } + + /// Record a naughty relay (adds new entry or updates existing) + /// + /// # Arguments + /// + /// * `relay_url` - The relay URL + /// * `category` - The naughty category + /// * `reason` - The full error message + /// + /// # Returns + /// + /// `true` if this is a new naughty entry (first occurrence), `false` if updating existing + pub fn record(&self, relay_url: &str, category: NaughtyCategory, reason: String) -> bool { + let now = Instant::now(); + + if let Some(mut entry) = self.entries.get_mut(relay_url) { + // Update existing entry + entry.last_seen = now; + entry.occurrence_count = entry.occurrence_count.saturating_add(1); + entry.reason = reason; // Update with latest error message + false + } else { + // Create new entry + self.entries.insert( + relay_url.to_string(), + NaughtyEntry { + category, + reason, + first_seen: now, + last_seen: now, + occurrence_count: 1, + }, + ); + true + } + } + + /// Check if a relay is on the naughty list (not expired) + /// + /// # Arguments + /// + /// * `relay_url` - The relay URL to check + /// + /// # Returns + /// + /// `true` if the relay is currently on the naughty list + pub fn is_naughty(&self, relay_url: &str) -> bool { + if let Some(entry) = self.entries.get(relay_url) { + let age = Instant::now().duration_since(entry.first_seen); + let expiration = std::time::Duration::from_secs(self.expiration_hours * 3600); + age < expiration + } else { + false + } + } + + /// Get a naughty entry if it exists and hasn't expired + /// + /// # Arguments + /// + /// * `relay_url` - The relay URL to look up + /// + /// # Returns + /// + /// A cloned `NaughtyEntry` if the relay is on the naughty list and not expired + pub fn get_entry(&self, relay_url: &str) -> Option { + self.entries.get(relay_url).map(|e| e.clone()) + } + + /// Remove expired entries from the naughty list + /// + /// Entries older than `expiration_hours` are removed to allow relays + /// to be retried after infrastructure issues are potentially fixed. + /// + /// # Returns + /// + /// Vector of relay URLs that were removed from the naughty list + pub fn expire_old_entries(&self) -> Vec { + let now = Instant::now(); + let expiration = std::time::Duration::from_secs(self.expiration_hours * 3600); + let mut expired = Vec::new(); + + // Collect expired relay URLs + self.entries.retain(|url, entry| { + let age = now.duration_since(entry.first_seen); + if age >= expiration { + expired.push(url.clone()); + false // Remove this entry + } else { + true // Keep this entry + } + }); + + expired + } + + /// Get all naughty relays (for metrics and monitoring) + /// + /// # Returns + /// + /// Vector of (relay_url, entry) tuples for all relays currently on the naughty list + pub fn get_all(&self) -> Vec<(String, NaughtyEntry)> { + self.entries + .iter() + .map(|entry| (entry.key().clone(), entry.value().clone())) + .collect() + } + + /// Get the count of relays in a specific category + /// + /// # Arguments + /// + /// * `category` - The category to count + /// + /// # Returns + /// + /// Number of relays in the specified category + pub fn count_by_category(&self, category: NaughtyCategory) -> usize { + self.entries + .iter() + .filter(|entry| entry.value().category == category) + .count() + } + + /// Get total number of relays on the naughty list + pub fn total_count(&self) -> usize { + self.entries.len() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_classify_dns_errors() { + assert_eq!( + NaughtyListTracker::classify_error("failed to lookup address information"), + Some(NaughtyCategory::DnsLookupFailed) + ); + assert_eq!( + NaughtyListTracker::classify_error("Name or service not known"), + Some(NaughtyCategory::DnsLookupFailed) + ); + assert_eq!( + NaughtyListTracker::classify_error("nodename nor servname provided"), + Some(NaughtyCategory::DnsLookupFailed) + ); + assert_eq!( + NaughtyListTracker::classify_error("dns error: NXDOMAIN"), + Some(NaughtyCategory::DnsLookupFailed) + ); + } + + #[test] + fn test_classify_tls_errors() { + assert_eq!( + NaughtyListTracker::classify_error("certificate not valid for 'example.com'"), + Some(NaughtyCategory::TlsCertificateInvalid) + ); + assert_eq!( + NaughtyListTracker::classify_error("SSL certificate problem"), + Some(NaughtyCategory::TlsCertificateInvalid) + ); + assert_eq!( + NaughtyListTracker::classify_error("TLS handshake failed"), + Some(NaughtyCategory::TlsCertificateInvalid) + ); + + // TLS timeout should NOT be classified as naughty + assert_eq!( + NaughtyListTracker::classify_error("TLS connection timed out"), + None + ); + } + + #[test] + fn test_classify_protocol_errors() { + assert_eq!( + NaughtyListTracker::classify_error("websocket protocol error"), + Some(NaughtyCategory::ProtocolError) + ); + assert_eq!( + NaughtyListTracker::classify_error("invalid frame header"), + Some(NaughtyCategory::ProtocolError) + ); + + // WebSocket connection errors should NOT be classified as naughty + assert_eq!( + NaughtyListTracker::classify_error("websocket connection refused"), + None + ); + } + + #[test] + fn test_classify_transient_errors() { + // Timeouts are transient + assert_eq!( + NaughtyListTracker::classify_error("connection timed out"), + None + ); + assert_eq!( + NaughtyListTracker::classify_error("operation timed out"), + None + ); + + // Connection refused is transient + assert_eq!( + NaughtyListTracker::classify_error("connection refused"), + None + ); + + // Generic network errors are transient + assert_eq!( + NaughtyListTracker::classify_error("network unreachable"), + None + ); + } + + #[test] + fn test_record_new_entry() { + let tracker = NaughtyListTracker::with_defaults(); + let url = "wss://bad-relay.example.com"; + + let is_new = tracker.record( + url, + NaughtyCategory::DnsLookupFailed, + "failed to lookup address".to_string(), + ); + + assert!(is_new); + assert!(tracker.is_naughty(url)); + + let entry = tracker.get_entry(url).unwrap(); + assert_eq!(entry.category, NaughtyCategory::DnsLookupFailed); + assert_eq!(entry.occurrence_count, 1); + } + + #[test] + fn test_record_updates_existing() { + let tracker = NaughtyListTracker::with_defaults(); + let url = "wss://bad-relay.example.com"; + + // First occurrence + let is_new1 = tracker.record(url, NaughtyCategory::DnsLookupFailed, "error 1".to_string()); + assert!(is_new1); + + // Second occurrence + let is_new2 = tracker.record(url, NaughtyCategory::DnsLookupFailed, "error 2".to_string()); + assert!(!is_new2); + + let entry = tracker.get_entry(url).unwrap(); + assert_eq!(entry.occurrence_count, 2); + assert_eq!(entry.reason, "error 2"); // Updated to latest + } + + #[test] + fn test_is_naughty() { + let tracker = NaughtyListTracker::with_defaults(); + let url = "wss://bad-relay.example.com"; + + assert!(!tracker.is_naughty(url)); + + tracker.record( + url, + NaughtyCategory::TlsCertificateInvalid, + "cert error".to_string(), + ); + + assert!(tracker.is_naughty(url)); + } + + #[test] + fn test_get_all() { + let tracker = NaughtyListTracker::with_defaults(); + + tracker.record( + "wss://relay1.example.com", + NaughtyCategory::DnsLookupFailed, + "dns error".to_string(), + ); + tracker.record( + "wss://relay2.example.com", + NaughtyCategory::TlsCertificateInvalid, + "tls error".to_string(), + ); + + let all = tracker.get_all(); + assert_eq!(all.len(), 2); + } + + #[test] + fn test_count_by_category() { + let tracker = NaughtyListTracker::with_defaults(); + + tracker.record( + "wss://relay1.example.com", + NaughtyCategory::DnsLookupFailed, + "error".to_string(), + ); + tracker.record( + "wss://relay2.example.com", + NaughtyCategory::DnsLookupFailed, + "error".to_string(), + ); + tracker.record( + "wss://relay3.example.com", + NaughtyCategory::TlsCertificateInvalid, + "error".to_string(), + ); + + assert_eq!( + tracker.count_by_category(NaughtyCategory::DnsLookupFailed), + 2 + ); + assert_eq!( + tracker.count_by_category(NaughtyCategory::TlsCertificateInvalid), + 1 + ); + assert_eq!(tracker.count_by_category(NaughtyCategory::ProtocolError), 0); + } + + #[test] + fn test_total_count() { + let tracker = NaughtyListTracker::with_defaults(); + assert_eq!(tracker.total_count(), 0); + + tracker.record( + "wss://relay1.example.com", + NaughtyCategory::DnsLookupFailed, + "error".to_string(), + ); + assert_eq!(tracker.total_count(), 1); + + tracker.record( + "wss://relay2.example.com", + NaughtyCategory::TlsCertificateInvalid, + "error".to_string(), + ); + assert_eq!(tracker.total_count(), 2); + } + + #[test] + fn test_expire_old_entries() { + // Use very short expiration for testing + let tracker = NaughtyListTracker::new(0); // Expire immediately (0 hours) + + tracker.record( + "wss://relay1.example.com", + NaughtyCategory::DnsLookupFailed, + "error".to_string(), + ); + + // Entry should exist in the map + assert_eq!(tracker.total_count(), 1); + + // But is_naughty should return false since it's already expired (0 hours) + assert!(!tracker.is_naughty("wss://relay1.example.com")); + + // Sleep to ensure time passes + std::thread::sleep(std::time::Duration::from_millis(10)); + + // Expire old entries (should remove the 0-hour expired entry) + let expired = tracker.expire_old_entries(); + assert_eq!(expired.len(), 1); + assert_eq!(expired[0], "wss://relay1.example.com"); + + // Entry should be gone + assert!(!tracker.is_naughty("wss://relay1.example.com")); + assert_eq!(tracker.total_count(), 0); + } + + #[test] + fn test_category_display() { + assert_eq!( + NaughtyCategory::DnsLookupFailed.to_string(), + "dns_lookup_failed" + ); + assert_eq!( + NaughtyCategory::TlsCertificateInvalid.to_string(), + "tls_certificate_invalid" + ); + assert_eq!(NaughtyCategory::ProtocolError.to_string(), "protocol_error"); + } + + #[test] + fn test_category_as_str() { + assert_eq!( + NaughtyCategory::DnsLookupFailed.as_str(), + "dns_lookup_failed" + ); + assert_eq!( + NaughtyCategory::TlsCertificateInvalid.as_str(), + "tls_certificate_invalid" + ); + assert_eq!(NaughtyCategory::ProtocolError.as_str(), "protocol_error"); + } +}