From 3ed439384240cd6ed5d8c5134afc335a8e285d9e Mon Sep 17 00:00:00 2001 From: Taksh Date: Mon, 5 Oct 2026 16:16:07 +0530 Subject: [PATCH] Use binary recipient IDs in cached delivery and deterministic tests --- .../android/mesh/StoreForwardManager.kt | 11 ++--- .../android/mesh/StoreForwardManagerTest.kt | 40 +++++++++++++++---- docs/client-rewrite-contracts.md | 10 +++++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/mesh/StoreForwardManager.kt b/app/src/main/java/com/bitchat/android/mesh/StoreForwardManager.kt index 5fc2eab7..80896f8a 100644 --- a/app/src/main/java/com/bitchat/android/mesh/StoreForwardManager.kt +++ b/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 diff --git a/app/src/test/kotlin/com/bitchat/android/mesh/StoreForwardManagerTest.kt b/app/src/test/kotlin/com/bitchat/android/mesh/StoreForwardManagerTest.kt index 9772923f..513678eb 100644 --- a/app/src/test/kotlin/com/bitchat/android/mesh/StoreForwardManagerTest.kt +++ b/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() - 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) } diff --git a/docs/client-rewrite-contracts.md b/docs/client-rewrite-contracts.md index 1abed243..25278157 100644 --- a/docs/client-rewrite-contracts.md +++ b/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