mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
Merge #3509c127: fix(sync): suppress naughty relay connection attempts
nostr:nevent1qgsx2lyl2e4zvfadwcvkd9fkrcwczj7mf858hy85mwqclwgut8wpg2spz3mhxue69uhhyetvv9ujumn8d96zuer9wcq3yamnwvaz7tm8d96xummnw3ezucm0d5q3kamnwvaz7tmwva5hgtnyv9hxxmmwwashjer9wchxxmmdqqsr2zwpylk5cw0654n7hhrqgdd0jhc4pyyr8fmwf0lccwmks4f2fdqumxzdg PR-Author: DanConwayDev's Agent nostr:npub1v47f74n2ycn66asev62nv8sas99akj0g0wg0fkup37u3ckwuzs4q7cwtp0 CoverNote: Persistent DNS, TLS, and protocol failures were recorded in the relay naughty list, but reconnect eligibility and every scheduling path ignored that state. Production consequently kept announcing and launching retries after normal health backoff. Consult the existing tracker both when deciding reconnect eligibility and at the shared connection-scheduling boundary before lifecycle mutation or worker-capacity reservation. This preserves the required first dial, suppresses all later scheduling paths while the entry is live, and restores eligibility through the existing 12-hour expiry behavior. The scenario regression uses a deterministic broken WebSocket endpoint. It proves that the classification dial is the only connection accepted across multiple reconnect ticks and that no reconnect intent is emitted after the endpoint becomes naughty. This deliberately leaves error classification, expiry duration, Git-domain throttling, and transient relay backoff unchanged. ## Validation - Final PR commit: `ff388ce184430869c0e5e6fecadb4bf96aec951c`. - Full workspace suite passed: 617 library tests plus every integration and doc-test target. - Final sync target passed: 79 passed, 1 pre-existing ignored. - Focused health tests passed: 16 passed. - `git diff --check` passed. ## Production verification Deployed unmerged PR commit `ff388ce184430869c0e5e6fecadb4bf96aec951c` to gitnostr.com via nixos-fi1 commit `f030cfc987b128356ff856c45d45984f040c1ffc`. The exact flake pin, live ExecStart path `/nix/store/4clqvqy04lmz1q3fq6964r020x985kaf-ngit-grasp-2.0.0/bin/ngit-grasp`, and Nix registration time 2026-08-05 14:55:05 UTC independently identify the deployed revision. Activation began at 14:55:20 UTC. Observed production window: 2026-08-05 14:55:20–15:10:37 UTC (15m17s). Fourteen relays organically entered the naughty list. Every one had zero subsequent reconnect-intent lines, zero repeated event-directed target rejections, and zero repeated naughty-failure lines. During the same window, 110 legitimate reconnect attempts continued for non-naughty degraded relays, demonstrating that the reconnect scheduler remained active rather than globally stalled. No regression signal appeared during the soak: `NRestarts=0`; no error-priority journal entries, panics, scheduler failures, or stale-attempt results; memory peaked at approximately 1.26 GB. This production result, together with the deterministic scenario regression and green test suite, is sufficient for merge. Tracked under nostr:nevent1qqspxxvcaqj96zt8sdj520h9nl4rtq3873m6za6keg2gm2xn6dgp55cpz3mhxue69uhhyetvv9ujumn8d96zuer9wc85zw09
This commit is contained in:
@@ -276,7 +276,7 @@ ngit-grasp implements multiple layers of defense against abuse, spam, and denial
|
|||||||
|
|
||||||
**Relay Sync Protection (GRASP-02):**
|
**Relay Sync Protection (GRASP-02):**
|
||||||
- **Exponential backoff** - Failed connections: 5s → 10s → 20s → ... → 1 hour max
|
- **Exponential backoff** - Failed connections: 5s → 10s → 20s → ... → 1 hour max
|
||||||
- **Naughty list** - Track relays with infrastructure issues separately (12h expiry)
|
- **Naughty list** - Suppress relays with infrastructure issues until expiry (12h default)
|
||||||
- **Rate limit detection** - Auto 65s cooldown when remote relays rate limit us
|
- **Rate limit detection** - Auto 65s cooldown when remote relays rate limit us
|
||||||
- **Domain throttling** - Max 5 concurrent, 30/min per domain for git data fetching
|
- **Domain throttling** - Max 5 concurrent, 30/min per domain for git data fetching
|
||||||
|
|
||||||
|
|||||||
@@ -89,7 +89,7 @@ These limits prevent individual connections from overwhelming the relay.
|
|||||||
- Tracks relays with persistent infrastructure issues (DNS, TLS, protocol errors)
|
- Tracks relays with persistent infrastructure issues (DNS, TLS, protocol errors)
|
||||||
- Separate from normal connection failures
|
- Separate from normal connection failures
|
||||||
- 12-hour expiration (configurable)
|
- 12-hour expiration (configurable)
|
||||||
- Reduces retry frequency for broken relays
|
- Suppresses connection attempts until the entry expires
|
||||||
|
|
||||||
**Rate Limit Detection:**
|
**Rate Limit Detection:**
|
||||||
- Detects when remote relay rate limits us
|
- Detects when remote relay rate limits us
|
||||||
|
|||||||
@@ -481,6 +481,14 @@ impl RelayHealthTracker {
|
|||||||
/// - The relay is healthy
|
/// - The relay is healthy
|
||||||
/// - The backoff period has elapsed
|
/// - The backoff period has elapsed
|
||||||
pub fn should_attempt_connection(&self, relay_url: &str) -> bool {
|
pub fn should_attempt_connection(&self, relay_url: &str) -> bool {
|
||||||
|
if self
|
||||||
|
.naughty_list
|
||||||
|
.as_ref()
|
||||||
|
.is_some_and(|naughty_list| naughty_list.is_naughty(relay_url))
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
let entry = self.health.get(relay_url);
|
let entry = self.health.get(relay_url);
|
||||||
|
|
||||||
match entry {
|
match entry {
|
||||||
|
|||||||
@@ -3299,6 +3299,17 @@ impl SyncManager {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
if self
|
||||||
|
.health_tracker
|
||||||
|
.naughty_list()
|
||||||
|
.is_some_and(|naughty_list| naughty_list.is_naughty(&relay_url))
|
||||||
|
{
|
||||||
|
tracing::debug!(
|
||||||
|
relay = %relay_url,
|
||||||
|
"Suppressing connection attempt for naughty relay"
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
let Some(result_tx) = self.connect_attempt_result_tx.clone() else {
|
let Some(result_tx) = self.connect_attempt_result_tx.clone() else {
|
||||||
tracing::error!(relay = %relay_url, "Connection scheduler is not running");
|
tracing::error!(relay = %relay_url, "Connection scheduler is not running");
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -38,6 +38,7 @@ mod sync {
|
|||||||
pub mod live_sync;
|
pub mod live_sync;
|
||||||
pub mod maintainer_reprocessing;
|
pub mod maintainer_reprocessing;
|
||||||
pub mod metrics;
|
pub mod metrics;
|
||||||
|
pub mod naughty_list_scheduling;
|
||||||
pub mod neg_concurrency;
|
pub mod neg_concurrency;
|
||||||
pub mod purgatory_fetch;
|
pub mod purgatory_fetch;
|
||||||
pub mod req_concurrency;
|
pub mod req_concurrency;
|
||||||
|
|||||||
+1
-1
@@ -138,4 +138,4 @@ pub mod metrics;
|
|||||||
pub mod neg_concurrency;
|
pub mod neg_concurrency;
|
||||||
pub mod purgatory_fetch;
|
pub mod purgatory_fetch;
|
||||||
pub mod req_concurrency;
|
pub mod req_concurrency;
|
||||||
pub mod tag_variations;
|
pub mod tag_variations;
|
||||||
|
|||||||
@@ -0,0 +1,113 @@
|
|||||||
|
//! Scenario regression coverage for relay naughty-list scheduling.
|
||||||
|
|
||||||
|
use std::path::Path;
|
||||||
|
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||||
|
use std::sync::Arc;
|
||||||
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use crate::common::{TestClient, TestRelay};
|
||||||
|
use nostr_sdk::prelude::*;
|
||||||
|
use tokio::io::AsyncWriteExt;
|
||||||
|
|
||||||
|
const OBSERVATION_DEADLINE: Duration = Duration::from_secs(30);
|
||||||
|
const RECONNECT_OBSERVATION: Duration = Duration::from_secs(4);
|
||||||
|
|
||||||
|
async fn wait_for_log_line<F>(log_path: &Path, predicate: F) -> bool
|
||||||
|
where
|
||||||
|
F: Fn(&str) -> bool,
|
||||||
|
{
|
||||||
|
let deadline = tokio::time::Instant::now() + OBSERVATION_DEADLINE;
|
||||||
|
loop {
|
||||||
|
if let Ok(content) = tokio::fs::read_to_string(log_path).await {
|
||||||
|
if content.lines().any(&predicate) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if tokio::time::Instant::now() >= deadline {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A reachable TCP endpoint that deliberately violates the WebSocket
|
||||||
|
/// handshake. This produces a persistent protocol failure without relying on
|
||||||
|
/// external DNS or network state.
|
||||||
|
async fn start_broken_websocket() -> (String, Arc<AtomicUsize>) {
|
||||||
|
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||||
|
.await
|
||||||
|
.expect("bind broken websocket endpoint");
|
||||||
|
let address = listener.local_addr().expect("broken endpoint address");
|
||||||
|
let accepted = Arc::new(AtomicUsize::new(0));
|
||||||
|
let task_accepted = Arc::clone(&accepted);
|
||||||
|
|
||||||
|
tokio::spawn(async move {
|
||||||
|
while let Ok((mut stream, _)) = listener.accept().await {
|
||||||
|
task_accepted.fetch_add(1, Ordering::SeqCst);
|
||||||
|
let _ = stream.write_all(b"not a websocket handshake\r\n").await;
|
||||||
|
let _ = stream.shutdown().await;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
(format!("ws://{address}"), accepted)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn naughty_relay_is_not_scheduled_for_reconnection() {
|
||||||
|
let relay = TestRelay::start_with_sync(None).await;
|
||||||
|
let (broken_relay, accepted) = start_broken_websocket().await;
|
||||||
|
let keys = Keys::generate();
|
||||||
|
let identifier = "naughty-relay-scheduling";
|
||||||
|
let npub = keys.public_key().to_bech32().expect("npub");
|
||||||
|
let own_clone = format!("http://{}/{npub}/{identifier}.git", relay.domain());
|
||||||
|
let own_relay = format!("ws://{}", relay.domain());
|
||||||
|
let announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "")
|
||||||
|
.tags(vec![
|
||||||
|
Tag::identifier(identifier),
|
||||||
|
Tag::custom("clone", vec![own_clone]),
|
||||||
|
Tag::custom("relays", vec![own_relay, broken_relay.clone()]),
|
||||||
|
])
|
||||||
|
.finalize(&keys)
|
||||||
|
.expect("sign announcement");
|
||||||
|
|
||||||
|
let client = TestClient::new(relay.url(), keys)
|
||||||
|
.await
|
||||||
|
.expect("connect client");
|
||||||
|
client
|
||||||
|
.send_event(&announcement)
|
||||||
|
.await
|
||||||
|
.expect("publish announcement");
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
wait_for_log_line(&relay.log_path(), |line| {
|
||||||
|
line.contains("added to naughty list") && line.contains(&broken_relay)
|
||||||
|
})
|
||||||
|
.await,
|
||||||
|
"broken endpoint must be classified as naughty"
|
||||||
|
);
|
||||||
|
assert_eq!(accepted.load(Ordering::SeqCst), 1, "first dial is required");
|
||||||
|
|
||||||
|
let reconnect_deadline = tokio::time::Instant::now() + RECONNECT_OBSERVATION;
|
||||||
|
while tokio::time::Instant::now() < reconnect_deadline
|
||||||
|
&& accepted.load(Ordering::SeqCst) == 1
|
||||||
|
{
|
||||||
|
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||||
|
}
|
||||||
|
assert_eq!(
|
||||||
|
accepted.load(Ordering::SeqCst),
|
||||||
|
1,
|
||||||
|
"a naughty relay must not receive another dial across reconnect ticks"
|
||||||
|
);
|
||||||
|
let log = tokio::fs::read_to_string(relay.log_path())
|
||||||
|
.await
|
||||||
|
.expect("read relay log");
|
||||||
|
assert!(
|
||||||
|
!log.lines().any(|line| {
|
||||||
|
line.contains("Attempting reconnection relay=") && line.contains(&broken_relay)
|
||||||
|
}),
|
||||||
|
"a naughty relay must be excluded before reconnect intent is logged"
|
||||||
|
);
|
||||||
|
|
||||||
|
client.disconnect().await;
|
||||||
|
relay.stop().await;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user