mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-29 07:27:16 +00:00
fix: require mutual-favorite + unblocked for launch DM restore (#1064)
Drops the `isGeoDM` short-circuit in `isDirectChatRestorable`: a restored `nostr_` geohash-DM id cannot rebuild its full Nostr key at launch (that map is repopulated only from inbound ephemeral events), so it opened an unsendable phantom DM. The predicate now mirrors the open-path gate `ChatPeerIdentityCoordinator.startPrivateChat` — restorable iff the peer is a MUTUAL favorite (we favorite them AND they favorite us) and NOT blocked. The gate's `isConnected` relaxation term is always false at launch, so it drops out of the launch-effective predicate. Threads durable `theyFavoritedUs`/`isPeerBlocked` lookups into the pure predicate the same way `isPeerFavorited` was injected; the production overload wires them via `FavoritesPersistenceService` and the gate's `unifiedIsBlocked` source. Also corrects the doc comment that falsely claimed geohash/Nostr DMs carry a self-resolving identity. Updates the predicate tests: geoDM-without-favorite is now NOT restorable, the "favorite" cases require mutuality, and adds one-way-favorite / blocked-mutual negative cases at both the pure and production-wired layers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7db98c4d40
commit
2ed52ebf4f
@ -136,7 +136,13 @@ final class AppRuntime: ObservableObject {
|
||||
/// `activeChannel`), so there is no race.
|
||||
private func restoreLastActiveConversationOnLaunch() {
|
||||
let presentation = conversations.restoreLastActiveConversation(
|
||||
isPeerResolvable: { Self.isDirectChatRestorable($0, favorites: .shared) }
|
||||
isPeerResolvable: {
|
||||
Self.isDirectChatRestorable(
|
||||
$0,
|
||||
favorites: .shared,
|
||||
isPeerBlocked: { chatViewModel.isPeerBlocked($0) }
|
||||
)
|
||||
}
|
||||
)
|
||||
var didOpenDirectChat = false
|
||||
if case .restoredDirectChat(let peerID) = presentation {
|
||||
@ -156,37 +162,55 @@ final class AppRuntime: ObservableObject {
|
||||
}
|
||||
|
||||
/// Whether a persisted last-active DM peer is genuinely restorable at
|
||||
/// launch — validated against *durable* conversation state, never live
|
||||
/// launch — validated against *durable* relationship state, never live
|
||||
/// presence (mesh discovery is async, so no peer is connected yet). A
|
||||
/// syntactically valid `PeerID` is NOT sufficient: an unknown peer would
|
||||
/// otherwise fall straight through `startPrivateChat` into an empty phantom
|
||||
/// DM. Restorable iff the peerID is a geohash/Nostr DM (its stable Nostr
|
||||
/// identity is embedded in the id itself) or the peer is a persisted
|
||||
/// favorite (keychain-backed, stored by stable Noise public key across
|
||||
/// launches). Presence-independent and pure, so it is unit-testable.
|
||||
/// DM. Mirrors the open-path gate
|
||||
/// (`ChatPeerIdentityCoordinator.startPrivateChat`): restorable iff the peer
|
||||
/// is a MUTUAL favorite (we favorite them AND they favorite us) and NOT
|
||||
/// blocked. The gate's third relaxation term, `isConnected`, is always false
|
||||
/// at launch, so it drops out of the launch-effective predicate. A geohash/
|
||||
/// Nostr DM is *not* special-cased: its full Nostr key is rebuilt only from
|
||||
/// inbound ephemeral events, so at launch a restored `nostr_` id cannot
|
||||
/// resolve and would open an unsendable phantom unless it is also a mutual
|
||||
/// favorite. Favorites are keychain-backed and keyed by stable Noise public
|
||||
/// key, so this is presence-independent and pure, hence unit-testable.
|
||||
static func isDirectChatRestorable(
|
||||
_ peerID: PeerID,
|
||||
isPeerFavorited: (PeerID) -> Bool
|
||||
isPeerFavorited: (PeerID) -> Bool,
|
||||
theyFavoritedUs: (PeerID) -> Bool,
|
||||
isPeerBlocked: (PeerID) -> Bool
|
||||
) -> Bool {
|
||||
if peerID.isGeoDM { return true }
|
||||
return isPeerFavorited(peerID)
|
||||
guard !isPeerBlocked(peerID) else { return false }
|
||||
return isPeerFavorited(peerID) && theyFavoritedUs(peerID)
|
||||
}
|
||||
|
||||
/// Production wiring of `isDirectChatRestorable`, extracted so the real
|
||||
/// favorites lookup (not just a stub predicate) is unit-testable via an
|
||||
/// injected in-memory-keychain-backed `FavoritesPersistenceService`.
|
||||
/// `migrateSelectedConversationIfNeeded` can persist the last-active peer in
|
||||
/// full 64-hex Noise-key form, but the favorites store is keyed by the
|
||||
/// short, Noise-key-derived id — so normalize with `toShort()` (a no-op on
|
||||
/// an already-short id) before the lookup, or favorited DMs silently fail to
|
||||
/// restore.
|
||||
/// favorites/block lookups (not just stub predicates) are unit-testable via
|
||||
/// an injected in-memory-keychain-backed `FavoritesPersistenceService` and a
|
||||
/// block closure. `migrateSelectedConversationIfNeeded` can persist the
|
||||
/// last-active peer in full 64-hex Noise-key form, but the favorites store is
|
||||
/// keyed by the short, Noise-key-derived id — so normalize with `toShort()`
|
||||
/// (a no-op on an already-short id) before the lookup, or favorited DMs
|
||||
/// silently fail to restore. The block lookup mirrors the open-path gate's
|
||||
/// `unifiedIsBlocked` (fingerprint-resolved, so it works for offline
|
||||
/// favorites).
|
||||
static func isDirectChatRestorable(
|
||||
_ peerID: PeerID,
|
||||
favorites: FavoritesPersistenceService
|
||||
favorites: FavoritesPersistenceService,
|
||||
isPeerBlocked: (PeerID) -> Bool
|
||||
) -> Bool {
|
||||
isDirectChatRestorable(peerID, isPeerFavorited: {
|
||||
favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false
|
||||
})
|
||||
isDirectChatRestorable(
|
||||
peerID,
|
||||
isPeerFavorited: {
|
||||
favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false
|
||||
},
|
||||
theyFavoritedUs: {
|
||||
favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false
|
||||
},
|
||||
isPeerBlocked: isPeerBlocked
|
||||
)
|
||||
}
|
||||
|
||||
/// Pure launch-effect decision, extracted so the fallback is unit-testable
|
||||
|
||||
@ -115,25 +115,72 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
// (The syntax-only `{ $0.isValid }` resolver missed exactly this.)
|
||||
XCTAssertTrue(peerID.isValid)
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in false })
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
peerID,
|
||||
isPeerFavorited: { _ in false },
|
||||
theyFavoritedUs: { _ in false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_favoritePeerIsRestorable() {
|
||||
// A persisted favorite is stored by stable Noise public key and survives
|
||||
// restart — the one durable, presence-independent mesh relationship.
|
||||
func test_mutualFavoritePeerIsRestorable() {
|
||||
// A persisted MUTUAL favorite is stored by stable Noise public key and
|
||||
// survives restart — the one durable, presence-independent mesh
|
||||
// relationship. Mirrors the open-path gate, which only lets an offline
|
||||
// favorite through when the favorite is mutual.
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(peerID, isPeerFavorited: { _ in true })
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
peerID,
|
||||
isPeerFavorited: { _ in true },
|
||||
theyFavoritedUs: { _ in true },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_geoDMPeerIsRestorable_withoutFavorite() {
|
||||
// Geohash/Nostr DM ids embed a stable Nostr identity in the id itself,
|
||||
// so they are restorable even though they are not favorites.
|
||||
func test_oneWayFavoritePeerIsNotRestorable() {
|
||||
// We favorite them but they do NOT favorite us: the open-path gate
|
||||
// rejects this at launch (isConnected is false), so auto-restoring it
|
||||
// would inject a "requires favorite" system message into the public
|
||||
// timeline. The predicate must refuse it up front.
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
peerID,
|
||||
isPeerFavorited: { _ in true },
|
||||
theyFavoritedUs: { _ in false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_blockedMutualFavoriteIsNotRestorable() {
|
||||
// A blocked peer is never restorable, even if the favorite is mutual —
|
||||
// mirrors the gate's first (block) reject.
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
peerID,
|
||||
isPeerFavorited: { _ in true },
|
||||
theyFavoritedUs: { _ in true },
|
||||
isPeerBlocked: { _ in true }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_geoDMPeerWithoutMutualFavoriteIsNotRestorable() {
|
||||
// #1064 phantom-DM fix: a geohash/Nostr DM id is NO LONGER special-cased
|
||||
// as restorable. Its full Nostr key is rebuilt only from inbound
|
||||
// ephemeral events, so at launch a restored `nostr_` id cannot resolve
|
||||
// and would open an unsendable phantom. Only a mutual favorite restores.
|
||||
let geoDMPeer = PeerID(str: "nostr_0123456789abcdef")
|
||||
XCTAssertTrue(geoDMPeer.isGeoDM)
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(geoDMPeer, isPeerFavorited: { _ in false })
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
geoDMPeer,
|
||||
isPeerFavorited: { _ in false },
|
||||
theyFavoritedUs: { _ in false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@ -147,7 +194,12 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
|
||||
let restored = ConversationStore(storage: storage).restoreLastActiveConversation(
|
||||
isPeerResolvable: {
|
||||
AppRuntime.isDirectChatRestorable($0, isPeerFavorited: { _ in false })
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
$0,
|
||||
isPeerFavorited: { _ in false },
|
||||
theyFavoritedUs: { _ in false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
@ -156,19 +208,64 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
|
||||
// MARK: - Production wiring (real FavoritesPersistenceService)
|
||||
|
||||
func test_production_fullHexFavoritePeerIsRestorable() {
|
||||
func test_production_fullHexMutualFavoritePeerIsRestorable() {
|
||||
// migrateSelectedConversationIfNeeded persists the peer in FULL 64-hex
|
||||
// Noise-key form; the favorites store is keyed by the short derived id.
|
||||
// The production resolver must normalize (`toShort()`) so a favorited DM
|
||||
// still restores. Regression for fix-round-3 finding 1.
|
||||
// still restores. Regression for fix-round-3 finding 1. The favorite must
|
||||
// be MUTUAL to mirror the open-path gate.
|
||||
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
|
||||
let noiseKey = Data((0..<32).map(UInt8.init))
|
||||
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
|
||||
favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true)
|
||||
|
||||
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
|
||||
XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
fullHexPeer,
|
||||
favorites: favorites,
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_production_oneWayFavoriteIsNotRestorable() {
|
||||
// We favorite them but they never favorited us: not mutual, so the
|
||||
// open-path gate would reject it at launch. The production resolver must
|
||||
// refuse it rather than auto-open a gated DM.
|
||||
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
|
||||
let noiseKey = Data((0..<32).map(UInt8.init))
|
||||
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
|
||||
|
||||
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
|
||||
XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites)
|
||||
XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite)
|
||||
XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs)
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
fullHexPeer,
|
||||
favorites: favorites,
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_production_blockedMutualFavoriteIsNotRestorable() {
|
||||
// A mutual favorite we have since blocked must NOT restore — mirrors the
|
||||
// gate's block reject. Block state is injected (it lives in the identity
|
||||
// manager, not the favorites store).
|
||||
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
|
||||
let noiseKey = Data((0..<32).map(UInt8.init))
|
||||
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
|
||||
favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true)
|
||||
|
||||
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
fullHexPeer,
|
||||
favorites: favorites,
|
||||
isPeerBlocked: { _ in true }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@ -179,7 +276,11 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
|
||||
XCTAssertTrue(peerID.isValid)
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(peerID, favorites: favorites)
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
peerID,
|
||||
favorites: favorites,
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@ -200,7 +301,11 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort()))
|
||||
XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite)
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(fullHexPeer, favorites: favorites)
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
fullHexPeer,
|
||||
favorites: favorites,
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user