mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-15 07:06:11 +00:00
Let private groups restore, and stop claiming geoDM restore works
Two gaps a review found in the launch-restore predicate. A private group could never restore. A group id is `group_` plus 32 hex, so both peer lookups guard on `isShort` and return empty without ever consulting the group — leaving `isDirectChatRestorable` to refuse every group, silently, every time. That is backwards: `startPrivateChat` gates group re-entry on nothing at all, because a group is local state rather than a claim about reaching a peer, so there is no phantom to guard against. Groups are now admitted on their own branch, ahead of the peer terms and behind the block veto. One test pins the admission and a second pins that neither peer term is consulted, so a group cannot be let in by an unrelated term happening to match. The geoDM reasoning was also describing behaviour the code does not have. It claimed a geoDM restores on its own favorite record; in fact `FavoritesPersistenceService` is keyed by Noise public key alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding `PeerID(publicKey:)`, which carries no prefix and so can never equal a `nostr_`-prefixed id. A geoDM therefore never restores today. The branch is the right shape for the day a Nostr-keyed lookup exists; until then it resolves to false. Documented rather than fixed, because adding that lookup means new favorites plumbing and this change does not touch that service. Mutation-proven: removing the group branch fails both group tests and nothing else, while the blocked-group test keeps passing on its own veto. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c6b15a7c58
commit
53b74bd1e9
@ -244,21 +244,45 @@ 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 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.
|
||||
/// A private group is a virtual conversation, so none of the peer terms
|
||||
/// apply to it and none of them would pass: a group id is `group_` plus 32
|
||||
/// hex, and both lookups guard on `isShort` (a 16-hex bare), so they return
|
||||
/// empty for every group without ever consulting the group. Left to the
|
||||
/// peer terms a group would therefore never restore — silently, every
|
||||
/// time. `startPrivateChat` gates group re-entry on nothing at all ("no
|
||||
/// peer identity, favorites, handshake … just select the chat"), and there
|
||||
/// is no phantom to guard against: the conversation is local state, not a
|
||||
/// claim about reachability. So groups are admitted outright.
|
||||
///
|
||||
/// Geohash/Nostr ids are screened before the peer terms, 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
|
||||
/// only conceivable durable anchor is our own favorite record.
|
||||
///
|
||||
/// In practice that anchor does not exist yet, so **a geoDM never restores
|
||||
/// today**. `FavoritesPersistenceService` is keyed by Noise public key
|
||||
/// alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding
|
||||
/// `PeerID(publicKey:)` — which carries no prefix — so it can never equal a
|
||||
/// `nostr_`-prefixed id no matter what is favorited. The geoDM branch below
|
||||
/// is the correct shape for the day a Nostr-keyed lookup exists; until
|
||||
/// then it resolves to `false` and the fallback to the conversation list is
|
||||
/// the whole behaviour. Documented rather than fixed here: adding that
|
||||
/// lookup means new favorites plumbing, which is not this change.
|
||||
static func isDirectChatRestorable(
|
||||
_ peerID: PeerID,
|
||||
isPeerFavorited: (PeerID) -> Bool,
|
||||
hasStoredCryptographicIdentity: (PeerID) -> Bool,
|
||||
isPeerBlocked: (PeerID) -> Bool
|
||||
) -> Bool {
|
||||
// Blocked is a veto, never one term among several.
|
||||
// Blocked is a veto, never one term among several. Kept ahead of the
|
||||
// group branch so the veto is unconditional; a group id is never
|
||||
// blocked in practice, so the order costs nothing.
|
||||
guard !isPeerBlocked(peerID) else { return false }
|
||||
// A private group is local state, not a claim about reaching a peer.
|
||||
// The peer terms below cannot represent it and would refuse it.
|
||||
if peerID.isGroup { return true }
|
||||
// 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
|
||||
|
||||
@ -339,6 +339,55 @@ final class ConversationStoreLastActiveTests: XCTestCase {
|
||||
)
|
||||
}
|
||||
|
||||
func test_privateGroupIsRestorable() {
|
||||
// A group id is `group_` plus 32 hex, so both peer lookups guard on
|
||||
// `isShort` and return empty for it. Left to those terms a group would
|
||||
// never restore, silently, every time — even though `startPrivateChat`
|
||||
// gates group re-entry on nothing at all.
|
||||
let group = PeerID(str: "group_" + String(repeating: "ab", count: 16))
|
||||
XCTAssertTrue(group.isGroup, "test fixture is not a group id")
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
group,
|
||||
isPeerFavorited: { _ in false },
|
||||
hasStoredCryptographicIdentity: { _ in false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_privateGroupNeverConsultsThePeerTerms() {
|
||||
// Admitting groups by accident — because some peer term happened to
|
||||
// match — would be a different bug wearing the same green check. The
|
||||
// group must be admitted on its own branch, before either lookup runs.
|
||||
let group = PeerID(str: "group_" + String(repeating: "cd", count: 16))
|
||||
var favoriteLookups = 0
|
||||
var identityLookups = 0
|
||||
XCTAssertTrue(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
group,
|
||||
isPeerFavorited: { _ in favoriteLookups += 1; return false },
|
||||
hasStoredCryptographicIdentity: { _ in identityLookups += 1; return false },
|
||||
isPeerBlocked: { _ in false }
|
||||
)
|
||||
)
|
||||
XCTAssertEqual(favoriteLookups, 0, "group restore consulted the favorites term")
|
||||
XCTAssertEqual(identityLookups, 0, "group restore consulted the identity term")
|
||||
}
|
||||
|
||||
func test_blockedGroupIsNotRestorable() {
|
||||
// Block stays an unconditional veto, ahead of the group branch.
|
||||
let group = PeerID(str: "group_" + String(repeating: "ef", count: 16))
|
||||
XCTAssertFalse(
|
||||
AppRuntime.isDirectChatRestorable(
|
||||
group,
|
||||
isPeerFavorited: { _ in false },
|
||||
hasStoredCryptographicIdentity: { _ in false },
|
||||
isPeerBlocked: { _ in true }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func test_blockedMutualFavoriteIsNotRestorable() {
|
||||
// A blocked peer is never restorable, even if the favorite is mutual —
|
||||
// mirrors the gate's first (block) reject.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user