mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-08 06:56:10 +00:00
Drop theyFavoritedUs as a restore term, and screen geohash ids first
Both cross-model reviewers landed on the same two defects in the previous commit, and they agreed with the doubt I had flagged for the PR. `theyFavoritedUs` was admitting the exact phantom this predicate exists to refuse. A peer who favorited us, whom we never favorited and hold no stored identity for, has no durable local evidence we can address them at all — their opinion of us is not a key. It is gone as a term, so restoring now requires local evidence: our own favorite, or a stored cryptographic identity. That also settles the behaviour change I was going to ask Jack to arbitrate. The earlier review's guard — a peer we deliberately unfavorited must not reopen on restart — SURVIVES intact rather than being reversed, because it never depended on #1415's premise. Its test goes back to its original name and expectation. With a stored identity such a peer does restore, and that is the intended line: the evidence is then our own capability, not the other side's opinion. Unfavoriting is not blocking; blocking still refuses. The geohash screen also ran too late. It sat after the favorite term, so a favorited geoChat id — a public channel identifier, not a direct chat at all — was reported restorable as a DM. Both the geoChat rejection and the geoDM restriction now run before any other term, so nothing can match past them. Mutation-verified, each guard by the test that fails without it: moving the geoChat screen back after the favorite term fails the geoChat case, re-adding an unconditional term fails five phantom-guard tests, and letting geoDMs reach the identity term fails the geoDM case. Two reviewer points I did not act on. Codex reads the identity lookup's `hasPrefix` as a loose prefix match that a collision could satisfy; it is exact by construction, since a short peer ID *is* the first 16 hex of the fingerprint, and every peer-ID lookup in the app shares that property. Codex also proposed making a retained `isFavorite == false` an explicit veto; that would refuse to restore a conversation the user was in when they quit, and the last-active pointer is the more recent signal of intent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang
parent
baacc6a85b
commit
a3ee2fbb46
@ -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
|
||||
)
|
||||
|
||||
@ -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 }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user