Merge pull request #3665 from alltheseas/fix/relay-disconnect-freeze

Fix stale relay list: replace UserDefaults hex lookup with ndb query

damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift

@@ -305,9 +305,6 @@ extension NostrNetworkManager {
         /// The keypair to use for relay authentication and updating relay lists
         var keypair: Keypair { get }
 
-        /// The latest relay list event id hex
-        var latestRelayListEventIdHex: String? { get set }  // TODO: Update this once we have full NostrDB query support
-        
         /// The latest contact list `NostrEvent`
         ///
         /// Note: Read-only access, because `NostrNetworkManager` does not manage contact lists.

damus/Core/Networking/NostrNetworkManager/UserRelayListManager.swift

@@ -22,6 +22,11 @@ extension NostrNetworkManager {
         private var relayListObserverTask: Task<Void, Never>? = nil
         private var walletUpdatesObserverTask: AnyCancellable? = nil
 
+        /// In-memory cache of the most recently set relay list.
+        /// Bridges the gap between sending an event to nostrdb (async write) and it being queryable.
+        @MainActor
+        private var lastSetRelayList: NIP65.RelayList?
+        
         init(delegate: Delegate, pool: RelayPool, reader: SubscriptionManager) {
             self.delegate = delegate
             self.pool = pool
@@ -60,9 +65,11 @@ extension NostrNetworkManager {
         
         /// Gets the user's current relay list.
         ///
-        /// It attempts to get a NIP-65 relay list from the local database, or falls back to a legacy list.
+        /// It attempts to get the in-memory cache first (to bridge the nostrdb async write gap),
+        /// then a NIP-65 relay list from the local database, or falls back to a legacy list.
         @MainActor
         func getUserCurrentRelayList() -> NIP65.RelayList? {
+            if let lastSetRelayList { return lastSetRelayList }
             if let latestRelayListEvent = try? self.getLatestNIP65RelayList() { return latestRelayListEvent }
             if let latestRelayListEvent = try? self.getLatestKind3RelayList() { return latestRelayListEvent }
             if let latestRelayListEvent = try? self.getLatestUserDefaultsRelayList() { return latestRelayListEvent }
@@ -80,7 +87,7 @@ extension NostrNetworkManager {
             return list
         }
         
-        /// Gets the latest NIP-65 relay list event from NostrDB.
+        /// Gets the latest NIP-65 relay list event from NostrDB via query.
         ///
         /// This is `private` because it is part of internal logic. Callers should use the higher level functions.
         ///
@@ -88,9 +95,10 @@ extension NostrNetworkManager {
         ///
         /// - Returns: The latest NIP-65 relay list NdbNote
         private func getLatestNIP65RelayListEvent() -> NdbNote? {
-            guard let latestRelayListEventId = delegate.latestRelayListEventIdHex else { return nil }
+            let filter = NostrFilter(kinds: [.relay_list], limit: 1, authors: [delegate.keypair.pubkey])
-            guard let latestRelayListEventId = NoteId(hex: latestRelayListEventId) else { return nil }
+            guard let ndbFilter = try? NdbFilter(from: filter) else { return nil }
-            return try? delegate.ndb.lookup_note_and_copy(latestRelayListEventId)
+            guard let noteKey = try? delegate.ndb.query(filters: [ndbFilter], maxResults: 1).first else { return nil }
+            return try? delegate.ndb.lookup_note_by_key_and_copy(noteKey)
         }
         
         /// Gets the latest `kind:3` relay list from NostrDB.
@@ -177,16 +185,18 @@ extension NostrNetworkManager {
             guard let fullKeypair = delegate.keypair.to_full() else { throw .notAuthorizedToChangeRelayList }
             guard let relayListEvent = userRelayList.toNostrEvent(keypair: fullKeypair) else { throw .cannotFormRelayListEvent }
 
+            await MainActor.run { self.lastSetRelayList = userRelayList }
+
             await self.apply(newRelayList: self.computeRelaysToConnectTo(with: userRelayList))
 
             await self.pool.send(.event(relayListEvent))   // This will send to NostrDB as well, which will locally save that NIP-65 event
-            self.delegate.latestRelayListEventIdHex = relayListEvent.id.hex()   // Make sure we are able to recall this event from NostrDB
         }
         
         // MARK: - Syncing our saved user relay list with the active `RelayPool`
         
         /// Loads the current user relay list
         func load() async {
+            await MainActor.run { self.lastSetRelayList = nil }  // Clear cache; ndb has had time to commit by now
             await self.apply(newRelayList: self.relaysToConnectTo())
         }
         

damus/Core/Storage/DamusState.swift

@@ -230,11 +230,6 @@ fileprivate extension DamusState {
         var ndb: Ndb
         var keypair: Keypair
         
-        var latestRelayListEventIdHex: String? {
-            get { self.settings.latestRelayListEventIdHex }
-            set { self.settings.latestRelayListEventIdHex = newValue }
-        }
-        
         @MainActor
         var latestContactListEvent: NostrEvent? { self.contacts.event }
         var bootstrapRelays: [RelayURL] { get_default_bootstrap_relays() }

damus/Features/Onboarding/Views/SaveKeysView.swift

@@ -166,7 +166,6 @@ struct SaveKeysView: View {
         // Save the ID to user settings so that we can easily find it later.
         let settings = UserSettingsStore.globally_load_for(pubkey: account.pubkey)
         settings.latest_contact_event_id_hex = first_contact_event.id.hex()
-        settings.latestRelayListEventIdHex = first_relay_list_event.id.hex()
     }
 
     func handle_event(relay: RelayURL, ev: NostrConnectionEvent) async {

damus/Features/Settings/Models/UserSettingsStore.swift

@@ -411,10 +411,6 @@ class UserSettingsStore: ObservableObject {
     @Setting(key: "draft_event_ids", default_value: nil)
     var draft_event_ids: [String]?
     
-    // TODO: Get rid of this once we have NostrDB query capabilities integrated
-    @Setting(key: "latest_relay_list_event_id", default_value: nil)
-    var latestRelayListEventIdHex: String?
-    
     // MARK: Helper types
     
     enum NotificationsMode: String, CaseIterable, Identifiable, StringCodable, Equatable {

damusTests/EntityPreloaderTests.swift

@@ -869,7 +869,6 @@ final class EntityPreloaderTests: XCTestCase {
 private final class TestNetworkDelegate: NostrNetworkManager.Delegate {
     var ndb: Ndb
     var keypair: Keypair
-    var latestRelayListEventIdHex: String?
     var latestContactListEvent: NostrEvent?
     var bootstrapRelays: [RelayURL]
     var developerMode: Bool = false

damusTests/NostrNetworkManagerTests/NostrNetworkManagerTests.swift

@@ -192,6 +192,204 @@ class NostrNetworkManagerTests: XCTestCase {
         XCTAssertEqual(manager.setCallCount, 1)
         XCTAssertEqual(manager.appliedRelayLists.first?.relays.count, validRelayList.relays.count)
     }
+
+    // MARK: - Relay list stale-data regression tests
+
+    /// Regression: removing a relay must not fall back to bootstrap relays.
+    ///
+    /// Before the fix, `getLatestNIP65RelayListEvent()` used a UserDefaults hex lookup
+    /// that could go stale, causing `getUserCurrentRelayList()` to return nil and
+    /// `remove()` to throw `.noInitialRelayList`. The user could never disconnect a relay.
+    func testRemoveRelayDoesNotFallBackToBootstrapList() async throws {
+        let ndb = Ndb.test
+        defer { ndb.close() }
+
+        let relayA = RelayURL("wss://relay-a.example.com")!
+        let relayB = RelayURL("wss://relay-b.example.com")!
+        let relayC = RelayURL("wss://relay-c.example.com")!
+        let bootstrapRelay = RelayURL("wss://bootstrap.example.com")!
+
+        let initialList = NIP65.RelayList(relays: [relayA, relayB, relayC])
+        let initialEvent = initialList.toNostrEvent(keypair: test_keypair_full)!
+        let eventJson = encode_json(initialEvent)!
+        let processed = ndb.processEvent("[\"EVENT\",\"subid\",\(eventJson)]")
+        XCTAssertTrue(processed, "Failed to process relay list event into ndb")
+        try await Task.sleep(for: .milliseconds(100))
+
+        let delegate = MockNetworkDelegate(
+            ndb: ndb,
+            keypair: test_keypair,
+            bootstrapRelays: [bootstrapRelay]
+        )
+        let pool = RelayPool(ndb: nil, keypair: test_keypair)
+        let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
+        let manager = NostrNetworkManager.UserRelayListManager(
+            delegate: delegate, pool: pool, reader: reader
+        )
+
+        // Remove relay B from [A, B, C] — should yield [A, C]
+        try await manager.remove(relayURL: relayB)
+
+        let currentList = await manager.getUserCurrentRelayList()
+        XCTAssertNotNil(currentList, "Relay list must not be nil after remove")
+        let urls = Set(currentList!.relays.keys)
+        XCTAssertEqual(urls, [relayA, relayC], "List should be [A, C] after removing B")
+        XCTAssertFalse(urls.contains(relayB), "Removed relay B must not be present")
+        XCTAssertFalse(urls.contains(bootstrapRelay), "Must not fall back to bootstrap relays")
+    }
+
+    /// Regression: the in-memory cache must bridge the nostrdb async write gap.
+    ///
+    /// After `set()`, `getUserCurrentRelayList()` must immediately return the new list,
+    /// even before nostrdb's async worker has committed the event.
+    func testCacheBridgesAsyncWriteGap() async throws {
+        let ndb = Ndb.test
+        defer { ndb.close() }
+
+        let delegate = MockNetworkDelegate(
+            ndb: ndb,
+            keypair: test_keypair,
+            bootstrapRelays: [RelayURL("wss://bootstrap.example.com")!]
+        )
+        let pool = RelayPool(ndb: nil, keypair: test_keypair)
+        let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
+        let manager = NostrNetworkManager.UserRelayListManager(
+            delegate: delegate, pool: pool, reader: reader
+        )
+
+        let relayA = RelayURL("wss://relay-a.example.com")!
+        let relayC = RelayURL("wss://relay-c.example.com")!
+        let newList = NIP65.RelayList(relays: [relayA, relayC])
+
+        // set() should populate the cache immediately
+        try await manager.set(userRelayList: newList)
+
+        // Query immediately — no sleep — ndb may not have committed yet
+        let currentList = await manager.getUserCurrentRelayList()
+        XCTAssertNotNil(currentList, "Cache must serve the list immediately after set()")
+        XCTAssertEqual(Set(currentList!.relays.keys), [relayA, relayC])
+    }
+
+    /// Regression: rapid sequential removes must not reintroduce removed relays.
+    ///
+    /// Scenario: start with [A, B, C], remove B, then immediately remove C.
+    /// Without the cache, the second `remove()` might read a stale [A, B, C] from ndb
+    /// (because the first set hasn't committed yet), producing [A, B] — relay B is back.
+    func testRapidSequentialRemovesDoNotReintroduceRelays() async throws {
+        let ndb = Ndb.test
+        defer { ndb.close() }
+
+        let relayA = RelayURL("wss://relay-a.example.com")!
+        let relayB = RelayURL("wss://relay-b.example.com")!
+        let relayC = RelayURL("wss://relay-c.example.com")!
+
+        let initialList = NIP65.RelayList(relays: [relayA, relayB, relayC])
+        let initialEvent = initialList.toNostrEvent(keypair: test_keypair_full)!
+        let eventJson = encode_json(initialEvent)!
+        XCTAssertTrue(ndb.processEvent("[\"EVENT\",\"subid\",\(eventJson)]"))
+        try await Task.sleep(for: .milliseconds(100))
+
+        let delegate = MockNetworkDelegate(
+            ndb: ndb,
+            keypair: test_keypair,
+            bootstrapRelays: []
+        )
+        let pool = RelayPool(ndb: nil, keypair: test_keypair)
+        let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
+        let manager = NostrNetworkManager.UserRelayListManager(
+            delegate: delegate, pool: pool, reader: reader
+        )
+
+        // Remove B then immediately remove C — no sleep between
+        try await manager.remove(relayURL: relayB)
+        try await manager.remove(relayURL: relayC)
+
+        let currentList = await manager.getUserCurrentRelayList()
+        XCTAssertNotNil(currentList)
+        let urls = Set(currentList!.relays.keys)
+        XCTAssertEqual(urls, [relayA], "Only relay A should remain after removing B and C")
+        XCTAssertFalse(urls.contains(relayB), "Relay B must not reappear after sequential removes")
+        XCTAssertFalse(urls.contains(relayC), "Relay C must stay removed")
+    }
+
+    /// Verify that `load()` clears the in-memory cache so ndb becomes the source of truth again.
+    func testLoadClearsCacheAndReadsFromNdb() async throws {
+        let ndb = Ndb.test
+        defer { ndb.close() }
+
+        let relayA = RelayURL("wss://relay-a.example.com")!
+        let relayB = RelayURL("wss://relay-b.example.com")!
+
+        // Seed ndb with [A, B]
+        let ndbList = NIP65.RelayList(relays: [relayA, relayB])
+        let ndbEvent = ndbList.toNostrEvent(keypair: test_keypair_full)!
+        XCTAssertTrue(ndb.processEvent("[\"EVENT\",\"subid\",\(encode_json(ndbEvent)!)]"))
+        try await Task.sleep(for: .milliseconds(100))
+
+        let delegate = MockNetworkDelegate(
+            ndb: ndb,
+            keypair: test_keypair,
+            bootstrapRelays: []
+        )
+        let pool = RelayPool(ndb: nil, keypair: test_keypair)
+        let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
+        let manager = NostrNetworkManager.UserRelayListManager(
+            delegate: delegate, pool: pool, reader: reader
+        )
+
+        // set() populates cache with [A] only
+        try await manager.set(userRelayList: NIP65.RelayList(relays: [relayA]))
+        let cachedList = await manager.getUserCurrentRelayList()
+        XCTAssertEqual(Set(cachedList!.relays.keys), [relayA], "Cache should serve [A]")
+
+        // load() must clear the cache so ndb is queried again
+        await manager.load()
+
+        // After load(), the list should come from ndb.
+        // ndb now has the [A] event from set() (committed by now) or the original [A,B].
+        // Either way, the cache is cleared — the manager reads from ndb, not stale cache.
+        let afterLoad = await manager.getUserCurrentRelayList()
+        XCTAssertNotNil(afterLoad, "Must return a relay list from ndb after load()")
+    }
+
+    /// Regression: ndb query must find the relay list without a stored hex ID.
+    ///
+    /// Before the fix, a fresh session with no `latestRelayListEventIdHex` in UserDefaults
+    /// meant `getLatestNIP65RelayListEvent()` returned nil, even though ndb had the event.
+    func testNdbQueryFindsRelayListWithoutStoredHex() async throws {
+        let ndb = Ndb.test
+        defer { ndb.close() }
+
+        let relayA = RelayURL("wss://relay-a.example.com")!
+        let relayB = RelayURL("wss://relay-b.example.com")!
+
+        // Seed ndb — simulates a relay list that arrived via sync, not user action
+        let list = NIP65.RelayList(relays: [relayA, relayB])
+        let event = list.toNostrEvent(keypair: test_keypair_full)!
+        XCTAssertTrue(ndb.processEvent("[\"EVENT\",\"subid\",\(encode_json(event)!)]"))
+        try await Task.sleep(for: .milliseconds(100))
+
+        // No hex stored anywhere — fresh delegate with no latestRelayListEventIdHex
+        let delegate = MockNetworkDelegate(
+            ndb: ndb,
+            keypair: test_keypair,
+            bootstrapRelays: [RelayURL("wss://bootstrap.example.com")!]
+        )
+        let pool = RelayPool(ndb: nil, keypair: test_keypair)
+        let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
+        let manager = NostrNetworkManager.UserRelayListManager(
+            delegate: delegate, pool: pool, reader: reader
+        )
+
+        let currentList = await manager.getUserCurrentRelayList()
+        XCTAssertNotNil(currentList, "ndb query must find relay list without stored hex")
+        let urls = Set(currentList!.relays.keys)
+        XCTAssertEqual(urls, [relayA, relayB])
+        XCTAssertFalse(
+            urls.contains(RelayURL("wss://bootstrap.example.com")!),
+            "Must not fall back to bootstrap when ndb has the event"
+        )
+    }
 }
 
 // MARK: - Test doubles
@@ -199,7 +397,6 @@ class NostrNetworkManagerTests: XCTestCase {
 private final class MockNetworkDelegate: NostrNetworkManager.Delegate {
     var ndb: Ndb
     var keypair: Keypair
-    var latestRelayListEventIdHex: String?
     var latestContactListEvent: NostrEvent?
     var bootstrapRelays: [RelayURL]
     var developerMode: Bool = false

damusTests/SubscriptionManagerNegentropyTests.swift

@@ -520,7 +520,6 @@ final class SubscriptionManagerNegentropyTests: XCTestCase {
 private final class TestNetworkDelegate: NostrNetworkManager.Delegate {
     var ndb: Ndb
     var keypair: Keypair
-    var latestRelayListEventIdHex: String?
     var latestContactListEvent: NostrEvent?
     var bootstrapRelays: [RelayURL]
     var developerMode: Bool = false