mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 08:14:42 +00:00
nostr: per-peer NAT-traversal failure suppression and clock-skew handling
Public-test daemons with populous open-discovery caches generate
sustained NAT-traversal-failure WARN volume (~140/hour, ~3500/day)
against cache-learned peers that have gone offline — their adverts
are absent from major Nostr relays but cached entries persist until
their advertised `valid_until` expires. The daemon kept publishing
offers indefinitely under exponential backoff with no per-peer
suppression, drowning operator signal and hammering relays. A
parallel concern: the strict freshness check at signal.rs silently
rejected offers under modest clock skew (now_ms() anchors to
SystemTime once at startup, so post-startup NTP step adjustments
don't propagate on long-uptime daemons), indistinguishable from
"peer is offline."
Six independent improvements layered on the existing retry logic.
Per-npub WARN log rate-limit
----------------------------
New `FailureState` struct on `NostrDiscovery` records per-npub
`last_warn_at_ms`. Subsequent failures inside `warn_log_interval_secs`
(default 5 min) emit DEBUG instead of WARN. Each WARN now also
carries `consecutive_failures` and remaining `cooldown_secs` so
operators can read the trajectory without grepping multiple lines.
Per-npub consecutive-failure counter + extended cooldown
--------------------------------------------------------
After `failure_streak_threshold` (default 5) consecutive failures
against a peer, the next `extended_cooldown_secs` (default 1800)
of attempts are suppressed by pushing
`retry_pending[npub].retry_after_ms` past the cooldown wall. The
open-discovery sweep also consults `cooldown_until` and increments
a new `skipped_cooldown` counter so a peer whose `retry_pending`
was cleared by max_retries doesn't get re-enqueued during the
cooldown window. Caps offer-publish rate per dead peer regardless
of how often the sweep tries to re-enqueue.
Stale-advert eviction on streak-threshold transition
----------------------------------------------------
On the threshold-crossing transition (one-shot, not every
subsequent failure), `tokio::spawn` an active re-fetch of the
peer's Kind 37195 advert from `advert_relays`. Three outcomes:
- absent on relays → cache evicted; sweep won't re-enqueue
(peer is genuinely gone).
- newer `created_at` → cache refreshed + streak reset
(peer republished; allowed to retry immediately).
- same → cache untouched; cooldown stands.
Cost: ~one fetch per dead peer per 30-min cooldown cycle, vs
hundreds of offer publishes/hour today.
Clock-skew tolerance on freshness check
---------------------------------------
`signal.rs` `validate_offer_freshness` and
`validate_traversal_answer_for_offer` now allow ±60s grace beyond
strict TTL. Both return a new `FreshnessOutcome` enum so callers
can DEBUG-log when an offer/answer was only accepted via the grace
window. `FRESHNESS_SKEW_TOLERANCE_MS` is hard-coded — loosening
this past minutes erodes the freshness/replay security boundary
and operators tend to tune in the wrong direction.
NTP-style skew estimate (offer_received_at echo)
------------------------------------------------
Added optional `offerReceivedAt: Option<u64>` field to
`TraversalAnswer` payload. Responder fills it with `now_ms()` at
offer-receipt time. Initiator computes the standard NTP offset
formula `((T2-T1) + (T3-T4)) / 2` against the round-trip and
DEBUG-logs when `|skew| ≥ 30s`. Skew is also stashed in
`FailureState` and surfaced in `show_peers`. Non-breaking — older
responders that don't fill the field still produce valid answers,
and `estimate_clock_skew` returns `None`.
Per-peer state in `show_peers` JSON
-----------------------------------
Each peer entry in `show_peers` now carries:
"nostr_traversal": {
"consecutive_failures": <u32>,
"in_cooldown": <bool>,
"cooldown_until_ms": <u64 | null>,
"last_observed_skew_ms": <i64 | null>
}
Always emitted (schema-stable); values populated when discovery is
enabled and the npub has a recorded entry. Required a new public
`Node::nostr_discovery_handle()` accessor and refactored
`FailureState`'s internal Mutex from `tokio::sync` to `std::sync`
(operations never hold across await), which lets the synchronous
`show_peers` handler call `snapshot()` directly without the
dispatcher becoming async.
New config knobs (under `node.discovery.nostr`)
-----------------------------------------------
failure_streak_threshold: 5
extended_cooldown_secs: 1800
warn_log_interval_secs: 300
failure_state_max_entries: 4096
Tests
-----
12 new unit tests:
- 5 in `tests.rs` covering freshness strict / tolerated / rejected
outcomes, NTP skew estimation, and the backward-compat None case
when the responder didn't fill `offer_received_at`.
- 7 in `failure_state.rs` covering streak/warn-rate-limit state
transitions, cooldown active vs expired semantics,
success-resets-streak, observed-skew records, and size-cap
eviction by oldest `last_failure_at`.
CHANGELOG entries added under `[Unreleased]` Fixed.
This commit is contained in:
+68
-3
@@ -407,7 +407,56 @@ impl Node {
|
||||
peer_config,
|
||||
reason,
|
||||
} => {
|
||||
warn!(npub = %peer_config.npub, error = %reason, "NAT traversal failed");
|
||||
let now_ms = Self::now_ms();
|
||||
let decision = bootstrap.record_traversal_failure(&peer_config.npub, now_ms);
|
||||
if decision.should_warn {
|
||||
warn!(
|
||||
npub = %peer_config.npub,
|
||||
error = %reason,
|
||||
consecutive_failures = decision.consecutive_failures,
|
||||
cooldown_secs = decision
|
||||
.cooldown_until_ms
|
||||
.map(|t| t.saturating_sub(now_ms) / 1000),
|
||||
"NAT traversal failed"
|
||||
);
|
||||
} else {
|
||||
debug!(
|
||||
npub = %peer_config.npub,
|
||||
error = %reason,
|
||||
consecutive_failures = decision.consecutive_failures,
|
||||
"NAT traversal failed (suppressed by warn-rate-limit)"
|
||||
);
|
||||
}
|
||||
|
||||
// B6: stale-advert eviction on the streak-threshold
|
||||
// crossing. Fire-and-forget; the outcome is logged so
|
||||
// operators can see when peers get cleaned up.
|
||||
if decision.crossed_threshold {
|
||||
let bootstrap = bootstrap.clone();
|
||||
let npub = peer_config.npub.clone();
|
||||
tokio::spawn(async move {
|
||||
let outcome = bootstrap.refetch_advert_for_stale_check(&npub).await;
|
||||
match outcome {
|
||||
crate::discovery::nostr::NostrRefetchOutcome::Evicted => info!(
|
||||
npub = %npub,
|
||||
"stale-advert sweep: peer evicted from advert cache"
|
||||
),
|
||||
crate::discovery::nostr::NostrRefetchOutcome::Refreshed => info!(
|
||||
npub = %npub,
|
||||
"stale-advert sweep: peer republished, cache refreshed and streak reset"
|
||||
),
|
||||
crate::discovery::nostr::NostrRefetchOutcome::SameAdvert => debug!(
|
||||
npub = %npub,
|
||||
"stale-advert sweep: advert unchanged, cooldown stands"
|
||||
),
|
||||
crate::discovery::nostr::NostrRefetchOutcome::Skipped => debug!(
|
||||
npub = %npub,
|
||||
"stale-advert sweep: skipped (relay error or no advert_relays)"
|
||||
),
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
let peer_identity = match PeerIdentity::from_npub(&peer_config.npub) {
|
||||
Ok(identity) => identity,
|
||||
Err(_) => continue,
|
||||
@@ -421,7 +470,16 @@ impl Node {
|
||||
continue;
|
||||
}
|
||||
|
||||
self.schedule_retry(*peer_identity.node_addr(), Self::now_ms());
|
||||
let node_addr = *peer_identity.node_addr();
|
||||
self.schedule_retry(node_addr, now_ms);
|
||||
if let Some(cooldown_until_ms) = decision.cooldown_until_ms
|
||||
&& let Some(state) = self.retry_pending.get_mut(&node_addr)
|
||||
{
|
||||
// Push the next retry past the cooldown so the
|
||||
// open-discovery sweep doesn't re-enqueue and the
|
||||
// per-attempt backoff doesn't fire sooner.
|
||||
state.retry_after_ms = state.retry_after_ms.max(cooldown_until_ms);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1204,6 +1262,7 @@ impl Node {
|
||||
let mut skipped_connecting = 0usize;
|
||||
let mut skipped_no_endpoints = 0usize;
|
||||
let mut skipped_invalid_npub = 0usize;
|
||||
let mut skipped_cooldown = 0usize;
|
||||
|
||||
for (npub, endpoints, created_at_secs) in candidates {
|
||||
if enqueue_budget == 0 {
|
||||
@@ -1242,6 +1301,10 @@ impl Node {
|
||||
skipped_retry_pending = skipped_retry_pending.saturating_add(1);
|
||||
continue;
|
||||
}
|
||||
if bootstrap.cooldown_until(&npub, now_ms).is_some() {
|
||||
skipped_cooldown = skipped_cooldown.saturating_add(1);
|
||||
continue;
|
||||
}
|
||||
let connecting = self.connections.values().any(|conn| {
|
||||
conn.expected_identity()
|
||||
.map(|id| id.node_addr() == &node_addr)
|
||||
@@ -1309,7 +1372,8 @@ impl Node {
|
||||
+ skipped_retry_pending
|
||||
+ skipped_connecting
|
||||
+ skipped_no_endpoints
|
||||
+ skipped_invalid_npub;
|
||||
+ skipped_invalid_npub
|
||||
+ skipped_cooldown;
|
||||
let should_summarize = caller == "startup" || enqueued > 0;
|
||||
if should_summarize {
|
||||
info!(
|
||||
@@ -1324,6 +1388,7 @@ impl Node {
|
||||
skipped_connecting = skipped_connecting,
|
||||
skipped_no_endpoints = skipped_no_endpoints,
|
||||
skipped_invalid_npub = skipped_invalid_npub,
|
||||
skipped_cooldown = skipped_cooldown,
|
||||
skipped_total = total_skipped,
|
||||
"open-discovery sweep complete"
|
||||
);
|
||||
|
||||
@@ -1539,6 +1539,13 @@ impl Node {
|
||||
self.peers.values()
|
||||
}
|
||||
|
||||
/// Reference to the Nostr discovery handle if discovery is enabled.
|
||||
/// Used by control queries (`show_peers` per-peer Nostr-traversal
|
||||
/// state) to read failure-state without taking shared ownership.
|
||||
pub fn nostr_discovery_handle(&self) -> Option<&crate::discovery::nostr::NostrDiscovery> {
|
||||
self.nostr_discovery.as_deref()
|
||||
}
|
||||
|
||||
/// Iterate over all peer node IDs.
|
||||
pub fn peer_ids(&self) -> impl Iterator<Item = &NodeAddr> {
|
||||
self.peers.keys()
|
||||
|
||||
Reference in New Issue
Block a user