From 3d11a99b4adc9ad900190b797e6d6dc1f97116fa Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 11 Dec 2025 15:02:33 +0000 Subject: docs(tests): replace catchup.rs test with documentation The catchup sync mechanism (reconnection with since filter) is implemented in src/sync/mod.rs handle_connect_or_reconnect(), but cannot be reliably integration tested with current infrastructure: - TestRelay uses in-memory database (events lost on stop) - No way to force WebSocket disconnection without stopping relay - Stopping syncing relay creates new instance (fresh sync, not catchup) Convert the skeleton test file to comprehensive documentation explaining: - How catchup sync works (since filter on reconnect) - The 15-minute quick reconnect window logic - Why integration testing is not feasible - Alternative approaches that could enable testing - Related tests that cover adjacent functionality --- tests/sync/catchup.rs | 311 ++++++++++++++++++-------------------------------- 1 file changed, 110 insertions(+), 201 deletions(-) (limited to 'tests/sync') diff --git a/tests/sync/catchup.rs b/tests/sync/catchup.rs index 1ddafd1..22f513d 100644 --- a/tests/sync/catchup.rs +++ b/tests/sync/catchup.rs @@ -1,201 +1,110 @@ -//! Catchup Sync Tests -//! -//! Tests for the catchup synchronization feature (Test 0). -//! -//! # Catchup Sync Overview -//! -//! Catchup sync refers to the ability of a relay to synchronize historical events -//! that were published while it was offline or unreachable. This is critical for -//! ensuring data consistency across the relay network. -//! -//! ## Expected Behavior -//! -//! When a relay comes back online after being offline: -//! 1. Detect gap in event history by comparing timestamps -//! 2. Query connected relays for events in the gap period -//! 3. Backfill Layer 2 events (kind 1618) from bootstrap relays -//! 4. Discover and sync Layer 3 events (kinds 1, 1111) referencing Layer 2 events -//! 5. Maintain chronological ordering during backfill -//! -//! ## Implementation Status -//! -//! ⚠ **NOT YET IMPLEMENTED** - Tests marked with `#[ignore]` -//! -//! These tests are ready to enable once catchup sync is implemented in the relay. -//! -//! ## See Also -//! -//! - Bootstrap sync: [`tests/sync/bootstrap.rs`](bootstrap.rs) -//! - Live sync: [`tests/sync/live_sync.rs`](live_sync.rs) -//! - Discovery sync: [`tests/sync/discovery.rs`](discovery.rs) - -use std::time::Duration; - -use nostr_sdk::prelude::*; - -use crate::common::{sync_helpers::*, TestRelay}; - -/// Test that relay performs catchup sync after being offline -/// -/// # Scenario -/// -/// 1. Start two relays (relay1, relay2) with discovery configured -/// 2. Publish several Layer 2 events to relay2 -/// 3. Stop relay1 (simulating offline state) -/// 4. Publish more Layer 2 events to relay2 while relay1 is offline -/// 5. Restart relay1 -/// 6. Verify relay1 catches up and syncs events it missed -/// -/// # Expected Result -/// -/// All events published while relay1 was offline should be synced -/// to relay1 after it comes back online, maintaining chronological order. -/// -/// # TODO -/// -/// - Implement catchup sync mechanism in relay -/// - Add timestamp-based gap detection -/// - Add backfill query generation -/// - Enable this test by removing `#[ignore]` -#[tokio::test] -#[ignore = "Catchup sync not yet implemented"] -async fn test_catchup_sync_after_relay_restart() { - // NOTE: This is a skeleton implementation ready for when catchup sync is added - - // 1. Start two relays - let relay1 = TestRelay::start().await; - let relay2 = TestRelay::start().await; - - // 2. Set up discovery between relays via shared announcement - let keys = Keys::generate(); - let identifier = "catchup-test-repo"; - - // Create announcement listing both relays - let domain1 = relay1.domain(); - let domain2 = relay2.domain(); - let announcement = create_repo_announcement( - &keys, - &[&domain1, &domain2], - identifier, - ); - - // Publish announcement to both relays - let client1 = TestClient::new(relay1.url(), keys.clone()) - .await - .expect("Failed to connect to relay1"); - let client2 = TestClient::new(relay2.url(), keys.clone()) - .await - .expect("Failed to connect to relay2"); - - client1 - .send_event(&announcement) - .await - .expect("Failed to send announcement to relay1"); - client2 - .send_event(&announcement) - .await - .expect("Failed to send announcement to relay2"); - - // Wait for discovery connections to establish - tokio::time::sleep(Duration::from_secs(2)).await; - - // 3. Publish initial Layer 2 event (while both relays are online) - let repo_coord_str = repo_coord(&keys, identifier); - let event1 = build_layer2_issue_event(&keys, &repo_coord_str, "Issue 1 - before offline") - .expect("Failed to build event1"); - let event1_id = client2 - .send_event(&event1) - .await - .expect("Failed to send event1"); - - // Verify initial sync works (baseline check) - let synced = wait_for_event_on_relay( - relay1.url(), - Filter::new().id(event1_id), - Duration::from_secs(5), - ) - .await; - assert!(synced, "Initial event should sync normally via live sync"); - - // 4. Stop relay1 (simulating offline state) - // Note: In a real implementation, we'd need a way to stop and restart a relay - // For now, this skeleton demonstrates the intended test flow - relay1.stop().await; - - // Small delay to ensure relay1 is fully stopped - tokio::time::sleep(Duration::from_millis(500)).await; - - // 5. Publish events while relay1 is offline - let event2 = build_layer2_issue_event(&keys, &repo_coord_str, "Issue 2 - during offline") - .expect("Failed to build event2"); - let event2_id = client2 - .send_event(&event2) - .await - .expect("Failed to send event2"); - - let event3 = build_layer2_issue_event(&keys, &repo_coord_str, "Issue 3 - during offline") - .expect("Failed to build event3"); - let event3_id = client2 - .send_event(&event3) - .await - .expect("Failed to send event3"); - - // Give time for events to be stored in relay2 - tokio::time::sleep(Duration::from_secs(1)).await; - - // 6. Restart relay1 - // Note: TestRelay doesn't currently support restart, so we start a new instance - // A real implementation would need persistent storage and relay restart capability - let relay1_restarted = TestRelay::start().await; - - // Reconnect client to the new relay instance - let client1_restarted = TestClient::new(relay1_restarted.url(), keys.clone()) - .await - .expect("Failed to connect to restarted relay1"); - - // Re-publish announcement to establish discovery - let domain1_restarted = relay1_restarted.domain(); - let announcement_restarted = create_repo_announcement( - &keys, - &[&domain1_restarted, &domain2], - identifier, - ); - client1_restarted - .send_event(&announcement_restarted) - .await - .expect("Failed to send announcement to restarted relay1"); - - // 7. Wait for catchup sync to complete - // This is where the catchup sync mechanism would kick in - tokio::time::sleep(Duration::from_secs(5)).await; - - // 8. Verify missed events were synced via catchup - let event2_synced = wait_for_event_on_relay( - relay1_restarted.url(), - Filter::new().id(event2_id), - Duration::from_secs(5), - ) - .await; - - let event3_synced = wait_for_event_on_relay( - relay1_restarted.url(), - Filter::new().id(event3_id), - Duration::from_secs(5), - ) - .await; - - assert!( - event2_synced, - "Event 2 (missed while offline) should be synced via catchup" - ); - assert!( - event3_synced, - "Event 3 (missed while offline) should be synced via catchup" - ); - - // 9. Cleanup - client1_restarted.disconnect().await; - client2.disconnect().await; - relay1_restarted.stop().await; - relay2.stop().await; -} \ No newline at end of file +//! Catchup Sync - Documentation Only +//! +//! This file documents the catchup sync mechanism. No integration tests are included +//! because the functionality cannot be reliably tested with current test infrastructure. +//! +//! # What is Catchup Sync? +//! +//! Catchup sync ensures that when a relay's WebSocket connection to another relay drops +//! and reconnects, any events the source relay received during the disconnection are +//! fetched using a `since` filter based on the last connection timestamp. +//! +//! # Implementation Status: ✅ IMPLEMENTED +//! +//! The catchup sync mechanism is fully implemented in the sync module via: +//! +//! - [`handle_connect_or_reconnect()`](../../src/sync/mod.rs) - Detects reconnection and +//! applies appropriate sync strategy +//! - [`RelayState.last_connected`](../../src/sync/mod.rs) - Tracks when we last connected +//! to each relay +//! - [`filters::build_announcement_filter(since)`](../../src/sync/filters.rs) - Builds +//! Layer 1 filters with `since` timestamp +//! - [`filters::build_layer2_and_layer3_filters(since)`](../../src/sync/filters.rs) - +//! Builds Layer 2/3 filters with `since` timestamp +//! +//! ## Reconnection Logic +//! +//! When a relay reconnects to a source relay, the sync manager uses smart reconnection: +//! +//! | Scenario | Behavior | +//! |----------|----------| +//! | First connection ever | Full sync (no `since` filter) | +//! | Reconnect within 15 min | Quick reconnect with `since = last_connected - 15min` | +//! | Reconnect after >15 min | Full sync (clear state, treat as fresh connection) | +//! +//! The 15-minute buffer on the `since` filter accounts for clock drift and ensures +//! no events are missed at the boundary. +//! +//! # Why No Integration Tests? +//! +//! Testing catchup sync in integration tests is not feasible with current infrastructure: +//! +//! ## 1. Cannot Force WebSocket Disconnection +//! +//! The catchup mechanism is designed for same-process reconnection scenarios, such as: +//! - Network hiccup causing temporary disconnection +//! - Source relay temporarily unreachable +//! - WebSocket connection timeout +//! +//! Our [`TestRelay`](../common/relay.rs) fixture doesn't provide a way to force a +//! WebSocket disconnection without stopping the relay entirely. +//! +//! ## 2. Stopping a Relay Loses Events (In-Memory Database) +//! +//! `TestRelay` uses `NGIT_DATABASE_BACKEND=memory` for test isolation. If we stop +//! the source relay (to simulate disconnection), all events are lost. When a new +//! instance starts, there's nothing to "catch up" on. +//! +//! ## 3. Stopping the Syncing Relay Creates a New Instance +//! +//! If we stop the syncing relay and start a new one: +//! - `last_connected` is lost (in-memory state) +//! - New instance does a fresh full sync, not a `since`-filtered catchup +//! - This is correct behavior, but tests the bootstrap path, not catchup +//! +//! # Alternative Testing Approaches (Not Implemented) +//! +//! These could enable catchup testing but add significant complexity: +//! +//! 1. **Persistent database for source relay** - Use SQLite instead of in-memory, +//! allowing relay restart without data loss +//! +//! 2. **TestRelay restart capability** - Add `restart()` method that preserves the +//! same port and database path +//! +//! 3. **Network simulation** - Add ability to inject network failures between specific +//! relay pairs without stopping either relay +//! +//! 4. **Internal sync manager API** - Expose methods to force reconnection without +//! network-level disruption +//! +//! # Related Tests +//! +//! While catchup sync itself isn't directly tested, related functionality is covered: +//! +//! - [`bootstrap.rs`](bootstrap.rs) - Tests that a new relay syncs existing events +//! from a bootstrap relay (fresh full sync path) +//! - [`live_sync.rs`](live_sync.rs) - Tests real-time sync of new events after +//! connection is established +//! - [`discovery.rs`](discovery.rs) - Tests that relays discover each other via +//! repository announcements +//! +//! # Design Rationale +//! +//! The catchup mechanism prioritizes simplicity and correctness: +//! +//! - **Correctness over testing**: The `since` filter logic is straightforward and +//! uses well-tested nostr-sdk primitives. The risk of bugs is low. +//! +//! - **15-minute quick reconnect window**: Balances efficiency (avoid full resync for +//! brief outages) with simplicity (don't track complex state for long outages). +//! +//! - **Full sync fallback**: After 15 minutes, the relay does a complete resync. +//! This guarantees no events are missed, at the cost of redundant transfers. +//! +//! # See Also +//! +//! - [`src/sync/mod.rs`](../../src/sync/mod.rs) - Main sync module with reconnection logic +//! - [`src/sync/filters.rs`](../../src/sync/filters.rs) - Filter builders with `since` support +//! - [`src/sync/metrics.rs`](../../src/sync/metrics.rs) - Metrics tracking event sources +//! including `RECONNECT` for catchup events -- cgit v1.2.3