Reject unsendable Nostr DMs visibly

bitchat/Nostr/NostrProtocol.swift

@@ -52,6 +52,10 @@ struct NostrProtocol {
     static let maximumPrivateEnvelopeCiphertextBytes = 64 * 1024
 
     /// Bound the inner authenticated message JSON before allocation/parsing.
+    /// This is intentionally an envelope limit, not the generic composer
+    /// limit. Raising it alone would not make larger private-message packets
+    /// compatible with released readers; callers must surface a rejected
+    /// packet or envelope as a failed send.
     static let maximumPrivateEnvelopePlaintextBytes = 32 * 1024
 
     /// The outer authenticated seal JSON contains a Base64-encoded encrypted

bitchat/Services/NostrTransport.swift

@@ -323,6 +323,11 @@ final class NostrTransport: Transport, @unchecked Sendable {
             SecureLogger.debug("NostrTransport: preparing PM to \(recipientNpub.prefix(16))… id=\(messageID.prefix(8))…", category: .session)
             guard let embedded = NostrEmbeddedBitChat.encodePMForNostr(content: content, messageID: messageID, recipientPeerID: peerID, senderPeerID: senderPeerID) else {
                 SecureLogger.error("NostrTransport: failed to embed PM packet", category: .session)
+                handlePrivateEnvelopeFailure(
+                    events: [],
+                    registerPending: false,
+                    policy: .userMessage(messageID: messageID)
+                )
                 return
             }
             sendPrivateEnvelope(
@@ -387,22 +392,48 @@ extension NostrTransport {
     }
 
     // MARK: Geohash DMs (per-geohash identity)
-    func sendPrivateMessageGeohash(content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) {
+    /// Returns true only when the complete migration pair entered the relay
-        Task { @MainActor in
+    /// delivery queue. GeoDM callers use this synchronous admission result
-            guard !recipientHex.isEmpty else { return }
+    /// before showing "sent"; deterministic packet/envelope failures must not
-            SecureLogger.debug("GeoDM: send PM mid=\(messageID.prefix(8))…", category: .session)
+    /// be hidden behind an unobserved MainActor task.
-            guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient(content: content, messageID: messageID, senderPeerID: senderPeerID) else {
+    @MainActor
-                SecureLogger.error("NostrTransport: failed to embed geohash PM packet", category: .session)
+    @discardableResult
-                return
+    func sendPrivateMessageGeohash(
-            }
+        content: String,
-            sendPrivateEnvelope(
+        toRecipientHex recipientHex: String,
-                content: embedded,
+        from identity: NostrIdentity,
-                recipientHex: recipientHex,
+        messageID: String
-                senderIdentity: identity,
+    ) -> Bool {
-                registerPending: true,
+        let failurePolicy = PrivateEnvelopeFailurePolicy.userMessage(messageID: messageID)
-                failurePolicy: .userMessage(messageID: messageID)
+        guard !recipientHex.isEmpty else {
+            handlePrivateEnvelopeFailure(
+                events: [],
+                registerPending: false,
+                policy: failurePolicy
             )
+            return false
         }
+        SecureLogger.debug("GeoDM: send PM mid=\(messageID.prefix(8))…", category: .session)
+        guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient(
+            content: content,
+            messageID: messageID,
+            senderPeerID: senderPeerID
+        ) else {
+            SecureLogger.error("NostrTransport: failed to embed geohash PM packet", category: .session)
+            handlePrivateEnvelopeFailure(
+                events: [],
+                registerPending: false,
+                policy: failurePolicy
+            )
+            return false
+        }
+        return sendPrivateEnvelope(
+            content: embedded,
+            recipientHex: recipientHex,
+            senderIdentity: identity,
+            registerPending: true,
+            failurePolicy: failurePolicy
+        )
     }
 }
 
@@ -424,20 +455,35 @@ extension NostrTransport {
 
     /// Creates and sends a BitChat private-envelope event over Nostr.
     @MainActor
+    @discardableResult
     private func sendPrivateEnvelope(
         content: String,
         recipientHex: String,
         senderIdentity: NostrIdentity,
         registerPending: Bool = false,
         failurePolicy: PrivateEnvelopeFailurePolicy
-    ) {
+    ) -> Bool {
-        guard let events = try? NostrProtocol.createPrivateEnvelopePublicationBatch(
+        let events: [NostrEvent]
-            content: content,
+        do {
-            recipientPubkey: recipientHex,
+            events = try NostrProtocol.createPrivateEnvelopePublicationBatch(
-            senderIdentity: senderIdentity
+                content: content,
-        ) else {
+                recipientPubkey: recipientHex,
-            SecureLogger.error("NostrTransport: failed to build Nostr private-envelope batch", category: .session)
+                senderIdentity: senderIdentity
-            return
+            )
+        } catch {
+            SecureLogger.error(
+                "NostrTransport: failed to build Nostr private-envelope batch: \(error)",
+                category: .session
+            )
+            // Construction failures are deterministic. User-authored messages
+            // must become visibly failed; control payloads have no valid event
+            // pair to retain and retry.
+            handlePrivateEnvelopeFailure(
+                events: [],
+                registerPending: false,
+                policy: failurePolicy
+            )
+            return false
         }
         let accepted = dependencies.sendPrivateEnvelopeBatch(events) { [self] in
             handlePrivateEnvelopeFailure(
@@ -456,9 +502,10 @@ extension NostrTransport {
                 registerPending: registerPending,
                 policy: failurePolicy
             )
-            return
+            return false
         }
         registerPendingPrivateEnvelopesIfNeeded(events, registerPending: registerPending)
+        return true
     }
 
     @MainActor
@@ -477,6 +524,10 @@ extension NostrTransport {
                 ))
             ))
         case .retry(let retryKey):
+            // A deterministic packet/envelope construction failure has no
+            // events to retry. Only relay admission/delivery failures reach
+            // this branch with the complete atomic pair.
+            guard !events.isEmpty else { return }
             envelopeRetryQueue.enqueue(
                 key: retryKey,
                 events: events,

bitchat/ViewModels/ChatPrivateConversationCoordinator.swift

@@ -86,7 +86,13 @@ protocol ChatPrivateConversationContext: AnyObject {
     @discardableResult
     func routeReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) -> Bool
     func sendMeshReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID)
-    func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String)
+    @discardableResult
+    func sendGeohashPrivateMessage(
+        _ content: String,
+        toRecipientHex recipientHex: String,
+        from identity: NostrIdentity,
+        messageID: String
+    ) -> Bool
     func sendGeohashDeliveryAck(for messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity)
     func sendGeohashReadReceipt(_ messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity)
 
@@ -174,7 +180,13 @@ extension ChatViewModel: ChatPrivateConversationContext {
         meshService.sendReadReceipt(receipt, to: peerID)
     }
 
-    func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) {
+    @discardableResult
+    func sendGeohashPrivateMessage(
+        _ content: String,
+        toRecipientHex recipientHex: String,
+        from identity: NostrIdentity,
+        messageID: String
+    ) -> Bool {
         makeGeohashNostrTransport().sendPrivateMessageGeohash(
             content: content,
             toRecipientHex: recipientHex,
@@ -399,13 +411,21 @@ final class ChatPrivateConversationCoordinator {
                 "GeoDM: local send mid=\(messageID.prefix(8))… to=\(recipientHex.prefix(8))… conv=\(peerID)",
                 category: .session
             )
-            context.sendGeohashPrivateMessage(
+            let accepted = context.sendGeohashPrivateMessage(
                 content,
                 toRecipientHex: recipientHex,
                 from: identity,
                 messageID: messageID
             )
-            context.setPrivateDeliveryStatus(.sent, forMessageID: messageID, peerID: peerID)
+            let status: DeliveryStatus = accepted
+                ? .sent
+                : .failed(
+                    reason: String(
+                        localized: "content.delivery.reason.not_delivered",
+                        comment: "Failure reason shown when a private message could not enter the relay delivery queue"
+                    )
+                )
+            context.setPrivateDeliveryStatus(status, forMessageID: messageID, peerID: peerID)
         } catch {
             context.setPrivateDeliveryStatus(
                 .failed(

bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift

@@ -176,6 +176,7 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC
     private(set) var geoPrivateMessages: [(content: String, recipientHex: String, messageID: String)] = []
     private(set) var geoDeliveryAcks: [(messageID: String, recipientHex: String)] = []
     private(set) var geoReadReceipts: [(messageID: String, recipientHex: String)] = []
+    var geohashPrivateMessageAccepted = true
 
     func routePrivateMessage(_ content: String, to peerID: PeerID, recipientNickname: String, messageID: String) {
         routedPrivateMessages.append((content, peerID, messageID))
@@ -191,8 +192,14 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC
         meshReadReceipts.append((receipt.originalMessageID, peerID))
     }
 
-    func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) {
+    func sendGeohashPrivateMessage(
+        _ content: String,
+        toRecipientHex recipientHex: String,
+        from identity: NostrIdentity,
+        messageID: String
+    ) -> Bool {
         geoPrivateMessages.append((content, recipientHex, messageID))
+        return geohashPrivateMessageAccepted
     }
 
     func sendGeohashDeliveryAck(for messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) {
@@ -279,6 +286,11 @@ private func isRead(_ status: DeliveryStatus?, by expected: String) -> Bool {
     return false
 }
 
+private func isFailed(_ status: DeliveryStatus?) -> Bool {
+    if case .failed = status { return true }
+    return false
+}
+
 private func makeFavoriteRelationship(
     noiseKey: Data,
     nostrPublicKey: String? = nil,
@@ -412,6 +424,25 @@ struct ChatPrivateConversationCoordinatorContextTests {
         #expect(context.privateChats[convKey]?.count == 1)
     }
 
+    @Test @MainActor
+    func sendGeohashDM_rejectedAdmissionNeverBecomesSent() async {
+        let context = MockChatPrivateConversationContext()
+        let coordinator = ChatPrivateConversationCoordinator(context: context)
+        let recipientHex = String(repeating: "33", count: 32)
+        let peerID = PeerID(nostr_: recipientHex)
+        context.activeChannel = .location(
+            GeohashChannel(level: .city, geohash: "u4pruy")
+        )
+        context.nostrKeyMapping[peerID] = recipientHex
+        context.geohashPrivateMessageAccepted = false
+
+        coordinator.sendGeohashDM("rejected", to: peerID)
+
+        #expect(context.geoPrivateMessages.map(\.content) == ["rejected"])
+        #expect(context.privateChats[peerID]?.count == 1)
+        #expect(isFailed(context.privateChats[peerID]?.first?.deliveryStatus))
+    }
+
     @Test @MainActor
     func handleViewingThisChat_clearsUnreadAndSendsRoutedReadReceiptOnce() async {
         let context = MockChatPrivateConversationContext()

bitchatTests/NostrProtocolTests.swift

@@ -429,6 +429,27 @@ struct NostrProtocolTests {
         }
     }
 
+    @Test func privateEnvelopePublicationBatchRejectsMaximumComposerPayload() throws {
+        let sender = try NostrIdentity.generate()
+        let recipient = try NostrIdentity.generate()
+        let composerMaximum = String(
+            repeating: "x",
+            count: InputValidator.Limits.maxMessageLength
+        )
+
+        #expect(
+            composerMaximum.utf8.count
+                > NostrProtocol.maximumPrivateEnvelopePlaintextBytes
+        )
+        expectInvalidCiphertext {
+            _ = try NostrProtocol.createPrivateEnvelopePublicationBatch(
+                content: composerMaximum,
+                recipientPubkey: recipient.publicKeyHex,
+                senderIdentity: sender
+            )
+        }
+    }
+
     @Test func privateEnvelopeRejectsOversizedNestedJSONBeforeParsing() {
         let oversizedJSON = String(
             repeating: "{",

bitchatTests/Services/NostrTransportTests.swift

@@ -302,6 +302,128 @@ struct NostrTransportTests {
         #expect(reported)
     }
 
+    @Test("Envelope construction failure rejects visibly without publishing")
+    @MainActor
+    func envelopeConstructionFailureRejectsVisibly() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let sender = try NostrIdentity.generate()
+        let eventProbe = NostrTransportEventProbe()
+        let publicationProbe = NostrTransportProbe()
+        let transport = NostrTransport(
+            keychain: keychain,
+            idBridge: idBridge,
+            dependencies: makeDependencies(
+                currentIdentity: { sender },
+                sendPrivateEnvelopeBatch: { events, _ in
+                    publicationProbe.record(batch: events)
+                }
+            )
+        )
+        transport.senderPeerID = PeerID(str: "0123456789abcdef")
+        transport.eventDelegate = eventProbe
+
+        // Non-empty but invalid hex reaches envelope construction after the
+        // embedded BitChat packet succeeds, reproducing the throwing batch
+        // builder path without allocating an oversized test fixture.
+        let accepted = transport.sendPrivateMessageGeohash(
+            content: "must fail before sent",
+            toRecipientHex: "not-a-hex-public-key",
+            from: sender,
+            messageID: "build-reject"
+        )
+
+        #expect(!accepted)
+        #expect(publicationProbe.sentEvents.isEmpty)
+        #expect(eventProbe.failedMessageIDs == ["build-reject"])
+    }
+
+    @Test("Unencodable user message rejects visibly without publishing")
+    @MainActor
+    func unencodableUserMessageRejectsVisibly() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let sender = try NostrIdentity.generate()
+        let recipient = try NostrIdentity.generate()
+        let eventProbe = NostrTransportEventProbe()
+        let publicationProbe = NostrTransportProbe()
+        let transport = NostrTransport(
+            keychain: keychain,
+            idBridge: idBridge,
+            dependencies: makeDependencies(
+                currentIdentity: { sender },
+                sendPrivateEnvelopeBatch: { events, _ in
+                    publicationProbe.record(batch: events)
+                }
+            )
+        )
+        transport.senderPeerID = PeerID(str: "0123456789abcdef")
+        transport.eventDelegate = eventProbe
+
+        // PrivateMessagePacket uses the deployed UInt8 content length. Do not
+        // create a larger wire shape that released clients cannot decode.
+        let accepted = transport.sendPrivateMessageGeohash(
+            content: String(repeating: "x", count: 256),
+            toRecipientHex: recipient.publicKeyHex,
+            from: sender,
+            messageID: "packet-reject"
+        )
+
+        #expect(!accepted)
+        #expect(publicationProbe.sentEvents.isEmpty)
+        #expect(eventProbe.failedMessageIDs == ["packet-reject"])
+    }
+
+    @Test("Unencodable direct message emits a visible failure")
+    @MainActor
+    func unencodableDirectMessageEmitsVisibleFailure() async throws {
+        let keychain = MockKeychain()
+        let idBridge = NostrIdentityBridge(keychain: keychain)
+        let sender = try NostrIdentity.generate()
+        let recipient = try NostrIdentity.generate()
+        let noiseKey = Data((224..<256).map(UInt8.init))
+        let peerID = PeerID(hexData: noiseKey)
+        let relationship = makeRelationship(
+            peerNoisePublicKey: noiseKey,
+            peerNostrPublicKey: recipient.npub,
+            peerNickname: "Oversized peer"
+        )
+        let eventProbe = NostrTransportEventProbe()
+        let publicationProbe = NostrTransportProbe()
+        let transport = NostrTransport(
+            keychain: keychain,
+            idBridge: idBridge,
+            dependencies: makeDependencies(
+                favoriteStatusForNoiseKey: {
+                    $0 == noiseKey ? relationship : nil
+                },
+                currentIdentity: { sender },
+                sendPrivateEnvelopeBatch: { events, _ in
+                    publicationProbe.record(batch: events)
+                }
+            )
+        )
+        transport.senderPeerID = PeerID(str: "0123456789abcdef")
+        transport.eventDelegate = eventProbe
+
+        transport.sendPrivateMessage(
+            String(repeating: "x", count: 256),
+            to: peerID,
+            recipientNickname: "Oversized peer",
+            messageID: "direct-packet-reject"
+        )
+
+        let failed = await TestHelpers.waitUntil(
+            {
+                eventProbe.failedMessageIDs
+                    == ["direct-packet-reject"]
+            },
+            timeout: 5.0
+        )
+        #expect(failed)
+        #expect(publicationProbe.sentEvents.isEmpty)
+    }
+
     @Test("Rejected favorite notification retains and retries the exact pair")
     @MainActor
     func rejectedFavoriteNotificationRetriesExactPair() async throws {