diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index eda16d4b..77b7fc78 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -213,9 +213,16 @@ final class AppRuntime: ObservableObject { /// otherwise fall straight through `startPrivateChat` into an empty phantom /// DM. /// - /// Restorable iff the peer is NOT blocked and we hold durable evidence the - /// conversation is real: either side has favorited the other, or we have a - /// stored cryptographic identity for them. + /// Restorable iff the peer is NOT blocked and we hold durable evidence + /// *locally* that the conversation is addressable: we favorited them, or we + /// have a stored cryptographic identity for them. + /// + /// `theyFavoritedUs` is deliberately NOT a term. It is remote state, and on + /// its own it proves nothing about our ability to address the peer — a peer + /// who favorited us, whom we never favorited and hold no identity for, is + /// exactly the unaddressable phantom this predicate exists to refuse. It + /// would also override a deliberate local unfavorite on the strength of the + /// other side's opinion. /// /// This tracks `ChatPeerIdentityCoordinator.startPrivateChat`, which #1415 /// relaxed — it no longer requires a mutual favorite, on the grounds that @@ -237,28 +244,31 @@ final class AppRuntime: ObservableObject { /// defers loading until protected data is available and session state is /// in-memory, so both read empty at launch regardless of the truth. /// - /// Geohash/Nostr DMs stay excluded: a `nostr_` id's full Nostr key is - /// rebuilt only from inbound ephemeral events, so at launch it cannot - /// resolve, and `startPrivateChat` skips the handshake for geoDMs — a - /// phantom would open with no error at all. Their one durable anchor is a - /// favorite record, so for them the favorite terms decide and the identity - /// term is refused outright. + /// Geohash/Nostr ids are screened first, before any other term, so nothing + /// can bypass the check by matching earlier. A geoChat id is not a direct + /// chat at all. A geoDM's full Nostr key is rebuilt only from inbound + /// ephemeral events, so at launch it cannot resolve, and `startPrivateChat` + /// skips the handshake for geoDMs — a phantom would open with no error at + /// all. Its one durable anchor is OUR OWN favorite record, which carries + /// `peerNostrPublicKey`, so a geoDM restores on that and nothing else. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, - theyFavoritedUs: (PeerID) -> Bool, hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { - // Blocked stays a veto, never one term among several. + // Blocked is a veto, never one term among several. guard !isPeerBlocked(peerID) else { return false } - if isPeerFavorited(peerID) || theyFavoritedUs(peerID) { return true } - // Explicit rather than incidental: a `nostr_` id does satisfy - // `isShort` (the prefix is not part of the length check), and today it - // misses only because the identity lookup re-attaches that prefix and - // no hex fingerprint starts with it. That is luck, not a rule. - guard !peerID.isGeoDM, !peerID.isGeoChat else { return false } - return hasStoredCryptographicIdentity(peerID) + // A geohash channel is not a direct chat. + guard !peerID.isGeoChat else { return false } + // Screened before the identity term rather than after, so no earlier + // match can skip it. The identity lookup does not reject these on its + // own merits either: a `nostr_` id satisfies `isShort` (the prefix is + // not part of the length check), and today it misses only because the + // lookup re-attaches that prefix and no hex fingerprint starts with it. + // Luck, not a rule. + if peerID.isGeoDM { return isPeerFavorited(peerID) } + return isPeerFavorited(peerID) || hasStoredCryptographicIdentity(peerID) } /// Production wiring of `isDirectChatRestorable`, extracted so the real @@ -291,9 +301,6 @@ final class AppRuntime: ObservableObject { isPeerFavorited: { favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false }, - theyFavoritedUs: { - favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false - }, hasStoredCryptographicIdentity: hasStoredCryptographicIdentity, isPeerBlocked: isPeerBlocked ) diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index ab27e6bd..4c4df1ef 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -209,7 +209,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -225,7 +224,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -244,26 +242,42 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_peerWhoFavoritedUsIsRestorable() { - // The other direction of the same relaxation. - XCTAssertTrue( + func test_peerWhoOnlyFavoritedUsIsNotRestorable() { + // Remote state alone is not evidence we can address them. They favorited + // us, we never favorited them, and we hold no stored identity — the + // unaddressable phantom this predicate exists to refuse. Their opinion + // of us says nothing about whether we have a key for them. + XCTAssertFalse( AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } + func test_geoChatIdIsNeverRestorableAsADirectChat() { + // A geohash channel id is not a direct chat at all, and the screen runs + // before the favorite term so nothing can match past it. + let geoChatID = PeerID(str: "nostr:01234567") + XCTAssertTrue(geoChatID.isGeoChat) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + geoChatID, + isPeerFavorited: { _ in true }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + func test_establishedNonFavoritePeerIsRestorable() { // No favorite either way, but we hold a stored cryptographic identity — // durable, on disk, and enough for the router to deliver via courier or @@ -272,7 +286,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in false } ) @@ -286,7 +299,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in true } ) @@ -307,7 +319,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in true }, isPeerBlocked: { _ in false } ) @@ -322,7 +333,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -336,7 +346,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, - theyFavoritedUs: { _ in true }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) @@ -354,7 +363,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( geoDMPeer, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -374,7 +382,6 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( $0, isPeerFavorited: { _ in false }, - theyFavoritedUs: { _ in false }, hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) @@ -467,25 +474,20 @@ final class ConversationStoreLastActiveTests: XCTestCase { ) } - func test_production_unfavoritedPeerWhoStillFavoritesUsIsRestorable() { - // INVERTED by #1415, and the inversion worth arguing about. - // + func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() { // removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs: - // true) when the peer still favorites us. This previously refused to - // restore, so that a DM to a peer we deliberately unfavorited would not - // reopen on restart — the "is a persisted favorite" contract, added for - // an earlier Codex review. + // true) when the peer still favorites us. The resolver must key on + // isFavorite, not mere record existence — otherwise a DM to a peer we + // deliberately unfavorited reopens on restart. Regression for an + // earlier Codex review, and it SURVIVES the #1415 relaxation: the + // relaxed predicate admits our own favorite or a stored identity, and + // `theyFavoritedUs` is deliberately not a term, so the unfavorite still + // stands on its own. // - // #1415 dissolved that contract: chat entry no longer consults - // favorites at all, so this conversation is openable by hand and - // sendable through the router. Refusing to restore it would single out - // launch for a stricter rule than every other way in. - // - // Note the protection is largely moot in practice regardless: any peer - // we have actually corresponded with has a stored cryptographic - // identity, which admits them through the identity term whatever the - // favorite record says. Unfavoriting is not blocking — blocking still - // refuses, and that is the control for "do not reopen this". + // With a stored identity this peer would restore — see + // test_establishedNonFavoritePeerIsRestorable. That is the intended + // line: the durable evidence is then local capability rather than the + // other side's opinion of us. let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -497,23 +499,12 @@ final class ConversationStoreLastActiveTests: XCTestCase { XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertTrue( - AppRuntime.isDirectChatRestorable( - fullHexPeer, - favorites: favorites, - hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in false } - ) - ) - - // Blocking is the control that still refuses, with everything else - // about this peer unchanged. XCTAssertFalse( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, hasStoredCryptographicIdentity: { _ in false }, - isPeerBlocked: { _ in true } + isPeerBlocked: { _ in false } ) ) }