diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 018c7e5df..8350460b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -998,13 +998,13 @@ jobs: PeerStreamSession.swift ReticulumManager.swift mesh/Mesh.swift ble/MeshConnectionRegistry.swift \ ble/BleDiscoveryBootstrapPolicy.swift \ PeripheralRestorationAgeOutPolicy.swift \ - BleHelloPolicy.swift BleMessageNotificationPolicy.swift \ + BleHelloPolicy.swift BleMessageNotificationPolicy.swift BleWriteStallPolicy.swift \ BleAppTag.swift BleDensityPolicy.swift BleServiceInstanceSelection.swift \ Generated/offline_protocol.swift ForcedPresenceCheckQueue.swift \ AddressDeclarationPolicy.swift \ GatewayAttachPolicy.swift GatewayVerdictTracker.swift \ InboundFragmentBuffer.swift OutboundFragmentQueue.swift \ - PeerIdentityBinding.swift RelayAnswerPrefixes.swift \ + PeerIdentityBinding.swift PeerLinkResolution.swift RelayAnswerPrefixes.swift \ RelayControlOpTranslator.swift RelayGroupSnapshotBridge.swift \ RelayRateLimiter.swift LegacyRelayMessage.swift MonotonicClock.swift \ NostrQueryTracker.swift \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 0820a42c2..39a083f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,124 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **A phone no longer loses a Bluetooth peer it is still linked to.** In the + 2026-10-19 volume test an iPhone and an Android phone held two Bluetooth LE + links between them; the one that stalled was torn down, the iPhone's redial + of its address timed out and reported Android lost, and the core refused + every send to Android for ten minutes while the second link sat idle. No + reconnect could succeed, because the radios complete one connection between + two devices. Now iOS retires the dropped identifier instead of reporting the + peer lost while another link to it is up, and skips dials to a peer it is + linked to; Android closes a client link to a peer already subscribed to its + server, and restarts its advertisement (which gives it a new address) on its + two-minute refresh only while no identified peer is linked, or when the + advertisement has stopped. Evicting a peer on iOS now drops every + identifier it maps from, so a second link no longer survives a peer the + core was told is lost. See ADR 0030. +- **A full outbox refuses the newest message instead of failing the oldest.** + At 500 entries the outbox failed its least recently sent entry to admit any + new one, so the same outage failed about 2,500 queued messages per phone, + each to make room for a newer message to the same peer. Now a message to the + recipient with the largest backlog is refused (`message_failed`, "Outbox + capacity exceeded", no attempts, while `send` returns its id); a message to + a recipient with a smaller backlog evicts from the largest one (its newest + message held by the per-peer window, else its oldest entry); and a message + awaiting its acknowledgement is never evicted. +- **A link carrying less than it is offered no longer collapses.** Two phones + sending 5 messages a second each way over Bluetooth LE delivered 70.5%: the + Android phone's indication path carried 12 to 16 frames a second against 15 + offered, acknowledgements queued behind data until every message timed out, + each resend cost the receiver one more acknowledgement on the saturated + link, and both phones retried 50,000 times and failed 6,000 messages as + "Outbox capacity exceeded". Four changes: + - A sender keeps at most 16 frames awaiting acknowledgement in flight to + one peer over a mesh link. Further messages wait in the outbox, are + reported as `message_deferred` with reason `peer_window`, and go out + oldest first as acknowledgements return. The relay is not windowed, and + neither is session control (key packages, Welcomes, confirmation + probes), which heals the session the held frames are sealed under. + - The Bluetooth LE carrier queues an acknowledgement ahead of data. + - Delivery acknowledgements to a peer that advertises `ack_versions` entry 2 + are coalesced, up to five per frame within 500 ms, in a new `ack_more` + metadata key. Older peers get one acknowledgement per frame. + - On Android, the indication path sends the next fragment as soon as the + peer confirms the last, instead of 20 ms later. +- **A resend the receiver can no longer decrypt is re-sealed.** Past the + re-seal budget a resend went out verbatim, and a verbatim copy sealed more + than 32 generations before the newest was refused by the receiver, + unanswered on a mesh link, until its retries ran out. It is now re-sealed, + up to 512 generations past the last acknowledgement. +- **A full acknowledgement table no longer stops the engine.** Once 1,000 + messages were waiting for acknowledgements, a retry with no slot returned + an error out of `process()` before the step that frees slots, on every + tick, and the engine never recovered: every later send failed and nothing + more was retried. A frame is now never transmitted without a slot to track + it. At the ceiling a send is held for the retry queue, reported as + `message_deferred` with reason `ack_capacity`, and sent once an + acknowledgement frees a slot; `process()` runs every step on every tick. + Two leaks that kept the table full are fixed with it: a refused + registration orphaned every timed-out entry so it was never retried, and + the prune of timed-out entries ran at exactly the retry ladder's maximum + delay, so a ninth retry found its slot gone. +- **`sendMessage` no longer reports a failure for a message it sent.** The + capacity error used to surface after the frame was on the wire, so an + application showed messages as failed that the recipient had received. +- **A receiver that falls far behind a sender re-keys instead of going + deaf.** A frame more than the forward window ahead of the receiver's + ratchet failed as an ordinary decryption error, and so did every later + frame and every resend from that sender, permanently. It now triggers the + same rate-limited re-key as an epoch desync. The trigger is authenticated: + it needs the epoch's sender-data secret. The window is also raised from + 1,000 to 8,192 generations, pinned at four times the in-flight ceiling. +- **A frame still waiting in the BLE queue is not retried.** Its + acknowledgement timeout is extended, for up to a minute, instead of + queueing a second re-sealed copy behind it. On a link carrying less than it + was offered, those copies grew the queue, the round trip and the + receiver's ratchet gap together. +- **Large BLE messages are no longer torn under load.** The Android and iOS + outbound fragment queues discarded everything queued for a peer when they + filled, and they could fill: Android's periodic poll pulled fragments from + the core with no backpressure check, and both drains checked only after a + fragment had been pulled. With a cap of 100, one 16 KB message was enough. + Both drains now stop before pulling while any peer with a link is backed + up, and the cap is 768, above the largest message the core fragments. A + peer with no link does not stop the pull, so its undeliverable fragments + no longer hold back every other peer's. +- **Resends stop burning encryption keys while nothing is acknowledged.** + Each resend used to be re-sealed under a fresh ratchet generation, so a + link that stopped carrying acknowledgements walked the receiver past its + forward window: one phone re-sealed 20,919 resends during a 17-minute + stall. A sender now re-seals only while it has sealed fewer than 256 + frames past the peer's last acknowledgement, and resends verbatim beyond + that until the next one arrives. 256 is a quarter of 1,024, the receiver + window of a leaf node, which mls-rs fixes and which is smaller than a + phone's; `tools/mls-interop` pins it. +- **An iOS central link whose write buffer never drains is torn down and + reconnected.** CoreBluetooth kept refusing every write on such a link for + as long as it lived while still delivering the peer's notifications, so + nothing reported it down. After 15 s with no accepted write the link is + cancelled. A connect that has not completed in 10 s in the foreground is + cancelled too: CoreBluetooth never times one out, and the pending connect + refused every reconnect attempt as "already connecting". +- **A message sent while its sender was blocked is never delivered.** Its id + is now remembered, so a retry arriving after an unblock is answered rather + than delivered. +- **An iOS phone keeps talking to a peer that changed its Bluetooth + address.** The peer reappears under a new identifier, and the old one still + mapped to it, so the send path could pick the old, disconnected one and + report "no connected peripheral" indefinitely while the verified link to + the same peer sat idle: 25 minutes in one test. Sends now go to the + connected link, verifying a peer retires its other disconnected + identifiers, and the connection monitor stops dialling a peripheral it + never identified once it stops advertising. +- **The BLE bridges stop pulling far ahead of the link.** Both outbound + fragment queues pulled from the core until a peer held 576 fragments, so + the core saw nothing queued, could not extend the acknowledgement timeout + of what was still waiting, and resent it: 19,404 resends from one Android + phone on an indication-paced link. They now stop at 32 fragments, a few + seconds of link time, and the rest waits in the core where the extension + sees it. The 768 cap is unchanged. + - **Link-time optimisation reaches the libraries the release ships.** The release profile has always asked for it, but cargo runs it on a library only when every crate type in the library's list can take it, and the FFI diff --git a/bindings/react-native/MeshSdk.podspec b/bindings/react-native/MeshSdk.podspec index 0275fb802..131e1efcb 100644 --- a/bindings/react-native/MeshSdk.podspec +++ b/bindings/react-native/MeshSdk.podspec @@ -37,9 +37,11 @@ Pod::Spec.new do |s| "ios/BleAppTag.swift", "ios/BleDensityPolicy.swift", "ios/BleHelloPolicy.swift", + "ios/BleWriteStallPolicy.swift", "ios/BleMessageNotificationPolicy.swift", "ios/BleServiceInstanceSelection.swift", "ios/PeerIdentityBinding.swift", + "ios/PeerLinkResolution.swift", "ios/RelayAnswerPrefixes.swift", "ios/RelayControlOpTranslator.swift", "ios/RelayGroupSnapshotBridge.swift", diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/AdvertisingRefreshPolicy.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/AdvertisingRefreshPolicy.kt new file mode 100644 index 000000000..9fd1b2d0a --- /dev/null +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/AdvertisingRefreshPolicy.kt @@ -0,0 +1,30 @@ +package com.offlineprotocol.ble + +/** + * Whether the two-minute forced BLE refresh restarts the advertisement. + * + * Restarting starts a new advertising set, and the stack gives every set a + * new random address. A peer linked to us then sees a new device every two + * minutes, dials it, and waits out its connect timeout, because the Link Layer + * does not complete a second connection between two devices that already have + * one (ADR 0030). So the refresh leaves the advertisement alone while a peer + * is linked and identified. + * + * Two exceptions keep that from stranding the phone undiscoverable: + * - An advertisement the advertiser reports as not running is always + * restarted. Skipping the restart while linked used to leave a stopped + * advertisement stopped for as long as any link stayed up, so a third + * device could not find this phone. + * - Only identified links hold it. An inbound central that has not proved who + * it is, or the server's echo of a dial still in flight, may be closed in a + * moment, and must not keep an advertisement from being refreshed. + * + * What it cannot see: an advertisement the stack stopped without telling the + * advertiser. While linked that one is recovered by the next membership + * change, disconnect or peer loss, each of which refreshes it (ADR 0030 records + * the trade). + */ +internal object AdvertisingRefreshPolicy { + fun restartsAdvertising(identifiedLinks: Int, isAdvertising: Boolean): Boolean = + identifiedLinks == 0 || !isAdvertising +} diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt index 7a0dbaf2b..eb942bb3e 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt @@ -497,6 +497,9 @@ class BleTransportFacade( override val bluetoothAdapter: BluetoothAdapter? get() = this@BleTransportFacade.bluetoothAdapter override val selfDeviceId: String get() = deviceId + override fun isCentralSubscribed(address: String): Boolean = + peripheralGattServer?.isSubscribed(address) == true + override fun isShuttingDown(): Boolean = shuttingDown override fun isRunning(): Boolean = state == TransportState.RUNNING @@ -2747,13 +2750,25 @@ class BleTransportFacade( if (logThrottler.shouldLog("forced_ble_refresh", intervalMs = FORCED_BLE_REFRESH_MS)) { Log.i(TAG, "Performing forced BLE refresh for reliability") emitDiagnostic("info", "Forced BLE refresh for reliability", mapOf( + "establishedLinks" to connections.establishedLinkCount(), "connectedPeers" to connections.connectionCount(), "discoveredPeers" to connections.discoveredPeerCount() )) } - // Stop and restart both scanning and advertising + // Restart scanning always; the advertisement only as + // [AdvertisingRefreshPolicy] allows: not while an identified peer + // is linked (each restart gives the advertisement a new random + // address, and a linked peer then dials an address pair the Link + // Layer will never connect, ADR 0030), unless the advertiser says + // it is not running. Membership changes and disconnects still + // refresh it. + val identifiedLinks = connections.identifiedLinkCount() stopScanning("forced_refresh") - refreshAdvertising("forced_refresh") + if (AdvertisingRefreshPolicy.restartsAdvertising(identifiedLinks, leAdvertiser.isAdvertising)) { + refreshAdvertising("forced_refresh") + } else if (logThrottler.shouldLog("forced_refresh_keeps_advertising", intervalMs = FORCED_BLE_REFRESH_MS)) { + Log.i(TAG, "Forced refresh keeps the advertisement: $identifiedLinks identified link(s) up") + } bleHandler.postDelayed({ if (state == TransportState.RUNNING) { startScanning("forced_refresh") @@ -2810,7 +2825,10 @@ class BleTransportFacade( return } - if (connections.isSuppressedDuplicate(device.address, SystemClock.elapsedRealtime())) { + if (connections.isSuppressedDuplicate(device.address, SystemClock.elapsedRealtime()) { + peripheralGattServer?.isSubscribed(it) == true + } + ) { if (logThrottler.shouldLog("duplicate_skip_${device.address}", intervalMs = 30000)) { Log.d(TAG, "Not redialing ${device.address}: a second link to a peer already held") } @@ -3296,6 +3314,18 @@ class BleTransportFacade( } iterations++ + // Stop before the pop, not after it. The pop is destructive and + // shared by every peer, so a fragment pulled for a peer whose + // queue is past the mark has nowhere to go but further past it; + // the checks further down run only once that has happened. + // Only a recipient with a link counts: one with none cannot + // drain, so counting it stopped every other peer until its + // fragments expired ([OutboundFragmentQueue.anyBackedUp]). + if (outboundQueue.anyBackedUp { hasDeliverableLink(it) }) { + hitBackpressure = true + break + } + val fragment = try { protocol.bleGetNextFragment() } catch (e: Exception) { @@ -3307,14 +3337,7 @@ class BleTransportFacade( val data = fragment.data.map { it.toByte() }.toByteArray() val address = resolveTargetAddress(recipientId) - // A peer that subscribed on the link it opened to our server is - // reachable by notify, which [sendFragmentData] prefers. A hello - // maps such a peer at its central-role address, where we hold no - // client link; requiring one here would queue every fragment and - // dial that address, the dial the hello exists to remove (#512). - val hasConnection = address?.let { connections.getGatt(it) } != null || - subscribedNotifyAddressFor(recipientId) != null - if (!hasConnection) { + if (!hasDeliverableLink(recipientId, address)) { outboundQueue.enqueue(recipientId, data) // Proactively attempt reconnection if we know the address (once per peer per drain). // Never toward a central connected to our server: that address need not @@ -3340,22 +3363,10 @@ class BleTransportFacade( // Maintain FIFO ordering: if this recipient already has // fragments waiting, enqueue instead of sending directly. if (outboundQueue.enqueueIfBlocked(recipientId, data)) { - // Backpressure against the write gate: once this peer's - // queue is backed up, stop pulling more fragments out of - // the Rust core (bleGetNextFragment is a destructive pop) - // into the bounded per-peer queue. The write gate paces - // sends to ~1 fragment / WRITE_GATE_WATCHDOG_MS when the - // completion callback is absent, which is far slower than - // this loop can pull; without this stop the loop spins the - // whole backlog into the queue, overflows maxPerPeer, and - // OutboundFragmentQueue.enqueue discards the in-flight - // message. Leaving the backlog in Rust keeps delivery - // lossless — the scheduled re-drain below resumes once - // flush() has drained the queue back down. - if (outboundQueue.isBackedUp(recipientId)) { - hitBackpressure = true - break - } + // Queued behind this peer's earlier fragments, so its + // stream stays in order. The check at the top of the loop + // stops the next pop once this queue reaches the mark; + // the scheduled re-drain resumes once flush() drains it. continue } @@ -3420,7 +3431,18 @@ class BleTransportFacade( * calls back-to-back and Android 13+ rejects the concurrent ones with * ERROR_GATT_WRITE_REQUEST_BUSY (201), silently dropping fragments. */ - fun onWriteCompleted(address: String) { + fun onWriteCompleted(address: String) = onWriteCompleted(address, peerConfirmed = false) + + /** + * [peerConfirmed] is true for an ATT indication, whose completion is the + * central's confirmation: the peer has the fragment, so the next one goes + * at once. The pacing below exists for completions that only say our own + * controller took the op. On the indicate path it was dead time: one + * fragment per confirmation plus 20 ms held the Android-to-iPhone link to + * 12 to 16 fragments a second in the 2026-10-19 volume test, below the 15 + * it was offered. + */ + fun onWriteCompleted(address: String, peerConfirmed: Boolean) { assertOnBleThread("onWriteCompleted") // Address-keyed, not per-write: a stale onCharacteristicWrite for an // earlier write that fires after the watchdog/stale-check already advanced @@ -3430,13 +3452,15 @@ class BleTransportFacade( // single ERROR_GATT_WRITE_REQUEST_BUSY (201) on that newer write, which // self-heals via the caller's re-enqueue + BACKPRESSURE_RETRY — no data loss. writeInFlight.remove(address) - if (state == TransportState.RUNNING) { + if (state == TransportState.RUNNING && peerConfirmed) { + drainAndSendFragments() + } else if (state == TransportState.RUNNING) { // Pace the next completion-driven send by one BLE connection - // interval instead of draining immediately. onNotificationSent / - // onCharacteristicWrite signal that OUR controller accepted the - // op, not that the peer has drained it — firing the next notify - // instantly out-runs a slower central (iOS) and drops fragments - // mid-burst, so a large multi-fragment Welcome never reassembles. + // interval instead of draining immediately. onCharacteristicWrite + // signals that OUR controller accepted the op, not that the peer + // has drained it; firing the next write instantly out-runs a + // slower peer and drops fragments mid-burst, so a large + // multi-fragment Welcome never reassembles. // The brief spacing lets the peer keep up; small messages are // unaffected in practice. bleHandler.postDelayed({ @@ -3468,7 +3492,13 @@ class BleTransportFacade( // Still poll for new fragments even if there are unsent pending ones // This prevents deadlock where old fragments block new ones - // Poll for next fragment from protocol + // Poll for next fragment from protocol — but never while a peer's + // queue is past the mark. This floor used to pop with no check at + // all, so every 2 s it pushed one more fragment into a queue the + // drain had stopped feeding, until the cap discarded the lot. + if (outboundQueue.anyBackedUp { hasDeliverableLink(it) }) { + return + } val fragment = try { protocol.bleGetNextFragment() } catch (e: Exception) { @@ -3528,6 +3558,22 @@ class BleTransportFacade( } } + /** + * Whether [recipientId] has a link its fragments can leave on now: a + * client link at its resolved address, or its subscription on the link it + * opened to our server. A peer that subscribed is reachable by notify, + * which [sendFragmentData] prefers. A hello maps such a peer at its + * central-role address, where we hold no client link; requiring one would + * queue every fragment and dial that address, the dial the hello exists + * to remove (#512). BLE thread only. + */ + private fun hasDeliverableLink( + recipientId: String, + address: String? = resolveTargetAddress(recipientId), + ): Boolean = + address?.let { connections.getGatt(it) } != null || + subscribedNotifyAddressFor(recipientId) != null + private fun resolveTargetAddress(recipientId: String): String? { if (recipientId == deviceId) { return null @@ -4315,9 +4361,22 @@ class BleTransportFacade( // (status 0 = GATT_SUCCESS). If "Replying via notify" appears but this // never does, the notify is stuck in the stack queue / not going OTA. Log.i(TAG, "Notification flushed to $address status=$status") + // A failed indication lost its fragment: the queue already gave it + // up, so its message reassembles torn and the core's resend is + // the recovery. Say so, so a tearing link is attributable, and + // pace the next one instead of draining at once into a link that + // is refusing. + val confirmed = status == BluetoothGatt.GATT_SUCCESS + if (!confirmed && logThrottler.shouldLog("notify_failed_$address", intervalMs = 5_000)) { + Log.w(TAG, "Indication to $address failed with status=$status; its fragment is lost") + emitDiagnostic("warning", "BLE indication failed; fragment lost", mapOf( + "address" to address, + "status" to status + )) + } bleHandler.post { if (shuttingDown) return@post - onWriteCompleted(address) + onWriteCompleted(address, peerConfirmed = confirmed) } } diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/CentralGattClient.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/CentralGattClient.kt index 444301e44..5c6ecc261 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/CentralGattClient.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/CentralGattClient.kt @@ -119,6 +119,11 @@ internal class CentralGattClient( * fragment. Called on the BLE thread. */ fun onWriteCompleted(address: String) + /** Whether the central at [address] is subscribed to our GATT server. + * A subscribed server link to a peer makes a client link to it a + * duplicate ([MeshConnectionRegistry.hasOtherUsableLink]). */ + fun isCentralSubscribed(address: String): Boolean = false + /** Hand a received fragment to the facade (either via client * notify or via central-side write). Called on the BLE thread. */ fun handleInboundFragment(address: String, data: ByteArray) @@ -1260,7 +1265,7 @@ internal class CentralGattClient( return } is PeerIdentityBinding.Outcome.Verified -> { - if (host.connections.hasOtherClientLink(outcome.peerId, address)) { + if (host.connections.hasOtherUsableLink(outcome.peerId, address, host::isCentralSubscribed)) { refuseDuplicateLink(gatt, outcome.peerId) return } @@ -1354,15 +1359,21 @@ internal class CentralGattClient( } /** - * Closes a client link that proved an identity we already hold a client - * link to at another address. + * Closes a client link that proved an identity we already hold a usable + * link to at another address: a client link of ours, or the peer's own + * subscribed link to our server. * * A macOS peripheral rotates its address and advertises under the old and * the new one at once, so the scanner dials both and both verify as the * same peer. Kept, the pair spends two of [BleTransportFacade]'s four * connection slots on one peer, and with two such peers the cap refuses * every inbound central, which is how such a peripheral's own central - * would have learned who we are. The first link to verify stays; this one + * would have learned who we are. And a phone that rotates its address + * while subscribed to our server reappears as a new device: dialled and + * kept, that was a second Link Layer connection between the same two + * radios, after which neither side could reconnect when the other link + * dropped (run 5 of the 2026-10-19 volume test, ten minutes apart). + * The first link to verify stays; this one * is closed unannounced, and its address is not redialed for * [DUPLICATE_LINK_SUPPRESS_MS]. */ diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/MeshConnectionRegistry.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/MeshConnectionRegistry.kt index c2a93cf7d..2dcc5ce94 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/MeshConnectionRegistry.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/MeshConnectionRegistry.kt @@ -119,13 +119,35 @@ class MeshConnectionRegistry { } /** - * True when we hold a client link to [deviceId] at an address other than - * [excluding]. Server links do not count: a peer may hold one link per - * direction, and only a second link in the SAME direction is a duplicate. + * True when we hold a usable link to [deviceId] at an address other than + * [excluding]: a client link of ours, or a central connected to our server + * and subscribed to it ([isSubscribed]), which is the path the peer's + * writes and our notifications already take. + * + * A client link beside such a server link is a second Link Layer + * connection between the same two radios, and two devices hold one + * (Core 5.4, Vol 6 Part B, 1.1.1). It forms only when one side's address + * has rotated, and once it exists neither side can reconnect after the + * other link drops: every dial is from an address the peer already has a + * link with. In run 5 of the 2026-10-19 volume test that kept the phones + * apart for ten minutes. + * + * A server link is seen only through [addressToDevice], which the hello + * handler sets for a central's address. A subscribed central that writes + * no hello (a Mac, a leaf, a service without it) is invisible here, so the + * second client link beside it is kept (ADR 0030 names that case). A dial + * still in flight counts as the kept link: [gattClients] holds it from + * registration, and the duplicate is the one that verified second. */ - fun hasOtherClientLink(deviceId: String, excluding: String): Boolean = + fun hasOtherUsableLink( + deviceId: String, + excluding: String, + isSubscribed: (String) -> Boolean, + ): Boolean = addressToDevice.any { (address, id) -> - id == deviceId && address != excluding && gattClients.containsKey(address) + id == deviceId && address != excluding && + (gattClients.containsKey(address) || + (serverConnections.contains(address) && isSubscribed(address))) } /** @@ -139,14 +161,18 @@ class MeshConnectionRegistry { /** * Whether a dial to [address] is still inside a duplicate refusal. * - * Lapses early once the client link it duplicated is gone: otherwise a + * Lapses early once the link it duplicated is gone: otherwise a * peer whose kept link died just after the refusal stayed undialed at the * address it still advertises for the rest of the window. */ - fun isSuppressedDuplicate(address: String, nowMs: Long): Boolean { + fun isSuppressedDuplicate( + address: String, + nowMs: Long, + isSubscribed: (String) -> Boolean = { false }, + ): Boolean { val entry = duplicateUntil[address] ?: return false val (deviceId, until) = entry - if (nowMs < until && hasOtherClientLink(deviceId, address)) return true + if (nowMs < until && hasOtherUsableLink(deviceId, address, isSubscribed)) return true duplicateUntil.remove(address, entry) return false } @@ -216,6 +242,15 @@ class MeshConnectionRegistry { */ fun establishedLinkCount(): Int = (establishedClients + serverConnections).size + /** + * [establishedLinkCount] restricted to links whose peer has proved who it + * is (an address mapped to a device id). An unverified inbound central, or + * the server's echo of a dial still being verified, may be closed in a + * moment; the forced refresh must not keep the advertisement for it. + */ + fun identifiedLinkCount(): Int = + (establishedClients + serverConnections).count { addressToDevice.containsKey(it) } + fun clear() { gattClients.clear() addressToDevice.clear() diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/OutboundFragmentQueue.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/OutboundFragmentQueue.kt index 64b6ed263..df2457625 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/OutboundFragmentQueue.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/OutboundFragmentQueue.kt @@ -49,13 +49,48 @@ internal data class FlushResult( * unsafe: fragments are slices of a single application message and * evicting slice 0 of a five-slice message leaves four orphan slices that * would reassemble into garbage at the receiver. Dropping the whole - * queue gives clean backpressure at message boundary granularity — we - * lose messages that hadn't started delivery yet, but we never split one. + * queue avoids orphaning the slices behind a dropped one, but it is still + * a loss, and it tears the message whose first slices already went out. + * + * So the cap is a memory backstop and must stay unreachable in normal + * operation. The drain stops pulling from the Rust core while any recipient + * with a link to drain into is past the [isBackedUp] mark ([anyBackedUp]), + * checked **before** each pop, because the pop is destructive and nothing can + * hand a fragment back. Every fragment in this queue arrived through one such + * pop, so a linked recipient never holds more than the mark, and the mark sits + * below the cap. A recipient with no link is left out of the stop (see + * [anyBackedUp]), so only its undeliverable queue can reach the cap. The cap also + * sits above the largest message the core will fragment (512 fragments). + * With the old cap of 100, and a poll path that popped without the check, + * the 2026-10-19 volume test never delivered a 16 KB or 30 KB message from + * Android. + * + * ### The mark is small on purpose + * + * [DEFAULT_BACKPRESSURE_MARK] is a few seconds of link time, not a fraction of + * the cap, because the Rust core can see only what it still holds. A message + * counts as queued, and its acknowledgement timer is extended instead of + * retried, until its first fragment is pulled. With the mark at 3/4 of the cap + * the drain pulled 576 fragments ahead of an indication-paced link, the core + * saw nothing queued, and every message behind the first few seconds was + * resent: 19,404 resends from Android in the third volume run, each one more + * load on the link that was already the bottleneck. A fragment left in Rust + * costs nothing; one pulled early only waits here, expires after [timeoutMs], + * and tears its message. + * + * Rust guards pin the cap and the mark equal to the iOS queue's. */ internal class OutboundFragmentQueue( private val bleThreadCheck: () -> Unit = DEFAULT_BLE_THREAD_CHECK, private val maxPerPeer: Int = DEFAULT_MAX_PER_PEER, private val timeoutMs: Long = DEFAULT_TIMEOUT_MS, + /** + * Below [maxPerPeer]; see the class note. Smaller caps keep the old 3/4, + * and never less than one: a mark of zero (a cap of 1) read every created + * queue, even an empty one, as backed up. + */ + private val backpressureMark: Int = + minOf(DEFAULT_BACKPRESSURE_MARK, maxPerPeer * 3 / 4).coerceAtLeast(1), private val clock: () -> Long = System::currentTimeMillis, private val onDropped: (recipientId: String, reason: DropReason, count: Int) -> Unit = { _, _, _ -> }, @@ -84,8 +119,8 @@ internal class OutboundFragmentQueue( } /** - * True once [recipientId]'s queue has filled past a high-water mark - * (3/4 of [maxPerPeer]). The drain loop uses this as a backpressure + * True once [recipientId]'s queue has filled to [backpressureMark]. The + * drain loop uses this as a backpressure * signal to STOP pulling more fragments out of the Rust core — which is * a destructive pop — into this bounded queue. Without it, when the BLE * write gate paces sends slower than the loop can pull, the loop spins @@ -98,7 +133,30 @@ internal class OutboundFragmentQueue( fun isBackedUp(recipientId: String): Boolean { bleThreadCheck() val size = queues[recipientId]?.size ?: 0 - return size >= maxPerPeer * 3 / 4 + return size >= backpressureMark + } + + /** + * True if any recipient that [deliverable] accepts is past the + * [isBackedUp] mark. The drain checks it before every pop from the Rust + * core: the pop is one FIFO across all peers and cannot be undone, so the + * only lossless moment to stop is before it. Safe only on the BLE thread, + * like every other read here. + * + * Only a recipient with a link to drain into may stop the pull. One with + * none (a peer whose link closed, in the grace before it is reported + * lost) keeps the fragments the core still hands out for it until they + * expire, and counting it stopped every other peer's egress until then, + * in 30-second rounds, for as long as the core held its backlog. Such a + * recipient's queue can pass the mark and, at worst, reach the cap: its + * fragments cannot be delivered either way, and the core's + * acknowledgement timeout resends them once a link returns. + */ + fun anyBackedUp(deliverable: (String) -> Boolean = { true }): Boolean { + bleThreadCheck() + return queues.entries.any { (recipientId, queue) -> + queue.size >= backpressureMark && deliverable(recipientId) + } } /** @@ -219,7 +277,10 @@ internal class OutboundFragmentQueue( } companion object { - const val DEFAULT_MAX_PER_PEER: Int = 100 + /** Above 4/3 of the core's 512-fragment message ceiling; see the class note. */ + const val DEFAULT_MAX_PER_PEER: Int = 768 + /** About 2.5 s of an indication-paced link; see the class note. */ + const val DEFAULT_BACKPRESSURE_MARK: Int = 32 const val DEFAULT_TIMEOUT_MS: Long = 30_000L val DEFAULT_BLE_THREAD_CHECK: () -> Unit = { diff --git a/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/AdvertisingRefreshPolicyTest.kt b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/AdvertisingRefreshPolicyTest.kt new file mode 100644 index 000000000..07050bdc5 --- /dev/null +++ b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/AdvertisingRefreshPolicyTest.kt @@ -0,0 +1,22 @@ +package com.offlineprotocol.ble + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class AdvertisingRefreshPolicyTest { + @Test + fun `with no identified link the advertisement is refreshed`() { + assertTrue(AdvertisingRefreshPolicy.restartsAdvertising(identifiedLinks = 0, isAdvertising = true)) + } + + @Test + fun `a running advertisement is left alone while a peer is linked`() { + assertFalse(AdvertisingRefreshPolicy.restartsAdvertising(identifiedLinks = 1, isAdvertising = true)) + } + + @Test + fun `a stopped advertisement is restarted even while linked`() { + assertTrue(AdvertisingRefreshPolicy.restartsAdvertising(identifiedLinks = 2, isAdvertising = false)) + } +} diff --git a/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/DuplicateClientLinkTest.kt b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/DuplicateClientLinkTest.kt index d93ea52ad..217f1bfba 100644 --- a/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/DuplicateClientLinkTest.kt +++ b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/DuplicateClientLinkTest.kt @@ -30,21 +30,21 @@ class DuplicateClientLinkTest { registry.setDeviceIdentifier("AA:AA:AA:AA:AA:01", "off1mac") registry.registerGatt("AA:AA:AA:AA:AA:02", gatt("AA:AA:AA:AA:AA:02")) - assertTrue(registry.hasOtherClientLink("off1mac", excluding = "AA:AA:AA:AA:AA:02")) + assertTrue(registry.hasOtherUsableLink("off1mac", excluding = "AA:AA:AA:AA:AA:02") { false }) assertFalse( "the kept link is never its own duplicate", - registry.hasOtherClientLink("off1mac", excluding = "AA:AA:AA:AA:AA:01"), + registry.hasOtherUsableLink("off1mac", excluding = "AA:AA:AA:AA:AA:01") { false }, ) } @Test - fun `a server link to the same peer is not a duplicate`() { + fun `an unsubscribed server link to the same peer is not a duplicate`() { val registry = MeshConnectionRegistry() registry.setDeviceIdentifier("BB:BB:BB:BB:BB:01", "off1mac") registry.trackServerConnection("BB:BB:BB:BB:BB:01") registry.registerGatt("BB:BB:BB:BB:BB:02", gatt("BB:BB:BB:BB:BB:02")) - assertFalse(registry.hasOtherClientLink("off1mac", excluding = "BB:BB:BB:BB:BB:02")) + assertFalse(registry.hasOtherUsableLink("off1mac", excluding = "BB:BB:BB:BB:BB:02") { false }) } @Test @@ -54,7 +54,7 @@ class DuplicateClientLinkTest { registry.setDeviceIdentifier("CC:CC:CC:CC:CC:01", "off1mac") registry.removeGatt("CC:CC:CC:CC:CC:01") - assertFalse(registry.hasOtherClientLink("off1mac", excluding = "CC:CC:CC:CC:CC:02")) + assertFalse(registry.hasOtherUsableLink("off1mac", excluding = "CC:CC:CC:CC:CC:02") { false }) } /** A registry holding a client link to `off1mac` at the kept address. */ @@ -104,4 +104,92 @@ class DuplicateClientLinkTest { assertFalse(registry.isSuppressedDuplicate("EE:EE:EE:EE:EE:02", nowMs = 0L)) } + + /** + * Run 5 of the 2026-10-19 volume test: the iPhone held a link to our + * server at its old address and was subscribed; its address rotated, we + * dialled the "new" device and kept it. That second Link Layer connection + * between the same two radios is what made the reconnect impossible after + * the first link dropped. A subscribed server link makes it a duplicate. + */ + @Test + fun `a subscribed server link to the peer makes a client link a duplicate`() { + val registry = MeshConnectionRegistry() + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + registry.trackServerConnection("41:9B:ED:3F:CC:72") + registry.registerGatt("4A:2A:C8:EB:B3:4D", gatt("4A:2A:C8:EB:B3:4D")) + + assertTrue( + registry.hasOtherUsableLink("off1iphone", excluding = "4A:2A:C8:EB:B3:4D") { + it == "41:9B:ED:3F:CC:72" + }, + ) + assertFalse( + "a central that never subscribed carries nothing, so our link is the usable one", + registry.hasOtherUsableLink("off1iphone", excluding = "4A:2A:C8:EB:B3:4D") { false }, + ) + } + + @Test + fun `a client link over the peer's own link to us is not a duplicate`() { + // Android reports our dial to an address the peer is connected from + // as the same address on both sides: one Link Layer connection. + val registry = MeshConnectionRegistry() + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + registry.trackServerConnection("41:9B:ED:3F:CC:72") + registry.registerGatt("41:9B:ED:3F:CC:72", gatt("41:9B:ED:3F:CC:72")) + + assertFalse( + registry.hasOtherUsableLink("off1iphone", excluding = "41:9B:ED:3F:CC:72") { true }, + ) + } + + @Test + fun `a refusal held by a subscribed server link lapses when that link goes`() { + val registry = MeshConnectionRegistry() + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + registry.trackServerConnection("41:9B:ED:3F:CC:72") + registry.suppressDuplicate("4A:2A:C8:EB:B3:4D", "off1iphone", untilMs = Long.MAX_VALUE) + val subscribed: (String) -> Boolean = { it == "41:9B:ED:3F:CC:72" } + + assertTrue(registry.isSuppressedDuplicate("4A:2A:C8:EB:B3:4D", nowMs = 0L, isSubscribed = subscribed)) + registry.untrackServerConnection("41:9B:ED:3F:CC:72") + assertFalse(registry.isSuppressedDuplicate("4A:2A:C8:EB:B3:4D", nowMs = 0L, isSubscribed = subscribed)) + } + + @Test + fun `a subscription at an address with no server link is not a link`() { + // The AND with the server connection: a stale subscription entry + // alone must never block dialling the peer. + val registry = MeshConnectionRegistry() + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + registry.registerGatt("4A:2A:C8:EB:B3:4D", gatt("4A:2A:C8:EB:B3:4D")) + + assertFalse( + registry.hasOtherUsableLink("off1iphone", excluding = "4A:2A:C8:EB:B3:4D") { true }, + ) + } + + @Test + fun `a refusal lapses when the peer unsubscribes but stays connected`() { + val registry = MeshConnectionRegistry() + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + registry.trackServerConnection("41:9B:ED:3F:CC:72") + registry.suppressDuplicate("4A:2A:C8:EB:B3:4D", "off1iphone", untilMs = Long.MAX_VALUE) + var subscribed = true + + assertTrue(registry.isSuppressedDuplicate("4A:2A:C8:EB:B3:4D", nowMs = 0L) { subscribed }) + subscribed = false + assertFalse(registry.isSuppressedDuplicate("4A:2A:C8:EB:B3:4D", nowMs = 0L) { subscribed }) + } + + @Test + fun `only identified links hold the advertisement`() { + val registry = MeshConnectionRegistry() + registry.trackServerConnection("41:9B:ED:3F:CC:72") + assertTrue(registry.establishedLinkCount() == 1) + assertTrue("an unverified central does not count", registry.identifiedLinkCount() == 0) + registry.setDeviceIdentifier("41:9B:ED:3F:CC:72", "off1iphone") + assertTrue(registry.identifiedLinkCount() == 1) + } } diff --git a/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/OutboundFragmentQueueTest.kt b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/OutboundFragmentQueueTest.kt index feec4ec01..1b28f3ae8 100644 --- a/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/OutboundFragmentQueueTest.kt +++ b/bindings/react-native/android/src/test/java/com/offlineprotocol/ble/OutboundFragmentQueueTest.kt @@ -138,6 +138,90 @@ class OutboundFragmentQueueTest { assertArrayEquals(byteArrayOf(3), sent[0]) } + @Test + fun `anyBackedUp trips when any one recipient reaches the mark`() { + // The drain checks this before every destructive pop, so it has to see + // a backed-up peer even when the fragment about to be popped is for + // someone else: the pop is one FIFO across all of them. + val h = Harness(maxPerPeer = 8) + assertFalse(h.queue.anyBackedUp()) + repeat(5) { h.queue.enqueue("alice", byteArrayOf(it.toByte())) } + h.queue.enqueue("bob", byteArrayOf(9)) + assertFalse("5 of 8 is below the 3/4 mark", h.queue.anyBackedUp()) + h.queue.enqueue("alice", byteArrayOf(6)) + assertTrue(h.queue.anyBackedUp()) + assertTrue(h.queue.isBackedUp("alice")) + assertFalse(h.queue.isBackedUp("bob")) + } + + @Test + fun `a recipient with no link does not stop the pull for others`() { + // Counting a linkless recipient parked every other peer's fragments + // in the core until its queue expired. + val h = Harness(maxPerPeer = 8) + repeat(6) { h.queue.enqueue("gone", byteArrayOf(it.toByte())) } + assertTrue(h.queue.anyBackedUp()) + assertFalse(h.queue.anyBackedUp { it != "gone" }) + repeat(6) { h.queue.enqueue("bob", byteArrayOf(it.toByte())) } + assertTrue(h.queue.anyBackedUp { it != "gone" }) + } + + @Test + fun `the pull resumes after a flush or a removal`() { + val h = Harness(maxPerPeer = 8) + repeat(6) { h.queue.enqueue("bob", byteArrayOf(it.toByte())) } + assertTrue(h.queue.anyBackedUp()) + h.queue.flush { _, _ -> true } + assertFalse(h.queue.anyBackedUp()) + repeat(6) { h.queue.enqueue("bob", byteArrayOf(it.toByte())) } + assertTrue(h.queue.anyBackedUp()) + h.queue.removeAll("bob") + assertFalse(h.queue.anyBackedUp()) + } + + @Test + fun `a cap of one still has a mark of at least one`() { + // A mark of zero read every created queue, even an empty one, as + // backed up. + val h = Harness(maxPerPeer = 1) + h.queue.enqueue("bob", byteArrayOf(0)) + h.queue.flush { _, _ -> true } + assertFalse(h.queue.anyBackedUp()) + } + + @Test + fun `a whole maximum-size message fits under the default cap with nothing discarded`() { + val h = Harness(maxPerPeer = OutboundFragmentQueue.DEFAULT_MAX_PER_PEER) + repeat(512) { h.queue.enqueue("bob", byteArrayOf(it.toByte())) } + assertTrue(h.drops.isEmpty()) + assertEquals(512, h.queue.totalCount()) + } + + @Test + fun `the default cap holds a whole maximum-size message and the mark sits below it`() { + // The core fragments a message into at most 512 pieces, and the drain + // stops pulling at the mark, so the overflow discard that tore 16 KB + // messages stays out of reach only while the mark is below the cap. + assertTrue(OutboundFragmentQueue.DEFAULT_MAX_PER_PEER >= 512) + assertTrue(OutboundFragmentQueue.DEFAULT_BACKPRESSURE_MARK < OutboundFragmentQueue.DEFAULT_MAX_PER_PEER) + } + + @Test + fun `the default queue backs up after a few seconds of link time, not three quarters of the cap`() { + // The core counts a message as queued, and defers its retry, only + // until its first fragment is pulled. A mark at 3/4 of the cap let the + // drain pull 576 fragments ahead of the link, so the core resent + // everything behind them. + val h = Harness(maxPerPeer = OutboundFragmentQueue.DEFAULT_MAX_PER_PEER) + repeat(OutboundFragmentQueue.DEFAULT_BACKPRESSURE_MARK - 1) { + h.queue.enqueue("alice", byteArrayOf(it.toByte())) + } + assertFalse(h.queue.anyBackedUp()) + h.queue.enqueue("alice", byteArrayOf(0)) + assertTrue(h.queue.isBackedUp("alice")) + assertTrue(h.queue.anyBackedUp()) + } + @Test fun `per-peer cap overflow does not affect other recipients`() { val h = Harness(maxPerPeer = 2) diff --git a/bindings/react-native/ios/BleManager.swift b/bindings/react-native/ios/BleManager.swift index a5e66a9a6..7947a0462 100644 --- a/bindings/react-native/ios/BleManager.swift +++ b/bindings/react-native/ios/BleManager.swift @@ -59,6 +59,9 @@ public class BleManager: NSObject, TransportManager { // been reported yet. Keeping the constant here would duplicate the // floor and go stale the first time Rust changes it. private let CONNECTION_TIMEOUT: TimeInterval = 10.0 + // Tears down a central link whose write buffer never drains again (see + // BleWriteStallPolicy). Without it such a link is never reported down. + private let writeStallPolicy = BleWriteStallPolicy() private let MAX_CONNECTIONS_PER_DEVICE = 4 private let ADVERTISE_RESTART_MIN: TimeInterval = 0.2 private let ADVERTISE_RESTART_MAX: TimeInterval = 1.2 @@ -101,6 +104,16 @@ public class BleManager: NSObject, TransportManager { return centralManager?.state ?? .unknown } private var discoveredPeripherals: [UUID: CBPeripheral] = [:] + /// When each peripheral last passed the mesh filter in a scan. The + /// connection monitor reads it to stop dialling a peripheral it never + /// identified once it stops advertising (`PeerLinkResolution`). Main only. + private var lastAdvertisedAt: [UUID: Date] = [:] + /// The connect attempt each pending connect timeout belongs to. A timer + /// cancels only the attempt that armed it: it captured only the + /// peripheral before, so a timer from a dial that had already failed + /// cancelled a healthy redial armed after it, and the cancel was then + /// reported as the peer lost. Main only. + private var connectTimeoutTokens: [UUID: UUID] = [:] private var peripheralRSSI: [UUID: Int16] = [:] // Peripheral (advertiser/server) components @@ -280,11 +293,17 @@ public class BleManager: NSObject, TransportManager { private let PENDING_FRAGMENT_TIMEOUT: TimeInterval = 15.0 private let PENDING_OUTBOUND_FRAGMENT_TIMEOUT: TimeInterval = 30.0 // For outbound fragments that failed to send private let MAX_PENDING_FRAGMENTS_PER_PEER = 100 + // The outbound queues' cap. Not the inbound one above: that buffer only + // holds fragments while a central's device id resolves, but an outbound + // queue must hold a whole maximum-size message below its mark (see the + // overflow note on OutboundFragmentQueue). At 100, one 16 KB message + // overran it. + private let MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER = OutboundFragmentQueue.defaultMaxPerPeer // Outbound fragments that could not be sent immediately — same contract as // `inboundFragments` above. private lazy var outboundFragments = OutboundFragmentQueue( queueCheck: { [weak self] in self?.assertOnFragmentQueue() }, - maxPerPeer: MAX_PENDING_FRAGMENTS_PER_PEER, + maxPerPeer: MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER, timeout: PENDING_OUTBOUND_FRAGMENT_TIMEOUT, onDropped: { [weak self] recipientId, reason, count in guard let self = self else { return } @@ -297,7 +316,7 @@ public class BleManager: NSObject, TransportManager { } else { self.emitDiagnostic("warning", "Pending outbound fragment queue capped, discarding queue", context: ["recipientId": recipientId, "dropped": count, - "max": self.MAX_PENDING_FRAGMENTS_PER_PEER]) + "max": self.MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER]) } } ) @@ -319,7 +338,7 @@ public class BleManager: NSObject, TransportManager { /// fragments from both: the drain would pull past the mark and the next /// `enqueue` would discard the lot. See `OutboundFragmentQueue`. private lazy var notifyFragments = OutboundFragmentQueue( - maxPerPeer: MAX_PENDING_FRAGMENTS_PER_PEER, + maxPerPeer: MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER, timeout: PENDING_OUTBOUND_FRAGMENT_TIMEOUT, onDropped: { [weak self] recipientId, reason, count in guard let self = self else { return } @@ -331,7 +350,7 @@ public class BleManager: NSObject, TransportManager { } else { self.emitDiagnostic("warning", "NOTIFY outbound queue capped, discarding queue", context: ["recipientId": recipientId, "dropped": count, - "max": self.MAX_PENDING_FRAGMENTS_PER_PEER]) + "max": self.MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER]) } } ) @@ -368,6 +387,11 @@ public class BleManager: NSObject, TransportManager { // `notifyFragments` and hands the send to the main-queue pump. private let notifyLock = NSLock() private var subscribedCentralsById: [UUID: CBCentral] = [:] + /// When each subscribed central last subscribed or wrote to our server. + /// Guarded by `notifyLock` with the map above. A subscription counts as a + /// live link in `otherLiveLinks` only while this is recent + /// (`PeerLinkResolution.subscriptionIsLive`). + private var centralLastHeardAt: [UUID: Date] = [:] private var lastMeshAdvertisement: MeshAdvertisementData? // Logging & monitoring @@ -798,6 +822,7 @@ public class BleManager: NSObject, TransportManager { // The mesh counts the same links; without this it stays full. meshController.registerAllDisconnected() discoveredPeripherals.removeAll() + lastAdvertisedAt.removeAll() peripheralRSSI.removeAll() inboundFragments.clear() outboundFragments.clear() @@ -829,7 +854,10 @@ public class BleManager: NSObject, TransportManager { estimatedMeshPeerCount = 0 notifyLock.lock() subscribedCentralsById.removeAll() + centralLastHeardAt.removeAll() notifyLock.unlock() + connectTimeoutTokens.removeAll() + writeStallPolicy.reset() pendingRestoredPeripherals = [:] @@ -1123,9 +1151,25 @@ public class BleManager: NSObject, TransportManager { guard let self = self else { return } let now = Date() for peripheral in self.discoveredPeripherals.values { - if self.connections.connectedPeripheral(for: peripheral.identifier) == nil { - self.attemptConnection(to: peripheral, reason: "monitor") + guard self.connections.connectedPeripheral(for: peripheral.identifier) == nil else { continue } + // Not every peripheral it ever saw: one never identified that + // stopped advertising is an address a peer has left, and a + // pending connect to it only crowds out the one to where the + // peer went (`PeerLinkResolution`). + let identified = self.connections.hasAnyDevice(for: peripheral.identifier) + guard PeerLinkResolution.monitorShouldRedial( + identified: identified, + lastAdvertisedAt: self.lastAdvertisedAt[peripheral.identifier], + now: now + ) else { + // Past the window it is never read again unless the + // peripheral advertises anew (which sets it afresh), so + // drop it: one entry per rotated address otherwise + // accumulated for the life of the process. + self.lastAdvertisedAt.removeValue(forKey: peripheral.identifier) + continue } + self.attemptConnection(to: peripheral, reason: "monitor") } let pendingKeys: [UUID] = self.inboundFragments.pendingIds() for centralId in pendingKeys { @@ -1165,6 +1209,15 @@ public class BleManager: NSObject, TransportManager { return } + // 1b. A known peer we already hold a link to on another identifier. + // The Link Layer completes one connection between two devices, so + // this dial would only time out, and while it is pending it holds + // the identifier the send path may still fall back to. + if let deviceId = connections.peripheralDeviceId(for: peripheral.identifier), + !otherLiveLinks(of: deviceId, excluding: peripheral.identifier).isEmpty { + return + } + // 2. Recent attempt cooldown check let now = Date() if let lastAttempt = connectionAttemptTimestamps[peripheral.identifier], now.timeIntervalSince(lastAttempt) < MIN_RECONNECT_INTERVAL { @@ -1226,6 +1279,30 @@ public class BleManager: NSObject, TransportManager { } centralManager?.connect(peripheral, options: nil) + // CoreBluetooth never times a connect out: a pending one waits until + // the peripheral answers, and while it waits every later attempt is + // refused as "already connecting". In the 2026-10-19 rerun that held + // a dead link in place for 17 minutes, because the connect to the + // peer's new advertising identity never completed. In the foreground + // a connect that has not landed in CONNECTION_TIMEOUT is cancelled so + // the next discovery can try again. In the background a pending + // connect is how iOS reconnects a peer that comes back into range, so + // it is left alone. + let attempt = UUID() + connectTimeoutTokens[peripheral.identifier] = attempt + DispatchQueue.main.asyncAfter(deadline: .now() + CONNECTION_TIMEOUT) { [weak self] in + guard let self = self, + self.connectTimeoutTokens[peripheral.identifier] == attempt, + peripheral.state == .connecting, + UIApplication.shared.applicationState == .active else { return } + self.connectTimeoutTokens.removeValue(forKey: peripheral.identifier) + print("[BleManager] Connect to \(peripheral.identifier) did not complete in \(Int(self.CONNECTION_TIMEOUT)) s; cancelling so it can be retried") + self.emitDiagnostic("warning", "BLE connect timed out; cancelled", context: [ + "identifier": peripheral.identifier.uuidString, + "timeoutSeconds": self.CONNECTION_TIMEOUT + ]) + self.centralManager?.cancelPeripheralConnection(peripheral) + } if logThrottler.shouldLog(key: "connect_attempt_\(peripheral.identifier.uuidString)", interval: 10) { print("[BleManager] Attempting connection to \(peripheral.identifier) (reason: \(reason))") var context: [String: Any] = [ @@ -1239,6 +1316,90 @@ public class BleManager: NSObject, TransportManager { } } + /// The peer just proved its id on `verified`. Any other identifier it + /// mapped from and is not connected on is an address it rotated away + /// from: forget it, so neither the send path nor the monitor dials it + /// again, and cancel a connect still pending to it. Main only. + private func retireSupersededPeripherals(of deviceId: String, verifiedOn verified: UUID) { + for identifier in connections.retirePeripheralIdentifiers(supersededBy: verified, deviceId: deviceId) { + let peripheral = discoveredPeripherals.removeValue(forKey: identifier) + lastAdvertisedAt.removeValue(forKey: identifier) + connectionAttemptTimestamps.removeValue(forKey: identifier) + connectionRetryCount.removeValue(forKey: identifier) + peripheralRSSI.removeValue(forKey: identifier) + if let peripheral, peripheral.state == .connecting { + centralManager?.cancelPeripheralConnection(peripheral) + } + print("[BleManager] Retired \(identifier) for \(deviceId): verified on \(verified)") + emitDiagnostic("info", "Retired a superseded peripheral identifier", context: [ + "identifier": identifier.uuidString, + "verifiedOn": verified.uuidString, + "deviceId": deviceId + ]) + } + } + + /// The other identifiers `deviceId` holds a usable link on (see + /// `PeerLinkResolution.otherLiveLinks`). Any thread. + private func otherLiveLinks(of deviceId: String, excluding identifier: UUID) -> [UUID] { + let now = Date() + notifyLock.lock() + let subscribed = Set(subscribedCentralsById.keys.filter { + PeerLinkResolution.subscriptionIsLive(lastHeardAt: centralLastHeardAt[$0], now: now) + }) + notifyLock.unlock() + return connections.otherLiveLinks( + of: deviceId, + excluding: identifier, + isSubscribed: { subscribed.contains($0) } + ) + } + + private func isSubscribedCentral(_ identifier: UUID) -> Bool { + notifyLock.lock() + defer { notifyLock.unlock() } + return subscribedCentralsById[identifier] != nil + } + + /// A subscribed central subscribed or wrote: its link is up. + private func noteCentralHeard(_ identifier: UUID) { + notifyLock.lock() + if subscribedCentralsById[identifier] != nil { + centralLastHeardAt[identifier] = Date() + } + notifyLock.unlock() + } + + /// Retires `identifier` when the peer it maps to is still linked on + /// another one, instead of reporting the peer lost or redialling it. + /// Returns whether it did. Main only. + @discardableResult + private func retireIfLinkedElsewhere(_ identifier: UUID, wasConnected: Bool) -> Bool { + guard let deviceId = connections.peripheralDeviceId(for: identifier), + let kept = otherLiveLinks(of: deviceId, excluding: identifier).first else { + return false + } + connectionRetryCount.removeValue(forKey: identifier) + _ = connections.consumePendingRole(for: identifier) + retireSupersededPeripherals(of: deviceId, verifiedOn: kept) + // The ACL link to this identifier is gone, so a central-role mapping + // for it keys nothing, unless it is itself a subscribed central. + if !isSubscribedCentral(identifier) { + connections.removeCentralDeviceId(for: identifier) + } + // The peer's egress MTU was whichever link verified last; it may + // have been this one. + reflectEgressMtu(forDeviceId: deviceId) + refreshSelfMetrics() + emitDiagnostic("info", "BLE link dropped; peer still linked on another identifier", context: [ + "identifier": identifier.uuidString, + "kept": kept.uuidString, + "deviceId": deviceId, + "wasConnected": wasConnected + ]) + return true + } + private func ensureDeviceId(for centralId: UUID) { DispatchQueue.main.async { [weak self] in guard let self = self else { return } @@ -1685,7 +1846,26 @@ public class BleManager: NSObject, TransportManager { // and the fragments waited for the expiry that tears them. var hitBackpressure = self.flushPendingOutboundFragments() - while let fragment = self.protocolInstance.bleGetNextFragment() { + while true { + // Stop before the pop, not after it. The pop is destructive and + // shared by every peer, so a fragment pulled while some queue + // is past its mark has nowhere to go but further past it, and + // the post-enqueue checks below only see that once it has + // happened. Both egress queues count: either one discarding + // tears a message. + // + // Only a recipient with a link counts: one with none cannot + // drain, so counting it stopped every other peer until its + // fragments expired (`OutboundFragmentQueue.anyBackedUp`). + let deliverable: (String) -> Bool = { recipient in + self.findPeripheral(for: recipient) != nil || self.notifyTarget(for: recipient) != nil + } + if self.outboundFragments.anyBackedUp(deliverable: deliverable) + || self.notifyFragments.anyBackedUp(deliverable: deliverable) { + hitBackpressure = true + break + } + guard let fragment = self.protocolInstance.bleGetNextFragment() else { break } let recipientId = fragment.recipientId let data = Data(fragment.data) @@ -1741,7 +1921,7 @@ public class BleManager: NSObject, TransportManager { // destructive pop) into the bounded per-peer queue. The // write buffer drains far slower than this loop can pull; // without this stop the loop spins the whole backlog into - // the queue, overflows MAX_PENDING_FRAGMENTS_PER_PEER, and + // the queue, overflows MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER, and // OutboundFragmentQueue.enqueue discards the in-flight // message. Leaving the backlog in Rust keeps delivery // lossless. Mirrors Android's drainAndSendFragments. @@ -1760,18 +1940,13 @@ public class BleManager: NSObject, TransportManager { // arms it too, so the floor is there before this queue // reaches the mark. // - // The mark is checked after the enqueue because the pop is - // destructive and nothing can hand a fragment back to Rust, - // so each drain that meets a stalled peer at the head adds - // one fragment past it. The headroom to the cap is a budget - // of such drains, not a guarantee: a peer stalled while - // other peers keep drains coming can still reach the cap, - // and the whole-queue discard is then the intended outcome. - // Those fragments were headed for the 30 s expiry anyway, - // which drops them fragment by fragment and can tear a - // message; the discard never does. Holding the fragment - // back in Swift instead would trade that for every other - // peer waiting behind a stalled one. + // This check runs after the enqueue; the one at the top of + // the loop is what stops the next pop. Together they keep + // a stalled peer's queue at the mark plus at most one + // fragment, far below the cap, however many drains other + // peers trigger. The cost is that every peer waits while + // one is backed up, because the pop is one FIFO; the + // re-drain below is the floor that resumes it. if self.outboundFragments.isBackedUp(recipientId) { hitBackpressure = true break @@ -2028,7 +2203,19 @@ public class BleManager: NSObject, TransportManager { } private func evictPeer(_ deviceId: String, reason: String) { - guard let identifier = connections.peripheralIdentifier(for: deviceId) else { + // Every identifier the peer maps from, not one. A peer can hold two + // (ADR 0030: a second link persists with a peer the bridge does not + // control, such as a Mac rotating its address). Evicting one of them + // and then reporting the peer lost dropped it in the core while the + // other link stayed up, and nothing re-announced it until that link + // cycled. Each pass unmaps the identifier it took, so this ends. + var identifiers: [UUID] = [] + while identifiers.count < 16, let identifier = connections.peripheralIdentifier(for: deviceId) { + identifiers.append(identifier) + connections.removePeripheralDeviceId(for: identifier) + connections.removeCentralDeviceId(for: identifier) + } + guard !identifiers.isEmpty else { if logThrottler.shouldLog(key: "mesh_evict_missing_\(deviceId)") { print("[BleManager] Cannot evict \(deviceId): missing identifier") } @@ -2036,23 +2223,25 @@ public class BleManager: NSObject, TransportManager { } if logThrottler.shouldLog(key: "mesh_evict_\(deviceId)", interval: 5) { - print("[BleManager] Evicting \(deviceId) to reclaim capacity (reason: \(reason))") + print("[BleManager] Evicting \(deviceId) to reclaim capacity (reason: \(reason), identifiers: \(identifiers.count))") } - if let peripheral = connections.connectedPeripheral(for: identifier) { - centralManager?.cancelPeripheralConnection(peripheral) - } - - _ = connections.removePeripheral(identifier) - connections.removePeripheralDeviceId(for: identifier) - connections.removeCentralDeviceId(for: identifier) connections.removeConnectionRole(for: deviceId) - peripheralRSSI.removeValue(forKey: identifier) - // Removals are synchronous and thread-safe, so `refreshSelfMetrics()` - // below still sees the post-removal counts — without the - // `fragmentQueue.sync` that used to park the main thread here. - inboundFragments.removeAll(identifier) outboundFragments.removeAll(deviceId) + for identifier in identifiers { + if let peripheral = connections.connectedPeripheral(for: identifier) { + centralManager?.cancelPeripheralConnection(peripheral) + } + _ = connections.removePeripheral(identifier) + peripheralRSSI.removeValue(forKey: identifier) + // Removals are synchronous and thread-safe, so `refreshSelfMetrics()` + // below still sees the post-removal counts, without the + // `fragmentQueue.sync` that used to park the main thread here. + inboundFragments.removeAll(identifier) + connectionAttemptTimestamps.removeValue(forKey: identifier) + connectTimeoutTokens.removeValue(forKey: identifier) + connectionRetryCount.removeValue(forKey: identifier) + } // A fragment block dispatched before this point runs after the removals // above and would resurrect the evicted peer's buffer. Chase it on the // owning queue: serial and FIFO, so this drops exactly the backlog that @@ -2061,12 +2250,13 @@ public class BleManager: NSObject, TransportManager { // — same deinit rule as `stopUnsafe`. let inbound = inboundFragments let outbound = outboundFragments + let evicted = identifiers fragmentQueue.async { - inbound.removeAll(identifier) + for identifier in evicted { + inbound.removeAll(identifier) + } outbound.removeAll(deviceId) } - connectionAttemptTimestamps.removeValue(forKey: identifier) - connectionRetryCount.removeValue(forKey: identifier) meshController.registerDisconnection(peerId: deviceId) refreshSelfMetrics() @@ -2141,11 +2331,29 @@ public class BleManager: NSObject, TransportManager { print("[BleManager] Cannot send fragment yet, write buffer full for recipient: \(recipientId)") emitDiagnostic("info", "BLE write buffer full, retrying", context: ["recipientId": recipientId]) } + if writeStallPolicy.noteRefused(peripheral.identifier) { + print("[BleManager] ⚠️ Write buffer full for \(Int(BleWriteStallPolicy.defaultStallSeconds)) s on \(peripheral.identifier); tearing the link down to reconnect") + // Which links the peer had when the write side died, for + // the open question of what stalls it (a second link + // between the same two radios is the suspect). + let others = otherLiveLinks(of: recipientId, excluding: peripheral.identifier) + print("[BleManager] Stall on \(peripheral.identifier): peer's other live links \(others.map { $0.uuidString })") + emitDiagnostic("warning", "BLE link refused every write; tearing it down", context: [ + "recipientId": recipientId, + "identifier": peripheral.identifier.uuidString, + "stallSeconds": BleWriteStallPolicy.defaultStallSeconds, + "otherLiveLinks": others.map { $0.uuidString } + ]) + DispatchQueue.main.async { [weak self] in + self?.centralManager?.cancelPeripheralConnection(peripheral) + } + } return false } } peripheral.writeValue(data, for: characteristic, type: .withoutResponse) + writeStallPolicy.noteWritable(peripheral.identifier) meshController.markPeerActive(recipientId) meshController.markPeerActive(deviceId) return true @@ -3070,6 +3278,7 @@ extension BleManager: CBCentralManagerDelegate { // unknown-bootstrap path below it is rate limited. Recording above it // would spend the 200-entry cap on passing headphones. peripheralRestorationPolicy.recordSeen(uuid: peripheral.identifier, at: now, source: .advertisement) + lastAdvertisedAt[peripheral.identifier] = now // Mesh density is counted here too, for the same reasons: after the // gate, so it counts mesh candidates and not every Bluetooth device in @@ -3194,6 +3403,8 @@ extension BleManager: CBCentralManagerDelegate { // objects of an earlier one are not this link's. clearServiceInstanceSelection(for: peripheral.identifier) connectionAttemptTimestamps.removeValue(forKey: peripheral.identifier) + connectTimeoutTokens.removeValue(forKey: peripheral.identifier) + writeStallPolicy.forget(peripheral.identifier) connectionRetryCount.removeValue(forKey: peripheral.identifier) // Reset retry count on successful connection // `.linkActivity`, not `.advertisement`: a completed connect proves // this peer was reachable, not that the app was scanning, so it @@ -3218,7 +3429,15 @@ extension BleManager: CBCentralManagerDelegate { "retryCount": retryCount ]) connectionAttemptTimestamps.removeValue(forKey: peripheral.identifier) + connectTimeoutTokens.removeValue(forKey: peripheral.identifier) _ = connections.consumePendingRole(for: peripheral.identifier) + + // The same rule as `didDisconnectPeripheral`: a peer linked on + // another identifier is not redialled here (the dial cannot complete + // while that link stands), and this identifier is retired. + if retireIfLinkedElsewhere(peripheral.identifier, wasConnected: false) { + return + } // Give up after max retries guard retryCount <= MAX_CONNECTION_RETRIES else { @@ -3240,8 +3459,9 @@ extension BleManager: CBCentralManagerDelegate { // `discoveredPeripherals` was dropped deliberately — by `stop()`, // or by the restoration age-out, which cancels the OS-side connect // request precisely so it stops being retried. Reconnecting here - // re-arms what the age-out just cleared, and a cancelled pending - // connect is a plausible source of this very callback. + // re-arms what the age-out just cleared. (CoreBluetooth reports a + // cancelled pending connect through `didDisconnectPeripheral`; + // this guard holds whichever callback a cancel arrives on.) guard self.discoveredPeripherals[peripheral.identifier] != nil else { return } self.attemptConnection(to: peripheral, reason: "retry_fail") } @@ -3250,6 +3470,11 @@ extension BleManager: CBCentralManagerDelegate { public func centralManager(_ central: CBCentralManager, didDisconnectPeripheral peripheral: CBPeripheral, error: Error?) { let wasConnected = connections.connectedPeripheral(for: peripheral.identifier) != nil _ = connections.removePeripheral(peripheral.identifier) + connectTimeoutTokens.removeValue(forKey: peripheral.identifier) + // The link is gone, so is its write stall. A refusal between the + // teardown verdict and this callback re-armed the clock, and the + // identifier survives the reconnect. + writeStallPolicy.forget(peripheral.identifier) // Drop the handshake state with the link. Keeping it would let a // reconnect pair a device id read from the previous connection with an @@ -3271,6 +3496,20 @@ extension BleManager: CBCentralManagerDelegate { ]) } + // The peer is still reachable on another identifier: this one is an + // address it left, or a second link to it. Retire it instead of + // reporting the peer lost or redialling. Both branches below used to + // do one of those, and in run 5 of the 2026-10-19 volume test a + // redial of the link the stall teardown cancelled timed out, landed + // here as "wasn't connected", and reported Android lost while the + // iPhone held a second, working link to it: the core refused every + // send for ten minutes. CoreBluetooth reports a cancelled connect + // here, so this covers the connect timeout as well; `didFailToConnect` + // applies the same rule in case a cancel ever arrives there. + if retireIfLinkedElsewhere(peripheral.identifier, wasConnected: wasConnected) { + return + } + // Don't remove from discovered list - keep trying to reconnect // Only remove RSSI if it's a permanent error if let error = error as? CBError, error.code == .connectionTimeout { @@ -3336,12 +3575,26 @@ extension BleManager: CBCentralManagerDelegate { } } } else { - // Wasn't connected, just notify if we had device ID + // A dial that never connected: a reconnect that failed, or one the + // connect timeout cancelled. The peer is reported lost once, and + // the identifier is unmapped, so it is no longer "identified" and + // the monitor re-dials it only while it is still advertising + // (`PeerLinkResolution.monitorShouldRedial`). It stayed mapped + // before, so a verified peer that had walked away was dialled, + // cancelled at 10 s and reported lost again on every monitor + // tick for as long as the app was in the foreground, restarting + // the advertisement each time. A peer that comes back is + // re-identified on its next verified link. if let deviceId = connections.peripheralDeviceId(for: peripheral.identifier) { notifyBlePeerLost(deviceId: deviceId) meshController.registerDisconnection(peerId: deviceId) refreshSelfMetrics() connections.removeConnectionRole(for: deviceId) + connections.removePeripheralDeviceId(for: peripheral.identifier) + if !isSubscribedCentral(peripheral.identifier) { + connections.removeCentralDeviceId(for: peripheral.identifier) + } + connectionRetryCount.removeValue(forKey: peripheral.identifier) DispatchQueue.main.async { self.refreshAdvertising(reason: "disconnect") } @@ -3668,6 +3921,7 @@ extension BleManager: CBPeripheralDelegate { connections.setPeripheralDeviceId(peerId, for: peripheral.identifier) connections.setCentralDeviceId(peerId, for: peripheral.identifier) connectionAttemptTimestamps.removeValue(forKey: peripheral.identifier) + retireSupersededPeripherals(of: peerId, verifiedOn: peripheral.identifier) // ORDERING INVARIANT — DO NOT REORDER. // @@ -4074,6 +4328,7 @@ extension BleManager: CBPeripheralDelegate { /// Flow-control signal: the BLE write buffer has drained for this peripheral. /// Resume sending queued fragments instead of waiting for a timer tick. public func peripheralIsReady(toSendWriteWithoutResponse peripheral: CBPeripheral) { + writeStallPolicy.noteWritable(peripheral.identifier) guard let recipientId = connections.peripheralDeviceId(for: peripheral.identifier) else { return } if logThrottler.shouldLog(key: "flow_ready_\(recipientId)", interval: 1.0) { emitDiagnostic("debug", "BLE write buffer drained, resuming sends", context: ["recipientId": recipientId]) @@ -4276,6 +4531,7 @@ extension BleManager: CBPeripheralManagerDelegate { public func peripheralManager(_ peripheral: CBPeripheralManager, didReceiveWrite requests: [CBATTRequest]) { for request in requests { + noteCentralHeard(request.central.identifier) print("[BleManager] 📨 GATT WRITE REQUEST from \(request.central.identifier), char: \(request.characteristic.uuid), size: \(request.value?.count ?? 0)") emitDiagnostic("info", "GATT write request received", context: [ "centralId": request.central.identifier.uuidString, @@ -4360,6 +4616,7 @@ extension BleManager: CBPeripheralManagerDelegate { // fragment drain tests notify-reachability off the main queue. notifyLock.lock() subscribedCentralsById[central.identifier] = central + centralLastHeardAt[central.identifier] = Date() notifyLock.unlock() print("[BleManager] Central subscribed: \(central.identifier)") emitDiagnostic("info", "Central subscribed to characteristic", context: [ @@ -4383,6 +4640,7 @@ extension BleManager: CBPeripheralManagerDelegate { ?? connections.peripheralDeviceId(for: central.identifier) notifyLock.lock() subscribedCentralsById.removeValue(forKey: central.identifier) + centralLastHeardAt.removeValue(forKey: central.identifier) notifyLock.unlock() emitDiagnostic("info", "Central unsubscribed", context: [ "central": central.identifier.uuidString, diff --git a/bindings/react-native/ios/BleWriteStallPolicy.swift b/bindings/react-native/ios/BleWriteStallPolicy.swift new file mode 100644 index 000000000..63b149c76 --- /dev/null +++ b/bindings/react-native/ios/BleWriteStallPolicy.swift @@ -0,0 +1,83 @@ +// +// BleWriteStallPolicy.swift +// OfflineProtocol +// +// Decides when a central link whose write buffer never drains is dead and +// must be torn down so it can reconnect. +// +// A write-without-response is refused while CoreBluetooth's buffer for the +// peripheral is full, and `peripheralIsReady(toSendWriteWithoutResponse:)` +// says when it has room again. Under load that cycle turns over in +// milliseconds. In the 2026-10-19 volume rerun it stopped: the buffer stayed +// full for 17 minutes while the same link kept delivering the peer's +// notifications, so nothing ever reported the link as down, every fragment +// queued for the peer expired, and the peer's acknowledgements never left. +// The link recovered only when an unrelated connection to the peer's next +// advertising identity happened to complete. +// +// The rule is time without a successful write, not a count of refusals: a +// refusal is normal flow control, and a busy link refuses constantly while +// still moving. Any accepted write, or a ready callback, ends the stall. +// Tearing a link down costs one reconnect, so the threshold sits far above +// any real backpressure wait and well below the 30 s fragment expiry. +// +// Safe from any thread: refusals are seen on the fragment queue and ready +// callbacks on main. +// + +import Foundation + +final class BleWriteStallPolicy { + /// How long a link may refuse every write before it is declared dead. + static let defaultStallSeconds: TimeInterval = 15 + + private let stallSeconds: TimeInterval + private let lock = NSLock() + private var blockedSince: [UUID: Date] = [:] + + init(stallSeconds: TimeInterval = BleWriteStallPolicy.defaultStallSeconds) { + self.stallSeconds = stallSeconds + } + + /// Records a refused write on `peripheral`. Returns true once the link + /// has refused everything for the threshold or longer: the caller tears + /// it down. The stall is forgotten when this returns true, so one dead + /// link produces one teardown. + func noteRefused(_ peripheral: UUID, now: Date = Date()) -> Bool { + lock.lock() + defer { lock.unlock() } + guard let since = blockedSince[peripheral] else { + blockedSince[peripheral] = now + return false + } + guard now.timeIntervalSince(since) >= stallSeconds else { return false } + blockedSince[peripheral] = nil + return true + } + + /// The link took a write, or said it has room: it is moving. + func noteWritable(_ peripheral: UUID) { + lock.lock() + blockedSince[peripheral] = nil + lock.unlock() + } + + /// The link is gone, or has just connected: a reconnect starts from a + /// clean slate. Called from `didConnect` and `didDisconnectPeripheral`. + /// + /// Without it the clock outlived the teardown. The cancel that ends a + /// stalled link is an asynchronous hop to main, and the redrain floor can + /// refuse another write in between, re-arming the clock for an identifier + /// CoreBluetooth keeps across the reconnect. The fresh link then met one + /// refusal before its first accepted write and was torn down at once. + func forget(_ peripheral: UUID) { + noteWritable(peripheral) + } + + /// Forgets every link: the bridge stopped, or the radio went away. + func reset() { + lock.lock() + blockedSince.removeAll() + lock.unlock() + } +} diff --git a/bindings/react-native/ios/OutboundFragmentQueue.swift b/bindings/react-native/ios/OutboundFragmentQueue.swift index 7befdf734..6749ec57c 100644 --- a/bindings/react-native/ios/OutboundFragmentQueue.swift +++ b/bindings/react-native/ios/OutboundFragmentQueue.swift @@ -57,8 +57,33 @@ import Foundation /// the sender's higher layer (ack-driven retry, data-sync anti-entropy) /// re-drives them. /// -/// Overflow should be rare: `isBackedUp` is the drain loop's signal to stop -/// pulling from the Rust core well before the cap is reached. +/// The cap is a memory backstop and must stay unreachable in normal +/// operation, because the discard is still a loss and it tears the message +/// whose first slices already went out. The drain stops pulling from the Rust +/// core while any recipient of either queue with a link to drain into is past +/// the `isBackedUp` mark (`anyBackedUp`), checked **before** each pop, since +/// the pop is destructive and nothing can hand a fragment back. Every fragment +/// here arrived through one such pop, so a linked recipient never holds more +/// than the mark, and the mark sits below the cap. A recipient with no link is +/// left out of the stop (see `anyBackedUp`), so only its undeliverable queue +/// can reach the cap. The cap also sits above the largest message the core +/// will fragment (512 fragments). With the old cap of 100, one sealed 16 KB +/// message was more than the queue could hold. +/// +/// ### The mark is small on purpose +/// +/// `defaultBackpressureMark` is a few seconds of link time, not a fraction of +/// the cap, because the Rust core can see only what it still holds. A message +/// counts as queued, and its acknowledgement timer is extended instead of +/// retried, until its first fragment is pulled. With the mark at 3/4 of the +/// cap a drain could pull 576 fragments ahead of a slow link, the core saw +/// nothing queued, and everything behind the first few seconds was resent: +/// Android's mirror held exactly 576 in the third volume run, and resent +/// 19,404 times. A fragment left in +/// Rust costs nothing; one pulled early only waits here, expires after +/// `timeout`, and tears its message. +/// +/// Rust guards pin the cap and the mark equal to Android's. final class OutboundFragmentQueue: @unchecked Sendable { enum DropReason { @@ -81,6 +106,7 @@ final class OutboundFragmentQueue: @unchecked Sendable { private let queueCheck: () -> Void private let maxPerPeer: Int + private let backpressureMark: Int private let timeout: TimeInterval private let clock: () -> Date private let onDropped: (String, DropReason, Int) -> Void @@ -88,12 +114,18 @@ final class OutboundFragmentQueue: @unchecked Sendable { init( queueCheck: @escaping () -> Void = {}, maxPerPeer: Int = OutboundFragmentQueue.defaultMaxPerPeer, + // Below `maxPerPeer`; see the class note. Smaller caps keep the old 3/4. + backpressureMark: Int? = nil, timeout: TimeInterval = OutboundFragmentQueue.defaultTimeout, clock: @escaping () -> Date = Date.init, onDropped: @escaping (String, DropReason, Int) -> Void = { _, _, _ in } ) { self.queueCheck = queueCheck self.maxPerPeer = maxPerPeer + // At least one: a mark of zero (a cap of 1) reads every created queue, + // even an empty one, as backed up. + self.backpressureMark = max(1, backpressureMark + ?? min(OutboundFragmentQueue.defaultBackpressureMark, maxPerPeer * 3 / 4)) self.timeout = timeout self.clock = clock self.onDropped = onDropped @@ -123,8 +155,8 @@ final class OutboundFragmentQueue: @unchecked Sendable { return !(queues[recipientId]?.isEmpty ?? true) } - /// True once `recipientId`'s queue has filled past a high-water mark - /// (3/4 of `maxPerPeer`). The drain loop uses this as a backpressure + /// True once `recipientId`'s queue has filled to `backpressureMark`. The + /// drain loop uses this as a backpressure /// signal to STOP pulling more fragments out of the Rust core — which is /// a destructive pop — into this bounded queue. Without it, when /// CoreBluetooth's write buffer paces sends slower than the loop can pull, @@ -137,7 +169,29 @@ final class OutboundFragmentQueue: @unchecked Sendable { func isBackedUp(_ recipientId: String) -> Bool { lock.lock() defer { lock.unlock() } - return (queues[recipientId]?.count ?? 0) >= maxPerPeer * 3 / 4 + return (queues[recipientId]?.count ?? 0) >= backpressureMark + } + + /// True if any recipient that `deliverable` accepts is past the + /// `isBackedUp` mark. The drain checks it before every pop from the Rust + /// core: the pop is one FIFO across all peers and cannot be undone, so + /// the only lossless moment to stop is before it. Mirrors Android's + /// `anyBackedUp`. Safe from any thread. + /// + /// Only a recipient with a link to drain into may stop the pull. One with + /// none (a peer whose link closed, in the grace before it is reported + /// lost) keeps the fragments the core still hands out for it until they + /// expire, and counting it stopped every other peer's egress until then, + /// in 30-second rounds, for as long as the core held its backlog. Such a + /// recipient's queue can pass the mark and, at worst, reach the cap: its + /// fragments cannot be delivered either way, and the core's acknowledgement + /// timeout resends them once a link returns. `deliverable` is called + /// outside this queue's lock. + func anyBackedUp(deliverable: (String) -> Bool = { _ in true }) -> Bool { + lock.lock() + let full = queues.compactMap { $0.value.count >= backpressureMark ? $0.key : nil } + lock.unlock() + return full.contains(where: deliverable) } // MARK: - Owning-queue mutations @@ -258,6 +312,9 @@ final class OutboundFragmentQueue: @unchecked Sendable { return hasUnsent } - static let defaultMaxPerPeer = 100 + /// Above 4/3 of the core's 512-fragment message ceiling; see the class note. + static let defaultMaxPerPeer = 768 + /// About 2.5 s of an indication-paced link; see the class note. + static let defaultBackpressureMark = 32 static let defaultTimeout: TimeInterval = 30.0 } diff --git a/bindings/react-native/ios/Package.swift b/bindings/react-native/ios/Package.swift index a7097c365..d8a4404b9 100644 --- a/bindings/react-native/ios/Package.swift +++ b/bindings/react-native/ios/Package.swift @@ -62,6 +62,7 @@ let package = Package( "BleAppTag.swift", "BleDensityPolicy.swift", "BleHelloPolicy.swift", + "BleWriteStallPolicy.swift", "BleMessageNotificationPolicy.swift", "BleServiceInstanceSelection.swift", "EncryptionConfigReader.swift", @@ -77,6 +78,7 @@ let package = Package( "NostrQueryTracker.swift", "OutboundFragmentQueue.swift", "PeerIdentityBinding.swift", + "PeerLinkResolution.swift", "PeerStreamFraming.swift", "PeerStreamSession.swift", "GatewayAttachPolicy.swift", @@ -121,6 +123,7 @@ let package = Package( "BleAppTagTests.swift", "BleDensityPolicyTests.swift", "BleHelloPolicyTests.swift", + "BleWriteStallPolicyTests.swift", "BleMessageNotificationPolicyTests.swift", "BleServiceInstanceSelectionTests.swift", "EncryptionConfigReaderTests.swift", @@ -138,6 +141,7 @@ let package = Package( "NostrQueryTrackerTests.swift", "OutboundFragmentQueueTests.swift", "PeerIdentityBindingTests.swift", + "PeerLinkResolutionTests.swift", "PeerStreamDialPolicyTests.swift", "PeerStreamFramingTests.swift", "PeerStreamReaderTests.swift", diff --git a/bindings/react-native/ios/PeerLinkResolution.swift b/bindings/react-native/ios/PeerLinkResolution.swift new file mode 100644 index 000000000..114522d40 --- /dev/null +++ b/bindings/react-native/ios/PeerLinkResolution.swift @@ -0,0 +1,146 @@ +// +// PeerLinkResolution.swift +// OfflineProtocol +// +// Which of a peer's peripheral identifiers the bridge sends on, retires and +// keeps dialling, once the peer has been seen under more than one. +// +// A peer that rotates its advertising address reappears to CoreBluetooth as a +// new CBPeripheral with a new identifier, and the bridge verifies it there +// while the identifier it held before still maps to the same device id. In +// the third 2026-10-19 volume run that left two identifiers mapped to the +// Android peer. The send path took whichever a dictionary walk met first, +// which was the stale one, so every fragment for the peer found "no connected +// peripheral" and re-dialled an address nobody advertised any more, for 25 +// minutes, while the verified link to the same peer sat idle. The connection +// monitor meanwhile kept a dozen connects pending to identifiers that had +// stopped advertising, each cancelled and re-armed by the 10 s connect +// timeout. +// +// Four rules, each closing one of those or the failure named on +// `otherLiveLinks`: +// - Send on a connected link when the peer has one, whatever else maps to it. +// - Verifying the peer on one identifier retires every other identifier that +// maps to it and is not connected: it is an address the peer has left. +// - The monitor re-dials a peripheral it never identified only while it is +// still advertising. One that was identified is retired by the rule above +// instead, so the monitor still reconnects a known peer that went quiet. +// - A peer with another live link is never reported lost when one of its +// identifiers drops, and an identifier of such a peer is never dialled. A +// subscribed central is a live link only while it is heard from. +// +// Pure functions over snapshots, so the rules are testable without +// CoreBluetooth. +// + +import Foundation + +enum PeerLinkResolution { + /// How long a never-identified peripheral stays worth dialling after its + /// last advertisement. Above the 30 s an idle scan can go between + /// sightings of a peer that is still there, and far below the hours a + /// stale identifier otherwise lingers. + static let unidentifiedRedialWindow: TimeInterval = 60 + + /// The identifier to send `deviceId`'s fragments on: a connected one if + /// the peer has one, else any peripheral-role identifier, else a + /// central-role one. Ties break on the identifier's string so the answer + /// does not depend on dictionary order. + static func preferredIdentifier( + for deviceId: String, + peripheralDeviceIds: [UUID: String], + centralDeviceIds: [UUID: String], + isConnected: (UUID) -> Bool + ) -> UUID? { + let peripheralMatches = matches(deviceId, in: peripheralDeviceIds) + if let connected = peripheralMatches.first(where: isConnected) { + return connected + } + if let any = peripheralMatches.first { + return any + } + return matches(deviceId, in: centralDeviceIds).first + } + + /// The peripheral-role identifiers that verifying `deviceId` on + /// `verified` retires: every other one mapped to the same device that is + /// not connected. A connected one is a live second link and stays. + static func superseded( + by verified: UUID, + deviceId: String, + peripheralDeviceIds: [UUID: String], + isConnected: (UUID) -> Bool + ) -> [UUID] { + matches(deviceId, in: peripheralDeviceIds).filter { $0 != verified && !isConnected($0) } + } + + /// Whether the connection monitor should dial a peripheral it holds and + /// is not connected to. + static func monitorShouldRedial( + identified: Bool, + lastAdvertisedAt: Date?, + now: Date, + window: TimeInterval = PeerLinkResolution.unidentifiedRedialWindow + ) -> Bool { + if identified { return true } + guard let seen = lastAdvertisedAt else { return false } + return now.timeIntervalSince(seen) < window + } + + /// How recently a subscribed central must have been heard from (its + /// subscription, or a write to our server) to count as a live link in + /// `otherLiveLinks`. A connected peripheral is known live from + /// CoreBluetooth's own state; a subscription is not: it ends only when + /// `didUnsubscribeFrom` arrives. One that never arrived would otherwise + /// keep a departed peer a neighbour for good, since the gate then retires + /// its real identifiers, refuses every dial and reports nothing lost. A + /// quiet but live central that misses the window costs only the older + /// behaviour: the peer is reported lost and re-announced on its next + /// verified frame. Under load the peer's acknowledgements arrive several + /// times a second, so a working link never misses it. + static let subscribedCentralLivenessWindow: TimeInterval = 60 + + /// Whether a subscribed central last heard from at `lastHeardAt` counts + /// as a live link at `now`. + static func subscriptionIsLive( + lastHeardAt: Date?, + now: Date, + window: TimeInterval = PeerLinkResolution.subscribedCentralLivenessWindow + ) -> Bool { + guard let heard = lastHeardAt else { return false } + return now.timeIntervalSince(heard) < window + } + + /// The identifiers, other than `excluding`, on which `deviceId` holds a + /// link the bridge can use right now: a connected peripheral it verified, + /// or a verified central subscribed to our server (the notify path) and + /// heard from recently (`subscriptionIsLive`). + /// + /// A non-empty answer means the peer is not lost when `excluding` goes, + /// and that dialling `excluding` again cannot succeed: two devices hold + /// one Link Layer connection between them (Core 5.4, Vol 6 Part B, + /// 1.1.1), so a dial from an address the peer already has a link with is + /// never completed. In run 5 of the 2026-10-19 volume test a redial of a + /// dead identifier timed out, was reported as the peer lost while the + /// iPhone held a second, working link to it, and the core refused every + /// send for ten minutes. + static func otherLiveLinks( + of deviceId: String, + excluding: UUID, + peripheralDeviceIds: [UUID: String], + centralDeviceIds: [UUID: String], + isConnected: (UUID) -> Bool, + isSubscribed: (UUID) -> Bool + ) -> [UUID] { + let peripherals = matches(deviceId, in: peripheralDeviceIds) + .filter { $0 != excluding && isConnected($0) } + let centrals = matches(deviceId, in: centralDeviceIds) + .filter { $0 != excluding && isSubscribed($0) && !peripherals.contains($0) } + return peripherals + centrals + } + + private static func matches(_ deviceId: String, in map: [UUID: String]) -> [UUID] { + map.compactMap { $0.value == deviceId ? $0.key : nil } + .sorted { $0.uuidString < $1.uuidString } + } +} diff --git a/bindings/react-native/ios/ble/MeshConnectionRegistry.swift b/bindings/react-native/ios/ble/MeshConnectionRegistry.swift index 15a0c6373..eaad2d154 100644 --- a/bindings/react-native/ios/ble/MeshConnectionRegistry.swift +++ b/bindings/react-native/ios/ble/MeshConnectionRegistry.swift @@ -75,15 +75,60 @@ final class MeshConnectionRegistry: @unchecked Sendable { return removed } + /// The identifier to send `deviceId`'s fragments on. A peer that rotated + /// its address maps from more than one, and the connected one must win: + /// see `PeerLinkResolution`. func peripheralIdentifier(for deviceId: String) -> UUID? { lock.lock() - let match = - peripheralDeviceIds.first(where: { $0.value == deviceId })?.key - ?? centralDeviceIds.first(where: { $0.value == deviceId })?.key + let connected = connectedPeripherals + let match = PeerLinkResolution.preferredIdentifier( + for: deviceId, + peripheralDeviceIds: peripheralDeviceIds, + centralDeviceIds: centralDeviceIds, + isConnected: { connected[$0] != nil } + ) lock.unlock() return match } + /// Unmaps every other disconnected peripheral identifier of `deviceId`, + /// now verified on `verified`, and returns them so the caller can stop + /// dialling them. Central-role mappings are left alone: they key inbound + /// fragments from a link the peer opened, which this says nothing about. + func retirePeripheralIdentifiers(supersededBy verified: UUID, deviceId: String) -> [UUID] { + lock.lock() + let connected = connectedPeripherals + let retired = PeerLinkResolution.superseded( + by: verified, + deviceId: deviceId, + peripheralDeviceIds: peripheralDeviceIds, + isConnected: { connected[$0] != nil } + ) + for identifier in retired { + peripheralDeviceIds.removeValue(forKey: identifier) + } + lock.unlock() + return retired + } + + /// The other identifiers `deviceId` holds a usable link on: see + /// `PeerLinkResolution.otherLiveLinks`. `isSubscribed` answers for the + /// centrals subscribed to our server, which this registry does not track. + func otherLiveLinks(of deviceId: String, excluding: UUID, isSubscribed: (UUID) -> Bool) -> [UUID] { + lock.lock() + let connected = connectedPeripherals + let links = PeerLinkResolution.otherLiveLinks( + of: deviceId, + excluding: excluding, + peripheralDeviceIds: peripheralDeviceIds, + centralDeviceIds: centralDeviceIds, + isConnected: { connected[$0] != nil }, + isSubscribed: isSubscribed + ) + lock.unlock() + return links + } + func setCentralDeviceId(_ deviceId: String, for identifier: UUID) { lock.lock() centralDeviceIds[identifier] = deviceId diff --git a/bindings/react-native/ios/tests/BleWriteStallPolicyTests.swift b/bindings/react-native/ios/tests/BleWriteStallPolicyTests.swift new file mode 100644 index 000000000..11471de2e --- /dev/null +++ b/bindings/react-native/ios/tests/BleWriteStallPolicyTests.swift @@ -0,0 +1,73 @@ +import XCTest +@testable import OfflineProtocol + +final class BleWriteStallPolicyTests: XCTestCase { + private let link = UUID() + private let start = Date(timeIntervalSince1970: 1_000) + + /// Refusals are normal flow control. Only a link that refuses everything + /// for the whole threshold is dead. + func testALinkIsDeadOnlyAfterRefusingEverythingForTheThreshold() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + XCTAssertFalse(policy.noteRefused(link, now: start)) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(14))) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(15))) + } + + /// A busy link refuses constantly while still moving; one accepted write + /// or ready callback restarts the clock. + func testAnyWriteTheLinkTakesEndsTheStall() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + XCTAssertFalse(policy.noteRefused(link, now: start)) + policy.noteWritable(link) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(14))) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(28))) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(29))) + } + + /// One dead link produces one teardown, and the reconnect starts clean. + func testATeardownIsReportedOnceAndTheNextStallStartsOver() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + XCTAssertFalse(policy.noteRefused(link, now: start)) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(20))) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(21))) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(35))) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(36))) + } + + /// Each link has its own clock. + func testLinksStallIndependently() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + let other = UUID() + XCTAssertFalse(policy.noteRefused(link, now: start)) + XCTAssertFalse(policy.noteRefused(other, now: start.addingTimeInterval(10))) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(16))) + XCTAssertFalse(policy.noteRefused(other, now: start.addingTimeInterval(16))) + } + + /// A link that disconnects and reconnects under the same identifier + /// starts clean, even if a refusal re-armed the clock after the teardown + /// verdict and before the disconnect landed. + func testForgetClearsAClockReArmedBetweenTheVerdictAndTheDisconnect() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + XCTAssertFalse(policy.noteRefused(link, now: start)) + XCTAssertTrue(policy.noteRefused(link, now: start.addingTimeInterval(15))) + // The redrain refuses again before the cancel reaches main. + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(16))) + policy.forget(link) + // The reconnected link's first refusal starts a new clock. + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(40))) + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(54))) + } + + /// Stopping the bridge forgets every link. + func testResetForgetsEveryLink() { + let policy = BleWriteStallPolicy(stallSeconds: 15) + let other = UUID() + XCTAssertFalse(policy.noteRefused(link, now: start)) + XCTAssertFalse(policy.noteRefused(other, now: start)) + policy.reset() + XCTAssertFalse(policy.noteRefused(link, now: start.addingTimeInterval(20))) + XCTAssertFalse(policy.noteRefused(other, now: start.addingTimeInterval(20))) + } +} diff --git a/bindings/react-native/ios/tests/OutboundFragmentQueueTests.swift b/bindings/react-native/ios/tests/OutboundFragmentQueueTests.swift index 6f9a48eb7..227e1209f 100644 --- a/bindings/react-native/ios/tests/OutboundFragmentQueueTests.swift +++ b/bindings/react-native/ios/tests/OutboundFragmentQueueTests.swift @@ -207,6 +207,102 @@ final class OutboundFragmentQueueTests: XCTestCase { XCTAssertFalse(queue.isBackedUp("bob")) } + /// The drain checks this before every destructive pop, so it has to see a + /// backed-up peer even when the fragment about to be popped is for someone + /// else: the pop is one FIFO across all of them. + func testAnyBackedUpTripsWhenAnyOneRecipientReachesTheMark() { + let queue = OutboundFragmentQueue(maxPerPeer: 8) + XCTAssertFalse(queue.anyBackedUp()) + for i in 0..<5 { queue.enqueue("bob", bytes(UInt8(i))) } + queue.enqueue("carol", bytes(9)) + XCTAssertFalse(queue.anyBackedUp()) + + queue.enqueue("bob", bytes(5)) + XCTAssertTrue(queue.anyBackedUp()) + XCTAssertFalse(queue.isBackedUp("carol")) + } + + /// A recipient with no link cannot drain, so it must not stop the pull + /// for everyone else. Counting it parked every other peer's fragments in + /// the core until the linkless queue expired. + func testARecipientWithNoLinkDoesNotStopThePullForOthers() { + let queue = OutboundFragmentQueue(maxPerPeer: 8) + for i in 0..<6 { queue.enqueue("gone", bytes(UInt8(i))) } + XCTAssertTrue(queue.anyBackedUp()) + XCTAssertFalse(queue.anyBackedUp(deliverable: { $0 != "gone" })) + for i in 0..<6 { queue.enqueue("bob", bytes(UInt8(i))) } + XCTAssertTrue(queue.anyBackedUp(deliverable: { $0 != "gone" })) + } + + /// The pull resumes once the backed-up recipient drains below the mark, + /// or is removed. + func testThePullResumesAfterAFlushOrARemoval() { + let queue = OutboundFragmentQueue(maxPerPeer: 8) + let sender = Sender() + for i in 0..<6 { queue.enqueue("bob", bytes(UInt8(i))) } + XCTAssertTrue(queue.anyBackedUp()) + _ = queue.flush(send: sender.send) + XCTAssertFalse(queue.anyBackedUp()) + for i in 0..<6 { queue.enqueue("bob", bytes(UInt8(i))) } + XCTAssertTrue(queue.anyBackedUp()) + _ = queue.removeAll("bob") + XCTAssertFalse(queue.anyBackedUp()) + } + + /// A cap of one never yields a mark of zero, which read an empty queue as + /// backed up. + func testATinyCapStillHasAMarkOfAtLeastOne() { + let queue = OutboundFragmentQueue(maxPerPeer: 1) + queue.enqueue("bob", bytes(0)) + _ = queue.flush(send: Sender().send) + XCTAssertFalse(queue.anyBackedUp()) + } + + /// A cap between the default mark and the default cap takes the default + /// mark, not three quarters of the cap. + func testAMidSizedCapTakesTheDefaultMark() { + let queue = OutboundFragmentQueue(maxPerPeer: 100) + for i in 0..<(OutboundFragmentQueue.defaultBackpressureMark - 1) { + queue.enqueue("bob", bytes(UInt8(truncatingIfNeeded: i))) + } + XCTAssertFalse(queue.isBackedUp("bob")) + queue.enqueue("bob", bytes(0)) + XCTAssertTrue(queue.isBackedUp("bob")) + } + + /// A whole maximum-size message fits under the default cap with nothing + /// discarded. + func testTheDefaultCapTakesAWholeMaximumSizeMessageWithoutADiscard() { + var drops = 0 + let queue = OutboundFragmentQueue(onDropped: { _, _, n in drops += n }) + for i in 0..<512 { queue.enqueue("bob", bytes(UInt8(truncatingIfNeeded: i))) } + XCTAssertEqual(drops, 0) + XCTAssertEqual(queue.totalCount(), 512) + } + + /// The core fragments a message into at most 512 pieces, and the drain + /// stops pulling at the mark, so the overflow discard that tore 16 KB + /// messages stays out of reach only while the mark is below the cap. + func testTheDefaultCapHoldsAWholeMaximumSizeMessageAndTheMarkSitsBelowIt() { + XCTAssertGreaterThanOrEqual(OutboundFragmentQueue.defaultMaxPerPeer, 512) + XCTAssertLessThan(OutboundFragmentQueue.defaultBackpressureMark, OutboundFragmentQueue.defaultMaxPerPeer) + } + + /// The core counts a message as queued, and defers its retry, only until + /// its first fragment is pulled. A mark at 3/4 of the cap let the drain + /// pull 576 fragments ahead of the link, so the core resent everything + /// behind them. + func testTheDefaultQueueBacksUpAfterAFewSecondsOfLinkTime() { + let queue = OutboundFragmentQueue() + for i in 0..<(OutboundFragmentQueue.defaultBackpressureMark - 1) { + queue.enqueue("bob", bytes(UInt8(truncatingIfNeeded: i))) + } + XCTAssertFalse(queue.anyBackedUp()) + queue.enqueue("bob", bytes(0)) + XCTAssertTrue(queue.isBackedUp("bob")) + XCTAssertTrue(queue.anyBackedUp()) + } + /// The NOTIFY queue is enqueued by the drain while main flushes it. A /// flush that took the queue out to send it hid those fragments from /// `isBackedUp`, so the drain pulled on past the mark, and from the cap, diff --git a/bindings/react-native/ios/tests/PeerLinkResolutionTests.swift b/bindings/react-native/ios/tests/PeerLinkResolutionTests.swift new file mode 100644 index 000000000..0ad18258e --- /dev/null +++ b/bindings/react-native/ios/tests/PeerLinkResolutionTests.swift @@ -0,0 +1,179 @@ +// +// PeerLinkResolutionTests.swift +// OfflineProtocol +// + +import XCTest +@testable import OfflineProtocol + +final class PeerLinkResolutionTests: XCTestCase { + private let peer = "off1peer" + // Ordered so the stale identifier sorts first: a lookup that took the + // first match it met, as the registry's dictionary walk could, picks it. + private let stale = UUID(uuidString: "00000000-0000-0000-0000-000000000001")! + private let fresh = UUID(uuidString: "00000000-0000-0000-0000-000000000002")! + private let other = UUID(uuidString: "00000000-0000-0000-0000-000000000003")! + + /// The third volume run: the peer rotated its address, was verified on + /// the new identifier, and the old one still mapped to it. Sending on the + /// old one found no connection for 25 minutes. + func testASendGoesToTheConnectedLinkWhenAStaleIdentifierAlsoMapsToThePeer() { + let chosen = PeerLinkResolution.preferredIdentifier( + for: peer, + peripheralDeviceIds: [stale: peer, fresh: peer, other: "off1other"], + centralDeviceIds: [:], + isConnected: { $0 == self.fresh } + ) + XCTAssertEqual(chosen, fresh) + } + + func testWithNoConnectedLinkTheChoiceIsStableAndPrefersThePeripheralRole() { + let chosen = PeerLinkResolution.preferredIdentifier( + for: peer, + peripheralDeviceIds: [fresh: peer, stale: peer], + centralDeviceIds: [other: peer], + isConnected: { _ in false } + ) + XCTAssertEqual(chosen, stale) + XCTAssertEqual( + PeerLinkResolution.preferredIdentifier( + for: peer, + peripheralDeviceIds: [:], + centralDeviceIds: [other: peer], + isConnected: { _ in false } + ), + other + ) + XCTAssertNil( + PeerLinkResolution.preferredIdentifier( + for: peer, + peripheralDeviceIds: [other: "off1other"], + centralDeviceIds: [:], + isConnected: { _ in true } + ) + ) + } + + func testVerifyingOnANewIdentifierRetiresTheDisconnectedOnesOnly() { + let third = UUID() + let retired = PeerLinkResolution.superseded( + by: fresh, + deviceId: peer, + peripheralDeviceIds: [stale: peer, fresh: peer, third: peer, other: "off1other"], + isConnected: { $0 == third } + ) + XCTAssertEqual(retired, [stale], "a connected second link stays; another peer's never goes") + } + + func testTheMonitorDialsAnUnidentifiedPeripheralOnlyWhileItAdvertises() { + let now = Date() + XCTAssertTrue(PeerLinkResolution.monitorShouldRedial(identified: false, lastAdvertisedAt: now.addingTimeInterval(-5), now: now)) + XCTAssertFalse(PeerLinkResolution.monitorShouldRedial(identified: false, lastAdvertisedAt: now.addingTimeInterval(-61), now: now)) + XCTAssertFalse(PeerLinkResolution.monitorShouldRedial(identified: false, lastAdvertisedAt: nil, now: now)) + // A known peer that went quiet is still reconnected: rotation is + // handled by retiring the identifier, not by the monitor giving up. + XCTAssertTrue(PeerLinkResolution.monitorShouldRedial(identified: true, lastAdvertisedAt: nil, now: now)) + } + + /// Run 5: Android advertised on `stale` (the link the stall teardown + /// cancelled) and was also connected and verified on `fresh`. The timed-out + /// redial of `stale` must not lose the peer, and `stale` must not be + /// dialled while `fresh` is up. + func testAPeerConnectedOnAnotherIdentifierHasALiveLink() { + let links = PeerLinkResolution.otherLiveLinks( + of: peer, + excluding: stale, + peripheralDeviceIds: [stale: peer, fresh: peer, other: "off1other"], + centralDeviceIds: [stale: peer, fresh: peer], + isConnected: { $0 == self.fresh }, + isSubscribed: { $0 == self.fresh } + ) + XCTAssertEqual(links, [fresh], "one link, counted once though it is in both roles") + } + + func testWithNoOtherLinkThePeerIsLost() { + XCTAssertEqual( + PeerLinkResolution.otherLiveLinks( + of: peer, + excluding: stale, + peripheralDeviceIds: [stale: peer, fresh: peer], + centralDeviceIds: [:], + isConnected: { _ in false }, + isSubscribed: { _ in false } + ), + [], + "a mapped identifier that is not connected is not a link" + ) + XCTAssertEqual( + PeerLinkResolution.otherLiveLinks( + of: peer, + excluding: stale, + peripheralDeviceIds: [stale: peer], + centralDeviceIds: [stale: peer], + isConnected: { $0 == self.stale }, + isSubscribed: { $0 == self.stale } + ), + [], + "the identifier that dropped never counts for itself" + ) + } + + func testAVerifiedSubscribedCentralIsALink() { + XCTAssertEqual( + PeerLinkResolution.otherLiveLinks( + of: peer, + excluding: stale, + peripheralDeviceIds: [stale: peer], + centralDeviceIds: [other: peer], + isConnected: { _ in false }, + isSubscribed: { $0 == self.other } + ), + [other] + ) + XCTAssertEqual( + PeerLinkResolution.otherLiveLinks( + of: peer, + excluding: stale, + peripheralDeviceIds: [stale: peer], + centralDeviceIds: [other: peer], + isConnected: { _ in false }, + isSubscribed: { _ in false } + ), + [], + "a central that unsubscribed carries nothing" + ) + } + + /// A subscribed central counts only while it is heard from. One whose + /// `didUnsubscribeFrom` never arrived would otherwise hold a departed + /// peer as a neighbour for good. + func testASubscriptionCountsOnlyWhileItsCentralIsHeardFrom() { + let now = Date(timeIntervalSince1970: 10_000) + XCTAssertTrue(PeerLinkResolution.subscriptionIsLive(lastHeardAt: now.addingTimeInterval(-59), now: now)) + XCTAssertFalse(PeerLinkResolution.subscriptionIsLive(lastHeardAt: now.addingTimeInterval(-60), now: now)) + XCTAssertFalse(PeerLinkResolution.subscriptionIsLive(lastHeardAt: nil, now: now)) + } + + /// The monitor's window is exclusive at its edge, like the liveness one. + func testTheMonitorStopsDiallingAnUnidentifiedPeripheralExactlyAtTheWindow() { + let now = Date(timeIntervalSince1970: 10_000) + let window = PeerLinkResolution.unidentifiedRedialWindow + XCTAssertTrue(PeerLinkResolution.monitorShouldRedial( + identified: false, lastAdvertisedAt: now.addingTimeInterval(-window + 1), now: now)) + XCTAssertFalse(PeerLinkResolution.monitorShouldRedial( + identified: false, lastAdvertisedAt: now.addingTimeInterval(-window), now: now)) + } + + /// The disconnect path retires on the kept link even when that link is a + /// subscribed central with no peripheral-role mapping of its own. + func testRetiringOnACentralOnlyLinkRetiresEveryDisconnectedPeripheral() { + let retired = PeerLinkResolution.superseded( + by: other, + deviceId: peer, + peripheralDeviceIds: [stale: peer, fresh: peer], + isConnected: { _ in false } + ) + XCTAssertEqual(retired, [stale, fresh]) + } +} + diff --git a/crates/offline-protocol-mls/src/group.rs b/crates/offline-protocol-mls/src/group.rs index 0778bd9b0..bc478cc96 100644 --- a/crates/offline-protocol-mls/src/group.rs +++ b/crates/offline-protocol-mls/src/group.rs @@ -7,6 +7,7 @@ use crate::provider::MlsProvider; use crate::storage::{MlsStorage, StorageError}; use crate::types::{GroupId, GroupInfo, GroupMetadata, GroupRole, StorageKeyType}; +use openmls::framing::errors::{MessageDecryptionError, SecretTreeError}; use openmls::prelude::*; use openmls_traits::signatures::Signer; use openmls_traits::OpenMlsProvider; @@ -21,12 +22,18 @@ pub const DEFAULT_CIPHERSUITE: Ciphersuite = /// with, re-exported from the sealed layer. /// /// They live there rather than here because the peer at the other end of a -/// session may be a leaf node running a different MLS implementation, and it -/// has to be configured with the same two numbers. A drifted copy does not -/// fail a build or a test: it produces a session that stops decrypting under -/// load, on a device, later. +/// session may be a leaf node running a different MLS implementation, and the +/// interop harness configures its OpenMLS side with the same two numbers. A +/// drifted copy does not fail a build or a test: it produces a session that +/// stops decrypting under load, on a device, later. +/// +/// The leaf's own receiver window is not these. mls-rs fixes it at +/// [`LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE`] and offers no setting, so +/// whatever the phone derives from how far ahead a receiver tolerates (the +/// re-seal budget in `offline-protocol`) takes the smaller of the two. pub use offline_protocol_sealed::{ - SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, + LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, + SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, }; /// Sender ratchet configuration applied to every created and joined group. @@ -396,11 +403,30 @@ impl GroupManager { // `test_forged_frame_reaches_session_desync_without_any_key_material`). // `SessionDesync` is an unauthenticated hint by construction; // the mitigation lives in what the protocol layer hangs off it. + // + // A frame more than `SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE` + // generations ahead of this receiver's ratchet is the third + // member of the family, and the one that is never transient: + // the sender's every later frame (and every re-sealed resend) + // is further ahead still, so without a re-key the session + // never decrypts from that sender again. The 2026-10-19 volume + // test lost its whole second half to exactly that, as 3,915 + // per-frame failures with nothing ever healing them. + // + // Unlike the epoch checks, this one IS authenticated: OpenMLS + // decrypts the sender data under AEAD before it reads the + // generation from it, so only a holder of this epoch's + // sender-data secret can produce a frame that fails here. + // `TooDistantInThePast` stays `Decryption`: the session is + // healthy, and the sender's re-sealed resend recovers it. match &e { ProcessMessageError::ValidationError(ValidationError::WrongEpoch) - | ProcessMessageError::ValidationError(ValidationError::NoPastEpochData) => { - MlsError::SessionDesync(e.to_string()) - } + | ProcessMessageError::ValidationError(ValidationError::NoPastEpochData) + | ProcessMessageError::ValidationError(ValidationError::UnableToDecrypt( + MessageDecryptionError::SecretTreeError( + SecretTreeError::TooDistantInTheFuture, + ), + )) => MlsError::SessionDesync(e.to_string()), _ => MlsError::Decryption(e.to_string()), } })?; diff --git a/crates/offline-protocol-mls/src/lib.rs b/crates/offline-protocol-mls/src/lib.rs index 90a00e97d..bbde24c1c 100644 --- a/crates/offline-protocol-mls/src/lib.rs +++ b/crates/offline-protocol-mls/src/lib.rs @@ -52,6 +52,7 @@ pub mod types; pub use error::{LeafSource, MlsError, Result}; pub use identity_assertion::verify_identity_assertion; pub use manager::{is_mls_framed, MlsManager, PushKeyPackage, MAX_PUSH_KEY_PACKAGES}; +pub use session::SealMark; pub use storage::{MlsStorage, StorageError}; pub use types::{ EncryptedMessage, GroupId, GroupInfo, GroupMetadata, GroupRole, KeyPackageBundle, diff --git a/crates/offline-protocol-mls/src/manager.rs b/crates/offline-protocol-mls/src/manager.rs index c313a2ce4..c5fee1896 100644 --- a/crates/offline-protocol-mls/src/manager.rs +++ b/crates/offline-protocol-mls/src/manager.rs @@ -1085,6 +1085,25 @@ impl MlsManager { .encrypt_message(other_user_id, plaintext, &signature_keys) } + /// The position of the most recent frame sealed for `other_user_id` (see + /// [`crate::session::SessionManager::last_seal_mark`]). + pub fn last_seal_mark(&self, other_user_id: &str) -> Option { + self.session_manager.last_seal_mark(other_user_id) + } + + /// Records that the peer received the frame at `mark` (see + /// [`crate::session::SessionManager::note_acknowledged_seal`]). + pub fn note_acknowledged_seal(&self, other_user_id: &str, mark: crate::session::SealMark) { + self.session_manager + .note_acknowledged_seal(other_user_id, mark); + } + + /// Frames sealed for `other_user_id` past the last one it acknowledged + /// (see [`crate::session::SessionManager::unacknowledged_seals`]). + pub fn unacknowledged_seals(&self, other_user_id: &str) -> u64 { + self.session_manager.unacknowledged_seals(other_user_id) + } + /// Encrypts a message for a 1:1 session that is known to exist. /// /// Unlike `encrypt_for_user`, this skips the `has_session()` storage check @@ -4082,6 +4101,98 @@ mod tests { ); } + /// A receiver that falls more than the forward window behind its sender + /// never decrypts from that sender again: every later frame, and every + /// re-sealed resend, is further ahead still. That must surface as the + /// recoverable `SessionDesync`, which the protocol layer answers with a + /// re-key, not as `Decryption`, which it answers by waiting for a resend + /// that can never decrypt either. The 2026-10-19 volume test lost a whole + /// direction this way. + /// + /// The gap inside the window still fast-forwards, so a receiver is not + /// re-keyed for an ordinary burst of lost frames. + #[test] + fn test_a_receiver_past_the_forward_window_classifies_as_session_desync() { + let (alice, bob) = create_test_session(); + + // Inside the window: skip far ahead and still decrypt. + let mut last = None; + for i in 0..100 { + last = Some( + alice + .encrypt_for_user(&addr("bob"), format!("in window {i}").as_bytes()) + .unwrap(), + ); + } + let pt = bob + .decrypt_from_user(&last.unwrap(), &addr("alice")) + .unwrap(); + assert_eq!(pt.as_deref(), Some(&b"in window 99"[..])); + + // Past it: the next frame lands more than the window ahead of bob. + let gap = crate::group::SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE + 1; + for _ in 0..gap { + alice.encrypt_for_user(&addr("bob"), b"lost").unwrap(); + } + let ahead = alice.encrypt_for_user(&addr("bob"), b"too far").unwrap(); + let err = bob.decrypt_from_user(&ahead, &addr("alice")).unwrap_err(); + assert!( + matches!(err, MlsError::SessionDesync(_)), + "a frame past the forward window must classify as SessionDesync, got {err:?}" + ); + } + + /// The other side of the window stays a plain decryption failure: a frame + /// more than the out-of-order tolerance behind means the session is + /// healthy and only that frame is lost, which the sender's re-sealed + /// resend recovers. Re-keying there would tear down a working session. + #[test] + fn test_a_frame_past_the_out_of_order_tolerance_is_not_a_desync() { + let (alice, bob) = create_test_session(); + let old = alice.encrypt_for_user(&addr("bob"), b"old").unwrap(); + let mut newest = None; + for _ in 0..=crate::group::SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE + 1 { + newest = Some(alice.encrypt_for_user(&addr("bob"), b"newer").unwrap()); + } + bob.decrypt_from_user(&newest.unwrap(), &addr("alice")) + .unwrap(); + let err = bob.decrypt_from_user(&old, &addr("alice")).unwrap_err(); + assert!( + matches!(err, MlsError::Decryption(_)), + "a frame behind the tolerance must stay Decryption, got {err:?}" + ); + } + + /// The seal count is what bounds a sender's re-seals, so it must move + /// with seals and acknowledgements, and an acknowledgement for a frame of + /// a session that has since been rebuilt must not vouch for the new one. + #[test] + fn test_unacknowledged_seals_track_seals_acks_and_session_rebuilds() { + let (alice, _bob) = create_test_session(); + let bob = addr("bob"); + assert_eq!(alice.unacknowledged_seals(&bob), 0); + + alice.encrypt_for_user(&bob, b"one").unwrap(); + let first = alice.last_seal_mark(&bob).unwrap(); + alice.encrypt_for_user(&bob, b"two").unwrap(); + alice.encrypt_for_user(&bob, b"three").unwrap(); + assert_eq!(alice.unacknowledged_seals(&bob), 3); + + alice.note_acknowledged_seal(&bob, first); + assert_eq!(alice.unacknowledged_seals(&bob), 2); + // An older acknowledgement never moves the mark back. + alice.note_acknowledged_seal(&bob, crate::session::SealMark { index: 0, ..first }); + assert_eq!(alice.unacknowledged_seals(&bob), 2); + + alice.delete_session(&bob).unwrap(); + assert_eq!(alice.unacknowledged_seals(&bob), 0); + assert!(alice.last_seal_mark(&bob).is_none()); + // A mark from before the rebuild is ignored, so it cannot hide seals + // of the new session. + alice.note_acknowledged_seal(&bob, crate::session::SealMark { index: 99, ..first }); + assert_eq!(alice.unacknowledged_seals(&bob), 0); + } + /// RFC 8032 §7.1 TEST 1 Ed25519 public key. const RFC8032_TV1_PK: [u8; 32] = [ 0xd7, 0x5a, 0x98, 0x01, 0x82, 0xb1, 0x0a, 0xb7, 0xd5, 0x4b, 0xfe, 0xd3, 0xc9, 0x64, 0x07, diff --git a/crates/offline-protocol-mls/src/session.rs b/crates/offline-protocol-mls/src/session.rs index 33b68578a..fdc824b81 100644 --- a/crates/offline-protocol-mls/src/session.rs +++ b/crates/offline-protocol-mls/src/session.rs @@ -12,9 +12,35 @@ use crate::types::{EncryptedMessage, GroupId, GroupInfo, MlsMessageType, Welcome use openmls::prelude::tls_codec::{Deserialize as TlsDeserialize, Serialize as TlsSerialize}; use openmls::prelude::*; use openmls_traits::signatures::Signer; -use std::sync::Arc; +use std::collections::HashMap; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use tracing::{debug, info}; +/// Where one sealed frame sits in its session's sending order: the +/// `index`-th frame this device sealed in that session, and which session +/// (`incarnation`) that was. +/// +/// The incarnation is what keeps an acknowledgement for a frame of a session +/// that has since been rebuilt from vouching for the new one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SealMark { + /// Which session the frame was sealed in. New on every create, join and + /// delete. + pub incarnation: u64, + /// The frame's position among this session's seals, from 1. + pub index: u64, +} + +/// How far this device's sealing has run ahead of what the peer has shown +/// it received, per session. +#[derive(Debug, Clone, Copy)] +struct SealProgress { + incarnation: u64, + sealed: u64, + acknowledged: u64, +} + /// Manages 1:1 encrypted sessions. pub struct SessionManager { /// The local user's ID. @@ -22,6 +48,15 @@ pub struct SessionManager { /// Group manager for underlying MLS operations. group_manager: GroupManager, + + /// Seals per session against the highest seal the peer has acknowledged. + /// In memory only: after a restart the count starts again from nothing, + /// which reads as "no gap", the permissive direction, until the first + /// acknowledgement. + seal_progress: Mutex>, + + /// Source of session incarnations. + next_incarnation: AtomicU64, } impl SessionManager { @@ -31,9 +66,75 @@ impl SessionManager { Self { user_id, group_manager, + seal_progress: Mutex::new(HashMap::new()), + next_incarnation: AtomicU64::new(1), + } + } + + fn progress(&self) -> std::sync::MutexGuard<'_, HashMap> { + self.seal_progress + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } + + fn fresh_progress(&self) -> SealProgress { + SealProgress { + incarnation: self.next_incarnation.fetch_add(1, Ordering::Relaxed), + sealed: 0, + acknowledged: 0, + } + } + + /// Starts a session's seal count from nothing: it was created, joined or + /// deleted, so nothing sealed before can vouch for it. + fn restart_seal_progress(&self, session_id: &GroupId) { + let fresh = self.fresh_progress(); + self.progress().insert(session_id.to_string(), fresh); + } + + /// The position of the most recent frame sealed for `other_user_id`, or + /// `None` when nothing has been sealed in this session since it started + /// (or since this process did). + pub fn last_seal_mark(&self, other_user_id: &str) -> Option { + let session_id = self.get_session_id(other_user_id).ok()?; + let progress = self.progress(); + let entry = progress.get(session_id.as_str())?; + (entry.sealed > 0).then_some(SealMark { + incarnation: entry.incarnation, + index: entry.sealed, + }) + } + + /// Records that the peer received the frame at `mark`, so it has + /// decrypted at least that far into the session. Ignored for a frame of + /// an earlier incarnation of the session. + pub fn note_acknowledged_seal(&self, other_user_id: &str, mark: SealMark) { + let Ok(session_id) = self.get_session_id(other_user_id) else { + return; + }; + if let Some(entry) = self.progress().get_mut(session_id.as_str()) { + if entry.incarnation == mark.incarnation { + entry.acknowledged = entry.acknowledged.max(mark.index); + } } } + /// How many frames this device has sealed for `other_user_id` past the + /// last one the peer acknowledged. + /// + /// A receiver cannot decrypt a frame more than the forward window past + /// the newest it has decrypted, and an acknowledged frame is one it has, + /// so keeping this below the window keeps every frame decryptable. The + /// sender is what controls it: each re-sealed resend adds one. + pub fn unacknowledged_seals(&self, other_user_id: &str) -> u64 { + let Ok(session_id) = self.get_session_id(other_user_id) else { + return 0; + }; + self.progress() + .get(session_id.as_str()) + .map_or(0, |entry| entry.sealed.saturating_sub(entry.acknowledged)) + } + /// Returns a reference to the underlying group manager. pub fn group_manager(&self) -> &GroupManager { &self.group_manager @@ -92,6 +193,7 @@ impl SessionManager { "Created new 1:1 session" ); + self.restart_seal_progress(&session_id); Ok(WelcomeMessage { group_id: session_id, welcome_data: welcome_bytes, @@ -176,6 +278,7 @@ impl SessionManager { "Joined/adopted 1:1 session (non-destructive)" ); + self.restart_seal_progress(&welcome_msg.group_id); Ok(info) } @@ -198,6 +301,13 @@ impl SessionManager { .encrypt_message(&mut group, plaintext, signer)?; self.group_manager.save_group(&session_id, &group)?; + { + let mut progress = self.progress(); + let entry = progress + .entry(session_id.to_string()) + .or_insert_with(|| self.fresh_progress()); + entry.sealed = entry.sealed.saturating_add(1); + } let ciphertext = mls_message .tls_serialize_detached() @@ -289,6 +399,7 @@ impl SessionManager { pub fn delete_session(&self, other_user_id: &str) -> Result<()> { let session_id = self.get_session_id(other_user_id)?; self.group_manager.delete_group(&session_id)?; + self.restart_seal_progress(&session_id); debug!(session_id = %session_id, "Deleted session"); Ok(()) } diff --git a/crates/offline-protocol-reliability/src/ack_manager.rs b/crates/offline-protocol-reliability/src/ack_manager.rs index ac6ab86db..1fda25098 100644 --- a/crates/offline-protocol-reliability/src/ack_manager.rs +++ b/crates/offline-protocol-reliability/src/ack_manager.rs @@ -48,6 +48,12 @@ pub struct PendingAck { /// Priority of the message (for eviction decisions). pub priority: MessagePriority, + + /// How far the deadline has been pushed past `sent_at + timeout_ms` while + /// the frame was still waiting in a local transport queue. Kept apart from + /// `sent_at`, which the sender reads to measure the round trip, and reset + /// on every new attempt. + pub extended_ms: u64, } impl PendingAck { @@ -58,7 +64,7 @@ impl PendingAck { } let elapsed = Utc::now().signed_duration_since(self.sent_at); - elapsed.num_milliseconds() >= self.timeout_ms as i64 + elapsed.num_milliseconds() >= self.timeout_ms.saturating_add(self.extended_ms) as i64 } } @@ -140,22 +146,23 @@ impl AckManager { timeout_ms: Option, priority: MessagePriority, ) -> crate::Result> { - // Check if we've hit the limit + // At the ceiling, only a strictly lower-priority entry may make room. + // + // Timed-out entries are deliberately NOT swept here. Marking them + // `TimedOut` without handing them to the caller orphaned them: the + // engine's timeout sweep reads `is_timed_out`, which is false for an + // entry already marked, so those messages were never retried and sat + // in the outbox until its seven-day expiry. Only the engine's own + // sweep (`drain_timed_out`) marks entries, because only it acts on + // the list it gets back. let mut eviction = None; - if self.pending_acks.len() >= self.config.max_pending_acks { - // Clean up timed out ACKs first - self.cleanup_timed_out(); - - // If still at limit, try priority-based eviction - if self.pending_acks.len() >= self.config.max_pending_acks { - match self.evict_lowest_priority(priority) { - Some(info) => eviction = Some(info), - None => { - return Err(crate::Error::Other( - "Maximum pending ACKs limit reached (no lower priority ACKs to evict)" - .to_string(), - )); - } + if !self.has_capacity_for(&message_id) { + match self.evict_lowest_priority(priority) { + Some(info) => eviction = Some(info), + None => { + return Err(crate::Error::AckCapacity { + max: self.config.max_pending_acks, + }); } } } @@ -167,6 +174,7 @@ impl AckManager { status: AckStatus::Pending, retry_count: 0, priority, + extended_ms: 0, }; self.pending_acks.insert(message_id, pending); @@ -260,10 +268,39 @@ impl AckManager { if let Some(pending) = self.pending_acks.get_mut(message_id) { pending.retry_count += 1; pending.sent_at = Utc::now(); // Reset timer for retry + pending.extended_ms = 0; pending.status = AckStatus::Pending; } } + /// Gives a timed-out entry another full timeout because its frame has not + /// left the device yet, as long as the attempt is younger than + /// `max_wait`. Returns whether it was extended; `false` means the caller + /// retries it as usual. + /// + /// The timer starts when a frame is queued, not when it is written, so on + /// a link carrying less than it is offered a frame can time out while it + /// is still waiting its turn. Retrying it then appends a second copy + /// behind the first, re-sealed under a fresh ratchet generation, which + /// makes the queue longer and the receiver's gap wider. `max_wait` bounds + /// the grace so a queue that never drains still falls through to the retry + /// ladder and the other carriers. + pub fn extend_while_queued(&mut self, message_id: &MessageId, max_wait: Duration) -> bool { + let Some(pending) = self.pending_acks.get_mut(message_id) else { + return false; + }; + let elapsed_ms = Utc::now() + .signed_duration_since(pending.sent_at) + .num_milliseconds() + .max(0) as u64; + if elapsed_ms >= max_wait.as_millis().min(u128::from(u64::MAX)) as u64 { + return false; + } + pending.extended_ms = elapsed_ms; + pending.status = AckStatus::Pending; + true + } + /// Carry an accumulated retry count onto a pending ACK. /// /// `register_pending_ack*` always starts a fresh ACK at `retry_count: 0`. @@ -290,14 +327,20 @@ impl AckManager { self.pending_acks.len() } - /// Checks if a message is waiting for ACK. - pub fn is_waiting_for_ack(&self, message_id: &MessageId) -> bool { + /// Whether registering `message_id` would succeed without evicting + /// anything: it is already tracked, or the table has a free slot. + /// + /// A sender asks this **before** it transmits. Asking after is too late: + /// the frame is on the wire, and the only honest outcomes left are to + /// track it or to have refused it. + pub fn has_capacity_for(&self, message_id: &MessageId) -> bool { self.pending_acks.contains_key(message_id) + || self.pending_acks.len() < self.config.max_pending_acks } - /// Cleans up timed out ACKs by marking them as timed out. - fn cleanup_timed_out(&mut self) { - self.drain_timed_out(); + /// Checks if a message is waiting for ACK. + pub fn is_waiting_for_ack(&self, message_id: &MessageId) -> bool { + self.pending_acks.contains_key(message_id) } /// Removes all ACKs with TimedOut status that are older than a given duration. @@ -441,9 +484,86 @@ mod tests { .register_pending_ack(MessageId::new(), None) .unwrap(); - // Fourth should fail (same priority can't evict) + // Fourth should fail (same priority can't evict), with the typed + // error a sender can recognise and defer on. let result = manager.register_pending_ack(MessageId::new(), None); - assert!(result.is_err()); + assert!(matches!(result, Err(crate::Error::AckCapacity { max: 3 }))); + } + + #[test] + fn has_capacity_for_admits_a_tracked_id_at_the_ceiling() { + let mut manager = AckManager::with_config(AckConfig { + default_timeout_ms: 5000, + max_pending_acks: 1, + }); + let tracked = MessageId::new(); + assert!(manager.has_capacity_for(&tracked)); + manager.register_pending_ack(tracked.clone(), None).unwrap(); + // A resend of the tracked message needs no new slot. + assert!(manager.has_capacity_for(&tracked)); + // A new message does. + assert!(!manager.has_capacity_for(&MessageId::new())); + } + + #[test] + fn extend_while_queued_grants_one_more_timeout_within_the_bound() { + let mut manager = AckManager::with_config(AckConfig { + default_timeout_ms: 10, + max_pending_acks: 10, + }); + let id = MessageId::new(); + manager.register_pending_ack(id.clone(), Some(10)).unwrap(); + thread::sleep(Duration::from_millis(15)); + assert_eq!(manager.drain_timed_out().len(), 1); + + assert!(manager.extend_while_queued(&id, Duration::from_secs(60))); + assert!( + manager.drain_timed_out().is_empty(), + "extended for one more timeout" + ); + thread::sleep(Duration::from_millis(15)); + assert_eq!(manager.drain_timed_out().len(), 1, "and only one"); + + // Past the bound it falls through to the retry ladder. + assert!(!manager.extend_while_queued(&id, Duration::from_millis(1))); + // A new attempt starts from a clean deadline. + manager.increment_retry_count(&id); + assert_eq!(manager.get_pending_ack(&id).unwrap().extended_ms, 0); + } + + /// A refused registration at the ceiling used to mark every timed-out + /// entry `TimedOut` and throw the list away. `is_timed_out` is false for a + /// marked entry, so the engine's own sweep never saw those messages again + /// and they were never retried. + #[test] + fn a_refused_registration_leaves_timed_out_entries_for_the_sweep() { + let mut manager = AckManager::with_config(AckConfig { + default_timeout_ms: 10, + max_pending_acks: 2, + }); + let first = MessageId::new(); + let second = MessageId::new(); + manager + .register_pending_ack(first.clone(), Some(10)) + .unwrap(); + manager + .register_pending_ack(second.clone(), Some(10)) + .unwrap(); + thread::sleep(Duration::from_millis(20)); + + assert!(manager + .register_pending_ack(MessageId::new(), None) + .is_err()); + + let mut drained: Vec<_> = manager + .drain_timed_out() + .into_iter() + .map(|ack| ack.message_id) + .collect(); + drained.sort_by_key(|id| id.as_str()); + let mut expected = vec![first, second]; + expected.sort_by_key(|id| id.as_str()); + assert_eq!(drained, expected); } #[test] @@ -523,7 +643,7 @@ mod tests { // Wait for timeout thread::sleep(Duration::from_millis(20)); - manager.cleanup_timed_out(); + assert_eq!(manager.drain_timed_out().len(), 1); assert_eq!(manager.pending_count(), 1); diff --git a/crates/offline-protocol-reliability/src/error.rs b/crates/offline-protocol-reliability/src/error.rs index a07f71289..b151b2bbf 100644 --- a/crates/offline-protocol-reliability/src/error.rs +++ b/crates/offline-protocol-reliability/src/error.rs @@ -15,6 +15,19 @@ pub enum Error { #[error("ACK timeout for message {0}")] AckTimeout(String), + /// The pending-ACK table is full and nothing in it may be evicted for + /// this message. + /// + /// Typed rather than a string so a caller can tell "this frame cannot be + /// tracked right now" (defer it) from a genuine fault. A caller that + /// treats it as fatal after the frame has been transmitted reports a + /// failure for a message that is already on the wire. + #[error("Pending ACK table is full ({max} entries)")] + AckCapacity { + /// The configured ceiling. + max: usize, + }, + /// Core error. #[error("Core error: {0}")] Core(#[from] offline_protocol_core::Error), diff --git a/crates/offline-protocol-sealed/src/constants.rs b/crates/offline-protocol-sealed/src/constants.rs index 2d316fb44..13a29fea0 100644 --- a/crates/offline-protocol-sealed/src/constants.rs +++ b/crates/offline-protocol-sealed/src/constants.rs @@ -43,8 +43,39 @@ pub const ACK_FOR_KEY: &str = "ack_for"; pub const SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE: u32 = 32; /// How far ahead of the highest seen generation a sender ratchet may be -/// fast-forwarded when messages are lost (OpenMLS default). -pub const SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE: u32 = 1000; +/// fast-forwarded when messages are lost. +/// +/// A receiver more than this far behind cannot decrypt anything from that +/// sender again until the pair re-keys, so the window has to sit well above +/// the gap a congested link can open. The engine keeps up to +/// `DEFAULT_MAX_PENDING_ACKS` (1000) messages in flight and re-seals each +/// resend, which spends a fresh generation every time; a receiver that drops +/// a resend as a duplicate of a message it already has skips that generation. +/// The OpenMLS default of 1000 was therefore reachable, and the 2026-10-19 +/// volume test reached it: the sender logged 12,152 retries against 2,434 +/// messages the receiver decrypted. 8192 keeps more than four in-flight +/// ceilings of headroom (pinned in `offline-protocol`), and the cost of using +/// it is one fast-forward of up to 8192 key derivations when a gap opens. +/// +/// It is receiver-local policy: nothing on the wire carries it, and a peer +/// configured with a different value still interoperates. It configures the +/// phone's OpenMLS groups only: a leaf's receiver window is +/// [`LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE`], which mls-rs does not let +/// anyone set. +pub const SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE: u32 = 8192; + +/// How far ahead a leaf node's receiver ratchet may be fast-forwarded: a frame +/// more than this many generations past the next one it expects is refused. +/// +/// Not configuration. mls-rs fixes it (`MAX_RATCHET_BACK_HISTORY` in its +/// secret tree, 1024 in the pinned 0.56.0) and exposes no setting, so a phone +/// sealing for a leaf has this much room, not +/// [`SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE`]. Anything a phone derives from +/// "how far ahead a receiver tolerates" has to take the smaller of the two: +/// the re-seal budget in `offline-protocol` does. Stated here so that +/// derivation has one name to read, and pinned against the real mls-rs by +/// `tools/mls-interop`, which fails if an mls-rs upgrade moves it. +pub const LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE: u32 = 1024; /// How long a leaf node's key package claims to be valid. /// diff --git a/crates/offline-protocol-sealed/src/keypackage.rs b/crates/offline-protocol-sealed/src/keypackage.rs index 518dceca0..0198efb12 100644 --- a/crates/offline-protocol-sealed/src/keypackage.rs +++ b/crates/offline-protocol-sealed/src/keypackage.rs @@ -271,6 +271,13 @@ pub const SVC_SEALED_V1: u8 = 1; /// recipient can never decrypt this frame" rather than as a delivery. pub const ACK_UNDECRYPTABLE_V1: u8 = 1; +/// Acknowledgement version advertised in [`KeyPackagePayload::ack_versions`]: +/// the sender reads the `ack_more` metadata on a delivery ACK, so one ACK +/// frame may answer several of its frames. Over a mesh link an ACK per frame +/// was a third of the frames a phone sent, on a link already carrying less +/// than it was offered. +pub const ACK_COALESCED_V1: u8 = 2; + /// The conformance vectors for the key package payload. /// /// The chapter these pin is `docs/spec/capability-negotiation.md`, and they pin diff --git a/crates/offline-protocol-sealed/src/lib.rs b/crates/offline-protocol-sealed/src/lib.rs index 5c3e548d9..e253a3183 100644 --- a/crates/offline-protocol-sealed/src/lib.rs +++ b/crates/offline-protocol-sealed/src/lib.rs @@ -69,8 +69,8 @@ pub use canonical::{ }; pub use constants::{ ACK_FOR_KEY, LEAF_KEY_PACKAGE_LIFETIME, LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS, - MAX_ACCEPTED_KEY_PACKAGE_LIFETIME, SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, - SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, + LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, MAX_ACCEPTED_KEY_PACKAGE_LIFETIME, + SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, }; pub use derive::{derive_address, ED25519_PUBLIC_KEY_LEN}; pub use envelope::{EncryptedMessage, GroupId, MlsMessageType, WelcomeMessage}; @@ -84,7 +84,8 @@ pub use identity_assertion::{ IDENTITY_ASSERTION_MIN_LEN, }; pub use keypackage::{ - KeyPackagePayload, ACK_UNDECRYPTABLE_V1, CTRL_SIGN_V2, MLS_ENVELOPE_COMPACT_V1, SVC_SEALED_V1, + KeyPackagePayload, ACK_COALESCED_V1, ACK_UNDECRYPTABLE_V1, CTRL_SIGN_V2, + MLS_ENVELOPE_COMPACT_V1, SVC_SEALED_V1, }; #[cfg(all(test, feature = "std"))] @@ -187,6 +188,7 @@ mod interop_harness_guard_tests { "derive_address", "SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE", "SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE", + "LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE", "LEAF_KEY_PACKAGE_LIFETIME", "LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS", "MAX_ACCEPTED_KEY_PACKAGE_LIFETIME", @@ -204,6 +206,7 @@ mod interop_harness_guard_tests { "fn derive_address", "const OUT_OF_ORDER_TOLERANCE", "const MAXIMUM_FORWARD_DISTANCE", + "const LEAF_FORWARD_DISTANCE", "const KEY_PACKAGE_LIFETIME", "const NOT_BEFORE_BACKDATE", "const MAX_ACCEPTED", diff --git a/crates/offline-protocol-transport/src/ble.rs b/crates/offline-protocol-transport/src/ble.rs index a9a42e2aa..3a6d3c291 100644 --- a/crates/offline-protocol-transport/src/ble.rs +++ b/crates/offline-protocol-transport/src/ble.rs @@ -28,6 +28,7 @@ use crate::constants::{ }; use crate::{Result, SharedCallback, Transport, TransportMetrics, TransportStatus, TransportType}; use offline_protocol_core::{Message, MutexExt}; +use offline_protocol_sealed::ACK_FOR_KEY; use std::collections::{HashMap, VecDeque}; use std::convert::TryInto; use std::sync::atomic::{AtomicU64, Ordering}; @@ -239,6 +240,30 @@ pub struct BleTransport { eviction_callback: Arc>>, } +/// Queues a message for the bridge, a delivery acknowledgement ahead of every +/// queued data frame (behind earlier acknowledgements, so they keep their +/// order). +/// +/// An acknowledgement is what stops its sender's retry timer. Queued behind +/// data on a link carrying less than it is offered, it arrived after that +/// timer had fired, and each resend it provoked cost the receiver another +/// acknowledgement on the same saturated link: in the 2026-10-19 volume test +/// the acknowledgement round trip reached 196 seconds against a 10-second +/// timeout. The fragments of a message already being written are not +/// overtaken; they are past this queue. +fn enqueue_send(queue: &mut VecDeque<(String, Message)>, entry: (String, Message)) { + let is_ack = |message: &Message| message.metadata.contains_key(ACK_FOR_KEY); + if is_ack(&entry.1) { + let at = queue + .iter() + .position(|(_, queued)| !is_ack(queued)) + .unwrap_or(queue.len()); + queue.insert(at, entry); + } else { + queue.push_back(entry); + } +} + impl BleTransport { /// Creates a new BLE transport. pub fn new(device_id: impl Into) -> Self { @@ -1046,10 +1071,10 @@ impl Transport for BleTransport { } } - { - let mut queue = self.send_queue.lock_or_recover(); - queue.push_back((recipient, message.clone())); - } + enqueue_send( + &mut self.send_queue.lock_or_recover(), + (recipient, message.clone()), + ); self.update_queue_metric(); self.notify_fragments_available(); @@ -1057,6 +1082,16 @@ impl Transport for BleTransport { Ok(()) } + /// A message leaves this list when the bridge pulls its first fragment: + /// from then on it is being written, and a timeout means what it says. + fn queued_message_ids(&self) -> Vec { + self.send_queue + .lock_or_recover() + .iter() + .map(|(_, message)| message.id.as_str()) + .collect() + } + /// Queues `message` for a specific connected peer. /// /// Identical to [`Transport::send`] except that the queue key — which is @@ -1082,10 +1117,10 @@ impl Transport for BleTransport { } } - { - let mut queue = self.send_queue.lock_or_recover(); - queue.push_back((peer_id.to_string(), message.clone())); - } + enqueue_send( + &mut self.send_queue.lock_or_recover(), + (peer_id.to_string(), message.clone()), + ); self.update_queue_metric(); self.notify_fragments_available(); @@ -1569,6 +1604,22 @@ mod tests { ); } + /// A message counts as queued until the bridge pulls its first fragment. + /// The sender reads this to tell "not sent yet" from "unanswered" when an + /// acknowledgement times out. + #[test] + fn queued_message_ids_lists_a_message_until_its_first_fragment_is_pulled() { + let transport = BleTransport::new("test-device"); + transport.on_status_changed(TransportStatus::Available); + transport.on_peer_discovered(peer_device("bob")); + let message = small_message(); + transport.send(&message).unwrap(); + + assert_eq!(transport.queued_message_ids(), vec![message.id.as_str()]); + assert!(transport.get_next_fragment().unwrap().is_some()); + assert!(transport.queued_message_ids().is_empty()); + } + #[test] fn test_ble_on_status_changed_available_to_available_preserves_state() { let transport = BleTransport::new("test-device"); @@ -2757,4 +2808,45 @@ mod tests { assert_eq!(metrics.delivery_ratio, None); assert_eq!(metrics.drop_rate, None); } + + #[test] + fn an_acknowledgement_is_queued_ahead_of_data_but_behind_earlier_acknowledgements() { + let transport = BleTransport::new("test-device"); + transport.on_status_changed(TransportStatus::Available); + transport.on_peer_discovered(peer_device("bob")); + + let data: Vec = (0..3).map(|_| small_message()).collect(); + let mut ack = small_message(); + ack.metadata + .insert(ACK_FOR_KEY.to_string(), "first".to_string()); + let mut second_ack = small_message(); + second_ack + .metadata + .insert(ACK_FOR_KEY.to_string(), "second".to_string()); + + transport.send(&data[0]).unwrap(); + transport.send(&data[1]).unwrap(); + transport.send(&ack).unwrap(); + transport.send(&data[2]).unwrap(); + transport.send(&second_ack).unwrap(); + + let order: Vec<_> = transport + .send_queue + .lock() + .unwrap() + .iter() + .map(|(_, message)| message.id.clone()) + .collect(); + assert_eq!( + order, + vec![ + ack.id.clone(), + second_ack.id.clone(), + data[0].id.clone(), + data[1].id.clone(), + data[2].id.clone() + ], + "acknowledgements stop retry timers; queued behind data they arrived too late" + ); + } } diff --git a/crates/offline-protocol-transport/src/traits.rs b/crates/offline-protocol-transport/src/traits.rs index 6b2fb4201..2bda4cc9d 100644 --- a/crates/offline-protocol-transport/src/traits.rs +++ b/crates/offline-protocol-transport/src/traits.rs @@ -272,6 +272,16 @@ pub trait Transport: Send + Sync + Any { // Confirmation loop (platform → Rust delivery outcomes) // ------------------------------------------------------------------ + /// Ids of messages this transport has accepted but not yet started + /// writing: still in its local queue, not yet handed to the radio. + /// + /// The sender uses it to tell "unanswered" from "not sent yet" when an + /// acknowledgement times out. The default is empty: a transport that + /// writes on `send`, or does not know, keeps the ordinary timeout. + fn queued_message_ids(&self) -> Vec { + Vec::new() + } + /// Platform confirms a message previously returned by /// [`Transport::get_next_message`] was sent over the wire. /// diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 957a89810..085df7d57 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -13795,6 +13795,234 @@ mod tests { /// GATT read later, together; if a refactor separates them, /// `fragment_fallback_count` starts ticking in production and nothing /// fails at compile time. + /// The two outbound fragment queues share one cap and one backed-up mark, + /// the cap holds a whole maximum-size message, and the mark sits below it. + /// + /// The drain checks the mark before every pop, so a peer never holds more + /// than the mark and the overflow discard stays unreachable. With a cap of + /// 100 the 2026-10-19 volume test never delivered a 16 KB or 30 KB message + /// from Android. The mark is small because the core sees only what it + /// still holds: at 3/4 of the cap the bridge pulled 576 fragments ahead of + /// the link and the core resent everything behind them. The values are + /// hand-mirrored (C5), so a drifted copy would leave one platform tearing + /// or resending what the other delivers. iOS also has to hand the cap to + /// both of its outbound queues, since it constructs them explicitly. + #[test] + fn the_outbound_fragment_queue_cap_holds_a_maximum_message_on_both_platforms() { + fn value_after(source: &str, marker: &str) -> usize { + let rest = &source[source + .find(marker) + .unwrap_or_else(|| panic!("`{marker}` is gone")) + + marker.len()..]; + rest.trim_start() + .chars() + .take_while(char::is_ascii_digit) + .collect::() + .parse() + .unwrap_or_else(|_| panic!("`{marker}` is not followed by a number")) + } + let kotlin = rn_source_code_only( + "android/src/main/java/com/offlineprotocol/ble/OutboundFragmentQueue.kt", + ); + let swift = rn_source_code_only("ios/OutboundFragmentQueue.swift"); + let manager = rn_source_code_only("ios/BleManager.swift"); + + let android = value_after(&kotlin, "const val DEFAULT_MAX_PER_PEER: Int ="); + let ios = value_after(&swift, "static let defaultMaxPerPeer ="); + assert_eq!( + android, ios, + "the two outbound fragment queue caps must match" + ); + let largest = offline_protocol_transport::constants::BLE_MAX_FRAGMENT_COUNT; + assert!( + android >= largest, + "the cap ({android}) must hold a whole {largest}-fragment message" + ); + let android_mark = value_after(&kotlin, "const val DEFAULT_BACKPRESSURE_MARK: Int ="); + let ios_mark = value_after(&swift, "static let defaultBackpressureMark ="); + assert_eq!( + android_mark, ios_mark, + "the two outbound fragment queue marks must match" + ); + assert!( + 0 < android_mark && android_mark < android, + "the backed-up mark ({android_mark}) must sit below the cap ({android})" + ); + assert!( + manager.contains( + "private let MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER = OutboundFragmentQueue.defaultMaxPerPeer" + ) && manager.matches("maxPerPeer: MAX_PENDING_OUTBOUND_FRAGMENTS_PER_PEER,").count() == 2, + "both iOS outbound queues must be built with the shared outbound cap" + ); + } + + /// The bridge wiring the 2026-10-19 volume-test fixes depend on, which CI + /// only type-checks. Each line below names the failure its absence + /// brought back: + /// + /// - Both drains stop before the destructive pop only for a recipient with + /// a link. Counting every recipient stopped every peer's egress behind + /// one whose link had closed; checking after the pop overflowed the cap. + /// - The iOS connect timeout cancels only the attempt that armed it. + /// Capturing only the peripheral cancelled a healthy redial armed after + /// a failed dial, and the cancel was reported as the peer lost. + /// - iOS forgets a link's write stall on connect and disconnect, so a + /// reconnected link is not torn down by the clock of the dead one. + /// - iOS retires a dropped identifier, instead of reporting the peer lost, + /// on both connect-failure callbacks while the peer is linked elsewhere, + /// and only a write the link took ends a stall. + /// - Android's forced refresh restarts the advertisement only through the + /// policy that keeps a linked peer's address pair stable (ADR 0030). + #[test] + fn the_ble_bridges_keep_the_volume_test_wiring() { + fn swift_body<'a>(source: &'a str, signature: &str) -> &'a str { + let start = source + .find(signature) + .unwrap_or_else(|| panic!("BleManager.swift lost `{signature}`")); + let rest = &source[start + signature.len()..]; + let end = [" func ", " private func ", " public func "] + .iter() + .filter_map(|marker| rest.find(marker)) + .min() + .unwrap_or(rest.len()); + &rest[..end] + } + let swift = rn_source_code_only("ios/BleManager.swift"); + let kotlin = rn_source_code_only( + "android/src/main/java/com/offlineprotocol/ble/BleTransportFacade.kt", + ); + + // Stop before the pop, for linked recipients only. + let pop = swift + .find("self.protocolInstance.bleGetNextFragment()") + .expect("the iOS drain pops from the core"); + let stop = swift + .find("self.outboundFragments.anyBackedUp(deliverable: deliverable) || self.notifyFragments.anyBackedUp(deliverable: deliverable)") + .expect("the iOS drain must stop on backed-up recipients that have a link, in both queues"); + assert!( + stop < pop, + "the iOS drain must stop before the pop, not after it" + ); + assert_eq!(swift.matches("bleGetNextFragment()").count(), 1); + assert!( + !swift.contains("anyBackedUp() ||"), + "an unfiltered anyBackedUp() is back in the iOS drain" + ); + + let kotlin_pops: Vec = kotlin + .match_indices("protocol.bleGetNextFragment()") + .map(|(at, _)| at) + .collect(); + assert_eq!( + kotlin_pops.len(), + 2, + "the Android drain and the poll floor are the two pops" + ); + let kotlin_stops: Vec = kotlin + .match_indices("outboundQueue.anyBackedUp { hasDeliverableLink(it) }") + .map(|(at, _)| at) + .collect(); + assert_eq!( + kotlin_stops.len(), + 2, + "each Android pop needs its own linked-only stop" + ); + for (stop, pop) in kotlin_stops.iter().zip(&kotlin_pops) { + assert!(stop < pop, "an Android pop runs before its backed-up stop"); + } + assert!( + !kotlin.contains("anyBackedUp()"), + "an unfiltered anyBackedUp() is back on Android" + ); + + // The connect timeout belongs to its attempt. + let attempt = swift_body(&swift, "private func performConnectionAttempt("); + assert!( + attempt.contains("connectTimeoutTokens[peripheral.identifier] = attempt") + && attempt.contains("self.connectTimeoutTokens[peripheral.identifier] == attempt,"), + "the iOS connect timeout must be bound to the attempt that armed it" + ); + + // The stall clock and the timeout token do not outlive a link. + let did_connect = swift_body(&swift, "func centralManager(_ central: CBCentralManager, didConnect peripheral: CBPeripheral) {"); + let did_fail = swift_body(&swift, "func centralManager(_ central: CBCentralManager, didFailToConnect peripheral: CBPeripheral, error: Error?) {"); + let did_disconnect = swift_body(&swift, "func centralManager(_ central: CBCentralManager, didDisconnectPeripheral peripheral: CBPeripheral, error: Error?) {"); + for (name, body) in [ + ("didConnect", did_connect), + ("didDisconnectPeripheral", did_disconnect), + ] { + assert!( + body.contains("writeStallPolicy.forget(peripheral.identifier)"), + "{name} must forget the link's write stall" + ); + assert!( + body.contains("connectTimeoutTokens.removeValue(forKey: peripheral.identifier)"), + "{name} must drop the link's connect timeout" + ); + } + assert!( + did_fail.contains("connectTimeoutTokens.removeValue(forKey: peripheral.identifier)") + ); + + // A peer linked elsewhere is retired, not lost, on both callbacks, and + // the check runs before anything reports or redials. + for (name, body, before) in [ + ( + "didFailToConnect", + did_fail, + "guard retryCount <= MAX_CONNECTION_RETRIES", + ), + ( + "didDisconnectPeripheral", + did_disconnect, + "if wasConnected {", + ), + ] { + let retire = body + .find("if retireIfLinkedElsewhere(peripheral.identifier, wasConnected:") + .unwrap_or_else(|| { + panic!("{name} must retire an identifier of a peer linked elsewhere") + }); + let next = body + .find(before) + .unwrap_or_else(|| panic!("{name} lost `{before}`")); + assert!( + retire < next, + "{name} must retire before it reports or redials" + ); + } + // A dial that never connected unmaps the identifier, so the monitor + // does not redial a departed peer forever. + assert!( + did_disconnect + .contains("connections.removePeripheralDeviceId(for: peripheral.identifier)"), + "the never-connected branch must unmap the identifier" + ); + + // Only a write the link took, or a ready callback, ends a stall. + let ready = swift_body( + &swift, + "public func peripheralIsReady(toSendWriteWithoutResponse peripheral: CBPeripheral) {", + ); + assert!(ready.contains("writeStallPolicy.noteWritable(peripheral.identifier)")); + assert!(swift.contains( + "peripheral.writeValue(data, for: characteristic, type: .withoutResponse) writeStallPolicy.noteWritable(peripheral.identifier)" + )); + assert_eq!(swift.matches("writeStallPolicy.noteRefused(").count(), 1); + assert!( + swift.contains("writeStallPolicy.reset()"), + "stop must reset every link's stall" + ); + + // Android's forced refresh goes through the policy. + assert!( + kotlin.contains( + "if (AdvertisingRefreshPolicy.restartsAdvertising(identifiedLinks, leAdvertiser.isAdvertising)) { refreshAdvertising(\"forced_refresh\")" + ), + "the forced refresh must restart the advertisement only through AdvertisingRefreshPolicy" + ); + } + #[test] fn react_native_ble_announces_only_cross_checked_addresses() { let swift = rn_source_code_only("ios/BleManager.swift"); @@ -14162,8 +14390,13 @@ mod tests { .find("is PeerIdentityBinding.Outcome.Verified ->") .expect("handleIdentityRead must have a verified arm"); let check = identity - .find("if (host.connections.hasOtherClientLink(outcome.peerId, address)) {") - .expect("the verified arm must check for another client link to the peer"); + .find( + "if (host.connections.hasOtherUsableLink(outcome.peerId, address, host::isCentralSubscribed)) {", + ) + .expect( + "the verified arm must check for another usable link to the peer: a client \ + link, or its subscribed link to our server (ADR 0030)", + ); let refuse = identity .find("refuseDuplicateLink(gatt, outcome.peerId)") .expect("a duplicate must be refused through refuseDuplicateLink"); @@ -15012,8 +15245,19 @@ mod tests { let drain = &facade[drain_start..]; let drain = &drain[..rn_kotlin_body_end(drain)]; assert!( - drain.contains( - "val hasConnection = address?.let { connections.getGatt(it) } != null || \ + drain.contains("if (!hasDeliverableLink(recipientId, address)) {"), + "the drain must decide a peer is linked through hasDeliverableLink" + ); + let link_decl = "private fun hasDeliverableLink("; + let link_start = facade + .find(link_decl) + .expect("the facade decides deliverability in one place") + + link_decl.len(); + let link = &facade[link_start..]; + let link = &link[..rn_kotlin_body_end(link)]; + assert!( + link.contains( + "address?.let { connections.getGatt(it) } != null || \ subscribedNotifyAddressFor(recipientId) != null" ), "the drain must treat a notify-subscribed peer as connected" diff --git a/crates/offline-protocol/src/constants.rs b/crates/offline-protocol/src/constants.rs index 7ab7b819d..d1030a2c4 100644 --- a/crates/offline-protocol/src/constants.rs +++ b/crates/offline-protocol/src/constants.rs @@ -28,6 +28,24 @@ pub const ACK_STATUS_KEY: &str = "ack_status"; /// the frame drops it. pub const ACK_STATUS_UNDECRYPTABLE: &str = "undecryptable"; +/// Metadata key on a delivery ACK that answers more than one frame: the ids +/// after the one in [`ACK_FOR_KEY`], comma-separated. Sent only to a peer that +/// advertised [`ACK_COALESCED_V1`](offline_protocol_sealed::ACK_COALESCED_V1), +/// and never on an `undecryptable` answer. A peer that ignores it still +/// settles the first id, and the rest through their resends' duplicate +/// re-acknowledgement. +pub const ACK_MORE_KEY: &str = "ack_more"; + +/// How many frames one coalesced delivery ACK may answer. Five keeps a +/// coalesced ACK inside one fragment at the 185-byte floor's neighbour, the +/// 512-byte fragment both phones negotiate: 264 bytes plus four 37-byte ids. +pub const ACK_COALESCE_MAX: usize = 5; + +/// The longest a delivery ACK waits to be coalesced with others to the same +/// peer. Short against the 10-second acknowledgement timeout; long enough +/// that at a few messages a second each ACK frame answers two or three. +pub const ACK_COALESCE_WINDOW: std::time::Duration = std::time::Duration::from_millis(500); + /// Metadata key marking a frame the SDK sent on its own: control traffic, /// key packages, data-sync frames, receipts, typing, presence, and a 1:1 /// Welcome no user message is waiting on. @@ -121,6 +139,21 @@ pub const MAX_MEDIA_DESCRIPTORS: usize = 16; /// tolerance and permanently stall its transfer. pub const MAX_CONCURRENT_MEDIA_TRANSFERS_PER_PEER: usize = 2; +/// The receiver's forward window has to sit well above the number of +/// messages this engine keeps in flight to one peer. +/// +/// Every resend is re-sealed under a fresh ratchet generation, and a receiver +/// that drops a resend as a duplicate of a message it already holds skips +/// that generation. A congested link can therefore open a gap of several +/// in-flight ceilings, and a receiver more than the window behind decrypts +/// nothing from that sender until the pair re-keys. The 2026-10-19 volume +/// test hit the old window of 1000 at exactly the in-flight ceiling of 1000. +/// Stated to the compiler so raising the ceiling past it fails the build. +const _: () = assert!( + offline_protocol_mls::group::SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE as usize + >= 4 * offline_protocol_reliability::constants::DEFAULT_MAX_PENDING_ACKS +); + /// How many of a peer's transfer slots the SDK's own document-layer traffic /// may occupy at once. /// diff --git a/crates/offline-protocol/src/group_mesh.rs b/crates/offline-protocol/src/group_mesh.rs index f298eff86..bd4070af1 100644 --- a/crates/offline-protocol/src/group_mesh.rs +++ b/crates/offline-protocol/src/group_mesh.rs @@ -40,7 +40,7 @@ pub(crate) const MAX_BASE64_PAYLOAD_SIZE: usize = 1_048_576; /// the generation every member must reach to decrypt anything later from /// this device. A frame delivered to one member advances it for all of them /// and is observed by one, and OpenMLS refuses a generation more than -/// [`SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE`] (1000) ahead of the highest +/// [`SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE`] (8192) ahead of the highest /// a receiver has seen. Past that gap the absent member cannot decrypt this /// device's next frame, and cannot decrypt the one after that either, /// because nothing advanced their ratchet: the group silently stops @@ -50,7 +50,7 @@ pub(crate) const MAX_BASE64_PAYLOAD_SIZE: usize = 1_048_576; /// Directed replication answers are the first traffic in this SDK that /// advances the ratchet without every member seeing it, and they are /// produced by rediscovery rather than by anything the user does, so -/// unbounded they cross 1000 on their own. Promoting one frame per budget +/// unbounded they cross that distance on their own. Promoting one frame per budget /// to the whole roster puts a decryptable rung in every member's outbox and /// bounds the gap at the budget, restoring the property group messaging /// always had: generations advance roughly with roster-wide traffic. @@ -361,7 +361,7 @@ pub(crate) struct GroupMeshState { /// others must reach. This counts how far that has run ahead of what /// the roster has been given a chance to observe; /// [`MAX_ROSTER_INVISIBLE_GROUP_GENERATIONS`] documents what happens - /// when it is allowed to run to 1000. + /// when it is allowed to run past the forward distance. /// /// **A missing entry means "unknown", not "zero".** The MLS sender /// ratchet is persisted with the group state and this map is not, so a diff --git a/crates/offline-protocol/src/protocol/blocking.rs b/crates/offline-protocol/src/protocol/blocking.rs index fc0b6ad47..7ccb4cf56 100644 --- a/crates/offline-protocol/src/protocol/blocking.rs +++ b/crates/offline-protocol/src/protocol/blocking.rs @@ -145,6 +145,7 @@ impl OfflineProtocol { // protection back, requestable by anyone who can get a user to do it. self.peer_ctrl_freshness.remove(user_id); self.peer_undecryptable_ack.remove(user_id); + self.peer_coalesced_ack.remove(user_id); self.peer_rich_payload.remove(user_id); self.peer_rich_attested.remove(user_id); // Advertised half only, as above: `svc_sealed_proved_peers` is a @@ -494,6 +495,58 @@ mod tests { assert_eq!(received.unwrap().id, msg_id); } + /// A message sent while its sender was blocked is never delivered, even + /// when the sender's retry arrives after the block is lifted. The retry is + /// answered instead, so the sender stops. Before, a ladder still running + /// at unblock delivered everything sent during the block. + #[test] + fn test_a_message_sent_during_a_block_is_not_delivered_after_unblock() { + use offline_protocol_core::{AppId, Message, UserId}; + use offline_protocol_transport::{mock::MockTransport, TransportType}; + + let mut proto = make_protocol("alice"); + let mock = MockTransport::new(TransportType::BLE); + mock.start().unwrap(); + let handle = mock.clone(); + proto + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(mock)); + proto.start().unwrap(); + proto.block_user("bob").unwrap(); + + let msg = Message::new( + UserId::new("bob").unwrap(), + UserId::new("alice").unwrap(), + AppId::new("test-app").unwrap(), + "sent while blocked", + ); + assert!( + msg.requires_ack, + "precondition: the sender is waiting for an answer" + ); + handle.queue_message(msg.clone()); + assert!(proto.receive_message().is_none()); + assert!( + handle.sent_messages().is_empty(), + "nothing is answered while the block lasts" + ); + + proto.unblock_user("bob").unwrap(); + handle.clear_sent_messages(); + handle.queue_message(msg.clone()); + assert!( + proto.receive_message().is_none(), + "a retry of a message sent during the block must not be delivered" + ); + assert!( + handle.sent_messages().iter().any(|m| m + .metadata + .get(offline_protocol_sealed::ACK_FOR_KEY) + .is_some_and(|id| *id == msg.id.as_str())), + "the retry is answered so the sender stops retrying" + ); + } + #[test] fn test_send_to_blocked_user_rejected() { use offline_protocol_transport::{mock::MockTransport, TransportType}; diff --git a/crates/offline-protocol/src/protocol/config_accessors.rs b/crates/offline-protocol/src/protocol/config_accessors.rs index 10fc82ec2..7909a34e8 100644 --- a/crates/offline-protocol/src/protocol/config_accessors.rs +++ b/crates/offline-protocol/src/protocol/config_accessors.rs @@ -357,9 +357,28 @@ impl OfflineProtocol { self.cleanup_stale_media_state(StdDuration::from_secs(MEDIA_TRANSFER_STALE_TIMEOUT_SECS)); #[cfg(feature = "data")] self.expire_attachment_fetches(); - // Prune old timed-out ACKs that weren't cleaned up by normal retry flow + // Prune old timed-out ACKs that weren't cleaned up by normal retry flow. + // + // A timed-out entry still holds a slot its scheduled resend will + // reuse, so it must outlive the longest wait the retry ladder can + // schedule. A fixed five minutes equalled the default ladder's cap + // exactly: the slot of a message on its ninth retry was pruned just + // before that resend fired, the resend needed a fresh slot, and at + // capacity there was none. self.ack_manager - .prune_old_timeouts(std::time::Duration::from_secs(300)); // 5 minutes + .prune_old_timeouts(self.timed_out_ack_retention()); + } + + /// How long a timed-out pending ACK is kept before the prune drops it: + /// twice the longest gap between a timeout and its resend (the ACK timeout + /// plus the retry ladder's maximum delay). + pub(crate) fn timed_out_ack_retention(&self) -> std::time::Duration { + let reliability = &self.config.reliability; + let longest_gap_ms = reliability + .ack + .default_timeout_ms + .saturating_add(reliability.retry.max_delay_ms); + std::time::Duration::from_millis(longest_gap_ms.saturating_mul(2)) } /// Evicts known peers not re-seen within `KNOWN_PEER_TTL_SECS`. diff --git a/crates/offline-protocol/src/protocol/data_sync.rs b/crates/offline-protocol/src/protocol/data_sync.rs index 3c00132fc..28a83f0e6 100644 --- a/crates/offline-protocol/src/protocol/data_sync.rs +++ b/crates/offline-protocol/src/protocol/data_sync.rs @@ -1125,8 +1125,10 @@ impl OfflineProtocol { // this and spends its whole retry budget on dead bytes; there is no // reason to inherit that. Staged after the duplicate check so the // entry belongs to a frame that is actually on the ladder. + let recipient = message.recipient.as_str().to_string(); self.stage_outbox_reseal( &message.id, + &recipient, crate::protocol::types::OutboxReseal { content: plaintext, priority: message.priority, @@ -1135,6 +1137,7 @@ impl OfflineProtocol { content_type: message.content_type, media_metadata: None, rich: None, + seal_mark: None, }, ); diff --git a/crates/offline-protocol/src/protocol/message_dispatch.rs b/crates/offline-protocol/src/protocol/message_dispatch.rs index 12b590e0f..664a34560 100644 --- a/crates/offline-protocol/src/protocol/message_dispatch.rs +++ b/crates/offline-protocol/src/protocol/message_dispatch.rs @@ -194,6 +194,19 @@ impl OfflineProtocol { } else { self.peer_undecryptable_ack.remove(sender); } + if payload + .ack_versions + .contains(&offline_protocol_sealed::ACK_COALESCED_V1) + { + if !self.peer_coalesced_ack.contains(sender) + && self.peer_coalesced_ack.len() >= MAX_KEY_PACKAGE_SENT_TO + { + self.peer_coalesced_ack.clear(); + } + self.peer_coalesced_ack.insert(sender.to_string()); + } else { + self.peer_coalesced_ack.remove(sender); + } // Record whether this peer replicates documents, so the data // layer may push `__DATA_V1__` frames to them. Same shape again: diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index 7239e6053..04472facf 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -193,6 +193,14 @@ pub struct OfflineProtocol { /// to prevent large file transfers from evicting regular messages. media_outbox: HashMap, + /// Outbox ids held back by the per-peer send window, per recipient, in + /// the order the app sent them. Sent oldest first as the peer's + /// acknowledgements free the window (`flush_peer_windows`). An id whose + /// outbox entry is gone (settled, expired, evicted) is skipped and dropped + /// there, so no removal path has to know about this. In-memory only: a + /// restart re-drives the outbox, and the gate holds it again. + window_held: HashMap>, + /// Consecutive unreachable-park counter per recipient for plain DMs, /// driving the escalating reachability-probe interval on every carrier /// (see `handle_recipient_unreachable_for_message` — @@ -390,6 +398,21 @@ pub struct OfflineProtocol { /// [`ACK_UNDECRYPTABLE_V1`]: offline_protocol_sealed::ACK_UNDECRYPTABLE_V1 pub(crate) peer_undecryptable_ack: std::collections::HashSet, + /// Peers that read `ack_more` on a delivery ACK ([`ACK_COALESCED_V1`] in + /// `ack_versions`), so several of their frames may be answered by one ACK + /// frame. Same lifecycle as `peer_undecryptable_ack` above. + /// + /// [`ACK_COALESCED_V1`]: offline_protocol_sealed::ACK_COALESCED_V1 + pub(crate) peer_coalesced_ack: std::collections::HashSet, + + /// Delivery ACKs waiting to be coalesced, per peer: the carrier the + /// answered frames arrived on, when the first was held, and the answered + /// ids with the first one's hop count. Sent as one frame once + /// `ACK_COALESCE_MAX` are held or `ACK_COALESCE_WINDOW` has passed + /// (`flush_coalesced_acks`). In-memory only: an ACK lost with it is + /// answered again when the resend arrives as a duplicate. + coalesced_acks_out: HashMap, + /// Peers that have presented a control-frame signature over the /// freshness-bound payload, so their frames under the older one are /// refused. @@ -1192,6 +1215,7 @@ impl OfflineProtocol { pending_reseal: HashMap::new(), undecryptable_resent: HashSet::new(), media_outbox: HashMap::new(), + window_held: HashMap::new(), dm_unreachable_parks: HashMap::new(), unrouted_acks: VecDeque::new(), reachability: reachability::ReachabilityFacts::default(), @@ -1209,6 +1233,8 @@ impl OfflineProtocol { peer_compact_envelope: std::collections::HashSet::new(), peer_ctrl_freshness: std::collections::HashSet::new(), peer_undecryptable_ack: std::collections::HashSet::new(), + peer_coalesced_ack: std::collections::HashSet::new(), + coalesced_acks_out: HashMap::new(), control_freshness_peers: std::collections::HashSet::new(), control_reset_watermark: std::collections::HashMap::new(), peer_rich_payload: std::collections::HashSet::new(), @@ -1418,6 +1444,7 @@ impl OfflineProtocol { let previous_peer_compact_envelope = self.peer_compact_envelope.clone(); let previous_peer_ctrl_freshness = self.peer_ctrl_freshness.clone(); let previous_peer_undecryptable_ack = self.peer_undecryptable_ack.clone(); + let previous_peer_coalesced_ack = self.peer_coalesced_ack.clone(); // Snapshotted with its siblings so a failed re-initialize restores it // too. It is a ratchet, so the direction that matters is that a // rollback must not *lose* it: a peer we hold to the newer payload has @@ -1590,6 +1617,7 @@ impl OfflineProtocol { self.peer_compact_envelope = previous_peer_compact_envelope; self.peer_ctrl_freshness = previous_peer_ctrl_freshness; self.peer_undecryptable_ack = previous_peer_undecryptable_ack; + self.peer_coalesced_ack = previous_peer_coalesced_ack; self.control_freshness_peers = previous_control_freshness_peers; self.control_reset_watermark = previous_control_reset_watermark; self.peer_rich_payload = previous_peer_rich_payload; @@ -2836,6 +2864,19 @@ impl OfflineProtocol { attempt_count: u32, transport_override: Option, ) -> Option { + // No slot to track it: leave the send to the retry queue, which waits + // for one, instead of transmitting a frame nobody tracks. + if message.requires_ack && !self.ack_manager.has_capacity_for(&message.id) { + self.ensure_outbox_entry(&message); + let _ = self.retry_queue.enqueue(message, attempt_count); + return None; + } + // A full peer window holds a frame that has not gone out yet; the + // window flush sends it when an acknowledgement frees a slot. + if !self.peer_window_admits(&message) { + self.hold_for_peer_window(&message); + return None; + } self.ensure_outbox_entry(&message); // Tier 2: re-seal against the peer's current session before flushing, so // a post-re-key epoch change no longer leaves the bytes undecryptable. @@ -2876,6 +2917,58 @@ impl OfflineProtocol { } } + /// Sends held messages into each peer's free window slots, oldest first, + /// at most a flush batch per peer per tick. + /// + /// Runs every tick rather than from the acknowledgement handler alone, + /// because a slot is also freed by give-up, expiry and eviction, and a + /// peer whose mesh link went away stops being governed by the window, so + /// everything held for it goes to transport selection. + fn flush_peer_windows(&mut self) { + if self.window_held.is_empty() { + return; + } + let peers: Vec = self.window_held.keys().cloned().collect(); + for peer in peers { + let mut budget = crate::constants::FLUSH_BATCH_LIMIT; + while budget > 0 { + let Some(message_id) = self + .window_held + .get(&peer) + .and_then(|held| held.front().cloned()) + else { + break; + }; + let Some((message, attempt_count)) = self + .outbox + .get(&message_id) + .map(|entry| (entry.message.clone(), entry.attempt_count)) + else { + // Settled, expired or evicted while held. + if let Some(held) = self.window_held.get_mut(&peer) { + held.pop_front(); + } + continue; + }; + if !self.peer_window_admits(&message) { + break; + } + if let Some(held) = self.window_held.get_mut(&peer) { + held.pop_front(); + } + budget -= 1; + let _ = self.try_flush_send_via(message, attempt_count, None); + } + if self + .window_held + .get(&peer) + .is_some_and(|held| held.is_empty()) + { + self.window_held.remove(&peer); + } + } + } + /// Called when a neighbor is lost. /// /// Cleans up tracking state for the lost peer. @@ -3621,11 +3714,27 @@ impl OfflineProtocol { // end. Throttled inside; hours-long holds need no per-tick walk. self.sweep_custody(); - self.process_retry_queue()?; - self.process_welcome_retry_queue()?; + // One step's failure never starves another. Every fallible step runs + // on every tick and the first error is reported once they all have. + // The steps are coupled: the retry queue fills the pending-ACK table + // and the timeout sweep is what empties it, so a retry error that + // returned early used to stop the sweep for good. + let mut first_error: Option = None; + let mut note = |step: &'static str, result: Result<()>| { + if let Err(err) = result { + warn!(step, error = %err, "process() step failed; continuing the tick"); + first_error.get_or_insert(err); + } + }; + // Before the retry ladder, so a frame the app sent first is not + // overtaken by one that only reached the retry queue later. + self.flush_peer_windows(); + self.flush_due_coalesced_acks(); + note("retry_queue", self.process_retry_queue()); + note("welcome_retry_queue", self.process_welcome_retry_queue()); self.process_relay_register_retries(); self.process_relay_broadcast_report_timeouts(); - self.process_timed_out_acks()?; + note("timed_out_acks", self.process_timed_out_acks()); // Throttle session reconciliation to avoid expensive storage I/O // (list_sessions → Keychain/Keystore) on every tick. Only run when @@ -3649,7 +3758,10 @@ impl OfflineProtocol { self.evaluate_relay_role(); self.tick_telemetry_categories(); - Ok(()) + match first_error { + Some(err) => Err(err), + None => Ok(()), + } } /// Reports this device's condition to the forwarding governor and announces @@ -3893,6 +4005,31 @@ impl OfflineProtocol { }; processed += 1; + + // No slot to track this resend: put it back unchanged and try + // again shortly. Checked before the re-seal, so a held resend + // burns no ratchet generation, and before the send, so nothing + // goes out that cannot be tracked. The retry count is not + // advanced: being held is not an attempt, so the wait is the + // ladder's first rung rather than this entry's next one. + if entry.message.requires_ack && !self.ack_manager.has_capacity_for(&entry.message.id) { + let recheck_ms = self.config.reliability.retry.initial_delay_ms; + let _ = self.retry_queue.enqueue_with_delay( + entry.message, + entry.retry_count, + recheck_ms, + ); + continue; + } + + // A frame that has not gone out yet waits for the peer's window + // instead, out of this queue: re-checking it every second would + // spend the batch on frames that cannot move. + if !self.peer_window_admits(&entry.message) { + self.hold_for_peer_window(&entry.message); + continue; + } + let previous_transport = self.transport_manager.current_transport(); // Normally finds the entry already there: a retry entry outlives // its outbox entry only if a give-up path skipped @@ -3917,7 +4054,23 @@ impl OfflineProtocol { match send_result { Ok(()) => { - let ack_registered_now = self.ensure_ack_registration(&entry.message)?; + // Never `?`. This propagated once, and `process()` then + // returned before `process_timed_out_acks` on every tick: + // the step that frees slots never ran again, so the table + // stayed full and the engine never recovered. The capacity + // check above makes a failure here a priority-eviction + // race at most. + let ack_registered_now = match self.ensure_ack_registration(&entry.message) { + Ok(registered) => registered, + Err(err) => { + warn!( + message_id = %entry.message.id, + error = %err, + "ACK registration failed after a retry send" + ); + false + } + }; if ack_registered_now { // The resend registered a brand-new ACK (the prior one @@ -4002,9 +4155,26 @@ impl OfflineProtocol { /// Processes timed out ACKs and handles retry or failure. fn process_timed_out_acks(&mut self) -> Result<()> { let timed_out = self.ack_manager.drain_timed_out(); + if timed_out.is_empty() { + return Ok(()); + } + // A frame still waiting in a local queue has not been answered because + // it has not been sent. Retrying it would queue a second, re-sealed + // copy behind the first: on a congested link that is what grew the + // queue, the round trip and the receiver's ratchet gap together in the + // 2026-10-19 volume test. It gets a bounded extension instead. + let queued = self.transport_manager.queued_message_ids(); for pending in timed_out { let message_id = pending.message_id.clone(); + if queued.contains(&message_id.as_str()) + && self + .ack_manager + .extend_while_queued(&message_id, QUEUED_ACK_GRACE_MAX) + { + continue; + } + if pending.retry_count >= self.config.reliability.retry.max_retries { self.handle_max_retries_exceeded(&message_id, pending.retry_count)?; continue; diff --git a/crates/offline-protocol/src/protocol/receive.rs b/crates/offline-protocol/src/protocol/receive.rs index daa3a4cb8..349f0ba9b 100644 --- a/crates/offline-protocol/src/protocol/receive.rs +++ b/crates/offline-protocol/src/protocol/receive.rs @@ -192,15 +192,26 @@ impl OfflineProtocol { } // Block filter: silently drop messages from blocked users - // addressed to us. No ACK, no event, no side-effects. - // Checked before mark_seen so that if the user is later - // unblocked, retransmissions can still be delivered. + // addressed to us. No ACK and no event, so the block + // reveals nothing to the sender while it lasts. + // + // The id IS remembered. A message sent during a block is + // never delivered, whenever the block ends: that is what a + // user who blocked someone expects, and it is the inbound + // half of the post-unblock reset, which already drops our + // own queued messages to that peer. Without this, a sender + // whose retry ladder was still running when the block was + // lifted delivered everything it had sent meanwhile. Now + // such a resend meets the duplicate path above, which + // answers it once the block is gone, so the sender stops + // retrying. if sender_blocked { debug!( sender = %message.sender, message_id = %message.id, "Dropping message from blocked user" ); + self.mark_seen_persisted(message.id.clone()); continue; } diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index fad262288..daaf16b1c 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -2,7 +2,7 @@ use super::custody::CUSTODY_META_KEY; use super::{ - base64_encode, internal_prefixes, lifetime_expired, lock_shared_state, + base64_encode, internal_prefixes, lifetime_expired, lock_shared_state, CoalescedAcks, ConnectionAcceptedPayload, ConnectionRequestPayload, KeyPackagePayload, MediaSendOptions, MediaTransferDescriptor, OfflineProtocol, OutboundMediaTransfer, OutboundSendPreparation, OutboxEntry, OutboxReseal, PendingConnectionRequest, PendingMessage, PendingProvenance, @@ -11,15 +11,18 @@ use super::{ MAX_KEY_PACKAGE_SENT_TO, MAX_MESSAGE_CONTENT_BYTES, MAX_PENDING_CONNECTION_REQUESTS, MAX_PENDING_EXPIRIES_PER_PASS, MAX_PENDING_MESSAGES_GLOBAL, MAX_PENDING_MESSAGES_PER_PEER, MAX_PENDING_MESSAGE_BYTES_GLOBAL, MAX_PENDING_MESSAGE_BYTES_PER_PEER, MAX_READ_RECEIPT_IDS, - MAX_RICH_EXTRAS_BYTES, MLS_ENVELOPE_COMPACT_V1, PENDING_CONNECTION_REQUEST_TTL, - RICH_PAYLOAD_V1, SEND_FAIL_REASON_RECIPIENT_UNREACHABLE, SEND_FAIL_REASON_RELAY_PUSHED, + MAX_RICH_EXTRAS_BYTES, MLS_ENVELOPE_COMPACT_V1, PEER_SEND_WINDOW, + PENDING_CONNECTION_REQUEST_TTL, RESEAL_GAP_BUDGET, RESEAL_HARD_LIMIT, RICH_PAYLOAD_V1, + SEND_DEFER_REASON_ACK_CAPACITY, SEND_DEFER_REASON_PEER_WINDOW, + SEND_FAIL_REASON_RECIPIENT_UNREACHABLE, SEND_FAIL_REASON_RELAY_PUSHED, SEND_FAIL_REASON_RELAY_PUSHED_STORED, SEND_FAIL_REASON_RELAY_STORED, SEND_FAIL_REASON_TRANSPORT, WELCOME_NO_CARRIER_RETRY_SECS, WELCOME_UNREACHABLE_RETRY_CAP_SECS, }; use super::{classify_transport_send_error, send_failure_token}; use crate::constants::{ - ACK_FOR_KEY, ACK_HOP_COUNT_KEY, ACK_STATUS_KEY, ACK_STATUS_UNDECRYPTABLE, ACK_TRANSPORT_KEY, - MAX_FORWARD_COUNT, MAX_OUTBOX_ENTRIES, + ACK_COALESCE_MAX, ACK_COALESCE_WINDOW, ACK_FOR_KEY, ACK_HOP_COUNT_KEY, ACK_MORE_KEY, + ACK_STATUS_KEY, ACK_STATUS_UNDECRYPTABLE, ACK_TRANSPORT_KEY, MAX_FORWARD_COUNT, + MAX_OUTBOX_ENTRIES, }; use crate::events::{DecryptionFailureCode, Event, PresenceStatus}; use crate::file_transfer::{FileChunk, OutboundTransferState}; @@ -35,7 +38,7 @@ use offline_protocol_core::{ }; use offline_protocol_mls::{EncryptedMessage, MlsManager}; use offline_protocol_transport::TransportType; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::sync::{Arc, RwLock}; use std::time::{Duration as StdDuration, Instant}; use tracing::{debug, error, info, warn}; @@ -217,6 +220,7 @@ impl OfflineProtocol { if message.content.starts_with(internal_prefixes::ENCRYPTED) { self.stage_outbox_reseal( &message.id, + &recipient_str, OutboxReseal { content: content_str, priority, @@ -225,6 +229,7 @@ impl OfflineProtocol { content_type: message.content_type, media_metadata: None, rich: Some(rich).filter(|r| r.is_any()), + seal_mark: None, }, ); } @@ -356,6 +361,7 @@ impl OfflineProtocol { if message.content.starts_with(internal_prefixes::ENCRYPTED) { self.stage_outbox_reseal( &message.id, + &recipient_str, OutboxReseal { content: original_message.content.clone(), priority, @@ -364,6 +370,7 @@ impl OfflineProtocol { content_type: message.content_type, media_metadata: message.media_metadata.clone(), rich: Some(rich).filter(|r| r.is_any()), + seal_mark: None, }, ); } @@ -384,6 +391,13 @@ impl OfflineProtocol { self.deduplicator.mark_seen_local(message_id.clone()); + if self.refuse_app_message_at_capacity(&message) + || self.defer_app_message_for_ack_capacity(&message) + || self.defer_app_message_for_peer_window(&message) + { + return Ok(message_id); + } + let previous_transport = self.transport_manager.current_transport(); // A recipient this device holds a live mesh link to is reached over @@ -474,6 +488,13 @@ impl OfflineProtocol { self.deduplicator.mark_seen_local(message_id.clone()); + if self.refuse_app_message_at_capacity(&message) + || self.defer_app_message_for_ack_capacity(&message) + || self.defer_app_message_for_peer_window(&message) + { + return Ok(message_id); + } + let previous_transport = self.transport_manager.current_transport(); let send_result = self .transport_manager @@ -580,7 +601,7 @@ impl OfflineProtocol { let message = self.create_message(recipient, sealed, Some(priority), None)?; if let Some(reseal) = reseal { - self.stage_outbox_reseal(&message.id, reseal); + self.stage_outbox_reseal(&message.id, recipient, reseal); } self.dispatch_internal_message(message) } @@ -599,6 +620,11 @@ impl OfflineProtocol { self.deduplicator.mark_seen_local(message_id.clone()); + // Internal frames owe the app no event: the retry queue sends it. + if self.defer_for_ack_capacity(&message).is_some() { + return Ok(message_id); + } + let previous_transport = self.transport_manager.current_transport(); let send_result = self.transport_manager.send(&message); let current_transport = self.transport_manager.current_transport(); @@ -751,6 +777,10 @@ impl OfflineProtocol { } self.deduplicator.mark_seen_local(message.id.clone()); + if self.defer_for_ack_capacity(&message).is_some() { + return; + } + let previous_transport = self.transport_manager.current_transport(); match self.transport_manager.send(&message) { Ok(()) => { @@ -2366,6 +2396,7 @@ impl OfflineProtocol { if message.content.starts_with(internal_prefixes::ENCRYPTED) { self.stage_outbox_reseal( &message.id, + recipient, OutboxReseal { content: msg.content.clone(), priority: msg.priority, @@ -2374,6 +2405,7 @@ impl OfflineProtocol { content_type: msg.content_type, media_metadata: msg.media_metadata.clone(), rich: msg.rich.clone(), + seal_mark: None, }, ); } @@ -3004,17 +3036,25 @@ impl OfflineProtocol { .insert(message.id.clone(), (file_id.to_string(), chunk_index)); let previous_transport = self.transport_manager.current_transport(); - let send_result = self - .transport_manager - .send_via_transport(&message, pinned_transport); + // A chunk held for a slot is sent by the retry queue, pinned to + // the same carrier; the transfer's window counts it as in flight. + let send_result = if self.defer_for_ack_capacity(&message).is_some() { + None + } else { + Some( + self.transport_manager + .send_via_transport(&message, pinned_transport), + ) + }; let current_transport = Some(pinned_transport); match send_result { - Ok(()) => { + None => {} + Some(Ok(())) => { self.handle_send_success(&message, current_transport)?; self.emit_transport_switch_event(previous_transport, current_transport)?; } - Err(err) => { + Some(Err(err)) => { let _ = self .handle_send_failure(&message, current_transport.or(previous_transport))?; // send_via_transport does not record retry failures internally. @@ -3132,6 +3172,195 @@ impl OfflineProtocol { // OUTBOX // ======================================================================== + /// Whether a full outbox may give up `entry` to make room. + /// + /// Never a message awaiting its acknowledgement: it is on the wire, and + /// under load on a live link evicting it turned every new send into the + /// failure of a frame already sent, whose acknowledgement then settled + /// nothing. + /// + /// A message the per-peer window holds has not been sent, so it may go, + /// but last in its recipient's line ([`Self::eviction_order_key`]). It was + /// once never evictable on a live link, and then one slow peer whose held + /// line filled the outbox made every send to every other recipient a + /// refusal. + fn outbox_entry_evictable(&self, id: &MessageId, entry: &OutboxEntry) -> bool { + !(entry.message.requires_ack && self.ack_manager.is_waiting_for_ack(id)) + } + + /// Each message the per-peer window holds for a recipient this device + /// has a live mesh link to, with its place in that recipient's line + /// (0 is the front, the next to be sent). + fn held_line_positions(&self) -> HashMap { + self.window_held + .iter() + .filter(|(recipient, _)| { + self.transport_manager + .mesh_transport_for(recipient) + .is_some() + }) + .flat_map(|(_, held)| held.iter().cloned().enumerate().map(|(at, id)| (id, at))) + .collect() + } + + /// Orders the evictable entries of one recipient: lower goes first. + /// + /// Held messages for a live peer go first, from the back of the line, + /// so what survives is the front, the part about to be sent, and the + /// conversation stays contiguous. Their place in the line decides, not a + /// timestamp: a burst is held within one millisecond. Everything else + /// goes oldest first: a message to an unreachable peer that has waited + /// longest is the least likely to land. + fn eviction_order_key(held: &HashMap, entry: &OutboxEntry) -> (u8, i64) { + match held.get(&entry.message.id) { + Some(&at) => (0, -(at as i64)), + None => (1, entry.first_sent_at.timestamp_millis()), + } + } + + /// Evicts one entry from a full outbox, choosing from the recipient with + /// the largest backlog, oldest first, and only entries + /// [`Self::outbox_entry_evictable`] allows. With `newcomer` set, an + /// app send is asking: only a recipient with a strictly larger backlog + /// than the newcomer's own may pay, so a message never displaces another + /// to the same recipient and a tie refuses the newcomer. Returns whether + /// an entry was evicted. + /// + /// The failure this replaces: the outbox evicted the least recently sent + /// entry to admit any new one, whatever either was. In run 5 of the + /// 2026-10-19 volume test a ten-minute outage failed about 2,500 queued + /// messages per phone this way, each one so that a newer message to the + /// same unreachable peer could take its slot. + pub(super) fn make_outbox_room(&mut self, newcomer: Option<&Message>) -> bool { + let mut backlog: HashMap<&str, usize> = HashMap::new(); + for entry in self.outbox.values() { + *backlog.entry(entry.message.recipient.as_str()).or_default() += 1; + } + let floor = newcomer.map(|message| { + ( + message.recipient.as_str(), + backlog + .get(message.recipient.as_str()) + .copied() + .unwrap_or(0), + ) + }); + let held = self.held_line_positions(); + let victim = self + .outbox + .iter() + .filter(|(id, entry)| { + let recipient = entry.message.recipient.as_str(); + let heavier = match floor { + Some((own, own_count)) => { + recipient != own && backlog.get(recipient).copied().unwrap_or(0) > own_count + } + None => true, + }; + heavier && self.outbox_entry_evictable(id, entry) + }) + .min_by(|(_, left), (_, right)| { + let left_backlog = backlog + .get(left.message.recipient.as_str()) + .copied() + .unwrap_or(0); + let right_backlog = backlog + .get(right.message.recipient.as_str()) + .copied() + .unwrap_or(0); + right_backlog + .cmp(&left_backlog) + .then_with(|| { + Self::eviction_order_key(&held, left) + .cmp(&Self::eviction_order_key(&held, right)) + }) + .then_with(|| left.message.id.as_str().cmp(&right.message.id.as_str())) + }) + .map(|(id, _)| id.clone()); + let Some(victim) = victim else { + if newcomer.is_none() { + // Every entry is being delivered, so the outbox runs over + // its cap by internal frames until acknowledgements free + // room. Bounded by the pending-acknowledgement table. + warn!( + entries = self.outbox.len(), + "Outbox full of messages in delivery; admitting an internal frame over the cap" + ); + } + return false; + }; + // Retired, not just removed: the retry and flush paths re-create a + // missing entry before resending, so a retry entry left behind would + // bring the message back. + let attempt_count = self + .retire_undeliverable_message(&victim, "outbox eviction") + .map_or(0, |entry| entry.attempt_count); + self.emit_outbox_capacity_failure(&victim, attempt_count); + true + } + + /// The terminal events a message dropped for outbox capacity owes the + /// app: `message_failed`, and for a connection request its typed + /// undeliverable event. Shared by eviction and by a refused send. + fn emit_outbox_capacity_failure(&mut self, message_id: &MessageId, attempt_count: u32) { + self.emit_event(Event::message_failed( + message_id.clone(), + "Outbox capacity exceeded", + attempt_count, + )); + if let Some(recipient) = self.take_undeliverable_connection_request(message_id) { + warn!( + recipient = %recipient, + message_id = %message_id, + "Connection request undeliverable: outbox capacity exceeded" + ); + self.emit_event(Event::connection_request_undeliverable( + recipient, + message_id.as_str(), + "outbox_capacity_exceeded", + )); + } + } + + /// Admission for a message the app sent, before anything is sealed again, + /// held or transmitted. Returns false when the outbox is full and the + /// message is refused; the caller then owes the app the terminal event + /// (`message_failed`, "Outbox capacity exceeded", no attempts). + /// + /// A full outbox refuses the newest message for the recipient with the + /// largest backlog rather than failing an older one, so a queued + /// conversation stays contiguous and the app hears at the moment of + /// sending. A message to a recipient with a smaller backlog is admitted by + /// evicting from the largest one, so a week of messages to one + /// unreachable peer cannot block a message to a reachable one. + pub(super) fn admit_app_message(&mut self, message: &Message) -> bool { + if !message.requires_ack + || Self::is_media_outbox_message(message) + || self.outbox.contains_key(&message.id) + || self.outbox.len() < MAX_OUTBOX_ENTRIES + { + return true; + } + self.make_outbox_room(Some(message)) + } + + /// [`Self::admit_app_message`] with the refusal settled: staged re-seal + /// provenance dropped (it holds plaintext) and the terminal event emitted. + /// Returns true when the caller should stop, having returned the id. + fn refuse_app_message_at_capacity(&mut self, message: &Message) -> bool { + if self.admit_app_message(message) { + return false; + } + self.pending_reseal.remove(&message.id); + warn!( + message_id = %message.id, + recipient = %message.recipient, + "Outbox full and this recipient holds the largest backlog; send refused" + ); + self.emit_outbox_capacity_failure(&message.id, 0); + true + } + pub(super) fn ensure_outbox_entry(&mut self, message: &Message) { if !message.requires_ack { return; @@ -3146,42 +3375,27 @@ impl OfflineProtocol { }; if !outbox.contains_key(&message.id) && outbox.len() >= capacity { - let oldest_id = outbox - .iter() - .min_by_key(|(_, entry)| entry.last_sent_at) - .map(|(id, _)| id.clone()); - if let Some(oldest_id) = oldest_id { - // Retired, not just removed: this function is also how the - // retry and flush paths re-create an entry before resending, - // so a retry entry left behind here would bring the message - // back and evict another in its place. - let attempt_count = self - .retire_undeliverable_message(&oldest_id, "outbox eviction") - .map_or(0, |entry| entry.attempt_count); - if !is_media { - // Capacity eviction is as terminal as expiry: the entry - // and its persisted copy are gone, so without an event - // the app shows the message as pending forever. (Media - // chunks surface through the transfer abort above.) - self.emit_event(Event::message_failed( - oldest_id.clone(), - "Outbox capacity exceeded", - attempt_count, - )); - if let Some(recipient) = self.take_undeliverable_connection_request(&oldest_id) - { - warn!( - recipient = %recipient, - message_id = %oldest_id, - "Connection request undeliverable: outbox capacity exceeded" - ); - self.emit_event(Event::connection_request_undeliverable( - recipient, - oldest_id.as_str(), - "outbox_capacity_exceeded", - )); - } + if is_media { + // Media keeps its own cap and its oldest-first eviction: a + // transfer has its own window, and an evicted chunk surfaces + // through the transfer abort rather than a message event. + let oldest_id = outbox + .iter() + .min_by_key(|(_, entry)| entry.last_sent_at) + .map(|(id, _)| id.clone()); + if let Some(oldest_id) = oldest_id { + // Retired, not just removed: this function is also how the + // retry and flush paths re-create an entry before resending, + // so a retry entry left behind here would bring the message + // back and evict another in its place. + self.retire_undeliverable_message(&oldest_id, "outbox eviction"); } + } else { + // Reached only by frames that never passed app admission + // (internal frames, and the defensive re-creation on the + // resend paths): an app send made its room up front in + // `admit_app_message`, or was refused there. + self.make_outbox_room(None); } } @@ -3227,6 +3441,16 @@ impl OfflineProtocol { let now = Utc::now(); let is_media = Self::is_media_outbox_message(message); + // The success path creates the entry here, not in + // `ensure_outbox_entry`, so it honours the cap here too. An app send + // already made its room in `admit_app_message`; this is for internal + // frames, which are never refused. + if !is_media + && !self.outbox.contains_key(&message.id) + && self.outbox.len() >= MAX_OUTBOX_ENTRIES + { + self.make_outbox_room(None); + } // Consume the staged re-seal provenance only when first creating a // (non-media) outbox entry; on later attempts the entry already carries // it. Staging for media is never populated. @@ -3275,10 +3499,27 @@ impl OfflineProtocol { /// outbox entry created during dispatch carries it. Callers gate on the /// content being a sealed encrypted DM (`__MLS_ENC__`); media and plaintext /// never stage and thus always replay verbatim. - pub(super) fn stage_outbox_reseal(&mut self, id: &MessageId, reseal: OutboxReseal) { + /// Stages `reseal` for message `id`, just sealed for `recipient`. Called + /// straight after the seal, before anything else is sealed, so the + /// session's latest seal is this message's. + pub(super) fn stage_outbox_reseal( + &mut self, + id: &MessageId, + recipient: &str, + mut reseal: OutboxReseal, + ) { + reseal.seal_mark = self.last_seal_mark(recipient); self.pending_reseal.insert(id.clone(), reseal); } + fn last_seal_mark(&self, recipient: &str) -> Option { + self.mls_manager + .as_ref()? + .read() + .ok()? + .last_seal_mark(recipient) + } + /// Consumes any staged re-seal provenance for `message`, attaching it to a /// (non-media) main-outbox entry being created for the first time. On later /// attempts the entry already exists and carries its `reseal`, so this @@ -3325,6 +3566,47 @@ impl OfflineProtocol { return None; } let reseal = self.outbox.get(&message.id)?.reseal.as_ref()?.clone(); + // Every re-seal spends a ratchet generation the receiver has to + // fast-forward over, and one it drops as a duplicate is never + // decrypted at all. Past the budget, a resend goes out verbatim: it + // costs nothing, and it still decrypts unless the receiver has since + // moved more than the out-of-order tolerance past it. Without this, a + // link that stops carrying acknowledgements turns the retry ladder + // into a key-burner: in the 2026-10-19 rerun one phone re-sealed + // 20,919 resends during a 17-minute stall and left its peer past even + // the widened forward window. + let unacknowledged = self + .mls_manager + .as_ref() + .and_then(|mls| mls.read().ok()) + .map_or(0, |mls| mls.unacknowledged_seals(&recipient)); + // A verbatim copy this far behind cannot decrypt any more, so past + // the budget it is still re-sealed, up to the hard limit (see + // `RESEAL_HARD_LIMIT`). + let verbatim_is_dead = match (reseal.seal_mark, self.last_seal_mark(&recipient)) { + (Some(sealed_at), Some(newest)) => { + sealed_at.incarnation == newest.incarnation + && newest.index.saturating_sub(sealed_at.index) + > u64::from( + offline_protocol_mls::group::SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, + ) + } + _ => false, + }; + let budget_spent = if verbatim_is_dead { + unacknowledged >= RESEAL_HARD_LIMIT + } else { + unacknowledged >= RESEAL_GAP_BUDGET + }; + if budget_spent { + debug!( + message_id = %message.id, + recipient = %recipient, + unacknowledged, + "Re-seal budget spent; resending verbatim until the peer acknowledges" + ); + return None; + } match self.prepare_outbound_content( &recipient, &reseal.content, @@ -3338,7 +3620,17 @@ impl OfflineProtocol { Some(PendingProvenance::for_id(message.id.clone())), "resend_reseal", ) { - Ok(OutboundSendPreparation::Ready(sealed)) if sealed != message.content => Some(sealed), + Ok(OutboundSendPreparation::Ready(sealed)) if sealed != message.content => { + let mark = self.last_seal_mark(&recipient); + if let Some(provenance) = self + .outbox + .get_mut(&message.id) + .and_then(|entry| entry.reseal.as_mut()) + { + provenance.seal_mark = mark; + } + Some(sealed) + } // Identical bytes, or an error: replay verbatim this round. Tier 1 // keeps the sender retrying until the session heals. // @@ -3648,6 +3940,21 @@ impl OfflineProtocol { message.content_type == ContentType::FileChunk } + /// A frame that sets up or confirms a 1:1 session: a key package, a + /// Welcome, or a confirmation probe or its answer. Never windowed (see + /// [`Self::peer_window_governs`]). + pub(super) fn is_session_control_frame(message: &Message) -> bool { + let content = message.content.as_str(); + [ + internal_prefixes::KEY_PACKAGE, + internal_prefixes::WELCOME, + internal_prefixes::SESSION_CONFIRM_PROBE, + internal_prefixes::SESSION_CONFIRM_ACK, + ] + .iter() + .any(|prefix| content.starts_with(prefix)) + } + /// A session confirmation probe lives in memory only. It carries no user /// data, and a restarted process does not know which probe was the last, /// so a restored one could never be superseded; the next scan asks the @@ -3697,13 +4004,171 @@ impl OfflineProtocol { infrastructure_reaches_peer || self.transport_manager.mesh_can_address(peer_id) } + /// Holds `message` back when the pending-ACK table has no slot for it, + /// returning when the retry queue will try it again; `None` means it has a + /// slot and the caller transmits as usual. + /// + /// The check runs **before** the transmit, because nothing honest is left + /// to do after it. A frame sent without a slot is either untracked (its + /// loss is never noticed) or reported to the app as a failed send while it + /// is on the wire, which is what the 2026-10-19 volume test saw: the + /// receiver got 4,626 messages from a sender whose API had accepted 2,748, + /// and the retry path's copy of the same error aborted every later + /// `process()` tick, so the step that frees slots never ran again. + /// + /// A held message goes to the outbox and the retry queue at its first + /// attempt, so holding it spends nothing from its retry budget, and it is + /// not offered to the mesh: it was not sent, so nothing failed. + pub(super) fn defer_for_ack_capacity(&mut self, message: &Message) -> Option> { + if !message.requires_ack || self.ack_manager.has_capacity_for(&message.id) { + return None; + } + self.ensure_outbox_entry(message); + let next_retry_at = self + .retry_queue + .enqueue(message.clone(), 0) + .unwrap_or_else(Utc::now); + debug!( + message_id = %message.id, + pending_acks = self.ack_manager.pending_count(), + "Pending-ACK table full; message held for the retry queue" + ); + Some(next_retry_at) + } + + /// [`Self::defer_for_ack_capacity`] for a message the app sent, which is + /// owed a `message_deferred` so it does not read as stuck. + fn defer_app_message_for_ack_capacity(&mut self, message: &Message) -> bool { + let Some(next_retry_at) = self.defer_for_ack_capacity(message) else { + return false; + }; + self.emit_event(Event::message_deferred( + message.id.clone(), + message.recipient.as_str().to_string(), + SEND_DEFER_REASON_ACK_CAPACITY, + 0, + Some(next_retry_at.timestamp_millis()), + )); + true + } + + // ------------------------------------------------------------------------ + // Per-peer send window (see `PEER_SEND_WINDOW`) + // ------------------------------------------------------------------------ + + /// Frames to `recipient` that went out over a mesh carrier and still + /// await its acknowledgement. Derived from the outbox rather than kept as + /// a count, so no settle, expiry or eviction path can leak a slot. + pub(super) fn mesh_frames_in_flight_to(&self, recipient: &str) -> usize { + self.outbox + .values() + .filter(|entry| { + entry.attempt_count > 0 + && entry.message.recipient.as_str() == recipient + && matches!( + entry.last_transport, + Some(TransportType::BLE | TransportType::WiFiDirect) + ) + }) + .count() + } + + /// Whether `message` is one the window governs: a frame that waits for an + /// acknowledgement, is not a media chunk (transfers have their own + /// window), is not session control, and goes to a peer this device holds + /// a live mesh link to. + /// + /// Session control is exempt because it is what heals the session the + /// window's frames are sealed under. A key package or Welcome held behind + /// sixteen frames the peer cannot decrypt waits for acknowledgements that + /// never come, and the window then stays full of frames that are only + /// retried, never answered. + fn peer_window_governs(&self, message: &Message) -> bool { + message.requires_ack + && !Self::is_media_outbox_message(message) + && !Self::is_session_control_frame(message) + && self + .transport_manager + .mesh_transport_for(message.recipient.as_str()) + .is_some() + } + + /// Whether `message` may be transmitted now. A frame already in flight + /// always may (its resend is not a new slot); a new one only while the + /// peer has fewer than [`PEER_SEND_WINDOW`] frames in flight. + pub(super) fn peer_window_admits(&self, message: &Message) -> bool { + if !self.peer_window_governs(message) { + return true; + } + if self + .outbox + .get(&message.id) + .is_some_and(|entry| entry.attempt_count > 0) + { + return true; + } + self.mesh_frames_in_flight_to(message.recipient.as_str()) < PEER_SEND_WINDOW + } + + /// Parks `message` in the outbox, out of the retry queue, at the back of + /// its recipient's held line. No attempt is counted and nothing is + /// sealed again: being held is not an attempt. + pub(super) fn hold_for_peer_window(&mut self, message: &Message) { + self.ensure_outbox_entry(message); + self.retry_queue.remove(&message.id.as_str()); + let held = self + .window_held + .entry(message.recipient.as_str().to_string()) + .or_default(); + if !held.contains(&message.id) { + held.push_back(message.id.clone()); + } + } + + /// The window gate for a message the app just sent. Holds it when the + /// peer's window is full, or when earlier messages to that peer are + /// already held, so a new message never overtakes them. Emits + /// `message_deferred` so the app can show it as queued. + fn defer_app_message_for_peer_window(&mut self, message: &Message) -> bool { + if !self.peer_window_governs(message) { + return false; + } + let held_ahead = self + .window_held + .get(message.recipient.as_str()) + .is_some_and(|held| !held.is_empty()); + if !held_ahead && self.peer_window_admits(message) { + return false; + } + self.hold_for_peer_window(message); + debug!( + message_id = %message.id, + recipient = %message.recipient, + "Peer send window full; message held until an acknowledgement frees it" + ); + self.emit_event(Event::message_deferred( + message.id.clone(), + message.recipient.as_str().to_string(), + SEND_DEFER_REASON_PEER_WINDOW, + 0, + None, + )); + true + } + pub(super) fn handle_send_success( &mut self, message: &Message, transport: Option, ) -> Result<()> { self.mark_message_sent(message, transport, Some(1)); - self.ensure_ack_registration(message)?; + // Never `?`: the frame is already on the wire, so an error here would + // report a transmitted message as failed. Every transmit path asks + // `defer_for_ack_capacity` first, so this is only reachable through a + // priority eviction race, and a warning is the honest outcome. + if let Err(err) = self.ensure_ack_registration(message) { + warn!(message_id = %message.id, error = %err, "ACK registration failed after transmit"); + } // A transport reporting success is not always evidence the frame can // arrive. The internet transport and Reticulum accept any recipient @@ -5126,10 +5591,115 @@ impl OfflineProtocol { inbound_transport: TransportType, status: Option<&str>, ) -> Result<()> { + if status.is_none() && self.try_coalesce_ack(message, inbound_transport) { + return Ok(()); + } let ack = self.build_ack(message, inbound_transport, status)?; self.route_ack(&ack, inbound_transport) } + /// Holds the delivery ACK for `message` to go out with others to the same + /// peer, when that peer reads coalesced ACKs and the frame came over a + /// mesh link. Returns `false` when the ACK must go on its own. + /// + /// Only the mesh carriers: they are where a phone's ACKs were a third of + /// the frames it sent, on a link carrying less than it was offered (the + /// 2026-10-19 volume test). A relay frame costs a round trip to a server, + /// not a share of a radio link. An `undecryptable` answer is never held: + /// it is not a delivery, and only a delivery may share a frame. + fn try_coalesce_ack(&mut self, message: &Message, inbound_transport: TransportType) -> bool { + if !matches!( + inbound_transport, + TransportType::BLE | TransportType::WiFiDirect + ) || !self.peer_coalesced_ack.contains(message.sender.as_str()) + { + return false; + } + let peer = message.sender.as_str().to_string(); + if self + .coalesced_acks_out + .get(&peer) + .is_some_and(|batch| batch.transport != inbound_transport) + { + self.flush_coalesced_acks_for(&peer); + } + let batch = self + .coalesced_acks_out + .entry(peer.clone()) + .or_insert_with(|| CoalescedAcks { + transport: inbound_transport, + since: Instant::now(), + hop_count: message.hop_count.value().to_string(), + ids: Vec::new(), + }); + let id = message.id.as_str(); + if !batch.ids.contains(&id) { + batch.ids.push(id); + } + if batch.ids.len() >= ACK_COALESCE_MAX { + self.flush_coalesced_acks_for(&peer); + } + true + } + + /// Sends every held ACK batch older than [`ACK_COALESCE_WINDOW`]. + /// Called each tick. + pub(super) fn flush_due_coalesced_acks(&mut self) { + if self.coalesced_acks_out.is_empty() { + return; + } + let due: Vec = self + .coalesced_acks_out + .iter() + .filter(|(_, batch)| batch.since.elapsed() >= ACK_COALESCE_WINDOW) + .map(|(peer, _)| peer.clone()) + .collect(); + for peer in due { + self.flush_coalesced_acks_for(&peer); + } + } + + /// Sends the ACK batch held for `peer` as one frame: the first id in + /// [`ACK_FOR_KEY`], so even a peer that ignores [`ACK_MORE_KEY`] settles + /// one, and the rest in [`ACK_MORE_KEY`]. + fn flush_coalesced_acks_for(&mut self, peer: &str) { + let Some(batch) = self.coalesced_acks_out.remove(peer) else { + return; + }; + let Some((first, more)) = batch.ids.split_first() else { + return; + }; + let built = (|| -> Result { + let ttl = TTL::new(self.config.initial_ttl).unwrap_or_else(|_| TTL::default()); + let mut builder = Message::builder( + UserId::new(&self.local_id)?, + UserId::new(peer)?, + AppId::new(&self.config.app_id)?, + ) + .content(String::new()) + .priority(MessagePriority::Low) + .ttl(ttl) + .requires_ack(false) + .metadata(ACK_FOR_KEY, first.as_str()) + .metadata(ACK_HOP_COUNT_KEY, batch.hop_count.as_str()) + .metadata(ACK_TRANSPORT_KEY, Self::transport_label(batch.transport)); + if !more.is_empty() { + builder = builder.metadata(ACK_MORE_KEY, more.join(",")); + } + Ok(builder.build()) + })(); + let ack = match built { + Ok(ack) => ack, + Err(err) => { + warn!(peer = %peer, error = %err, "Failed to build a coalesced delivery ACK"); + return; + } + }; + if let Err(err) = self.route_ack(&ack, batch.transport) { + debug!(peer = %peer, error = %err, "Coalesced delivery ACK not routed"); + } + } + /// Builds the acknowledgement for `message`, optionally carrying /// [`ACK_STATUS_KEY`]. Routing is the caller's. fn build_ack( @@ -5334,6 +5904,30 @@ impl OfflineProtocol { } pub(super) fn handle_ack_message(&mut self, message: &Message) { + // A coalesced ACK is settled as the single ACKs it stands for, so + // each id passes every check below on its own, the sender-equality + // check first. Never on an `undecryptable` answer, which is not a + // delivery and is never coalesced; and no more ids than a sender + // coalesces, so a forged one reaches no further than five frames + // could. + if message.metadata.contains_key(ACK_MORE_KEY) { + let mut single = message.clone(); + let more = single.metadata.remove(ACK_MORE_KEY).unwrap_or_default(); + self.handle_ack_message(&single); + if !single.metadata.contains_key(ACK_STATUS_KEY) { + for id in more + .split(',') + .filter(|id| !id.is_empty()) + .take(ACK_COALESCE_MAX - 1) + { + single + .metadata + .insert(ACK_FOR_KEY.to_string(), id.to_string()); + self.handle_ack_message(&single); + } + } + return; + } if let Some(ack_for) = message.metadata.get(ACK_FOR_KEY) { if let Ok(message_id) = MessageId::from_str(ack_for) { // Whoever answers has to be whoever we sent it to. An @@ -5388,6 +5982,22 @@ impl OfflineProtocol { return; } + // The peer decrypted this frame, so it has reached at least + // its place in the session, which re-opens the re-seal budget + // up to there. Unauthenticated like the rest of this path: a + // forged acknowledgement can only give back re-seals, which is + // where every sender was before the budget existed. + if let Some(mark) = self + .outbox + .get(&message_id) + .and_then(|entry| entry.reseal.as_ref()) + .and_then(|provenance| provenance.seal_mark) + { + if let Some(mls) = self.mls_manager.as_ref().and_then(|m| m.read().ok()) { + mls.note_acknowledged_seal(message.sender.as_str(), mark); + } + } + // A delivery ack settles an outbound connection request: // the recipient provably received it, so a later stale // recipient_unreachable signal must not fire a false @@ -6082,8 +6692,12 @@ impl OfflineProtocol { // downgrade the peer's traffic to the bootstrap key as a side // effect of a local setting. // Read unconditionally: an `undecryptable` ACK is settled as "not - // delivered" whatever our config, so a peer may always send us one. - ack_versions: vec![offline_protocol_sealed::ACK_UNDECRYPTABLE_V1], + // delivered" whatever our config, so a peer may always send us one, + // and a coalesced one settles every id it names. + ack_versions: vec![ + offline_protocol_sealed::ACK_UNDECRYPTABLE_V1, + offline_protocol_sealed::ACK_COALESCED_V1, + ], nostr_pubkey: self.transport_manager.nostr_public_key(), } } diff --git a/crates/offline-protocol/src/protocol/service_seal.rs b/crates/offline-protocol/src/protocol/service_seal.rs index 8b20ea899..0a45e5701 100644 --- a/crates/offline-protocol/src/protocol/service_seal.rs +++ b/crates/offline-protocol/src/protocol/service_seal.rs @@ -61,6 +61,7 @@ impl OfflineProtocol { content_type: ContentType::Text, media_metadata: None, rich: None, + seal_mark: None, }; self.send_sealed_internal_message(recipient, sealed, priority, Some(reseal)) } diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 757a1f8fb..1cdfd0fa2 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -597,10 +597,11 @@ impl OfflineProtocol { ); } Err(err) => { - // The send can fail after the probe entered the outbox (a - // full ACK tracker refuses it), and then no id comes back to - // track. Any probe still queued for this peer is such an - // orphan, since the tracked one was withdrawn above. + // The send can fail after the probe entered the outbox, and + // then no id comes back to track. Any probe still queued for + // this peer is such an orphan, since the tracked one was + // withdrawn above. (A full ACK tracker no longer lands here: + // it holds the probe and returns its id.) let orphans: Vec<_> = self .outbox .values() diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index f8fe730c2..064d1caa1 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -550,8 +550,9 @@ pub(crate) const PENDING_DECRYPT_PERSISTED_MAX_AGE_MS: i64 = 7 * 24 * 60 * 60 * /// Restore-walk bound for the outbox. /// -/// The live insert path hard-caps the outbox at [`MAX_OUTBOX_ENTRIES`] -/// (`ensure_outbox_entry` evicts before admitting), so a durable store can +/// The live insert path caps the outbox at [`MAX_OUTBOX_ENTRIES`] (an app send +/// is admitted or refused, and internal frames evict before inserting, over +/// the cap only while every entry awaits delivery), so a durable store can /// never legitimately hold more than that plus what a merge restored. Four /// times the cap is ample headroom and avoids walking — and transiently /// allocating up to [`MAX_PROTOCOL_STATE_RECORD_BYTES`] for — thirty times more @@ -3227,6 +3228,12 @@ impl OfflineProtocol { { self.peer_undecryptable_ack.insert(peer_id.clone()); } + if caps + .ack_versions + .contains(&offline_protocol_sealed::ACK_COALESCED_V1) + { + self.peer_coalesced_ack.insert(peer_id.clone()); + } if self.config.encryption.rich_payload_enabled && caps.attested_rich_versions.contains(&RICH_PAYLOAD_V1) { diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 2d7bafd9e..cecbb4809 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -14,7 +14,8 @@ mod service_seal; use super::*; use crate::constants::{ - ACK_FOR_KEY, ACK_HOP_COUNT_KEY, ACK_STATUS_KEY, ACK_STATUS_UNDECRYPTABLE, ACK_TRANSPORT_KEY, + ACK_COALESCE_MAX, ACK_COALESCE_WINDOW, ACK_FOR_KEY, ACK_HOP_COUNT_KEY, ACK_MORE_KEY, + ACK_STATUS_KEY, ACK_STATUS_UNDECRYPTABLE, ACK_TRANSPORT_KEY, }; use crate::events::{DecryptionFailureCode, PresenceSource, PresenceStatus, SecurityWarningCode}; use crate::mls_observability::{ @@ -26921,7 +26922,7 @@ fn test_cleanup_outbox_expiry_settles_pending_connection_request() { /// copy are gone, so it must emit `message_failed` rather than leave the /// app showing the message as pending forever. #[test] -fn test_outbox_capacity_eviction_emits_message_failed() { +fn test_outbox_capacity_refuses_the_newest_send_to_the_heaviest_recipient() { let mut config = create_test_config(); config.reliability.retry.initial_delay_ms = 60_000; config.reliability.retry.max_delay_ms = 60_000; @@ -26942,46 +26943,50 @@ fn test_outbox_capacity_eviction_emits_message_failed() { let first_id = protocol .send_message( "bob", - "oldest, will be evicted", + "oldest, must survive", None::, None::, ) .unwrap(); - // Backdate so the eviction victim is deterministic regardless of - // timestamp resolution across the flood below. - protocol - .outbox - .get_mut(&first_id) - .expect("first message must be in the outbox") - .last_sent_at -= ChronoDuration::seconds(60); + let mut last_id = None; for i in 0..crate::constants::MAX_OUTBOX_ENTRIES { - let _ = protocol.send_message( - "bob", - &format!("filler-{i}"), - None::, - None::, + last_id = Some( + protocol + .send_message( + "bob", + &format!("filler-{i}"), + None::, + None::, + ) + .unwrap(), ); } + let last_id = last_id.unwrap(); assert_eq!( protocol.outbox_entry_count(), crate::constants::MAX_OUTBOX_ENTRIES ); + assert!(protocol.outbox.contains_key(&first_id)); + assert!(!protocol.outbox.contains_key(&last_id)); let captured = events.lock().unwrap(); - let evicted: Vec<_> = captured + let failed: Vec<_> = captured .iter() .filter_map(|e| match e { Event::MessageFailed { - message_id, reason, .. - } if reason == "Outbox capacity exceeded" => Some(message_id.clone()), + message_id, + reason, + retry_count, + } if reason == "Outbox capacity exceeded" => Some((message_id.clone(), *retry_count)), _ => None, }) .collect(); assert_eq!( - evicted, - vec![first_id.as_str()], - "capacity eviction must emit exactly one terminal message_failed, for the oldest entry" + failed, + vec![(last_id.as_str().to_string(), 0)], + "a full outbox refuses the newest send to the recipient with the largest backlog, \ + and fails nothing already queued" ); } @@ -27014,9 +27019,11 @@ fn test_outbox_capacity_eviction_settles_pending_connection_request() { .outbox .get_mut(&sent_id) .expect("connection request must be in the outbox") - .last_sent_at -= ChronoDuration::seconds(60); + .first_sent_at -= ChronoDuration::seconds(60); - for i in 0..crate::constants::MAX_OUTBOX_ENTRIES { + // Bob holds the whole outbox; a send to someone with a smaller backlog + // evicts bob's oldest entry, the connection request. + for i in 0..crate::constants::MAX_OUTBOX_ENTRIES - 1 { let _ = protocol.send_message( "bob", &format!("filler-{i}"), @@ -27024,6 +27031,12 @@ fn test_outbox_capacity_eviction_settles_pending_connection_request() { None::, ); } + let _ = protocol.send_message( + "carol", + "to a lighter recipient", + None::, + None::, + ); let captured = events.lock().unwrap(); let undeliverable = captured @@ -27066,7 +27079,7 @@ fn captured_failures(events: &Mutex>) -> Vec<(String, String)> { .collect() } -/// Fills the outbox one past capacity through the real failing-send path, so +/// Fills the outbox through the real failing-send path and then evicts, so /// every entry is also queued for retry, as on a device whose recipients are /// all unreachable. Returns the one message evicted: the first, backdated so it /// is deterministically the oldest. @@ -27098,8 +27111,8 @@ fn overflow_outbox_through_failed_sends( .outbox .get_mut(&evicted) .expect("first message must be in the outbox") - .last_sent_at -= ChronoDuration::seconds(60); - for i in 0..crate::constants::MAX_OUTBOX_ENTRIES { + .first_sent_at -= ChronoDuration::seconds(60); + for i in 0..crate::constants::MAX_OUTBOX_ENTRIES - 1 { let _ = protocol.send_message( "bob", &format!("filler-{i}"), @@ -27107,6 +27120,13 @@ fn overflow_outbox_through_failed_sends( None::, ); } + // A send to a recipient with a smaller backlog evicts bob's oldest. + let _ = protocol.send_message( + "carol", + "to a lighter recipient", + None::, + None::, + ); assert!( !protocol.outbox.contains_key(&evicted), "the backdated first message must be the one evicted" @@ -27175,14 +27195,18 @@ fn test_outbox_capacity_eviction_is_not_resurrected_by_a_flush() { assert!(!protocol.is_tracked_for_delivery(&evicted)); } -/// A message evicted while its acknowledgement is pending must not be settled -/// a second time. Left in the ACK tracker, its timeout found no outbox entry -/// and reported it failed again ("Message missing from outbox"), and an ACK -/// arriving first reported it delivered after it had been reported failed. -#[test] -fn test_outbox_capacity_eviction_while_awaiting_ack_settles_once() { +/// A message awaiting its acknowledgement is never evicted for capacity. +/// +/// It once was, and settling it took care: left in the ACK tracker, its +/// timeout reported it failed a second time, and an acknowledgement arriving +/// first reported it delivered after it had been reported failed. Under load +/// on a live link the old least-recently-sent rule failed such frames on +/// every new send. Now the outbox evicts only messages that are not moving, +/// and this one is delivered when its acknowledgement arrives. +#[test] +fn test_outbox_capacity_never_evicts_a_message_awaiting_its_ack() { let mut config = create_test_config(); - config.reliability.ack.default_timeout_ms = 10; + config.reliability.ack.default_timeout_ms = 60_000; config.reliability.retry.initial_delay_ms = 60_000; config.reliability.retry.max_delay_ms = 60_000; let mut protocol = OfflineProtocol::new(config).unwrap(); @@ -27214,10 +27238,10 @@ fn test_outbox_capacity_eviction_while_awaiting_ack_settles_once() { .outbox .get_mut(&in_flight) .expect("a sent message must be in the outbox") - .last_sent_at -= ChronoDuration::seconds(60); + .first_sent_at -= ChronoDuration::seconds(60); *failures.lock().unwrap() = u32::MAX; - for i in 0..crate::constants::MAX_OUTBOX_ENTRIES { + for i in 0..crate::constants::MAX_OUTBOX_ENTRIES - 1 { let _ = protocol.send_message( "bob", &format!("filler-{i}"), @@ -27225,11 +27249,18 @@ fn test_outbox_capacity_eviction_while_awaiting_ack_settles_once() { None::, ); } - assert!(!protocol.outbox.contains_key(&in_flight)); + let _ = protocol.send_message( + "carol", + "to a lighter recipient", + None::, + None::, + ); + assert!( + protocol.outbox.contains_key(&in_flight), + "the oldest entry is awaiting its ack, so a younger one pays" + ); + assert_eq!(captured_failures(&events).len(), 1); - // Past the acknowledgement timeout, then a late acknowledgement. - thread::sleep(Duration::from_millis(20)); - protocol.process().unwrap(); let ack = Message::builder( UserId::new("bob").unwrap(), UserId::new("user123").unwrap(), @@ -27240,20 +27271,15 @@ fn test_outbox_capacity_eviction_while_awaiting_ack_settles_once() { .build(); protocol.handle_ack_message(&ack); - assert_eq!( - captured_failures(&events), - vec![(in_flight.as_str(), "Outbox capacity exceeded".to_string())], - "the ACK timeout must not settle an evicted message a second time" - ); assert!( - !events - .lock() - .unwrap() - .iter() - .any(|e| matches!(e, Event::MessageDelivered { .. })), - "a message reported failed must not then be reported delivered" + events.lock().unwrap().iter().any( + |e| matches!(e, Event::MessageDelivered { message_id, .. } if *message_id == in_flight.as_str()) + ), + "the message the outbox kept is delivered by its acknowledgement" ); - assert!(!protocol.is_tracked_for_delivery(&in_flight)); + assert!(!captured_failures(&events) + .iter() + .any(|(id, _)| *id == in_flight.as_str())); } /// The media outbox evicts through the same function and leaked its retry @@ -41690,8 +41716,9 @@ fn a_confirmation_probe_is_withdrawn_when_nothing_could_supersede_it() { assert!(!alice.confirmation_probe_outstanding.contains_key(&bob)); assert!(!alice.confirmation_probe_superseded.contains_key(&bob)); - // A full ACK tracker fails the send after the probe entered the outbox, - // so no id comes back to supersede next time. The failed send clears it. + // A full ACK tracker holds the probe for the retry queue instead of + // failing its send, so its id still comes back and the next probe can + // supersede it: exactly one probe stays queued, and it is the tracked one. for _ in 0..100_000 { if alice .ack_manager @@ -41703,14 +41730,16 @@ fn a_confirmation_probe_is_withdrawn_when_nothing_could_supersede_it() { } alice.send_session_confirmation_probe(&bob, "test"); alice.send_session_confirmation_probe(&bob, "test"); + let queued: Vec = alice + .outbox + .values() + .filter(|e| OfflineProtocol::is_confirmation_probe(&e.message)) + .map(|e| e.message.id.clone()) + .collect(); assert_eq!( - alice - .outbox - .values() - .filter(|e| OfflineProtocol::is_confirmation_probe(&e.message)) - .count(), - 0, - "a probe whose send failed is not left queued" + queued, + vec![alice.confirmation_probe_outstanding[&bob].clone()], + "a probe held at the ACK ceiling stays tracked, so it never lingers unsuperseded" ); } @@ -42576,3 +42605,1032 @@ fn test_unrouted_ack_hold_is_bounded() { assert!(protocol.unrouted_acks.is_empty()); assert!(internet.sent_messages().is_empty()); } + +// --------------------------------------------------------------------------- +// The pending-ACK ceiling (2026-10-19 volume test) +// --------------------------------------------------------------------------- + +/// A protocol whose pending-ACK table holds `max_pending_acks` entries, on a +/// carrier that accepts every frame and never acknowledges one. +fn protocol_with_ack_ceiling( + max_pending_acks: usize, + ack_timeout_ms: u64, +) -> ( + OfflineProtocol, + Arc>>, + Arc>>, +) { + let mut config = create_test_config(); + config.reliability.ack.max_pending_acks = max_pending_acks; + config.reliability.ack.default_timeout_ms = ack_timeout_ms; + config.reliability.retry.initial_delay_ms = 1; + config.reliability.retry.max_delay_ms = 1; + let mut protocol = OfflineProtocol::new(config).unwrap(); + + let carrier = FlakyTransport::fail_first(TransportType::BLE, 0); + let sent = Arc::clone(&carrier.sent_messages); + protocol + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(carrier)); + protocol.start().unwrap(); + + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_handle = Arc::clone(&events); + protocol.on_event(move |event| events_handle.lock().unwrap().push(event)); + (protocol, sent, events) +} + +fn sent_ids(sent: &Arc>>) -> Vec { + sent.lock().unwrap().iter().map(|m| m.id.as_str()).collect() +} + +/// A full table holds the send back instead of failing it. The volume test +/// saw `sendMessage` throw for messages that were already on the wire: the +/// receiver got 4,626 from a sender whose API had accepted 2,748. +#[test] +fn test_ack_ceiling_defers_a_send_instead_of_failing_it() { + let (mut protocol, sent, events) = protocol_with_ack_ceiling(3, 60_000); + + let ids: Vec = (0..5) + .map(|i| { + protocol + .send_message( + "bob", + format!("m{i}"), + None::, + None::, + ) + .expect("a full ACK table must never fail a send") + }) + .collect(); + + assert_eq!( + sent_ids(&sent), + ids[..3].iter().map(MessageId::as_str).collect::>(), + "only messages with a slot to track them are transmitted" + ); + let deferred: Vec = events + .lock() + .unwrap() + .iter() + .filter_map(|e| match e { + Event::MessageDeferred { + message_id, reason, .. + } if reason == "ack_capacity" => Some(message_id.clone()), + _ => None, + }) + .collect(); + assert_eq!( + deferred, + ids[3..].iter().map(MessageId::as_str).collect::>() + ); + assert!(events + .lock() + .unwrap() + .iter() + .all(|e| !matches!(e, Event::MessageFailed { .. }))); + + // Held, not sent, on every tick while the table stays full. + for _ in 0..3 { + thread::sleep(Duration::from_millis(3)); + protocol + .process() + .expect("a full table must not fail the tick"); + } + assert_eq!(sent.lock().unwrap().len(), 3); + + // An acknowledgement frees a slot and the next held message goes out. + protocol.ack_manager.remove_ack(&ids[0]); + thread::sleep(Duration::from_millis(3)); + protocol.process().unwrap(); + assert_eq!(sent.lock().unwrap().len(), 4); + assert_eq!(sent_ids(&sent)[3], ids[3].as_str()); +} + +/// The wedge itself. A retry with no slot used to propagate the capacity +/// error out of `process_retry_queue`, so `process()` returned before +/// `process_timed_out_acks` on every tick, the step that frees slots never +/// ran, and the engine never recovered. +#[test] +fn test_a_retry_without_a_slot_does_not_starve_the_timeout_sweep() { + let (mut protocol, sent, events) = protocol_with_ack_ceiling(3, 1); + + for i in 0..3 { + protocol + .send_message( + "bob", + format!("m{i}"), + None::, + None::, + ) + .unwrap(); + } + // A fourth message already in the retry queue and holding no slot: the + // shape a pruned slot left behind in the field. + let orphan = protocol.create_message("bob", "held", None, None).unwrap(); + protocol.ensure_outbox_entry(&orphan); + protocol.retry_queue.enqueue(orphan.clone(), 0); + + thread::sleep(Duration::from_millis(5)); + protocol + .process() + .expect("a retry with no slot must not fail the tick"); + + assert!( + events + .lock() + .unwrap() + .iter() + .any(|e| matches!(e, Event::MessageRetrying { .. })), + "the timeout sweep must still run on the tick that met a full table" + ); + assert!( + !sent_ids(&sent).contains(&orphan.id.as_str()), + "a message with no slot to track it is never transmitted" + ); + assert!( + protocol.retry_queue.contains(&orphan.id.as_str()), + "a held retry stays queued instead of being dropped" + ); +} + +/// The prune must outlive the retry ladder's longest wait. A fixed five +/// minutes equalled the default cap, so the ninth retry's slot was gone just +/// before its resend fired, and that resend met a full table. +#[test] +fn test_timed_out_ack_retention_outlives_the_longest_retry_wait() { + let protocol = OfflineProtocol::new(create_test_config()).unwrap(); + let reliability = &protocol.config.reliability; + let longest_gap = + Duration::from_millis(reliability.ack.default_timeout_ms + reliability.retry.max_delay_ms); + assert!(protocol.timed_out_ack_retention() > longest_gap); +} + +/// A receiver that falls more than the forward window behind its sender used +/// to stay broken for good: every frame failed as an ordinary decryption +/// error, which waits for a re-sealed resend, and every resend was further +/// ahead still. It now re-keys, and the pair recovers. The 2026-10-19 volume +/// test lost its whole iPhone-to-Android direction to the old behaviour. +#[test] +fn test_a_receiver_past_the_forward_window_rekeys_and_recovers() { + let (mut alice, alice_h) = make_encrypted_protocol("alice"); + let (mut bob, bob_h) = make_encrypted_protocol("bob"); + + let bob_rx: Arc>> = Arc::new(Mutex::new(Vec::new())); + let alice_rx: Arc>> = Arc::new(Mutex::new(Vec::new())); + let bob_warnings: Arc>> = Arc::new(Mutex::new(Vec::new())); + { + let rx = Arc::clone(&bob_rx); + let warnings = Arc::clone(&bob_warnings); + bob.on_event(move |e| match e { + Event::MessageReceived { content, .. } => rx.lock().unwrap().push(content), + Event::SecurityWarning { reason_code, .. } => { + warnings.lock().unwrap().push(reason_code) + } + _ => {} + }); + let rx = Arc::clone(&alice_rx); + alice.on_event(move |e| { + if let Event::MessageReceived { content, .. } = e { + rx.lock().unwrap().push(content); + } + }); + } + alice.start().unwrap(); + bob.start().unwrap(); + establish_confirmed_session(&mut alice, &id("alice"), &mut bob, &id("bob")); + + // Alice spends more generations than bob's window covers, none of which + // reach him: the shape a stream of re-sealed resends leaves behind when + // the receiver drops each as a duplicate. + { + let m = alice.mls_manager.as_ref().unwrap().read().unwrap(); + for _ in 0..=offline_protocol_mls::group::SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE { + m.encrypt_for_user(&id("bob"), b"never delivered").unwrap(); + } + } + alice + .send_message(&id("bob"), "past-the-window", None, None::) + .unwrap(); + pump_between(&mut alice, &alice_h, &mut bob, &bob_h, 40); + + assert!( + bob_warnings + .lock() + .unwrap() + .contains(&SecurityWarningCode::SessionRekeyTriggered), + "a receiver past the window must re-key (warnings {:?})", + bob_warnings.lock().unwrap() + ); + + alice + .send_message(&id("bob"), "after-rekey-a2b", None, None::) + .unwrap(); + bob.send_message(&id("alice"), "after-rekey-b2a", None, None::) + .unwrap(); + pump_between(&mut alice, &alice_h, &mut bob, &bob_h, 40); + + assert!( + bob_rx + .lock() + .unwrap() + .iter() + .any(|c| c == "after-rekey-a2b"), + "bob must decrypt alice again once the pair re-keys (got {:?})", + bob_rx.lock().unwrap() + ); + assert!( + alice_rx + .lock() + .unwrap() + .iter() + .any(|c| c == "after-rekey-b2a"), + "alice must decrypt bob after the re-key (got {:?})", + alice_rx.lock().unwrap() + ); +} + +/// A carrier that accepts every frame and reports it as still waiting in its +/// local queue until `drained` is set, like a BLE link carrying less than it +/// is offered. +#[derive(Clone)] +struct BackloggedTransport { + sent: Arc>>, + drained: Arc, +} + +impl Transport for BackloggedTransport { + fn as_any(&self) -> &dyn std::any::Any { + self + } + fn transport_type(&self) -> TransportType { + TransportType::BLE + } + fn status(&self) -> TransportStatus { + TransportStatus::Available + } + fn metrics(&self) -> TransportMetrics { + TransportMetrics::default() + } + fn send(&self, message: &Message) -> offline_protocol_transport::Result<()> { + self.sent.lock().unwrap().push(message.clone()); + Ok(()) + } + fn receive(&self) -> offline_protocol_transport::Result> { + Ok(None) + } + fn start(&self) -> offline_protocol_transport::Result<()> { + Ok(()) + } + fn stop(&self) -> offline_protocol_transport::Result<()> { + Ok(()) + } + fn on_status_changed(&self, _status: TransportStatus) {} + fn queued_message_ids(&self) -> Vec { + if self.drained.load(std::sync::atomic::Ordering::SeqCst) { + return Vec::new(); + } + self.sent + .lock() + .unwrap() + .iter() + .map(|m| m.id.as_str()) + .collect() + } +} + +/// A frame that times out while still in the local queue is not retried: a +/// retry would append a second, re-sealed copy behind the first. Once the +/// frame has left the queue, its timeout means what it says again. +#[test] +fn test_a_frame_still_in_the_local_queue_is_not_retried() { + let mut config = create_test_config(); + config.reliability.ack.default_timeout_ms = 1; + config.reliability.retry.initial_delay_ms = 1; + config.reliability.retry.max_delay_ms = 1; + let mut protocol = OfflineProtocol::new(config).unwrap(); + let carrier = BackloggedTransport { + sent: Arc::new(Mutex::new(Vec::new())), + drained: Arc::new(std::sync::atomic::AtomicBool::new(false)), + }; + protocol + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(carrier.clone())); + protocol.start().unwrap(); + let retries = Arc::new(Mutex::new(0usize)); + let handle = Arc::clone(&retries); + protocol.on_event(move |e| { + if matches!(e, Event::MessageRetrying { .. }) { + *handle.lock().unwrap() += 1; + } + }); + + protocol + .send_message( + "bob", + "waiting its turn", + None::, + None::, + ) + .unwrap(); + for _ in 0..5 { + thread::sleep(Duration::from_millis(3)); + protocol.process().unwrap(); + } + assert_eq!( + *retries.lock().unwrap(), + 0, + "a queued frame must not be retried" + ); + assert_eq!( + carrier.sent.lock().unwrap().len(), + 1, + "and must not be queued twice" + ); + + carrier + .drained + .store(true, std::sync::atomic::Ordering::SeqCst); + thread::sleep(Duration::from_millis(3)); + protocol.process().unwrap(); + assert_eq!( + *retries.lock().unwrap(), + 1, + "a frame that has left the queue times out as before" + ); +} + +/// A sender stops re-sealing a peer's resends once it has sealed the budget +/// past that peer's last acknowledgement, and an acknowledgement re-opens it. +/// Every re-seal spends a generation the receiver must fast-forward over; in +/// the 2026-10-19 rerun 20,919 of them during a stall left the receiver past +/// its forward window. +#[test] +fn test_resends_stop_re_sealing_past_the_unacknowledged_budget() { + let (mut alice, alice_h) = make_encrypted_protocol("alice"); + let (mut bob, bob_h) = make_encrypted_protocol("bob"); + alice.start().unwrap(); + bob.start().unwrap(); + establish_confirmed_session(&mut alice, &id("alice"), &mut bob, &id("bob")); + + alice + .send_message(&id("bob"), "held back by a dead link", None, None::) + .unwrap(); + let mut on_the_wire = alice_h.sent_messages().last().unwrap().clone(); + alice_h.clear_sent_messages(); + let unacknowledged = |p: &OfflineProtocol| { + p.mls_manager + .as_ref() + .unwrap() + .read() + .unwrap() + .unacknowledged_seals(&id("bob")) + }; + + // Re-seal until the budget is spent, then once more: that one must go out + // verbatim and spend nothing. + while unacknowledged(&alice) < RESEAL_GAP_BUDGET { + let before = on_the_wire.content.clone(); + alice.reseal_for_resend_in_place(&mut on_the_wire); + assert_ne!( + on_the_wire.content, before, + "within the budget a resend re-seals" + ); + alice.outbox.get_mut(&on_the_wire.id).unwrap().message = on_the_wire.clone(); + } + let before = on_the_wire.content.clone(); + alice.reseal_for_resend_in_place(&mut on_the_wire); + assert_eq!( + on_the_wire.content, before, + "past the budget a resend is verbatim" + ); + assert_eq!(unacknowledged(&alice), RESEAL_GAP_BUDGET); + + // The newest copy still decrypts, and its acknowledgement re-opens the budget. + bob_h.queue_message_from(on_the_wire, id("alice").to_string()); + pump_between(&mut alice, &alice_h, &mut bob, &bob_h, 10); + assert_eq!( + unacknowledged(&alice), + 0, + "an acknowledgement for the newest seal clears the gap" + ); +} + +/// Past the budget, a resend whose bytes the receiver can no longer decrypt +/// is re-sealed anyway: verbatim it is refused (more than the out-of-order +/// tolerance behind), unacknowledged on a mesh link, and resent until its +/// ladder runs out. Run 4 of the 2026-10-19 volume test lost 525 messages +/// that way. A resend whose verbatim bytes still decrypt stays verbatim. +#[test] +fn test_a_dead_verbatim_resend_is_re_sealed_past_the_budget() { + let (mut alice, alice_h) = make_encrypted_protocol("alice"); + let (mut bob, _bob_h) = make_encrypted_protocol("bob"); + alice.start().unwrap(); + bob.start().unwrap(); + establish_confirmed_session(&mut alice, &id("alice"), &mut bob, &id("bob")); + + alice + .send_message(&id("bob"), "lost on the way", None, None::) + .unwrap(); + let mut old = alice_h.sent_messages().last().unwrap().clone(); + alice + .send_message(&id("bob"), "resent through a stall", None, None::) + .unwrap(); + let mut recent = alice_h.sent_messages().last().unwrap().clone(); + let unacknowledged = |p: &OfflineProtocol| { + p.mls_manager + .as_ref() + .unwrap() + .read() + .unwrap() + .unacknowledged_seals(&id("bob")) + }; + while unacknowledged(&alice) < RESEAL_GAP_BUDGET { + alice.reseal_for_resend_in_place(&mut recent); + alice.outbox.get_mut(&recent.id).unwrap().message = recent.clone(); + } + + let before = recent.content.clone(); + alice.reseal_for_resend_in_place(&mut recent); + assert_eq!( + recent.content, before, + "the newest seal still decrypts verbatim, so the budget holds it" + ); + + let before = old.content.clone(); + alice.reseal_for_resend_in_place(&mut old); + assert_ne!( + old.content, before, + "a copy sealed thousands of generations back can never decrypt verbatim" + ); + assert!(unacknowledged(&alice) < RESEAL_HARD_LIMIT); +} + +// ============================================================================ +// Per-peer send window +// ============================================================================ + +/// A started device holding a live BLE link to "bob", with its events +/// captured. Plain DMs (`create_test_config` opts out of encryption). +fn device_with_a_mesh_link_to_bob() -> (OfflineProtocol, MockTransport, Arc>>) { + let mut protocol = OfflineProtocol::new(create_test_config()).unwrap(); + let ble = MockTransport::new(TransportType::BLE); + ble.start().unwrap(); + ble.add_connected_peer("bob", -55); + protocol + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(ble.clone())); + protocol.start().unwrap(); + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_handle = Arc::clone(&events); + protocol.on_event(move |event| events_handle.lock().unwrap().push(event)); + (protocol, ble, events) +} + +fn send_dms_to_bob(protocol: &mut OfflineProtocol, count: usize) -> Vec { + (0..count) + .map(|i| { + protocol + .send_message( + "bob", + &format!("dm-{i}"), + None::, + None::, + ) + .unwrap() + }) + .collect() +} + +fn window_sent_ids(ble: &MockTransport) -> Vec { + ble.sent_messages() + .iter() + .map(|m| m.id.as_str().to_string()) + .collect() +} + +#[test] +fn a_frame_past_the_peer_window_is_held_not_sent() { + let (mut protocol, ble, events) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 2); + + assert_eq!( + window_sent_ids(&ble), + ids[..PEER_SEND_WINDOW] + .iter() + .map(|id| id.as_str().to_string()) + .collect::>(), + "only a window's worth of frames may be in flight to one peer" + ); + for held in &ids[PEER_SEND_WINDOW..] { + assert!( + protocol.outbox.contains_key(held), + "a held message is accepted: it waits in the outbox" + ); + assert!( + !protocol.ack_manager.is_waiting_for_ack(held), + "a held message was not sent, so nothing waits for its ACK" + ); + assert_eq!( + protocol.outbox[held].attempt_count, 0, + "holding is not an attempt" + ); + } + let deferred: Vec<_> = events + .lock() + .unwrap() + .iter() + .filter_map(|e| match e { + Event::MessageDeferred { + message_id, reason, .. + } if reason == SEND_DEFER_REASON_PEER_WINDOW => Some(message_id.clone()), + _ => None, + }) + .collect(); + assert_eq!( + deferred, + ids[PEER_SEND_WINDOW..] + .iter() + .map(|id| id.as_str()) + .collect::>(), + "the app is told each held message is queued" + ); +} + +#[test] +fn an_acknowledgement_releases_the_oldest_held_frame() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 2); + ble.clear_sent_messages(); + + ble.queue_message_from( + delivery_ack_frame("bob", "user123", &ids[0]), + "bob".to_string(), + ); + assert!(protocol.receive_message().is_none()); + protocol.process().unwrap(); + + assert_eq!( + window_sent_ids(&ble), + vec![ids[PEER_SEND_WINDOW].as_str().to_string()], + "one slot freed sends exactly one held frame, the oldest" + ); + assert!(protocol + .ack_manager + .is_waiting_for_ack(&ids[PEER_SEND_WINDOW])); +} + +#[test] +fn a_new_message_never_overtakes_one_already_held() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 1); + // Free a slot without a tick, then send again: the held one goes first. + protocol.forget_outbound_message(&ids[0]); + ble.clear_sent_messages(); + + let late = send_dms_to_bob(&mut protocol, 1); + assert!( + window_sent_ids(&ble).is_empty(), + "a slot is free, but an earlier message is held for it" + ); + protocol.process().unwrap(); + assert_eq!( + window_sent_ids(&ble), + vec![ids[PEER_SEND_WINDOW].as_str().to_string()], + "the held message takes the freed slot" + ); + assert!(protocol.outbox.contains_key(&late[0])); +} + +#[test] +fn a_give_up_frees_its_window_slot() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 1); + ble.clear_sent_messages(); + + protocol.retire_undeliverable_message(&ids[0], "test"); + protocol.process().unwrap(); + + assert_eq!( + window_sent_ids(&ble), + vec![ids[PEER_SEND_WINDOW].as_str().to_string()], + "a slot freed by giving up is as free as one freed by an ACK" + ); +} + +#[test] +fn a_frame_already_in_flight_is_always_admitted() { + let (mut protocol, _ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW); + let in_flight = protocol.outbox[&ids[0]].message.clone(); + assert!( + protocol.peer_window_admits(&in_flight), + "a resend reuses its slot; holding it would stall the window for good" + ); +} + +#[test] +fn the_window_does_not_govern_a_peer_without_a_mesh_link() { + let mut protocol = OfflineProtocol::new(create_test_config()).unwrap(); + let ble = MockTransport::new(TransportType::BLE); + ble.start().unwrap(); + protocol + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(ble.clone())); + protocol.start().unwrap(); + + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 4); + assert_eq!( + ble.sent_messages().len(), + ids.len(), + "store-and-forward carriers are not windowed: holding would strand messages" + ); +} + +#[test] +fn held_frames_go_to_transport_selection_when_the_mesh_link_drops() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW + 3); + ble.clear_sent_messages(); + + ble.remove_connected_peer("bob"); + protocol.process().unwrap(); + + let sent = window_sent_ids(&ble); + for held in &ids[PEER_SEND_WINDOW..] { + assert!( + sent.contains(&held.as_str().to_string()), + "with no mesh link the window no longer applies, so nothing stays held" + ); + } + assert!(protocol.window_held.is_empty()); +} + +// ============================================================================ +// Coalesced delivery acknowledgements +// ============================================================================ + +fn plain_dm_from_bob(text: &str) -> Message { + Message::builder( + UserId::new("bob").unwrap(), + UserId::new("user123").unwrap(), + AppId::new("test-app").unwrap(), + ) + .content(text.to_string()) + .requires_ack(true) + .build() +} + +fn receive_dms_from_bob( + protocol: &mut OfflineProtocol, + ble: &MockTransport, + count: usize, +) -> Vec { + (0..count) + .map(|i| { + let dm = plain_dm_from_bob(&format!("dm-{i}")); + let id = dm.id.as_str(); + ble.queue_message_from(dm, "bob".to_string()); + while protocol.receive_message().is_some() {} + id + }) + .collect() +} + +fn ack_frames(ble: &MockTransport) -> Vec { + ble.sent_messages() + .into_iter() + .filter(|m| m.metadata.contains_key(ACK_FOR_KEY)) + .collect() +} + +#[test] +fn acks_to_a_peer_that_reads_them_coalesce_into_one_frame() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + protocol.peer_coalesced_ack.insert("bob".to_string()); + + let ids = receive_dms_from_bob(&mut protocol, &ble, ACK_COALESCE_MAX); + + let acks = ack_frames(&ble); + assert_eq!(acks.len(), 1, "five answers ride one frame"); + assert_eq!(acks[0].metadata[ACK_FOR_KEY], ids[0]); + assert_eq!(acks[0].metadata[ACK_MORE_KEY], ids[1..].join(",")); + assert!(!acks[0].requires_ack); +} + +#[test] +fn a_partial_ack_batch_goes_out_when_its_window_passes() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + protocol.peer_coalesced_ack.insert("bob".to_string()); + + let ids = receive_dms_from_bob(&mut protocol, &ble, 2); + assert!(ack_frames(&ble).is_empty(), "held to be coalesced"); + + protocol.process().unwrap(); + assert!(ack_frames(&ble).is_empty(), "not yet due"); + + protocol.coalesced_acks_out.get_mut("bob").unwrap().since = + std::time::Instant::now() - ACK_COALESCE_WINDOW; + protocol.process().unwrap(); + let acks = ack_frames(&ble); + assert_eq!(acks.len(), 1); + assert_eq!(acks[0].metadata[ACK_FOR_KEY], ids[0]); + assert_eq!(acks[0].metadata[ACK_MORE_KEY], ids[1]); + assert!(protocol.coalesced_acks_out.is_empty()); +} + +#[test] +fn a_peer_that_never_advertised_coalescing_gets_an_ack_per_frame() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = receive_dms_from_bob(&mut protocol, &ble, 3); + + let acks = ack_frames(&ble); + assert_eq!( + acks.iter() + .map(|a| a.metadata[ACK_FOR_KEY].clone()) + .collect::>(), + ids, + "an older SDK ignores ack_more, so it must never be sent one" + ); + assert!(acks.iter().all(|a| !a.metadata.contains_key(ACK_MORE_KEY))); +} + +#[test] +fn a_coalesced_ack_settles_every_message_it_names() { + let (mut protocol, ble, events) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, 3); + + let mut ack = delivery_ack_frame("bob", "user123", &ids[0]); + ack.metadata.insert( + ACK_MORE_KEY.to_string(), + format!("{},{}", ids[1].as_str(), ids[2].as_str()), + ); + ble.queue_message_from(ack, "bob".to_string()); + assert!(protocol.receive_message().is_none()); + + for id in &ids { + assert!( + !protocol.outbox.contains_key(id), + "every named message is settled" + ); + } + let delivered = events + .lock() + .unwrap() + .iter() + .filter(|e| matches!(e, Event::MessageDelivered { .. })) + .count(); + assert_eq!( + delivered, 3, + "one message_delivered per message, as single ACKs give" + ); +} + +#[test] +fn ack_more_on_an_undecryptable_answer_settles_nothing_further() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + let ids = send_dms_to_bob(&mut protocol, 2); + + let mut ack = delivery_ack_frame("bob", "user123", &ids[0]); + ack.metadata.insert( + ACK_STATUS_KEY.to_string(), + ACK_STATUS_UNDECRYPTABLE.to_string(), + ); + ack.metadata + .insert(ACK_MORE_KEY.to_string(), ids[1].as_str()); + ble.queue_message_from(ack, "bob".to_string()); + assert!(protocol.receive_message().is_none()); + + assert!( + protocol.outbox.contains_key(&ids[1]), + "only a delivery may share a frame; an undecryptable answer names one id" + ); +} + +// ============================================================================ +// Outbox admission at capacity +// ============================================================================ + +fn capacity_failures(events: &Mutex>) -> Vec { + events + .lock() + .unwrap() + .iter() + .filter_map(|e| match e { + Event::MessageFailed { + message_id, reason, .. + } if reason == "Outbox capacity exceeded" => Some(message_id.clone()), + _ => None, + }) + .collect() +} + +/// The outbox never fails a message awaiting its acknowledgement, and one +/// slow live peer cannot block sends to anyone else. With a live link to bob, +/// sixteen of his messages are in flight and the rest are held by the window. +/// Another send to bob is refused (his is the largest backlog). A send to +/// carol evicts bob's newest held message, which was never sent, so the front +/// of his line, the part about to go out, stays intact. +#[test] +fn a_full_outbox_of_held_messages_admits_a_lighter_recipient() { + let (mut protocol, _ble, events) = device_with_a_mesh_link_to_bob(); + let queued = send_dms_to_bob(&mut protocol, crate::constants::MAX_OUTBOX_ENTRIES); + assert_eq!( + protocol.outbox_entry_count(), + crate::constants::MAX_OUTBOX_ENTRIES + ); + + let to_bob = protocol + .send_message("bob", "one more", None::, None::) + .unwrap(); + let to_carol = protocol + .send_message( + "carol", + "elsewhere", + None::, + None::, + ) + .unwrap(); + + let newest_held = queued.last().unwrap(); + assert_eq!( + capacity_failures(&events), + vec![ + to_bob.as_str().to_string(), + newest_held.as_str().to_string() + ] + ); + assert!(protocol.outbox.contains_key(&to_carol)); + for id in &queued[..PEER_SEND_WINDOW] { + assert!( + protocol.outbox.contains_key(id), + "a message awaiting its acknowledgement was failed" + ); + } + assert!( + protocol.outbox.contains_key(&queued[PEER_SEND_WINDOW]), + "the front of the held line survives" + ); +} + +/// When every entry awaits its acknowledgement, nothing may be evicted, and +/// a send to anyone is refused. +#[test] +fn a_full_outbox_of_messages_awaiting_acks_refuses_the_newcomer() { + let (mut protocol, ble, events) = device_with_a_mesh_link_to_bob(); + let queued = send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW); + // Pin the rest of the outbox as in flight, as a peer that drains fast + // enough to keep its window open would. + ble.remove_connected_peer("bob"); + let rest = send_dms_to_bob( + &mut protocol, + crate::constants::MAX_OUTBOX_ENTRIES - PEER_SEND_WINDOW, + ); + for id in &rest { + assert!(protocol.ack_manager.is_waiting_for_ack(id)); + } + + let to_carol = protocol + .send_message( + "carol", + "elsewhere", + None::, + None::, + ) + .unwrap(); + assert_eq!( + capacity_failures(&events), + vec![to_carol.as_str().to_string()] + ); + for id in queued.iter().chain(&rest) { + assert!(protocol.outbox.contains_key(id)); + } +} + +/// Session control is never held by the window: it is what heals the +/// session the window's frames are sealed under. +#[test] +fn session_control_frames_are_not_windowed() { + let (mut protocol, ble, _) = device_with_a_mesh_link_to_bob(); + send_dms_to_bob(&mut protocol, PEER_SEND_WINDOW); + ble.clear_sent_messages(); + + for prefix in [ + internal_prefixes::KEY_PACKAGE, + internal_prefixes::WELCOME, + internal_prefixes::SESSION_CONFIRM_PROBE, + internal_prefixes::SESSION_CONFIRM_ACK, + ] { + let frame = protocol + .create_message("bob", &format!("{prefix}payload"), None, None) + .unwrap(); + assert!( + frame.requires_ack, + "precondition: {prefix} waits for an ACK" + ); + assert!( + protocol.peer_window_admits(&frame), + "{prefix} must pass a full window" + ); + } + let dm = protocol + .create_message("bob", "ordinary", None, None) + .unwrap(); + assert!( + !protocol.peer_window_admits(&dm), + "precondition: the window is full" + ); +} + +/// With no carrier, a send to a recipient with a smaller backlog evicts the +/// oldest entry of the recipient holding the most, so one unreachable peer's +/// week of messages cannot block a message to someone else. +#[test] +fn a_full_outbox_evicts_from_the_heaviest_recipient_for_a_lighter_one() { + let mut config = create_test_config(); + config.reliability.retry.initial_delay_ms = 60_000; + config.reliability.retry.max_delay_ms = 60_000; + let mut protocol = OfflineProtocol::new(config).unwrap(); + protocol.transport_manager_mut().add_transport( + TransportType::BLE, + Box::new(FlakyTransport::fail_first(TransportType::BLE, u32::MAX)), + ); + protocol.start().unwrap(); + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_handle = Arc::clone(&events); + protocol.on_event(move |event| events_handle.lock().unwrap().push(event)); + + let send = |protocol: &mut OfflineProtocol, to: &str, i: usize| { + protocol + .send_message( + to, + &format!("m-{i}"), + None::, + None::, + ) + .unwrap() + }; + let bob_first = send(&mut protocol, "bob", 0); + protocol.outbox.get_mut(&bob_first).unwrap().first_sent_at -= ChronoDuration::seconds(60); + for i in 1..300 { + send(&mut protocol, "bob", i); + } + for i in 0..200 { + send(&mut protocol, "carol", i); + } + + let dave = send(&mut protocol, "dave", 0); + assert_eq!( + capacity_failures(&events), + vec![bob_first.as_str().to_string()] + ); + assert!(protocol.outbox.contains_key(&dave)); + + // Carol (200) is lighter than bob (299): bob pays again, carol keeps hers. + let carol_extra = send(&mut protocol, "carol", 200); + assert!(protocol.outbox.contains_key(&carol_extra)); + assert_eq!(capacity_failures(&events).len(), 2); + assert_eq!( + protocol.outbox_entry_count(), + crate::constants::MAX_OUTBOX_ENTRIES + ); +} + +/// A tie refuses the newcomer: a message never displaces one to a recipient +/// whose backlog is no larger than its own. +#[test] +fn a_full_outbox_refuses_on_a_tie() { + let mut config = create_test_config(); + config.reliability.retry.initial_delay_ms = 60_000; + config.reliability.retry.max_delay_ms = 60_000; + let mut protocol = OfflineProtocol::new(config).unwrap(); + protocol.transport_manager_mut().add_transport( + TransportType::BLE, + Box::new(FlakyTransport::fail_first(TransportType::BLE, u32::MAX)), + ); + protocol.start().unwrap(); + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_handle = Arc::clone(&events); + protocol.on_event(move |event| events_handle.lock().unwrap().push(event)); + + let half = crate::constants::MAX_OUTBOX_ENTRIES / 2; + for (to, n) in [("bob", half), ("carol", half)] { + for i in 0..n { + protocol + .send_message( + to, + &format!("m-{i}"), + None::, + None::, + ) + .unwrap(); + } + } + let refused = protocol + .send_message("bob", "tie", None::, None::) + .unwrap(); + assert_eq!( + capacity_failures(&events), + vec![refused.as_str().to_string()] + ); +} diff --git a/crates/offline-protocol/src/protocol/types.rs b/crates/offline-protocol/src/protocol/types.rs index 5239a7373..a6ec6c87b 100644 --- a/crates/offline-protocol/src/protocol/types.rs +++ b/crates/offline-protocol/src/protocol/types.rs @@ -387,6 +387,100 @@ pub(crate) fn classify_transport_send_error(raw: &str) -> &'static str { } } +/// The pending-ACK table was full, so the frame was held back instead of +/// transmitted. Not a transport fault: the retry queue sends it as soon as an +/// acknowledgement frees a slot. A link carrying less than the app offers +/// produces this steadily, and it is the signal to slow down. +pub(crate) const SEND_DEFER_REASON_ACK_CAPACITY: &str = "ack_capacity"; + +/// The peer already had [`PEER_SEND_WINDOW`] frames over a mesh link awaiting +/// its acknowledgement, so the frame waits in the outbox and goes out, oldest +/// first, as acknowledgements come back. Not a fault: it is what a link +/// carrying less than the app offers looks like. +pub(crate) const SEND_DEFER_REASON_PEER_WINDOW: &str = "peer_window"; + +/// How many frames this device keeps in flight to one peer over a mesh link +/// (Bluetooth LE, Wi-Fi Direct) before it holds the next one back. +/// +/// Without a window the sender handed the carrier every new frame, every +/// resend and an acknowledgement per received frame, into one unbounded queue, +/// on a timer that starts at enqueue. In the 2026-10-19 volume test (run 4) +/// the Android side's indication path carried 12 to 16 frames a second against +/// 15 offered: the acknowledgement round trip passed the timeout, every resend +/// cost the receiver another acknowledgement on the saturated path, that +/// path's rate halved, and both phones retried 50,000 times and evicted 6,000 +/// messages as "Outbox capacity exceeded". A window bounds the queue, and with +/// it the round trip, the resends and the receiver's ratchet gap, at any link +/// rate. It costs no throughput while it exceeds what the link carries in one +/// unloaded round trip: about six messages on that link, counting the +/// receiver's acknowledgement coalescing (`ACK_COALESCE_WINDOW`) and a +/// processing tick on each side. Sixteen leaves margin, and keeps the +/// round trip under about three seconds there, well inside the timeout. +/// +/// Only the mesh carriers: a relay mailbox is store-and-forward, and holding +/// a message back from it would strand it on a device the user may close. +pub(crate) const PEER_SEND_WINDOW: usize = 16; + +// Every in-window frame can be re-sealed on each of its attempts, and the +// re-seal budget must never trip under the window: past it resends go out +// verbatim, and a verbatim copy more than the out-of-order tolerance behind +// the receiver never decrypts (run 4: 3,308 such failures). +const _: () = assert!( + (PEER_SEND_WINDOW as u64) + * (1 + offline_protocol_reliability::constants::DEFAULT_MAX_RETRIES as u64) + < RESEAL_GAP_BUDGET +); + +/// The smallest forward window any receiver of a 1:1 session has: a phone's +/// OpenMLS group (configured) or a leaf's mls-rs one (fixed by mls-rs, and +/// smaller). The re-seal budget is a fraction of this, not of the phone's +/// window: derived from the phone's 8192, it let a sender burn 2,048 +/// generations past a leaf that can fast-forward over 1,024. +pub(crate) const RECEIVER_FORWARD_WINDOW_FLOOR: u64 = { + let phone = offline_protocol_mls::group::SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE as u64; + let leaf = offline_protocol_mls::group::LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE as u64; + if phone < leaf { + phone + } else { + leaf + } +}; + +/// How many frames a sender may seal for one peer past the last one that peer +/// acknowledged before resends stop being re-sealed and go out verbatim. +/// +/// A receiver cannot decrypt a frame more than its forward window past the +/// newest it has decrypted. A quarter of the smallest window any receiver has +/// ([`RECEIVER_FORWARD_WINDOW_FLOOR`]) leaves the rest to new messages, which +/// still seal. The per-peer window keeps unacknowledged seals growing only by +/// resends of at most [`PEER_SEND_WINDOW`] frames, so the budget trips only on +/// a link that has carried no acknowledgement for a whole retry ladder; the +/// compile-time assertion above keeps it above that ladder. +pub(crate) const RESEAL_GAP_BUDGET: u64 = RECEIVER_FORWARD_WINDOW_FLOOR / 4; + +/// Past [`RESEAL_GAP_BUDGET`], a resend whose bytes the receiver can no +/// longer decrypt is still re-sealed, until the gap reaches this. +/// +/// A verbatim copy sealed more than the out-of-order tolerance before the +/// newest frame this device sealed is refused by a receiver that has +/// decrypted anything since, and refused without an acknowledgement on a +/// mesh link, so it was resent until its ladder ran out: in run 4 of the +/// 2026-10-19 volume test, 525 such messages produced 3,308 decrypt +/// failures and were never delivered. Re-sealing one spends a generation, +/// which is what the budget rations, so the exception stops at half the +/// smallest forward window ([`RECEIVER_FORWARD_WINDOW_FLOOR`]): the receiver +/// going deaf is the failure the budget exists for, and a wasted resend is +/// not. +pub(crate) const RESEAL_HARD_LIMIT: u64 = RECEIVER_FORWARD_WINDOW_FLOOR / 2; + +/// The longest an acknowledgement timeout is postponed for a frame that is +/// still waiting in a local transport queue, measured from the attempt. +/// +/// Bounded so a queue that never drains (a link that died without telling +/// the transport) still reaches the retry ladder, and through it the other +/// carriers and the mesh, within a minute. +pub(crate) const QUEUED_ACK_GRACE_MAX: std::time::Duration = std::time::Duration::from_secs(60); + /// Classifies a locally raised send failure for an event field. /// /// The sibling of [`classify_transport_send_error`] for errors this device @@ -1423,6 +1517,21 @@ pub(crate) struct ReceivedKeyPackage { pub(crate) local_expires_at_ms: u64, } +/// Delivery ACKs held for one peer until they go out as one frame (see +/// `OfflineProtocol::coalesced_acks_out`). +#[derive(Debug, Clone)] +pub(crate) struct CoalescedAcks { + /// The carrier the answered frames arrived on, which the ACK goes back + /// over. A frame from another carrier starts a new batch. + pub(crate) transport: offline_protocol_transport::TransportType, + /// When the first held ACK was owed. + pub(crate) since: std::time::Instant, + /// The first answered frame's hop count, as a single ACK reports it. + pub(crate) hop_count: String, + /// The answered message ids, oldest first. + pub(crate) ids: Vec, +} + /// Durable record of the end-to-end capability versions a peer last /// advertised in its key package (`env_versions` / `rich_versions`). /// @@ -2471,6 +2580,12 @@ pub(crate) struct OutboxReseal { /// Sealed-only rich extras (reply context, rich media metadata, forward /// info) — re-evaluated against current capability at re-seal time. pub(crate) rich: Option, + /// Where the bytes currently on the ladder sit in the session's sealing + /// order. An acknowledgement for this message proves the peer decrypted + /// that far, which is what lets later re-seals go ahead (see + /// `RESEAL_GAP_BUDGET`). Set when the message is staged and on every + /// re-seal; `None` when it could not be read. + pub(crate) seal_mark: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/offline-protocol/src/transport_manager.rs b/crates/offline-protocol/src/transport_manager.rs index 715020a31..9cbd886d4 100644 --- a/crates/offline-protocol/src/transport_manager.rs +++ b/crates/offline-protocol/src/transport_manager.rs @@ -1202,6 +1202,15 @@ impl TransportManager { }) } + /// Ids of messages some carrier has accepted but not yet started writing + /// (see [`Transport::queued_message_ids`]). + pub fn queued_message_ids(&self) -> std::collections::HashSet { + self.transports + .values() + .flat_map(|transport| transport.queued_message_ids()) + .collect() + } + /// Whether a mesh carrier holds a live link straight to `peer_id`. /// /// The peer-specific half of [`Self::can_reach_without_carrying`], split out diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 500671007..74291dad3 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -2657,6 +2657,25 @@ builds the library from source with its own command and copies it from `target//mobile/`; the SDK's build scripts already do ([ADR 0029](adr/0029-mobile-libraries-are-sized-not-aborting.md)). +**A burst of direct messages to a nearby peer is sent sixteen at a time.** +Over Bluetooth LE or Wi-Fi Direct, at most 16 messages to one peer wait for +its acknowledgement at once. The 17th onward return their id as usual, emit +`message_deferred` with reason `peer_window` instead of `message_sent`, and +go out in order as acknowledgements return; `message_delivered` follows as +before. An app that shows a message as "sent" only on `message_sent` should +show a `peer_window` deferral as queued, not failed. Messages through the +relay are not affected. The burst used to be handed to the radio all at +once, which on a slow link ended in mass retries and "Outbox capacity +exceeded" failures. + +**A full outbox refuses the new message rather than failing an old one.** When +500 messages are already queued, `send` for a recipient that holds the largest +share of them still returns an id, but that id gets `message_failed` with +reason "Outbox capacity exceeded" and a retry count of 0, usually before +`send` returns. Treat it as "queue full, try later" for that conversation. +Messages already queued are no longer failed to make room for it, and a +message awaiting its acknowledgement is never failed for capacity. + --- ## Appendix A: limits reference diff --git a/docs/adr/0030-one-bluetooth-le-link-per-peer-pair.md b/docs/adr/0030-one-bluetooth-le-link-per-peer-pair.md new file mode 100644 index 000000000..065258b88 --- /dev/null +++ b/docs/adr/0030-one-bluetooth-le-link-per-peer-pair.md @@ -0,0 +1,86 @@ +# 0030. One Bluetooth LE link per peer pair + +**Status:** Accepted +**See also:** [BLE framing, invariants](../spec/ble-framing.md#invariants), +[Transport lifecycle](../state-machines/transport-lifecycle.md) + +## Context + +Android allowed one client link beside one server link to the same peer, one +per direction, and refused only a second client link (the macOS rotation case, +#548). Its 120-second "forced BLE refresh" restarted the advertisement, and the +stack gives every advertising set a new random address. iOS reported a peer +lost whenever a dial to one of its identifiers dropped, unless that same +identifier had been connected. + +In run 5 of the 2026-10-19 volume test (iPhone 11, Galaxy M36) the iPhone held +a link to Android's server. Its random address rotated, Android saw a new +device, dialled it, verified it as the iPhone and kept it: two Link Layer +connections between the same two radios. Two minutes later the iPhone's write +buffer on the first link stopped draining, and the stall policy tore that link +down. The iPhone kept writing on the second link for ten seconds, until its +redial of the first link's address timed out. CoreBluetooth reports a +cancelled connect as a disconnect, the identifier still mapped to Android, and +the bridge reported Android lost. The core then refused every send to a peer +the iPhone was connected to, for ten minutes, and about 2,500 queued messages +per phone failed as the outbox filled. + +No reconnect could end it. The Link Layer does not complete a second +connection between two devices that already have one (Core 5.4, Vol 6 Part B, +1.1.1), and every dial the iPhone made came from the address Android's second +link held. Seven of Android's advertising addresses in a row timed out, and +the dials that timed out before the stall show the same thing. The one that +completed followed the iPhone's own address rotation. + +## Decision + +One usable link per peer pair, and no peer is lost while it has one. + +- Android closes a client link that verifies as a peer it already holds a + usable link to: a client link of its own, or the peer's link to its server + with notifications subscribed. Before, only a second client link counted. +- Android's periodic refresh restarts the advertisement only while no + identified peer is linked, or when the advertiser reports it stopped. + Membership changes and disconnects still restart it. +- An Android server link counts only once its peer has written a Hello that + maps its address. A subscribed central that writes none (a Mac, a leaf, a + service without it) is invisible to the refusal, so the second client link + beside it is kept. +- iOS never reports a peer lost, and never redials, when the identifier that + dropped belongs to a peer connected and verified on another identifier or + subscribed to its server from one and heard from in the last 60 seconds. + The dropped identifier is retired. A subscription counts only while it is + heard from because it ends only when `didUnsubscribeFrom` arrives: one that + never arrived would otherwise keep a departed peer a neighbour for good. +- iOS evicts every identifier a peer maps from, not one, before reporting it + lost. + +## Consequences + +A second link no longer forms between two phones, so a link torn down by the +stall policy is redialled at an address pair the radios accept. A peer stays a +neighbour, and sendable, as long as any link to it is up. + +The partial versions each leave a failure. The iOS rule alone keeps the +second link, so whenever the iPhone's link dies while Android's is up, the +reconnect waits for an address rotation, up to fifteen minutes. The Android +rules alone leave the false loss on the iPhone whenever two links form anyway +(a peer the bridge does not control, such as a Mac, rotating its address). + +The write-buffer stall itself is not explained. It has been seen only while +two links existed (runs 2, 3 and 5; never in run 4, which had one); the stall +teardown logs the peer's other live links so the next run can confirm or +refute that. + +The refresh trade: while an identified peer is linked, Android's advertisement +is restarted only if the advertiser reports it stopped. One the stack stopped +without telling the advertiser stays stopped until the next membership change, +disconnect or peer loss, so a third device may not find this phone until then. +The alternative, restarting on the timer while linked, rotates the address +every two minutes, which is what made the second link form. + +## What would undo this + +Allowing a client link beside a subscribed server link again, or restarting +the advertisement on a timer while linked. Either brings back the address +churn that makes a second link form. diff --git a/docs/adr/README.md b/docs/adr/README.md index 506b361db..66f813abb 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -40,6 +40,7 @@ silently undo it. Decisions that follow from the obvious default do not need one | [0027](0027-ios-peer-streams-ride-network-framework.md) | iOS peer streams ride Network framework and keep the lower-opened stream | Accepted | | [0028](0028-android-peer-streams-join-the-lan.md) | Android peer streams join the LAN and keep the lower-opened stream | Accepted | | [0029](0029-mobile-libraries-are-sized-not-aborting.md) | The mobile libraries are optimised for size and still unwind | Accepted | +| [0030](0030-one-bluetooth-le-link-per-peer-pair.md) | One Bluetooth LE link per peer pair | Accepted | ## Format diff --git a/docs/api-reference.md b/docs/api-reference.md index 7fdbf62b1..c35059b9d 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -687,8 +687,18 @@ MessageDeferred { } ``` -Emitted when a message is queued for retry because no transport could deliver -it right now. Non-terminal. +Emitted when a message the app sent is accepted but not transmitted yet. +Non-terminal: `message_delivered` or `message_failed` follows. `reason` says +why it waits: + +| `reason` | Meaning | `next_retry_at` | +|----------|---------|-----------------| +| a transport failure token | No transport could deliver it right now; the retry queue tries again | When the retry is due | +| `ack_capacity` | The pending-acknowledgement table is full; it goes out as soon as an acknowledgement frees a slot | The retry queue's next look | +| `peer_window` | Sixteen messages to this peer already await acknowledgement over a Bluetooth LE or Wi-Fi Direct link; it goes out, in order, as they are answered | `None` | + +A deferred message never emits `message_sent`, even once it is transmitted. +Show it as queued until it is delivered or fails. #### MessageRetrying diff --git a/docs/configuration.md b/docs/configuration.md index c77608cef..e3ba5b3d2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -563,7 +563,7 @@ numbers mean and what a custodian owes. | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `defaultTimeoutMs` | number | 10000 | ACK timeout (milliseconds) | -| `maxPendingAcks` | number | 1000 | Max pending ACKs | +| `maxPendingAcks` | number | 1000 | Max pending ACKs. At the ceiling a send is held for the retry queue and reported as `message_deferred` with reason `ack_capacity`, never failed | **Retry Config**: | Parameter | Type | Default | Description | diff --git a/docs/message-delivery.md b/docs/message-delivery.md index aeb9bd141..87d39bda5 100644 --- a/docs/message-delivery.md +++ b/docs/message-delivery.md @@ -241,7 +241,7 @@ The outbox persists messages that require acknowledgment. It serves as the sourc | Max media entries | 100 | File chunk messages | | Max lifetime | 7 days | `outbox_max_lifetime_ms` | -When the outbox is full, the oldest entry is evicted with a terminal `message_failed` event (reason `"Outbox capacity exceeded"`). Eviction retires the message from the retry queue and from acknowledgement tracking as well, so an evicted message is never resent, and neither a later retry nor its acknowledgement settles it a second time. When a message exceeds its lifetime and has no pending ACK, it is dropped and a terminal `message_failed` event (reason `"Outbox lifetime exceeded"`) is emitted so the app can settle its UI state. +When the outbox is full, a new message to the recipient with the largest backlog is refused with a terminal `message_failed` event (reason `"Outbox capacity exceeded"`, no attempts) while `send` still returns its id; a message to a recipient with a smaller backlog is admitted by evicting the oldest entry of the largest one, with the same event. A message awaiting its acknowledgement is never evicted ([rules](state-machines/outbox-and-retries.md#at-outbox-capacity)). Eviction retires the message from the retry queue and from acknowledgement tracking as well, so an evicted message is never resent, and neither a later retry nor its acknowledgement settles it a second time. When a message exceeds its lifetime and has no pending ACK, it is dropped and a terminal `message_failed` event (reason `"Outbox lifetime exceeded"`) is emitted so the app can settle its UI state. **Important**: When a message storage backend is configured, regular-message outbox entries are persisted and restored on the next `start()` with a refreshed delivery window. Media chunks are never persisted — an interrupted transfer surfaces as `media_resend_required` instead. See [Client-Side Persistence](#client-side-persistence) for the app-side layer. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index ee84dcdf3..cd95b555a 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -230,6 +230,14 @@ sealed to an epoch the recipient has left is withheld like any other desync. Custody widens nobody's view of the acknowledgement channel in this version, because acknowledgements are not carried. +A coalesced acknowledgement (`ack_more`, see +[Delivery and acknowledgements](../state-machines/delivery-and-acks.md#acknowledgements-on-a-mesh-link)) +does not widen this. Each id it names is settled as a single acknowledgement, +including the check that it comes from the frame's recipient, and a receiver +settles at most five, so a forged one does what five forged single +acknowledgements could already do. Acknowledgements are unauthenticated either +way; binding one to its sender needs signed acknowledgements. + ## Known residual risks Stated plainly, because a threat model that lists only what it defeats is @@ -277,6 +285,13 @@ or signature check; and a 1:1 slot identifier is a public function of two public addresses. The MLS credential that would authenticate the sender exists only once decryption **succeeds**. +The desync class has one member this does not apply to: a frame more than the +forward window ahead of the receiver's ratchet. MLS decrypts the sender data +under AEAD before it reads the generation from it, so reaching that check takes +the epoch's sender-data secret, which only a group member holds. It was added +so a receiver that fell that far behind re-keys instead of failing every later +frame, and it leaves the unauthenticated surface exactly as described. + **Why it is tolerable:** the mitigation is that acting on the trigger is **harmless**, not that the trigger is trusted. diff --git a/docs/spec/ble-framing.md b/docs/spec/ble-framing.md index da629a217..07697047d 100644 --- a/docs/spec/ble-framing.md +++ b/docs/spec/ble-framing.md @@ -51,6 +51,25 @@ them is a wire break rather than a refactor: to keep a hostile header from costing memory or CPU, never to establish trust. +One more holds for the links that carry the frames, and it is a property of +the radio rather than of this format: + +- **Two devices hold one Bluetooth LE link between them.** The Link Layer + does not complete a second connection between two devices that already have + one (Core 5.4, Vol 6 Part B, 1.1.1), so a dial from an address the peer + already holds a link with times out, however often it is retried. A second + link can still form when one side's random address has rotated, because the + pair then looks new to both radios, and once it has formed neither side can + reconnect after the other link drops until an address rotates again. So a + device SHOULD close a link that proves the identity of a peer it already + holds a usable link to (a client link of its own, or the peer's subscribed + link to its server), and MUST NOT report a peer lost, or dial it, while such + a link is up. In run 5 of the 2026-10-19 volume test both rules were missing: the + phones held two links, the one that stalled was torn down, the redial of + its address timed out and reported a peer lost that was still linked, and + the pair was apart for ten minutes + ([ADR 0030](../adr/0030-one-bluetooth-le-link-per-peer-pair.md)). + ## The GATT contract A device offers one primary service for each application on it that runs this diff --git a/docs/spec/capability-negotiation.md b/docs/spec/capability-negotiation.md index 5ace03260..0519129c2 100644 --- a/docs/spec/capability-negotiation.md +++ b/docs/spec/capability-negotiation.md @@ -13,7 +13,7 @@ Peers advertise what they can parse in the key package payload, the body of a | `data_versions` | End to end | Whether we may send `__DATA_V1__` document sync frames, and which document encoding they carry. Entry 1 is 1:1 replication; entry 2 additionally means the peer intercepts these frames inside a *group* ciphertext; entry 3 additionally means the peer speaks the blob-fetch frames and routes a data-purposed media transfer into its document layer; entry 4 additionally means the peer reads the removals a version offer carries; entry 5 additionally means it answers inside the interest an offer declares; entry 6 additionally means it carries attachment bytes inside a group; entry 7 additionally means it parses the custody receipt, so a custodian may answer its deposits | No replication with that peer | | `ctrl_versions` | End to end | Which control-frame signing payload we build for this peer. Entry 2 means the peer verifies `offline-ctrl-v2`, which binds the frame's timestamp | Build `offline-ctrl-v1`, which states no freshness | | `svc_versions` | End to end | Whether a service request or response may travel sealed inside `__MLS_ENC__` toward this peer. Entry 1 means the peer routes a decrypted `__SVC_REQ__` or `__SVC_RESP__` into its service layer, with the MLS-authenticated sender as the requester | Signed plaintext service frames | -| `ack_versions` | End to end | Which delivery-ACK statuses we may send this peer. Entry 1 means the peer reads `ack_status: "undecryptable"` as "this frame can never decrypt" (it resends the message once, re-sealed when it can) rather than as a delivery | Never answer an undecryptable frame; leave it unacknowledged | +| `ack_versions` | End to end | Which delivery-ACK statuses we may send this peer. Entry 1 means the peer reads `ack_status: "undecryptable"` as "this frame can never decrypt" (it resends the message once, re-sealed when it can) rather than as a delivery. Entry 2 means the peer reads `ack_more`, so one delivery ACK may answer up to five of its frames | Never answer an undecryptable frame; leave it unacknowledged. Acknowledge each frame in its own ACK | | `nostr_pubkey` | End to end | Which key metadata is sealed to on the Nostr path | Seal to the publicly computable key | The key package payload also carries `user_id`, the MLS key package itself, a @@ -158,7 +158,9 @@ or whose advertisement this device has not yet received, is sent no `undecryptable` answer at all: an older sender reads any ACK as a delivery, so answering it would mark as delivered a message its recipient could not read. The cost of not knowing is the older behaviour, an unanswered frame that a relay -mailbox keeps redelivering until the sender's next key package arrives. +mailbox keeps redelivering until the sender's next key package arrives. Entry 2 +follows the same rule: a peer not known to read `ack_more` gets one ACK per +frame, since an older SDK would settle only the first id of a coalesced one. `wire_versions` is **hop-local** and deliberately in-memory only. It describes what the next hop decodes, and it is re-exchanged on connect. diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index fd1f37125..f7b8c3eda 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -339,6 +339,7 @@ observed on the wire: |-----|---------| | `ack_for` | This message acknowledges the named message id | | `ack_hop_count` | Hop count observed by the acknowledging party | +| `ack_more` | On a delivery `ack_for` frame: further acknowledged message ids, comma-separated, at most four. Sent only to a peer that advertised `ack_versions` entry 2 ([Capability negotiation](capability-negotiation.md)), only for frames that arrived over a mesh carrier, and never with `ack_status`. A receiver settles each id as if it arrived in its own `ack_for` frame; one that ignores the key still settles `ack_for` | | `ack_status` | On an `ack_for` frame: `"undecryptable"` when the recipient can never decrypt the named frame (spent ratchet generation, dead epoch, or the pending-decryption queue gave up on it). The sender resends the message once (re-sealed under its current session when it still holds the plaintext, verbatim otherwise) and never reports it delivered. Sent only for a frame that arrived over the internet relay, and only to a peer that advertised `ack_versions` entry 1 ([Capability negotiation](capability-negotiation.md)), since an SDK that does not know it reads it as a delivery ACK. Absent on a delivery ACK | | `ack_transport` | Transport the acknowledged message arrived on | | `transport_preference` | Requested transport for this message | diff --git a/docs/state-machines/delivery-and-acks.md b/docs/state-machines/delivery-and-acks.md index 49f1bb90b..65d63ade7 100644 --- a/docs/state-machines/delivery-and-acks.md +++ b/docs/state-machines/delivery-and-acks.md @@ -83,7 +83,19 @@ appears. `Duplicate` is a receive-loop disposition rather than a decrypt outcome, and its acknowledgement is conditional: a duplicate is re-acknowledged only when the -frame asked for an acknowledgement and the sender is not blocked. A duplicate of +frame asked for an acknowledgement and the sender is not blocked. + +**A message sent during a block is never delivered, whenever the block ends.** +A blocked sender's frame is dropped with no acknowledgement and no event, and +its identifier is remembered. The block reveals nothing while it lasts, and +once it is lifted a retry of that message meets the duplicate path, which +answers it, so the sender stops retrying. The sender's application sees the +message as delivered at that point; it never learns of the block. Before this, +the identifier was deliberately left unmarked, so a sender whose retry ladder +(about 15 minutes by default) was still running at unblock delivered everything +it had sent during the block. That was the opposite of what a user who blocked +someone expects, and of the post-unblock reset, which already drops our own +queued messages to that peer. A duplicate of a group message that is **still pending** in the buffer resolves to `Deferred` instead, because the original has not been delivered either. @@ -111,6 +123,29 @@ implementation's decrypt-result type also has five cases, but they are not the same five: it splits a delivered message out as its own case and does not model duplicates at all, because the duplicate check runs before it. +## Acknowledgements on a mesh link + +**An acknowledgement is queued ahead of data, and one frame may answer up to +five.** Both exist because an acknowledgement is what stops its sender's retry +timer, and on a mesh link it shares a radio link that may carry less than it +is offered. + +- **Ahead of data.** The Bluetooth LE carrier queues a frame carrying `ack_for` + ahead of every queued data frame, behind earlier acknowledgements. The + fragments of a message already being written are not overtaken. Behind data, + in run 4 of the 2026-10-19 volume test, the round trip reached 196 seconds + against a 10-second timeout. +- **Coalesced.** To a peer that advertised `ack_versions` entry 2, delivery + acknowledgements for frames that arrived over a mesh carrier are held for up + to 500 ms and sent as one frame of up to five: the first id in `ack_for`, the + rest comma-separated in `ack_more`. A sender settles each id exactly as a + single acknowledgement, every check included, and emits one + `message_delivered` per id. An `undecryptable` answer is never coalesced, and + a frame carrying both `ack_status` and `ack_more` settles only its `ack_for`. + An acknowledgement per frame was a third of the frames a phone sent in that + test. A held batch is in memory only; one lost with a restart is answered + again when the resend arrives as a duplicate. + ## The deferred-acknowledgement atom The `Deferred` outcome is not a single change. It is six interdependent pieces @@ -212,7 +247,8 @@ flowchart TD |-------|--------------|----------|--------|-----| | Session not ready | no | **yes** | no | It will become decryptable when the session arrives | | Session desync (epoch fork) | `undecryptable` | no | **yes** | Ciphertext is sealed to a dead epoch and can never drain | -| Crypto failure (AEAD, corrupt, ratchet generation) | `undecryptable` | no | **no** | The attempt spent the generation; a queued copy could never drain | +| Session desync (receiver past the forward window) | `undecryptable` | no | **yes** | Every later frame from that sender is further ahead, so only a re-key ends it; see [Session lifecycle](session-lifecycle.md#desync-and-heal) | +| Crypto failure (AEAD, corrupt, generation behind the tolerance) | `undecryptable` | no | **no** | The attempt spent the generation; a queued copy could never drain | | Transport failure | `undecryptable` | no | no | Classified with crypto failures: the attempt may have spent the generation, so recovery is the sender's re-sealed resend | | Envelope parse failure | no | no | no | Unparseable now is unparseable forever; the resend is the fix | | Policy refusal (commit not authorized) | **yes** | no | no | Can never become decryptable, so retries are pure waste | diff --git a/docs/state-machines/outbox-and-retries.md b/docs/state-machines/outbox-and-retries.md index 22f506f01..e710f3d74 100644 --- a/docs/state-machines/outbox-and-retries.md +++ b/docs/state-machines/outbox-and-retries.md @@ -30,6 +30,15 @@ persisted, and an entry restored after a restart replays verbatim. See **S4. Frames that cannot be acknowledged never enter the ladder.** See relay hint frames below. +**S5. A sender never has more than sixteen frames awaiting acknowledgement from +one peer over a mesh link.** The next one waits in the outbox and goes out, +oldest first, when an acknowledgement or a give-up frees a slot. See +[The per-peer window](#the-per-peer-window). + +**S6. A full outbox refuses the newest message for the recipient with the +largest backlog, and never fails a message awaiting its acknowledgement.** See +[At outbox capacity](#at-outbox-capacity). + ## States ```mermaid @@ -49,9 +58,9 @@ stateDiagram-v2 Parked --> Queued: probe succeeds / peer returns Parked --> Failed: lifetime expired - Queued --> Failed: evicted, outbox full - Pending --> Failed: evicted, outbox full - Parked --> Failed: evicted, outbox full + [*] --> Failed: refused, outbox full + Queued --> Failed: evicted for a lighter recipient + Parked --> Failed: evicted for a lighter recipient Delivered --> [*] Failed --> [*] @@ -89,9 +98,8 @@ should be: accepted, which is why there is a pending acknowledgement for the park to remove. -Eviction is the one edge that leaves every non-terminal state. When the outbox -is full, the least recently sent entry fails to make room, whatever it was -doing. +A message awaiting its acknowledgement is never evicted, so the eviction edges +leave `Queued` and `Parked` only. See [At outbox capacity](#at-outbox-capacity). ### Giving a message up @@ -122,12 +130,12 @@ reports it delivered after it was reported failed. | Parameter | Default | Notes | |-----------|---------|-------| -| Acknowledgement timeout | 10 s | Starts at **local enqueue**, not at wire write | +| Acknowledgement timeout | 10 s | Starts at **local enqueue**; extended up to 60 s while the frame is still queued | | Maximum retries | 10 | | | Backoff multiplier | 2.0 | Exponential | | Maximum backoff delay | 300 s | Caps the exponential | -| Maximum pending acknowledgements | 1000 | | -| Outbox capacity | 500 entries | When full, the least recently sent entry fails | +| Maximum pending acknowledgements | 1000 | At the ceiling a send is held, not failed (see below) | +| Outbox capacity | 500 entries | When full, the newest message to the heaviest recipient is refused (see below) | | Outbox lifetime | 7 days | Per entry, carrier-relative | | Absolute lifetime cap | 4 × lifetime (28 days) | Measured from first send, bounds restart refreshes | @@ -143,6 +151,145 @@ retransmitted before it was ever written. Deduplication absorbs the duplicates, so the cost is wasted work rather than loss, but the effect is real past roughly 118 members. +**A frame still in a local transport queue gets a bounded extension instead +of a retry.** When an acknowledgement times out, the sender asks its carriers +which frames they have accepted but not started writing (only BLE knows today: +a message counts as queued until the bridge pulls its first fragment, which is +why each bridge stops pulling once a peer holds 32 fragments; at 576 the +extension saw almost nothing). Such a +frame gets one more full timeout, repeatedly, until a minute has passed since +the attempt, and only then falls through to the ladder. The bound is what keeps +a link that died without telling the transport from pinning a message to it. + +The failure this prevents is a feedback loop on a link carrying less than it +is offered. Each retry appended a second copy behind the first, re-sealed under +a fresh ratchet generation, so the queue grew, the round trip grew past the +timeout, more frames were retried, and the receiver skipped one generation per +copy it dropped as a duplicate. In the 2026-10-19 volume test the sender logged +12,152 retries against 2,434 messages the receiver decrypted, and the receiver +ended up past its forward window (see +[Session lifecycle](session-lifecycle.md)). The extension restarts the deadline +without touching the send time the round-trip estimate reads. + +## At the acknowledgement ceiling + +**A frame is never transmitted without a slot to track it, and a full table +holds a send back rather than failing it.** Every transmit path asks the +pending-acknowledgement table for a slot before it writes. With none free, the +message goes to the outbox and the retry queue at its first attempt, the +application receives `message_deferred` with reason `ack_capacity`, and the send +returns its id as usual. The retry queue tries it again at the ladder's first +rung, without spending an attempt and without re-sealing it, and sends it as +soon as an acknowledgement frees a slot. + +Both halves exist because the other order failed. The check used to come after +the transmit, as a registration error. The direct send path returned that error +to the application for a message already on the wire, so in the volume test one +phone's API accepted 2,748 messages while the other received 4,626. The retry +path propagated the same error out of `process()`, which then returned before +the timeout sweep on every tick: the one step that frees slots never ran again, +and the engine never recovered. `process()` now runs every step on every tick +and reports the first error at the end. + +Two more things kept the table from draining, and both are gone. A refused +registration used to mark every timed-out entry and discard the list, and the +sweep skips a marked entry, so those messages were never retried. And the +prune of timed-out entries ran at a fixed five minutes, exactly the ladder's +maximum delay, so the slot of a message on its ninth retry was gone just before +that resend fired. The prune age is now twice the ACK timeout plus the maximum +delay. + +`ack_capacity` is the signal to slow down. It appears when the application +offers more than the link carries for long enough to fill the table. With the +per-peer window below, a single mesh peer can no longer fill it. + +## At outbox capacity + +**A full outbox refuses the newest message for the recipient holding the +largest backlog, and never fails a message awaiting its acknowledgement.** +Three rules, applied when the application sends and the outbox already holds +500 entries: + +1. A message awaiting its acknowledgement is never evicted: it is on the wire, + and failing it leaves an acknowledgement that settles nothing. +2. A message never displaces another to the same recipient. When the new + message's recipient holds the largest backlog (ties included), the new + message is refused: it gets no outbox entry, the application receives + `message_failed` with reason "Outbox capacity exceeded" and no attempts, + and `send` still returns its id. +3. Otherwise the recipient with the largest backlog pays, with the same + terminal event. A message the per-peer window holds for a live peer goes + newest first, so the front of that line, the part about to be sent, stays + and the conversation stays contiguous; any other entry goes oldest first. + A week of messages to one unreachable peer, or a long held line to one + slow peer, cannot block a message to anyone else. + +Held messages were once never evicted on a live link. Then one slow peer whose +held line filled the outbox made every send to every other recipient a +refusal: the outbox was full of messages that had never left the device. + +When nothing may be evicted (every entry awaits its acknowledgement), the new +message is refused. Internal frames +(key packages, Welcomes, receipts) are never refused: they evict by rules 1 +and 3 regardless of their own recipient, and when every entry is being +delivered they are admitted over the cap, which the acknowledgement table +bounds. Media chunks keep their own cap and their oldest-first eviction. + +The rule this replaces evicted the least recently sent entry to admit any new +one, whatever either was doing. In run 5 of the 2026-10-19 volume test a +ten-minute Bluetooth outage failed about 2,500 queued messages per phone that +way, each so that a newer message to the same unreachable peer could take its +slot, and the application learned of each failure minutes after it had shown +the message as queued. Under load on a live link the same rule failed frames +already on the wire, whose acknowledgements then settled nothing. Refusing the +newest instead keeps a queued conversation contiguous and tells the +application at the moment of sending. + +## The per-peer window + +**At most sixteen frames that wait for an acknowledgement are in flight to one +peer over a mesh carrier (Bluetooth LE, Wi-Fi Direct).** A seventeenth is held: +it goes to the outbox, the application receives `message_deferred` with reason +`peer_window`, and the send returns its id as usual. Held messages go out +oldest first as slots free, at most a flush batch per peer per tick. A new +message never overtakes one already held. Holding is not an attempt: nothing is +sealed and no retry is counted. + +Session control is never held: a key package, a Welcome, a confirmation probe +or its answer. They heal the session the window's frames are sealed under, and +held behind sixteen frames the peer cannot decrypt they would wait for +acknowledgements that never come. + +The count is derived from the outbox, entries to that peer with at least one +attempt whose last carrier was a mesh one, so a settle, give-up, expiry or +eviction frees its slot without a path having to remember to. A resend of a +frame already in flight always passes. When the peer's mesh link goes, the +window stops applying and everything held goes to transport selection. + +The failure it prevents is congestion collapse on a link carrying less than it +is offered. Without it the sender handed the carrier every new frame, every +resend and one acknowledgement per received frame, into one unbounded queue, +on a timer that starts at enqueue. In run 4 of the 2026-10-19 volume test the +Android phone's indication path carried 12 to 16 frames a second against 15 +offered. The acknowledgement round trip crossed the timeout after about two +minutes, each resend cost the receiver one more acknowledgement on the +saturated path, that path's rate halved, both phones filled their +acknowledgement tables, and every later send evicted a message as "Outbox +capacity exceeded": 70.5% delivered, 50,000 retries. The four earlier fixes on +that branch (the ceiling above, the re-seal budget, the BLE stall teardown, +the bridge pull mark) each removed one amplifier of the same loop. The window +removes the loop: it bounds the queue, and with it the round trip, the +resends and the receiver's ratchet gap, at any link rate. + +Sixteen is above what that link carries in one unloaded round trip (about six +messages, counting the receiver's acknowledgement coalescing and a processing +tick on each side), so it costs no throughput, and it keeps the loaded round +trip there near three seconds. A compile-time assertion keeps a full window's +worth of retries inside the re-seal budget. + +The relay is not windowed. Its mailbox is store-and-forward, and holding a +message back from it would strand it on a device the user may close. + ## Retry re-sealing A retry does not replay stored bytes, as long as the re-seal provenance staged @@ -161,6 +308,50 @@ This is the sender-side half of the desync recovery documented in withheld acknowledgement, it is what makes a session fork recoverable without message loss, for entries whose provenance is still in memory. +### The re-seal budget + +**A sender never seals more than a quarter of the smallest receiver forward +window past the last frame that receiver acknowledged by re-sealing +resends.** Every +re-seal spends a ratchet generation. The receiver has to fast-forward over it, +and one it drops as a duplicate of a message it already holds is never +decrypted at all. A receiver more than the forward window behind decrypts +nothing from that sender until the pair re-keys. So the sender counts, per +session, the frames it has sealed past the newest one the peer acknowledged +(an acknowledgement proves the peer decrypted that far). Once the count reaches +the budget, a resend goes out verbatim, +which spends nothing and still decrypts unless the receiver has since moved +more than the out-of-order tolerance past it. The next acknowledgement re-opens +the budget. New messages always seal; the budget leaves them the rest of the +window. + +The budget is 256: a quarter of 1,024, the window of a leaf node, which runs +mls-rs. mls-rs fixes that window and offers no setting, so it is smaller than +the 8,192 a phone's OpenMLS group is configured with, and a budget derived +from the phone's window (2,048) let a sender walk a leaf past its window by +re-sealing alone. `tools/mls-interop` pins the leaf's 1,024 against mls-rs +itself. With the per-peer window, unacknowledged seals grow only by resends of +at most sixteen frames, so the budget trips only after a whole retry ladder +with no acknowledgement; a compile-time assertion keeps it above that. + +**One exception: a resend whose verbatim bytes can no longer decrypt is +re-sealed past the budget, up to half the smallest forward window (512).** A copy sealed more +than the out-of-order tolerance (32) before the newest frame this device sealed +is refused by a receiver that has decrypted anything since, and a mesh link +carries no answer for it, so verbatim it was resent until its ladder ran out. +Run 4 of the volume test lost 525 messages that way, 3,308 decrypt failures on +the receiver. Re-sealing such a resend spends a generation, which is what the +budget rations, so the exception stops at half the window: the receiver going +deaf is the failure the budget exists for, a wasted resend is not. + +Without it, a link that stops carrying acknowledgements turns the retry ladder +into a key-burner. In the 2026-10-19 rerun one phone re-sealed 20,919 resends +during a 17-minute stall and left its peer past even the widened window. The +count is in memory and per session incarnation, so a rebuilt session starts +from nothing and an acknowledgement for a frame of the old one vouches for +nothing. Acknowledgements are unauthenticated, so a forged one can give +re-seals back, but never more than a sender had before the budget existed. + ### Provenance handling Re-seal provenance holds **plaintext**, so it is memory-only and never diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 6fce59730..4f8da47e1 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -279,6 +279,23 @@ epoch-disagreement decrypt failure. This is a dedicated recoverable class, kept strictly separate from ordinary decryption failure. See [Delivery and acknowledgements](delivery-and-acks.md#classifying-decrypt-failures). +**A receiver that has fallen past its forward window is in the same class.** +A frame more than the sender-ratchet forward distance (8192 generations) ahead +of what the receiver last decrypted can never decrypt, and neither can any +later frame from that sender, including every re-sealed resend, because each is +further ahead still. Classified as an ordinary failure, that waited forever for +a resend that could not succeed: the 2026-10-19 volume test lost a whole +direction this way, as thousands of per-frame `INVALID_CIPHERTEXT` failures +with nothing ever healing them. It is classified as a desync and heals the same +way. A frame too far *behind* (past the out-of-order tolerance) stays an +ordinary failure: the session is healthy, and the sender's re-sealed resend +recovers it. + +Unlike the epoch checks, this trigger is authenticated. MLS decrypts the +sender data under AEAD before it reads the generation, so only a holder of the +epoch's sender-data secret can produce it. It widens nothing in the +unauthenticated surface described below. + ### Tier 1: honest failure and heal (receiver side) On a desync the receiver: diff --git a/docs/state-machines/transport-lifecycle.md b/docs/state-machines/transport-lifecycle.md index d0f9d536b..c6625521a 100644 --- a/docs/state-machines/transport-lifecycle.md +++ b/docs/state-machines/transport-lifecycle.md @@ -189,6 +189,20 @@ connect timeout. Without the bound, links that closed cleanly kept the peer a neighbour until the reconnect backoff gave up, about two minutes, and a peer reached only through our server indefinitely. +**A Bluetooth LE peer with a live link is never reported lost.** On iOS a link +or a dial that drops while the peer is connected and verified on another +identifier, or subscribed to our server from one and heard from in the last +minute, retires the identifier that +dropped instead of reporting the peer lost or redialling it, and an identifier +of a peer linked elsewhere is not dialled. CoreBluetooth reports a cancelled +connect, including the one the connect timeout cancels, as a disconnect, so +without this a redial of a dead address reported a linked peer lost, and the +core refused every send to it until a new link was verified: ten minutes in +run 5 of the 2026-10-19 volume test. Android keeps one link per peer pair, and +its periodic refresh restarts the advertisement only while no link is up, +because each restart gives the advertisement a new random address +([ADR 0030](../adr/0030-one-bluetooth-le-link-per-peer-pair.md)). + ## What "sent" means at each layer This is a recurring source of confusion for application teams: diff --git a/examples/demo-app/src/context/ProtocolContext.tsx b/examples/demo-app/src/context/ProtocolContext.tsx index ac563e556..fe1e50bc4 100644 --- a/examples/demo-app/src/context/ProtocolContext.tsx +++ b/examples/demo-app/src/context/ProtocolContext.tsx @@ -936,6 +936,30 @@ export function ProtocolProvider({children}: {children: React.ReactNode}) { await proto.start(); setIsStarted(true); + // Sessions restored from storage never emit secure_session_established: + // that event marks an establishment, and these were established in an + // earlier run. Seed the set from the SDK so a restored contact shows its + // lock. The ids share the event's namespace (the peer's address). + try { + const restored = await proto.mlsListSessions(); + for (const peerId of restored) {sessionPeersRef.current.add(peerId);} + if (restored.length > 0) { + setContacts(prev => { + let next: Map | null = null; + for (const peerId of restored) { + const existing = prev.get(peerId); + if (existing && !existing.hasSession) { + next = next ?? new Map(prev); + next.set(peerId, {...existing, hasSession: true}); + } + } + return next ?? prev; + }); + } + } catch (err) { + console.warn('[ProtocolContext] could not list restored sessions:', err); + } + try { localAddressRef.current = await proto.localAddress(); const groupIds = await proto.meshListGroups(); diff --git a/tools/mls-interop/src/main.rs b/tools/mls-interop/src/main.rs index addd4eb55..00a837f90 100644 --- a/tools/mls-interop/src/main.rs +++ b/tools/mls-interop/src/main.rs @@ -52,12 +52,12 @@ use mls_rs::{CipherSuite, CipherSuiteProvider, Client, CryptoProvider, MlsMessag use mls_rs_crypto_rustcrypto::RustCryptoProvider; // The SDK's own declarations, not copies of them. The derivation rule and -// these four numbers are what this harness exists to test the phone against; +// these numbers are what this harness exists to test the phone against; // a local copy would leave it green while it stopped testing them. use offline_protocol_sealed::{ derive_address, LEAF_KEY_PACKAGE_LIFETIME, LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS, - MAX_ACCEPTED_KEY_PACKAGE_LIFETIME, SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, - SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, + LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, MAX_ACCEPTED_KEY_PACKAGE_LIFETIME, + SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE, SENDER_RATCHET_OUT_OF_ORDER_TOLERANCE, }; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -278,7 +278,10 @@ fn corrections_are_load_bearing() { fn the_lifetime_cap_is_ours_to_apply() { let (provider, _, _) = phone(); let (client, _) = leaf_client(MLS_RS_DEFAULT_LIFETIME); - let bytes = leaf_key_package(&client, unix_now() - LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS); + let bytes = leaf_key_package( + &client, + unix_now() - LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS, + ); if let Err(e) = openmls_admits(&provider, &bytes) { panic!( @@ -316,7 +319,10 @@ fn main() { let (leaf, leaf_address) = leaf_client(LEAF_KEY_PACKAGE_LIFETIME); step(1, &format!("leaf identity derives to {leaf_address}")); - let key_package_bytes = leaf_key_package(&leaf, unix_now() - LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS); + let key_package_bytes = leaf_key_package( + &leaf, + unix_now() - LEAF_KEY_PACKAGE_NOT_BEFORE_BACKDATE_SECONDS, + ); step( 2, &format!("leaf key package, {} bytes", key_package_bytes.len()), @@ -486,6 +492,54 @@ fn main() { other => panic!("expected an application message, got {other:?}"), } + // ---- The leaf's receiver window ------------------------------------- + // + // mls-rs fixes how far ahead a frame may be and exposes no setting. The + // phone's re-seal budget is derived from it, so an mls-rs upgrade that + // moves it has to fail here, not on a device under load. A frame exactly + // the window past the next generation the leaf expects decrypts; one + // further is refused. + let phone_frame = |group: &mut MlsGroup, text: &[u8]| { + group + .create_message(&provider, &phone_keys, text) + .expect("phone encrypts") + .tls_serialize_detached() + .expect("serialize") + }; + for _ in 0..LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE { + let _lost = phone_frame(&mut phone_group, b"lost"); + } + let at_the_edge = phone_frame(&mut phone_group, b"at the edge"); + match leaf_group + .process_incoming_message(MlsMessage::from_bytes(&at_the_edge).expect("leaf parses")) + .expect("leaf decrypts a frame exactly its window ahead") + { + mls_rs::group::ReceivedMessage::ApplicationMessage(app) => { + assert_eq!(app.data(), b"at the edge"); + step( + 11, + &format!( + "leaf decrypted a frame {LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE} generations ahead" + ), + ); + } + other => panic!("expected an application message, got {other:?}"), + } + for _ in 0..=LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE { + let _lost = phone_frame(&mut phone_group, b"lost"); + } + let past_the_edge = phone_frame(&mut phone_group, b"past the edge"); + let refused = leaf_group + .process_incoming_message(MlsMessage::from_bytes(&past_the_edge).expect("leaf parses")); + assert!( + refused.is_err(), + "mls-rs accepted a frame {} generations ahead: its receiver window moved, so \ + LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE and the phone's re-seal budget \ + derived from it are stale", + LEAF_SENDER_RATCHET_MAXIMUM_FORWARD_DISTANCE + 1 + ); + step(12, "leaf refused a frame one generation past its window"); + println!("\nPASS: the leaf stack interoperates with the phone's, both directions,"); println!("across a commit, with the SDK's ciphersuite and credential shape."); }