Use binary recipient IDs in cached delivery and deterministic tests

app/src/main/java/com/bitchat/android/mesh/StoreForwardManager.kt

@@ -4,6 +4,7 @@ import android.util.Log
 import com.bitchat.android.protocol.BitchatPacket
 import com.bitchat.android.protocol.MessageType
 import com.bitchat.android.protocol.SpecialRecipients
+import com.bitchat.android.util.hexEncodedString
 import kotlinx.coroutines.*
 import java.util.*
 import java.util.concurrent.ConcurrentHashMap
@@ -12,7 +13,7 @@ import java.util.concurrent.ConcurrentHashMap
  * Manages store-and-forward messaging for offline peers
  * Extracted from BluetoothMeshService for better separation of concerns
  */
-class StoreForwardManager {
+class StoreForwardManager(dispatcher: CoroutineDispatcher = Dispatchers.IO) {
     
     companion object {
         private const val TAG = "StoreForwardManager"
@@ -47,7 +48,7 @@ class StoreForwardManager {
     var delegate: StoreForwardManagerDelegate? = null
     
     // Coroutines
-    private val managerScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
+    private val managerScope = CoroutineScope(dispatcher + SupervisorJob())
     
     init {
         startPeriodicCleanup()
@@ -74,7 +75,7 @@ class StoreForwardManager {
         
         // Determine if this is for a favorite peer
         val recipientPeerID = packet.recipientID?.let { recipientID ->
-            String(recipientID).replace("\u0000", "")
+            recipientID.hexEncodedString()
         }
         
         if (recipientPeerID.isNullOrEmpty()) {
@@ -166,7 +167,7 @@ class StoreForwardManager {
             val recipientMessages = messageCache.filter { storedMessage ->
                 !deliveredMessages.contains(storedMessage.messageID) &&
                 storedMessage.packet.recipientID?.let { recipientID ->
-                    String(recipientID).replace("\u0000", "") == peerID
+                    recipientID.hexEncodedString() == peerID
                 } == true
             }
             messagesToSend.addAll(recipientMessages)
@@ -226,7 +227,7 @@ class StoreForwardManager {
         val favoriteCount = favoriteMessageQueue[peerID]?.size ?: 0
         val regularCount = messageCache.count { storedMessage ->
             storedMessage.packet.recipientID?.let { recipientID ->
-                String(recipientID).replace("\u0000", "") == peerID
+                recipientID.hexEncodedString() == peerID
             } == true
         }
         return favoriteCount + regularCount

app/src/test/kotlin/com/bitchat/android/mesh/StoreForwardManagerTest.kt

@@ -2,18 +2,25 @@ package com.bitchat.android.mesh
 
 import com.bitchat.android.protocol.BitchatPacket
 import com.bitchat.android.protocol.MessageType
-import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.StandardTestDispatcher
+import kotlinx.coroutines.test.TestCoroutineScheduler
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
 
 /**
  * Store-and-forward exists so mail for an offline peer is handed over when it
  * comes back. Two things stopped that working.
  */
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(RobolectricTestRunner::class)
 class StoreForwardManagerTest {
-    private val manager = StoreForwardManager()
+    private val scheduler = TestCoroutineScheduler()
+    private val manager = StoreForwardManager(StandardTestDispatcher(scheduler))
     private val delegate = RecordingDelegate()
 
     init {
@@ -24,7 +31,7 @@ class StoreForwardManagerTest {
     fun tearDown() = manager.shutdown()
 
     @Test
-    fun `mail cached after a peer was served is delivered on the next reconnect`() = runBlocking {
+    fun `mail cached after a peer was served is delivered on the next reconnect`() {
         // First contact: nothing held, peer is marked as served.
         manager.sendCachedMessages(PEER)
         waitForDelivery()
@@ -32,6 +39,7 @@ class StoreForwardManagerTest {
 
         // Peer goes away; a message is queued for it.
         manager.cacheMessage(privateMessage("m-1"), "m-1")
+        assertEquals(1, manager.getCachedMessageCount(PEER))
 
         // It comes back. Before this fix the send was refused outright,
         // because nothing ever removed the peer from the already-sent latch,
@@ -42,11 +50,26 @@ class StoreForwardManagerTest {
         assertEquals(1, delegate.sent.size)
     }
 
+    @Test
+    fun `favorite recipient keys use the same binary wire identity`() {
+        delegate.favorite = true
+        manager.sendCachedMessages(PEER)
+        waitForDelivery()
+        manager.cacheMessage(privateMessage("favorite-1"), "favorite-1")
+        assertEquals(1, manager.getCachedMessageCount(PEER))
+
+        manager.sendCachedMessages(PEER)
+        waitForDelivery()
+        assertEquals(1, delegate.sent.size)
+        assertEquals(0, manager.getCachedMessageCount(PEER))
+    }
+
     @Test
     fun `a peer with nothing new is still not re-sent the same batch`() {
         // The latch has to keep working, or every reconnect replays whatever
         // is still cached.
         manager.cacheMessage(privateMessage("m-1"), "m-1")
+        assertEquals(1, manager.getCachedMessageCount(PEER))
         manager.sendCachedMessages(PEER)
         waitForDelivery()
         val afterFirst = delegate.sent.size
@@ -91,16 +114,16 @@ class StoreForwardManagerTest {
     }
 
     private fun waitForDelivery() {
-        // sendCachedMessages dispatches on its own scope with a 10ms/message
-        // stagger; this is well clear of the single-message case under test.
-        Thread.sleep(300)
+        // Each tested batch contains one message, whose stagger is zero.
+        // Run queued work deterministically without advancing periodic cleanup.
+        scheduler.runCurrent()
     }
 
     private fun privateMessage(id: String): BitchatPacket = BitchatPacket(
         version = 1u,
         type = MessageType.MESSAGE.value,
         senderID = "1111222233334444".hexToBytes(),
-        recipientID = PEER.toByteArray(),
+        recipientID = PEER.hexToBytes(),
         timestamp = 1u,
         payload = id.toByteArray(),
         ttl = 7u
@@ -108,7 +131,8 @@ class StoreForwardManagerTest {
 
     private class RecordingDelegate : StoreForwardManagerDelegate {
         val sent = mutableListOf<BitchatPacket>()
-        override fun isFavorite(peerID: String) = false
+        var favorite = false
+        override fun isFavorite(peerID: String) = favorite
         override fun isPeerOnline(peerID: String) = false
         override fun sendPacket(packet: BitchatPacket) {
             synchronized(sent) { sent.add(packet) }

docs/client-rewrite-contracts.md

@@ -20,6 +20,16 @@ The remaining implementation work and milestone progress are tracked in
 | Nostr | Bech32, secp256k1 key derivation, NIP-01 event IDs/signatures, NIP-44 authenticated encryption, NIP-13 PoW, authenticated NIP-17 seals | `ClientRewriteNostrContractTest`, `NostrProtocolTest` |
 | Application state | Peer unions, canonical private conversations, chronological history, delivery/read behavior, media migration policy | `AppStateStoreTest`, `PrivateChatManagerTest`, `MediaSendingManagerMigrationTest` |
 
+## Store-forward recipient identity
+
+The recipient of a cached mesh packet is its eight-byte wire ID, rendered as
+16 lowercase hexadecimal characters for cache and peer lookups. It is not a
+UTF-8 string. The synthetic bytes `aa aa bb bb cc cc dd dd` therefore identify
+`aaaabbbbccccdddd` for regular queues, favorite queues, cache counts, and the
+already-sent latch. Caching new mail for that identity releases its latch so a
+later reconnect can deliver it. `StoreForwardManagerTest` uses this fixed wire
+identity and a controlled coroutine scheduler for the reconnect contract.
+
 ## Golden-vector policy
 
 Golden vectors compare literal externally visible bytes or hashes. Do not update