fix(sync): retain NIP-77 for small hydration residuals

A zero-progress residual retry previously disabled negentropy even when the relay had delivered almost every advertised event. Expiry, deletion, or transient indexing lag could therefore misclassify healthy relays and invoke an expensive semantic fallback.

Retain the first-pass hydration ratio and reserve semantic fallback for batches of at least 20 advertised IDs that delivered no more than 10 percent, after the residual retry also makes no progress. Small residuals keep NIP-77 enabled and enter the existing bounded missing-event recovery path.

The threshold deliberately preserves the observed high-failure compatibility fallback while excluding the production residual ratios. This does not change explicit NIP-77 unsupported-signal handling or the recovery schedule.

Extend the censoring-proxy scenario to prove a one-event residual retains NIP-77, add boundary and production-ratio unit coverage, and document the policy. Validation: nix develop -c cargo test (616 library tests; all integration and doc tests green).
This commit is contained in:
DanConwayDev
2026-08-05 12:52:23 +00:00
parent a2215b9b8c
commit a7da97e105
5 changed files with 118 additions and 16 deletions
+1 -1
View File
@@ -292,7 +292,7 @@ Each layer creates one or more `PendingBatch` entries tracked in `PendingSyncInd
**Why the double-check?** There's an async gap between receiving EOSE and the self-subscriber processing events to create Layer 2/3 filters. The 6-second wait (5s batch window + 1s buffer) ensures we don't prematurely mark sync complete while Layer 2/3 batches are being created. **Why the double-check?** There's an async gap between receiving EOSE and the self-subscriber processing events to create Layer 2/3 filters. The 6-second wait (5s batch window + 1s buffer) ensures we don't prematurely mark sync complete while Layer 2/3 batches are being created.
**Batch Failure Tracking**: When negentropy retry protection triggers (relay returns zero requested events on retry) and no fallback subscriptions can be created, the batch is marked as `failed = true`. This causes the relay to transition to `ConnectedHistoricSyncFailures` instead of `Connected`, signaling that live sync is active but historic sync is incomplete. The event IDs the relay failed to deliver are not dropped with the batch: they are registered for bounded background recovery (see "Missing-Event Recovery for Incomplete Batches" below), and a relay whose pending IDs are all eventually recovered — with no unrelated batch failures — is promoted back to `Connected`. **Batch Failure Tracking**: Semantic REQ+EOSE fallback is reserved for a material first-pass hydration incompatibility: at least 20 IDs were advertised and exact-ID REQ delivered no more than 10%. This preserves the fallback for relays that advertise inventory but cannot substantially serve it by ID, without disabling NIP-77 for small residuals caused by expiry, indexing lag, or concurrent deletion. Other incomplete batches retry the residual IDs once. If that retry makes no progress, the batch is marked as `failed = true` and NIP-77 remains enabled. This causes the relay to transition to `ConnectedHistoricSyncFailures` instead of `Connected`, signaling that live sync is active but historic sync is incomplete. The event IDs the relay failed to deliver are not dropped with the batch: they are registered for bounded background recovery (see "Missing-Event Recovery for Incomplete Batches" below), and a relay whose pending IDs are all eventually recovered — with no unrelated batch failures — is promoted back to `Connected`.
**Metrics tracking**: The `ngit_sync_relay_connected` metric shows: **Metrics tracking**: The `ngit_sync_relay_connected` metric shows:
- `0` = Disconnected - `0` = Disconnected
+3
View File
@@ -495,6 +495,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}], }],
@@ -601,6 +602,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}], }],
@@ -689,6 +691,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}], }],
+99 -12
View File
@@ -58,6 +58,14 @@ use nostr_sdk::prelude::LocalRelay;
const MAX_PURGATORY_DEPENDENCY_EVENTS_PER_TICK: usize = 32; const MAX_PURGATORY_DEPENDENCY_EVENTS_PER_TICK: usize = 32;
const MAX_PURGATORY_FILTER_ACTIONS_PER_TICK: usize = 1; const MAX_PURGATORY_FILTER_ACTIONS_PER_TICK: usize = 1;
const SEMANTIC_FALLBACK_MIN_REQUESTED_EVENTS: usize = 20;
const SEMANTIC_FALLBACK_MAX_DELIVERED_PERCENT: usize = 10;
fn should_use_semantic_fallback(requested_count: usize, received_count: usize) -> bool {
requested_count >= SEMANTIC_FALLBACK_MIN_REQUESTED_EVENTS
&& received_count.saturating_mul(100)
<= requested_count.saturating_mul(SEMANTIC_FALLBACK_MAX_DELIVERED_PERCENT)
}
fn purgatory_dependency_retry_after() -> Duration { fn purgatory_dependency_retry_after() -> Duration {
if std::env::var("NGIT_TEST").as_deref() == Ok("1") { if std::env::var("NGIT_TEST").as_deref() == Ok("1") {
@@ -458,6 +466,8 @@ pub struct PendingBatch {
/// Event IDs actually received for this batch (None for REQ+EOSE) /// Event IDs actually received for this batch (None for REQ+EOSE)
/// Compared against requested_event_ids to detect missing events /// Compared against requested_event_ids to detect missing events
pub received_event_ids: Option<HashSet<EventId>>, pub received_event_ids: Option<HashSet<EventId>>,
/// First-pass advertised and delivered counts, retained while retrying a residual.
pub initial_hydration_counts: Option<(usize, usize)>,
/// Number of retry attempts for missing events (Negentropy only) /// Number of retry attempts for missing events (Negentropy only)
/// Used to prevent infinite retry loops when relay consistently fails /// Used to prevent infinite retry loops when relay consistently fails
pub retry_count: usize, pub retry_count: usize,
@@ -1363,26 +1373,35 @@ impl SyncManager {
let requested_count = requested.len(); let requested_count = requested.len();
let received_count = received.len(); let received_count = received.len();
let retry_count = batch.retry_count; let retry_count = batch.retry_count;
let initial_hydration_counts = batch
.initial_hydration_counts
.unwrap_or((requested_count, received_count));
if retry_count == 0 {
batch.initial_hydration_counts = Some(initial_hydration_counts);
}
// Check if we made any progress (received ANY events we requested) // A semantic fallback is reserved for relays whose first exact-ID fetch is
// If received_count is 0, relay returned nothing useful - abort retry // materially incompatible with their advertised inventory. Small residuals
// // can be caused by indexing lag, expiry, or concurrent deletion and must not
// NOTE: Some relays (e.g., azzamo.net, snort.social) have been observed // disable NIP-77 for an otherwise healthy relay.
// returning zero events during negentropy retry even though manual queries if retry_count > 0
// (REQ by ID) show they DO have these events. This appears to be relay- && received_count == 0
// specific behavior where the relay refuses to serve events via negentropy && should_use_semantic_fallback(
// retry for unknown reasons (rate limiting, negentropy implementation bugs, initial_hydration_counts.0,
// or other internal logic). When retry returns zero, fall back to REQ+EOSE. initial_hydration_counts.1,
if retry_count > 0 && received_count == 0 { )
{
tracing::info!( tracing::info!(
relay = %relay_url, relay = %relay_url,
batch_id = batch.batch_id, batch_id = batch.batch_id,
retry_count = retry_count, retry_count = retry_count,
requested_count = requested_count, requested_count = requested_count,
missing_count = missing.len(), missing_count = missing.len(),
initial_requested_count = initial_hydration_counts.0,
initial_received_count = initial_hydration_counts.1,
missing_ids_sample = ?missing.iter().take(5).map(|id| id.to_hex()).collect::<Vec<_>>(), missing_ids_sample = ?missing.iter().take(5).map(|id| id.to_hex()).collect::<Vec<_>>(),
"Negentropy retry made no progress - relay returned zero requested events. \ "Negentropy exact-ID hydration delivered at most 10% of a material batch. \
Marking relay as not supporting negentropy and falling back to REQ+EOSE." Marking relay as incompatible with negentropy hydration and falling back to REQ+EOSE."
); );
// Mark relay as not supporting negentropy so future batches skip it // Mark relay as not supporting negentropy so future batches skip it
@@ -1524,6 +1543,53 @@ impl SyncManager {
} }
} }
if retry_count > 0 && received_count == 0 {
let relay_url_for_recovery = relay_url.to_string();
let batch_id = batch.batch_id;
let missing_count = missing.len();
drop(pending);
let relay_already_degraded = {
let index = self.relay_sync_index.read().await;
index
.get(&relay_url_for_recovery)
.map(|state| state.historic_sync_had_failures)
.unwrap_or(false)
};
let register_outcome =
self.missing_event_recovery.lock().unwrap().register(
&relay_url_for_recovery,
batch_id,
missing.iter().copied(),
relay_already_degraded,
Instant::now(),
);
tracing::warn!(
relay = %relay_url_for_recovery,
batch_id = batch_id,
retry_count = retry_count,
missing_count = missing_count,
pending_recovery = register_outcome.pending_total,
"Negentropy residual retry made no progress; retaining NIP-77 and scheduling bounded missing-event recovery"
);
let mut pending = self.pending_sync_index.write().await;
if let Some(batches) = pending.get_mut(&relay_url_for_recovery) {
if let Some(idx) = batches.iter().position(|b| b.batch_id == batch_id) {
let mut completed_batch = batches.remove(idx);
completed_batch.failed = true;
if batches.is_empty() {
pending.remove(&relay_url_for_recovery);
}
drop(pending);
self.confirm_batch(&relay_url_for_recovery, completed_batch)
.await;
}
}
return;
}
tracing::warn!( tracing::warn!(
relay = %relay_url, relay = %relay_url,
batch_id = batch.batch_id, batch_id = batch.batch_id,
@@ -5022,6 +5088,7 @@ impl SyncManager {
pagination_state: HashMap::new(), // Negentropy doesn't use pagination pagination_state: HashMap::new(), // Negentropy doesn't use pagination
requested_event_ids: None, // Will be set after negentropy diff requested_event_ids: None, // Will be set after negentropy diff
received_event_ids: None, // Will be set after negentropy diff received_event_ids: None, // Will be set after negentropy diff
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}; };
@@ -5279,6 +5346,7 @@ impl SyncManager {
pagination_state, pagination_state,
requested_event_ids: None, // Not used for REQ+EOSE requested_event_ids: None, // Not used for REQ+EOSE
received_event_ids: None, // Not used for REQ+EOSE received_event_ids: None, // Not used for REQ+EOSE
initial_hydration_counts: None,
retry_count: 0, // Not used for REQ+EOSE retry_count: 0, // Not used for REQ+EOSE
failed: false, failed: false,
}; };
@@ -5352,6 +5420,21 @@ impl SyncManager {
mod tests { mod tests {
use super::*; use super::*;
#[test]
fn semantic_fallback_requires_material_first_pass_incompatibility() {
for (requested, received) in [(118, 115), (203, 197), (380, 301), (1818, 1420)] {
assert!(
!should_use_semantic_fallback(requested, received),
"{received}/{requested} is a residual, not a hydration incompatibility"
);
}
assert!(should_use_semantic_fallback(20, 0));
assert!(should_use_semantic_fallback(20, 2));
assert!(!should_use_semantic_fallback(20, 3));
assert!(!should_use_semantic_fallback(19, 0));
}
#[test] #[test]
fn group_filters_for_req_respects_count_and_byte_budgets() { fn group_filters_for_req_respects_count_and_byte_budgets() {
// Many small filters group by the count cap. // Many small filters group by the count cap.
@@ -5711,6 +5794,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}; };
@@ -5742,6 +5826,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}; };
@@ -5990,6 +6075,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: Some(HashSet::new()), requested_event_ids: Some(HashSet::new()),
received_event_ids: Some(HashSet::new()), received_event_ids: Some(HashSet::new()),
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}; };
@@ -6012,6 +6098,7 @@ mod tests {
pagination_state: HashMap::new(), pagination_state: HashMap::new(),
requested_event_ids: None, requested_event_ids: None,
received_event_ids: None, received_event_ids: None,
initial_hydration_counts: None,
retry_count: 0, retry_count: 0,
failed: false, failed: false,
}; };
+3 -3
View File
@@ -857,9 +857,9 @@ impl RelayConnection {
/// Mark this relay as not supporting NIP-77 negentropy /// Mark this relay as not supporting NIP-77 negentropy
/// ///
/// Called only when the relay explicitly signals it cannot speak NIP-77 /// Called only when the relay explicitly signals it cannot speak NIP-77
/// (see [`classify_negentropy_failure`]), or when a negentropy retry /// (see [`classify_negentropy_failure`]), or when a material first-pass
/// returns zero events (see zero-progress handling in `sync::mod`). /// exact-ID hydration failure shows its advertised inventory is not usable.
/// Transient failures use a bounded cooldown instead. /// Small residuals and transient failures retain NIP-77.
/// ///
/// Future batches will skip negentropy and use REQ+EOSE directly. /// Future batches will skip negentropy and use REQ+EOSE directly.
pub fn mark_negentropy_unsupported(&self) { pub fn mark_negentropy_unsupported(&self) {
+12
View File
@@ -122,6 +122,18 @@ async fn incomplete_exact_id_fetch_recovers_after_event_becomes_available() {
"proxy should have censored the initial fetch and at least one retry, dropped: {}", "proxy should have censored the initial fetch and at least one retry, dropped: {}",
proxy.dropped_count() proxy.dropped_count()
); );
let syncing_log = std::fs::read_to_string(syncing.log_path())
.expect("read syncing relay log after historic batch completion");
assert!(
syncing_log.contains(
"Negentropy residual retry made no progress; retaining NIP-77 and scheduling bounded missing-event recovery"
),
"a small residual must retain NIP-77 and enter bounded recovery"
);
assert!(
!syncing_log.contains("Marking relay as incompatible with negentropy hydration"),
"a one-event residual must not trigger semantic fallback"
);
assert!( assert!(
!wait_for_event_on_relay( !wait_for_event_on_relay(
syncing.url(), syncing.url(),