From eaaafc9df5438f7b8360c1a377284dca6b760d7e Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Mon, 5 Oct 2026 22:52:21 +0400 Subject: [PATCH 01/15] fix(protocol): a new confirmation probe supersedes the last An unconfirmed session is probed every five seconds and each probe entered the outbox with its own retry ladder on top of the unanswered ones. Only a relay unreachable verdict backed that off, which a relay that pushes to offline users or a mesh-only link never produces. An account with ten such contacts stacked about two probes a second, filled the 500-entry outbox, and capacity eviction then failed the user's own messages. Each probe now retires the previous one (no event, no carrier penalty), so a peer holds one probe in the outbox on the same cadence. --- CHANGELOG.md | 12 +++ crates/offline-protocol/src/protocol/mod.rs | 10 +++ .../offline-protocol/src/protocol/session.rs | 15 +++- .../src/protocol/tests/mod.rs | 81 +++++++++++++++++++ docs/state-machines/session-lifecycle.md | 8 ++ 5 files changed, 125 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9de47f0b..151721f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -345,6 +345,18 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **Confirmation probes no longer crowd real messages out of the outbox.** A + session the peer never confirms is probed every five seconds, and each probe + used to enter the outbox with its own retry ladder on top of the unanswered + ones before it. Only a relay `unreachable` verdict backed that off, so a + relay that pushes to offline users, or any mesh-only link, never did. An + account with ten such contacts stacked about two probes a second, and once + the outbox hit its 500-entry cap, eviction failed the user's own messages + with `Outbox capacity exceeded`. Seen on a device test between a Samsung and + an iPhone, offline over BLE. Each new probe now supersedes the last, quietly + and without counting against the carrier, so a peer holds one probe in the + outbox and is still probed on the same cadence. + - **An old message no longer comes back as a push notification, again and again.** A direct message the relay had pushed stayed in the outbox well past a week, because each probe re-send refreshed its lifetime and so did each diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index 041abaae..e5c21e0b 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -709,6 +709,15 @@ pub struct OfflineProtocol { /// flap the connection. Reset on a reachability edge (`on_peer_presence`). confirmation_probe_unreachable_parks: HashMap, + /// The confirmation probe each pending peer last sent. At most one probe + /// per peer lives in the outbox: a new one supersedes the last. Stacking a + /// fresh probe (new id, full retry ladder) every + /// `CONFIRMATION_PROBE_INTERVAL_SECS` on top of the unanswered ones filled + /// the outbox whenever no `unreachable` verdict arrived to back the + /// schedule off: a relay that pushes to offline users, or a mesh-only + /// carrier. Capacity eviction then failed real messages. + confirmation_probe_outstanding: HashMap, + /// Token bucket that caps how fast the RETRY/PROBE path /// (`process_retry_queue`) re-sends, so a large backlog of resends to /// unreachable peers cannot burst past the relay's per-connection rate limit @@ -1176,6 +1185,7 @@ impl OfflineProtocol { confirmation_retry_due_at: HashMap::new(), confirmation_probe_due_at: HashMap::new(), confirmation_probe_unreachable_parks: HashMap::new(), + confirmation_probe_outstanding: HashMap::new(), retry_drain_tokens: 0.0, retry_drain_last: None, rekey_due_at: HashMap::new(), diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 6537286c..28427bb3 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -449,6 +449,7 @@ impl OfflineProtocol { self.confirmation_retry_due_at.remove(peer_id); self.confirmation_probe_due_at.remove(peer_id); self.confirmation_probe_unreachable_parks.remove(peer_id); + self.confirmation_probe_outstanding.remove(peer_id); } /// Fold a relay "recipient unreachable" verdict into the session-confirmation @@ -517,7 +518,9 @@ impl OfflineProtocol { internal_prefixes::SESSION_CONFIRM_PROBE.to_string(), MessagePriority::High, ) { - Ok(_) => { + Ok(message_id) => { + self.confirmation_probe_outstanding + .insert(peer_id.to_string(), message_id); info!( event = "session_confirmation_probe_sent", session_or_group_id = %peer_id, @@ -596,6 +599,8 @@ impl OfflineProtocol { .retain(|peer, _| pending_set.contains(peer)); self.confirmation_probe_unreachable_parks .retain(|peer, _| pending_set.contains(peer)); + self.confirmation_probe_outstanding + .retain(|peer, _| pending_set.contains(peer)); for peer_id in pending_peers { let due_at = self @@ -606,6 +611,14 @@ impl OfflineProtocol { if due_at > now { continue; } + // Superseded, not failed: the new probe asks the same question, so + // the old one's retry ladder would only stack onto it. Silent, and + // no delivery failure is recorded against the carrier. + if let Some(previous) = self.confirmation_probe_outstanding.remove(&peer_id) { + self.retry_queue.remove(&previous.as_str()); + self.ack_manager.remove_ack(&previous); + self.remove_outbox_entry(&previous); + } self.send_session_confirmation_probe(&peer_id, source_event); self.confirmation_probe_due_at.insert( diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 330b78bd..dd2bc80c 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40679,6 +40679,87 @@ fn test_confirmation_probe_unreachable_escalation() { ); } +/// A peer whose session never confirms, and that no `unreachable` verdict +/// backs off (a relay that pushes to offline users, or a mesh-only link), +/// holds one probe in the outbox, however many intervals pass. Each due scan +/// still probes, so a lost probe is replaced on the next one, but the new +/// probe supersedes the last instead of stacking a fresh retry ladder on it: +/// stacking filled the outbox at one probe per such peer per 5s, and capacity +/// eviction failed real messages. +#[test] +fn an_unanswered_confirmation_probe_supersedes_the_last_one() { + let mut alice_config = create_test_config_for_user("alice"); + alice_config.encryption.enabled = true; + let mut alice = OfflineProtocol::new(alice_config).unwrap(); + let mut bob = OfflineProtocol::new(create_test_config_for_user("bob")).unwrap(); + alice + .initialize_mls_for_test(Arc::new(InMemoryStorage::new())) + .unwrap(); + bob.initialize_mls_for_test(Arc::new(InMemoryStorage::new())) + .unwrap(); + + // A session alice holds but bob never confirms. + let bob_key_package = { + let manager = bob.mls_manager.as_ref().unwrap().read().unwrap(); + manager.get_or_create_key_package().unwrap() + }; + { + let manager = alice.mls_manager.as_ref().unwrap().read().unwrap(); + manager + .import_key_package(&id("bob"), &bob_key_package.key_package_data) + .unwrap(); + manager.create_session(&id("bob")).unwrap(); + } + alice + .ensure_session_state_entry(&id("bob"), "test_setup") + .unwrap(); + + let transport = MockTransport::new(TransportType::BLE); + transport.start().unwrap(); + let handle = transport.clone(); + alice + .transport_manager_mut() + .add_transport(TransportType::BLE, Box::new(transport)); + alice.start().unwrap(); + + let probes = |handle: &MockTransport| -> Vec { + handle + .sent_messages() + .into_iter() + .filter(|m| { + m.content + .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) + }) + .map(|m| m.id) + .collect() + }; + let make_due = |alice: &mut OfflineProtocol| { + alice.confirmation_probe_due_at.insert( + id("bob").to_string(), + Utc::now() - ChronoDuration::seconds(1), + ); + }; + + handle.clear_sent_messages(); + for _ in 0..6 { + make_due(&mut alice); + alice.kick_pending_session_reconciliation("test"); + } + let sent = probes(&handle); + assert_eq!(sent.len(), 6, "every due scan still probes"); + + let outstanding: Vec<_> = alice.outbox.keys().filter(|id| sent.contains(id)).collect(); + assert_eq!( + outstanding, + vec![sent.last().unwrap()], + "only the latest probe waits in the outbox; each superseded the last" + ); + assert!( + !alice.retry_queue.contains(&sent[0].as_str()), + "a superseded probe keeps no retry ladder" + ); +} + /// Pins component (D) of the flap fix: the opt-in edge-driven parking gate. /// With `edge_driven_unreachable_dm` on, a durably-unreachable DM keeps being /// timed-probed up to `DM_UNREACHABLE_PROBE_LIMIT` times, then stops (rests in diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 7847c659..8cf716ea 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -115,6 +115,14 @@ top so that the one side no other trigger can reach, the both-create owner, is still covered. Only that owner, waiting on the adopt path, depends on decrypt alone. +An unconfirmed session is probed on a fixed cadence, and **a peer holds at most +one probe in the outbox**: each new probe supersedes the last rather than +stacking its own retry ladder on top. The cadence only backs off on a relay +`unreachable` verdict, which a relay that pushes to offline users or a mesh-only +link never produces, so stacked probes once filled the outbox and capacity +eviction failed real messages. Superseding is not a delivery failure and is not +counted against the carrier. + The drain is downstream of the confirmation **transition**, not of decryption as such: a decrypt on a session that is already confirmed does not re-run it. From 44ba62e508cf66843e5e1eb619cc8a73ebc62fa2 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:08:20 +0400 Subject: [PATCH 02/15] chore(docs): update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76943101..885d8bba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -387,7 +387,7 @@ archived by series under [docs/changelog/](docs/changelog/); see the relay that pushes to offline users, or any mesh-only link, never did. An account with ten such contacts stacked about two probes a second, and once the outbox hit its 500-entry cap, eviction failed the user's own messages - with `Outbox capacity exceeded`. Seen on a device test between a Samsung and + with `Outbox capacity exceeded`. Seen on a device test between an Android and an iPhone, offline over BLE. Each new probe now supersedes the last, quietly and without counting against the carrier, so a peer holds one probe in the outbox and is still probed on the same cadence. From ae4d4f5dbdd0df2f9fbf067ad3a1caab29598068 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:23:04 +0400 Subject: [PATCH 03/15] fix(protocol): every confirmation probe supersedes, and none is restored Move the supersede into send_session_confirmation_probe so the Welcome fast path's probe is covered too, share the retry/ack/outbox teardown with retire_undeliverable_message via forget_outbound_message, and drop persisted probes at restore, where no later probe could supersede them. --- CHANGELOG.md | 6 +- crates/offline-protocol/src/protocol/send.rs | 18 +++++- .../offline-protocol/src/protocol/session.rs | 17 +++--- .../offline-protocol/src/protocol/storage.rs | 18 +++++- .../src/protocol/tests/mod.rs | 61 ++++++++++++++++--- docs/state-machines/outbox-and-retries.md | 5 +- docs/state-machines/session-lifecycle.md | 25 +++++--- 7 files changed, 118 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 885d8bba..500767e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -389,8 +389,10 @@ archived by series under [docs/changelog/](docs/changelog/); see the the outbox hit its 500-entry cap, eviction failed the user's own messages with `Outbox capacity exceeded`. Seen on a device test between an Android and an iPhone, offline over BLE. Each new probe now supersedes the last, quietly - and without counting against the carrier, so a peer holds one probe in the - outbox and is still probed on the same cadence. + and without counting against the carrier, whether the periodic scan or the + Welcome fast path sent it, and a probe persisted before a restart is not + restored. A peer holds one probe in the outbox and is still probed on the + same cadence. - **A key package refused by this device's clock is reported.** A peer's key package is valid from an hour before it was minted until 30 days after, diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 9ae73ef1..5837d659 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -3283,6 +3283,20 @@ impl OfflineProtocol { self.media_outbox.remove(message_id) } + /// Takes `message_id` out of the three places that can send it again: + /// the retry queue, the pending-ACK tracker and the outbox. One without + /// the others resurrects it (see [`Self::retire_undeliverable_message`]). + /// No failure is recorded against the carrier: that is the give-up + /// path's, and a message withdrawn for being superseded did not fail. + pub(super) fn forget_outbound_message( + &mut self, + message_id: &MessageId, + ) -> Option { + self.retry_queue.remove(&message_id.as_str()); + self.ack_manager.remove_ack(message_id); + self.remove_outbox_entry(message_id) + } + /// Takes a message this device has given up on out of everything that /// could send it again, returning its outbox entry if it still had one. /// @@ -3307,10 +3321,8 @@ impl OfflineProtocol { message_id: &MessageId, media_reason: &str, ) -> Option { - self.retry_queue.remove(&message_id.as_str()); - self.ack_manager.remove_ack(message_id); self.handle_outbound_media_chunk_failed(message_id, media_reason); - let entry = self.remove_outbox_entry(message_id); + let entry = self.forget_outbound_message(message_id); if let Some(transport) = entry.as_ref().and_then(|entry| entry.last_transport) { self.transport_manager.record_delivery_failure(transport); } diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 28427bb3..b29fbc11 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -513,6 +513,14 @@ impl OfflineProtocol { } pub(super) fn send_session_confirmation_probe(&mut self, peer_id: &str, source_event: &str) { + // Superseded, not failed: the new probe asks the same question, so + // the old one's retry ladder would only stack onto it. Here rather + // than in the scan because the Welcome fast path sends one too, and + // a probe it replaced would keep its full ladder. Silent, and no + // delivery failure is recorded against the carrier. + if let Some(previous) = self.confirmation_probe_outstanding.remove(peer_id) { + self.forget_outbound_message(&previous); + } match self.send_internal_message( peer_id, internal_prefixes::SESSION_CONFIRM_PROBE.to_string(), @@ -611,15 +619,6 @@ impl OfflineProtocol { if due_at > now { continue; } - // Superseded, not failed: the new probe asks the same question, so - // the old one's retry ladder would only stack onto it. Silent, and - // no delivery failure is recorded against the carrier. - if let Some(previous) = self.confirmation_probe_outstanding.remove(&peer_id) { - self.retry_queue.remove(&previous.as_str()); - self.ack_manager.remove_ack(&previous); - self.remove_outbox_entry(&previous); - } - self.send_session_confirmation_probe(&peer_id, source_event); self.confirmation_probe_due_at.insert( peer_id, diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index 4cc9f112..beb76fad 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -2,8 +2,8 @@ use super::state_crypto::{StateRecordCipher, SEALED_RECORD_OVERHEAD, STATE_RECORD_KEY_BYTES}; use super::{ - lifetime_expired, storage_keys, MediaTransferDescriptor, OfflineProtocol, OutboxEntry, - PeerCapabilities, PendingDecryptRecord, PendingMessage, PendingMessageRecord, + internal_prefixes, lifetime_expired, storage_keys, MediaTransferDescriptor, OfflineProtocol, + OutboxEntry, PeerCapabilities, PendingDecryptRecord, PendingMessage, PendingMessageRecord, ReceivedKeyPackage, SessionState, WelcomeDeliveryState, WelcomeLifecycleRecord, DATA_CUSTODY_V1, DATA_GROUP_BLOB_V1, DATA_GROUP_V1, DATA_INTEREST_V1, DATA_MEDIA_V1, DATA_SYNC_V1, DATA_TOMBSTONE_V1, MAX_BLOCKED_USERS, MAX_KEY_PACKAGE_SENT_TO, @@ -3997,6 +3997,20 @@ impl OfflineProtocol { continue; } + // A confirmation probe carries no user data, and the one sent + // before the restart is not in `confirmation_probe_outstanding`, + // so the next probe could not supersede it and it would keep a + // full retry ladder. The next scan asks the same question. + if entry + .message + .content + .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) + { + budget.claim(); + self.delete_outbox_key(&message_id); + continue; + } + restored.push(entry); } diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index ed5164c7..5c1adbb4 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40965,13 +40965,19 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { ); }; + // The scan and the Welcome fast path (`on_transport_send_confirmed`) + // both probe, interleaved: either may supersede the other's probe. handle.clear_sent_messages(); - for _ in 0..6 { - make_due(&mut alice); - alice.kick_pending_session_reconciliation("test"); + for round in 0..6 { + if round % 2 == 0 { + make_due(&mut alice); + alice.kick_pending_session_reconciliation("test"); + } else { + alice.send_session_confirmation_probe(&id("bob").to_string(), "transport_confirmed"); + } } let sent = probes(&handle); - assert_eq!(sent.len(), 6, "every due scan still probes"); + assert_eq!(sent.len(), 6, "every due scan and fast path still probes"); let outstanding: Vec<_> = alice.outbox.keys().filter(|id| sent.contains(id)).collect(); assert_eq!( @@ -40979,9 +40985,50 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { vec![sent.last().unwrap()], "only the latest probe waits in the outbox; each superseded the last" ); - assert!( - !alice.retry_queue.contains(&sent[0].as_str()), - "a superseded probe keeps no retry ladder" + for superseded in &sent[..sent.len() - 1] { + assert!( + !alice.retry_queue.contains(&superseded.as_str()), + "a superseded probe keeps no retry ladder" + ); + assert!( + !alice.ack_manager.is_waiting_for_ack(superseded), + "a superseded probe's ACK timeout would re-queue it" + ); + } +} + +/// A probe persisted before a restart is not in +/// `confirmation_probe_outstanding`, so no later probe could supersede it. +/// Restore drops it: it carries no user data, and the next scan asks again. +#[test] +fn a_persisted_confirmation_probe_is_not_restored() { + let storage = Arc::new(InMemoryStorage::new()); + let entry = |message: Message| OutboxEntry { + message, + attempt_count: 1, + first_sent_at: chrono::Utc::now(), + last_sent_at: chrono::Utc::now(), + last_transport: None, + reseal: None, + relay_pushed: false, + }; + let probe = test_message("bob", internal_prefixes::SESSION_CONFIRM_PROBE); + let real = test_message("bob", "hello"); + let real_id = real.id.clone(); + store_outbox_entry(&storage, &entry(probe)); + store_outbox_entry(&storage, &entry(real)); + + let mut protocol = OfflineProtocol::new(create_test_config()).unwrap(); + protocol + .enable_message_persistence_for_test(storage.clone()) + .unwrap(); + + let restored: Vec<_> = protocol.outbox_messages().map(|m| m.id.clone()).collect(); + assert_eq!(restored, vec![real_id.clone()]); + assert_eq!( + storage.list_keys(storage_keys::OUTBOX).unwrap(), + vec![real_id.as_str().to_string()], + "the probe's record is deleted, not left to be restored next launch" ); } diff --git a/docs/state-machines/outbox-and-retries.md b/docs/state-machines/outbox-and-retries.md index 417e6060..294928ef 100644 --- a/docs/state-machines/outbox-and-retries.md +++ b/docs/state-machines/outbox-and-retries.md @@ -92,7 +92,10 @@ doing. the outbox together.** Capacity eviction, lifetime expiry, retry exhaustion and an acknowledgement timeout that finds no outbox entry all go through one function (`retire_undeliverable_message`) for that reason, and the event each -one owes the application is emitted by the path itself. +one owes the application is emitted by the path itself. A session confirmation +probe superseded by a newer one leaves through the same three-way teardown +(`forget_outbound_message`), without the delivery failure a give-up records +against the carrier: it did not fail, it was replaced. Removing the outbox entry alone is not enough, because the retry and flush paths re-create a missing entry before they resend, with fresh timestamps. An id left diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 8cf716ea..f01d7ee4 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -93,6 +93,23 @@ re-enters the desync path. Repeating the reset flag would tear down a replacement session whose Welcome is in flight, so it belongs with the split-session work rather than here. +### An unconfirmed session holds one probe + +An unconfirmed session is probed on a fixed cadence, and **a peer holds at most +one probe in the outbox**: each new probe supersedes the last rather than +stacking its own retry ladder on top. The cadence only backs off on a relay +`unreachable` verdict, which a relay that pushes to offline users or a mesh-only +link never produces, so stacked probes once filled the outbox and capacity +eviction failed real messages. + +The supersede lives in the one function that sends a probe, because two paths +call it: the periodic scan and the fast path taken when the transport confirms +a Welcome went out. A supersede in the scan alone left the fast path's probe +with its full ladder. Superseding is not a delivery failure and is not counted +against the carrier. A probe is not restored after a restart either: the new +process does not know which probe was the last, so a restored one could never +be superseded, and the next scan asks the same question. + ### Both-create convergence Both peers can create a session simultaneously. The tiebreaker orders the two @@ -115,14 +132,6 @@ top so that the one side no other trigger can reach, the both-create owner, is still covered. Only that owner, waiting on the adopt path, depends on decrypt alone. -An unconfirmed session is probed on a fixed cadence, and **a peer holds at most -one probe in the outbox**: each new probe supersedes the last rather than -stacking its own retry ladder on top. The cadence only backs off on a relay -`unreachable` verdict, which a relay that pushes to offline users or a mesh-only -link never produces, so stacked probes once filled the outbox and capacity -eviction failed real messages. Superseding is not a delivery failure and is not -counted against the carrier. - The drain is downstream of the confirmation **transition**, not of decryption as such: a decrypt on a session that is already confirmed does not re-run it. From 734f91d3d8a742c5de71e46682419add23e76f99 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:29:28 +0400 Subject: [PATCH 04/15] fix(protocol): withdraw the outstanding probe with the confirmation tracking Confirming or tearing down a session dropped the probe's map entry but left the probe queued with its full retry ladder, where nothing could supersede it. The test now also pins that superseding records no carrier failure and emits no message_failed. --- .../offline-protocol/src/protocol/session.rs | 27 ++++++++++++++++--- .../src/protocol/tests/mod.rs | 27 +++++++++++++++++++ .../offline-protocol/src/transport_manager.rs | 7 +++++ docs/state-machines/session-lifecycle.md | 5 +++- 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index b29fbc11..0514b63d 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -449,7 +449,11 @@ impl OfflineProtocol { self.confirmation_retry_due_at.remove(peer_id); self.confirmation_probe_due_at.remove(peer_id); self.confirmation_probe_unreachable_parks.remove(peer_id); - self.confirmation_probe_outstanding.remove(peer_id); + // Withdrawn with the tracking: once the map forgets it, no later + // probe can supersede it, and it would keep its full retry ladder. + if let Some(probe) = self.confirmation_probe_outstanding.remove(peer_id) { + self.forget_outbound_message(&probe); + } } /// Fold a relay "recipient unreachable" verdict into the session-confirmation @@ -517,7 +521,10 @@ impl OfflineProtocol { // the old one's retry ladder would only stack onto it. Here rather // than in the scan because the Welcome fast path sends one too, and // a probe it replaced would keep its full ladder. Silent, and no - // delivery failure is recorded against the carrier. + // delivery failure is recorded against the carrier. Before the send, + // not after: if the send errors the peer has no probe in flight + // until the next scan, but a new probe can never evict a message at + // outbox capacity to make room beside the one it replaces. if let Some(previous) = self.confirmation_probe_outstanding.remove(peer_id) { self.forget_outbound_message(&previous); } @@ -607,8 +614,20 @@ impl OfflineProtocol { .retain(|peer, _| pending_set.contains(peer)); self.confirmation_probe_unreachable_parks .retain(|peer, _| pending_set.contains(peer)); - self.confirmation_probe_outstanding - .retain(|peer, _| pending_set.contains(peer)); + // Not a plain `retain`: a probe whose peer left the pending set + // without passing through `clear_confirmation_recovery_tracking` + // would be forgotten while still queued. + let settled: Vec = self + .confirmation_probe_outstanding + .keys() + .filter(|peer| !pending_set.contains(*peer)) + .cloned() + .collect(); + for peer in settled { + if let Some(probe) = self.confirmation_probe_outstanding.remove(&peer) { + self.forget_outbound_message(&probe); + } + } for peer_id in pending_peers { let due_at = self diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 5c1adbb4..ce5323f1 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40965,6 +40965,10 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { ); }; + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let events_handle = Arc::clone(&events); + alice.on_event(move |event| events_handle.lock().unwrap().push(event)); + // The scan and the Welcome fast path (`on_transport_send_confirmed`) // both probe, interleaved: either may supersede the other's probe. handle.clear_sent_messages(); @@ -40995,6 +40999,29 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { "a superseded probe's ACK timeout would re-queue it" ); } + assert_eq!( + alice + .transport_manager + .observed_failure_count(TransportType::BLE), + 0, + "superseding is not a delivery failure against the carrier" + ); + assert!( + !events + .lock() + .unwrap() + .iter() + .any(|e| matches!(e, Event::MessageFailed { .. })), + "a superseded probe is withdrawn silently" + ); + + // Confirming the session withdraws the last probe too: with the + // tracking gone, nothing could ever supersede it. + let last = sent.last().unwrap().clone(); + alice.clear_confirmation_recovery_tracking(&id("bob").to_string()); + assert!(!alice.outbox.contains_key(&last)); + assert!(!alice.retry_queue.contains(&last.as_str())); + assert!(!alice.ack_manager.is_waiting_for_ack(&last)); } /// A probe persisted before a restart is not in diff --git a/crates/offline-protocol/src/transport_manager.rs b/crates/offline-protocol/src/transport_manager.rs index 054fb1a1..a64fc6c4 100644 --- a/crates/offline-protocol/src/transport_manager.rs +++ b/crates/offline-protocol/src/transport_manager.rs @@ -1506,6 +1506,13 @@ impl TransportManager { stats.record_failure(); } + #[cfg(test)] + pub(crate) fn observed_failure_count(&self, transport_type: TransportType) -> u32 { + self.observations + .get(&transport_type) + .map_or(0, |stats| stats.failure_count) + } + /// Updates the DORS selector configuration at runtime, preserving /// accumulated state (transport history, retry counts, signal tracking). pub fn update_selector_config(&mut self, config: DorsConfig) { diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index f01d7ee4..b8da61a3 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -108,7 +108,10 @@ a Welcome went out. A supersede in the scan alone left the fast path's probe with its full ladder. Superseding is not a delivery failure and is not counted against the carrier. A probe is not restored after a restart either: the new process does not know which probe was the last, so a restored one could never -be superseded, and the next scan asks the same question. +be superseded, and the next scan asks the same question. For the same reason +a probe is withdrawn with the rest of the confirmation tracking when the +session confirms or is torn down: a probe the tracking no longer names could +never be superseded, and would keep its ladder. ### Both-create convergence From de77122964200172ac620932f6a838c17bf36990 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:37:26 +0400 Subject: [PATCH 05/15] fix(protocol): keep confirmation probes out of persistent storage Restore already discards every probe, so writing one per send and deleting it on each supersede was secure-storage I/O under the protocol lock for nothing. Also let the throttled scan run while a probe is outstanding, so a fast-path probe is superseded or pruned, and pin the pending-set prune in the test. --- CHANGELOG.md | 6 ++--- crates/offline-protocol/src/protocol/send.rs | 15 +++++++++++- .../offline-protocol/src/protocol/session.rs | 1 + .../offline-protocol/src/protocol/storage.rs | 24 +++++++++---------- .../src/protocol/tests/mod.rs | 19 +++++++++++++++ docs/state-machines/session-lifecycle.md | 7 +++--- 6 files changed, 53 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 500767e9..b3d5d21c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -390,9 +390,9 @@ archived by series under [docs/changelog/](docs/changelog/); see the with `Outbox capacity exceeded`. Seen on a device test between an Android and an iPhone, offline over BLE. Each new probe now supersedes the last, quietly and without counting against the carrier, whether the periodic scan or the - Welcome fast path sent it, and a probe persisted before a restart is not - restored. A peer holds one probe in the outbox and is still probed on the - same cadence. + Welcome fast path sent it. Probes are no longer written to storage, and any + an older build wrote are dropped at restore. A peer holds one probe in the + outbox and is still probed on the same cadence. - **A key package refused by this device's clock is reported.** A peer's key package is valid from an hour before it was minted until 30 days after, diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 5837d659..0768e134 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -3277,7 +3277,9 @@ impl OfflineProtocol { // nothing, and keeping it would only grow the map. self.custody_receipts.remove(message_id); if let Some(entry) = self.outbox.remove(message_id) { - self.clear_outbox_entry_from_storage(message_id); + if !Self::is_confirmation_probe(&entry.message) { + self.clear_outbox_entry_from_storage(message_id); + } return Some(entry); } self.media_outbox.remove(message_id) @@ -3546,6 +3548,17 @@ impl OfflineProtocol { message.content_type == ContentType::FileChunk } + /// 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 + /// same question. Persisting it cost a secure-storage write per probe and + /// a delete per supersede, under the protocol lock. + pub(super) fn is_confirmation_probe(message: &Message) -> bool { + message + .content + .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) + } + // ======================================================================== // DELIVERY TRACKING // ======================================================================== diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 0514b63d..6060c3a5 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -566,6 +566,7 @@ impl OfflineProtocol { // Fast path: nothing pending → skip entirely (no storage I/O) let has_pending_work = !self.pending_encrypted_messages.is_empty() || !self.confirmation_probe_due_at.is_empty() + || !self.confirmation_probe_outstanding.is_empty() || !self.confirmation_retry_due_at.is_empty(); if !has_pending_work { diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index beb76fad..c95c3959 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -2,8 +2,8 @@ use super::state_crypto::{StateRecordCipher, SEALED_RECORD_OVERHEAD, STATE_RECORD_KEY_BYTES}; use super::{ - internal_prefixes, lifetime_expired, storage_keys, MediaTransferDescriptor, OfflineProtocol, - OutboxEntry, PeerCapabilities, PendingDecryptRecord, PendingMessage, PendingMessageRecord, + lifetime_expired, storage_keys, MediaTransferDescriptor, OfflineProtocol, OutboxEntry, + PeerCapabilities, PendingDecryptRecord, PendingMessage, PendingMessageRecord, ReceivedKeyPackage, SessionState, WelcomeDeliveryState, WelcomeLifecycleRecord, DATA_CUSTODY_V1, DATA_GROUP_BLOB_V1, DATA_GROUP_V1, DATA_INTEREST_V1, DATA_MEDIA_V1, DATA_SYNC_V1, DATA_TOMBSTONE_V1, MAX_BLOCKED_USERS, MAX_KEY_PACKAGE_SENT_TO, @@ -3800,7 +3800,9 @@ impl OfflineProtocol { let Some(storage) = &self.protocol_state_storage else { return; }; - if Self::is_media_outbox_message(&entry.message) { + if Self::is_media_outbox_message(&entry.message) + || Self::is_confirmation_probe(&entry.message) + { return; } match serde_json::to_vec(entry) { @@ -3997,15 +3999,13 @@ impl OfflineProtocol { continue; } - // A confirmation probe carries no user data, and the one sent - // before the restart is not in `confirmation_probe_outstanding`, - // so the next probe could not supersede it and it would keep a - // full retry ladder. The next scan asks the same question. - if entry - .message - .content - .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) - { + // Probes are no longer persisted (`is_confirmation_probe`); drop + // any an older build wrote. Charged to the pool like any delete: + // the first launch after upgrading from the build that stacked + // them can spend most of it here, deferring the expiry and + // capacity prunes below to the next launch. That is the bound + // working, not a reason to make this delete unbudgeted. + if Self::is_confirmation_probe(&entry.message) { budget.claim(); self.delete_outbox_key(&message_id); continue; diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index ce5323f1..288ee5fb 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41022,6 +41022,20 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { assert!(!alice.outbox.contains_key(&last)); assert!(!alice.retry_queue.contains(&last.as_str())); assert!(!alice.ack_manager.is_waiting_for_ack(&last)); + + // A peer that leaves the pending set without that call (its session + // deleted under the engine) has its probe withdrawn by the next scan. + alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); + let orphan = probes(&handle).last().unwrap().clone(); + assert!(alice.outbox.contains_key(&orphan)); + { + let manager = alice.mls_manager.as_ref().unwrap().read().unwrap(); + manager.delete_session(&id("bob")).unwrap(); + } + alice.kick_pending_session_reconciliation("test"); + assert!(!alice.outbox.contains_key(&orphan)); + assert!(!alice.retry_queue.contains(&orphan.as_str())); + assert!(!alice.ack_manager.is_waiting_for_ack(&orphan)); } /// A probe persisted before a restart is not in @@ -41057,6 +41071,11 @@ fn a_persisted_confirmation_probe_is_not_restored() { vec![real_id.as_str().to_string()], "the probe's record is deleted, not left to be restored next launch" ); + + // Nor is a probe written in the first place. + let probe = test_message("bob", internal_prefixes::SESSION_CONFIRM_PROBE); + protocol.persist_outbox_entry(&entry(probe)); + assert_eq!(storage.list_keys(storage_keys::OUTBOX).unwrap().len(), 1); } /// Pins component (D) of the flap fix: the opt-in edge-driven parking gate. diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index b8da61a3..29c2dc0c 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -106,9 +106,10 @@ The supersede lives in the one function that sends a probe, because two paths call it: the periodic scan and the fast path taken when the transport confirms a Welcome went out. A supersede in the scan alone left the fast path's probe with its full ladder. Superseding is not a delivery failure and is not counted -against the carrier. A probe is not restored after a restart either: the new -process does not know which probe was the last, so a restored one could never -be superseded, and the next scan asks the same question. For the same reason +against the carrier. A probe is not persisted either, and one an older build +persisted is dropped at restore: the new process does not know which probe was +the last, so a restored one could never be superseded, and the next scan asks +the same question. For the same reason a probe is withdrawn with the rest of the confirmation tracking when the session confirms or is torn down: a probe the tracking no longer names could never be superseded, and would keep its ladder. From 2691dcb0f5b18677333ed0223d954a2681f10453 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:37:37 +0400 Subject: [PATCH 06/15] docs(protocol): rewrap the probe section --- docs/state-machines/session-lifecycle.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 29c2dc0c..bab7812e 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -109,10 +109,10 @@ with its full ladder. Superseding is not a delivery failure and is not counted against the carrier. A probe is not persisted either, and one an older build persisted is dropped at restore: the new process does not know which probe was the last, so a restored one could never be superseded, and the next scan asks -the same question. For the same reason -a probe is withdrawn with the rest of the confirmation tracking when the -session confirms or is torn down: a probe the tracking no longer names could -never be superseded, and would keep its ladder. +the same question. For the same reason a probe is withdrawn with the rest of +the confirmation tracking when the session confirms or is torn down: a probe +the tracking no longer names could never be superseded, and would keep its +ladder. ### Both-create convergence From e96f9848ec6608962677a9e3b711e60dbec5c988 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:45:37 +0400 Subject: [PATCH 07/15] docs(protocol): a relay_pushed verdict backs probes off too The docs said only an unreachable verdict backs the probe cadence off, but park_relay_pushed_dm escalates it as well; the case that never backs off is a mesh-only carrier. Also pin that a supersede clears a deferred probe's retry entry, and note that a superseded probe's verdict is dropped. --- CHANGELOG.md | 13 ++++++------- crates/offline-protocol/src/protocol/mod.rs | 4 ++-- crates/offline-protocol/src/protocol/session.rs | 4 +++- crates/offline-protocol/src/protocol/tests/mod.rs | 12 +++++++++++- docs/state-machines/session-lifecycle.md | 6 +++--- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3d5d21c..cb845eae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -383,14 +383,13 @@ archived by series under [docs/changelog/](docs/changelog/); see the - **Confirmation probes no longer crowd real messages out of the outbox.** A session the peer never confirms is probed every five seconds, and each probe used to enter the outbox with its own retry ladder on top of the unanswered - ones before it. Only a relay `unreachable` verdict backed that off, so a - relay that pushes to offline users, or any mesh-only link, never did. An - account with ten such contacts stacked about two probes a second, and once - the outbox hit its 500-entry cap, eviction failed the user's own messages - with `Outbox capacity exceeded`. Seen on a device test between an Android and - an iPhone, offline over BLE. Each new probe now supersedes the last, quietly + ones before it. Only a relay verdict backed that off, so a mesh-only link + never did. An account with ten such contacts stacked about two probes a + second, and once the outbox hit its 500-entry cap, eviction failed the user's + own messages with `Outbox capacity exceeded`. Seen on a device test between + an Android and an iPhone, offline over BLE. Each new probe now supersedes the last, quietly and without counting against the carrier, whether the periodic scan or the - Welcome fast path sent it. Probes are no longer written to storage, and any + Welcome fast path sent it. Probes are no longer written to storage, and those an older build wrote are dropped at restore. A peer holds one probe in the outbox and is still probed on the same cadence. diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index 95cff50c..dc8117f6 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -713,8 +713,8 @@ pub struct OfflineProtocol { /// per peer lives in the outbox: a new one supersedes the last. Stacking a /// fresh probe (new id, full retry ladder) every /// `CONFIRMATION_PROBE_INTERVAL_SECS` on top of the unanswered ones filled - /// the outbox whenever no `unreachable` verdict arrived to back the - /// schedule off: a relay that pushes to offline users, or a mesh-only + /// the outbox whenever no relay verdict (`unreachable` or + /// `relay_pushed`) arrived to back the schedule off, as on a mesh-only /// carrier. Capacity eviction then failed real messages. confirmation_probe_outstanding: HashMap, diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 6060c3a5..b429670c 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -524,7 +524,9 @@ impl OfflineProtocol { // delivery failure is recorded against the carrier. Before the send, // not after: if the send errors the peer has no probe in flight // until the next scan, but a new probe can never evict a message at - // outbox capacity to make room beside the one it replaces. + // outbox capacity to make room beside the one it replaces. A relay + // verdict that arrives for a superseded probe finds no entry and is + // dropped; the current probe's own verdict backs the schedule off. if let Some(previous) = self.confirmation_probe_outstanding.remove(peer_id) { self.forget_outbound_message(&previous); } diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 288ee5fb..051a3a0e 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41015,9 +41015,19 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { "a superseded probe is withdrawn silently" ); + // A probe whose send failed waits in the retry queue; the next probe + // takes it out of there too. + handle.set_fail_next_sends(1); + alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); + let deferred = alice.confirmation_probe_outstanding[&id("bob").to_string()].clone(); + assert!(alice.retry_queue.contains(&deferred.as_str())); + alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); + assert!(!alice.retry_queue.contains(&deferred.as_str())); + assert!(!alice.outbox.contains_key(&deferred)); + // Confirming the session withdraws the last probe too: with the // tracking gone, nothing could ever supersede it. - let last = sent.last().unwrap().clone(); + let last = alice.confirmation_probe_outstanding[&id("bob").to_string()].clone(); alice.clear_confirmation_recovery_tracking(&id("bob").to_string()); assert!(!alice.outbox.contains_key(&last)); assert!(!alice.retry_queue.contains(&last.as_str())); diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index bab7812e..40a3e978 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -98,9 +98,9 @@ split-session work rather than here. An unconfirmed session is probed on a fixed cadence, and **a peer holds at most one probe in the outbox**: each new probe supersedes the last rather than stacking its own retry ladder on top. The cadence only backs off on a relay -`unreachable` verdict, which a relay that pushes to offline users or a mesh-only -link never produces, so stacked probes once filled the outbox and capacity -eviction failed real messages. +verdict (`unreachable` or `relay_pushed`), which a mesh-only carrier never +produces, so stacked probes once filled the outbox and capacity eviction failed +real messages. The supersede lives in the one function that sends a probe, because two paths call it: the periodic scan and the fast path taken when the transport confirms From e23b24f9471c081f2a0c525442f3bccf0efbdf61 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 13:54:36 +0400 Subject: [PATCH 08/15] fix(protocol): clear a probe whose send failed after it was queued A full ACK tracker fails send_internal_message after the probe entered the outbox, so no id came back to supersede it next time. The failed send now withdraws any probe still queued for the peer. The probe sender also stamps the cadence, so the scan no longer supersedes a fast-path probe within two seconds, and restore logs how many legacy probes it dropped. --- CHANGELOG.md | 10 +++---- .../offline-protocol/src/protocol/session.rs | 29 +++++++++++++++--- .../offline-protocol/src/protocol/storage.rs | 8 +++++ .../src/protocol/tests/mod.rs | 30 +++++++++++++++++++ docs/state-machines/session-lifecycle.md | 3 +- 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb845eae..aafbf71e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -387,11 +387,11 @@ archived by series under [docs/changelog/](docs/changelog/); see the never did. An account with ten such contacts stacked about two probes a second, and once the outbox hit its 500-entry cap, eviction failed the user's own messages with `Outbox capacity exceeded`. Seen on a device test between - an Android and an iPhone, offline over BLE. Each new probe now supersedes the last, quietly - and without counting against the carrier, whether the periodic scan or the - Welcome fast path sent it. Probes are no longer written to storage, and those - an older build wrote are dropped at restore. A peer holds one probe in the - outbox and is still probed on the same cadence. + an Android and an iPhone, offline over BLE. Each new probe now supersedes + the last, quietly and without counting against the carrier, whether the + periodic scan or the Welcome fast path sent it. Probes are no longer written + to storage, and those an older build wrote are dropped at restore. A peer + holds one probe in the outbox and is still probed on the same cadence. - **A key package refused by this device's clock is reported.** A peer's key package is valid from an hour before it was minted until 30 days after, diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index b429670c..33c51f72 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -530,6 +530,15 @@ impl OfflineProtocol { if let Some(previous) = self.confirmation_probe_outstanding.remove(peer_id) { self.forget_outbound_message(&previous); } + // The sender owns the cadence, so a fast-path probe is not superseded + // by the next scan before its ACK can arrive, and a relay verdict for + // it can back the schedule off (`note_confirmation_probe_unreachable` + // only escalates a peer that has a due time). Stamped before the send + // so a verdict delivered during it is not overwritten. + self.confirmation_probe_due_at.insert( + peer_id.to_string(), + Utc::now() + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS), + ); match self.send_internal_message( peer_id, internal_prefixes::SESSION_CONFIRM_PROBE.to_string(), @@ -546,6 +555,22 @@ 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. + let orphans: Vec<_> = self + .outbox + .values() + .filter(|entry| { + entry.message.recipient.as_str() == peer_id + && Self::is_confirmation_probe(&entry.message) + }) + .map(|entry| entry.message.id.clone()) + .collect(); + for orphan in orphans { + self.forget_outbound_message(&orphan); + } warn!( event = "session_confirmation_probe_failed", session_or_group_id = %peer_id, @@ -642,10 +667,6 @@ impl OfflineProtocol { continue; } self.send_session_confirmation_probe(&peer_id, source_event); - self.confirmation_probe_due_at.insert( - peer_id, - now + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS), - ); } } diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index c95c3959..e4923a50 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -3943,6 +3943,7 @@ impl OfflineProtocol { // `PruneAllowance::pool`. let mut budget = allowance.counting(); let mut prune_bound_reached = false; + let mut legacy_probes_dropped = 0usize; for message_id in message_ids.into_iter().take(OUTBOX_RESTORE_KEY_CAP) { if budget.is_spent() { prune_bound_reached = true; @@ -4008,11 +4009,18 @@ impl OfflineProtocol { if Self::is_confirmation_probe(&entry.message) { budget.claim(); self.delete_outbox_key(&message_id); + legacy_probes_dropped += 1; continue; } restored.push(entry); } + if legacy_probes_dropped > 0 { + warn!( + count = legacy_probes_dropped, + "Dropped persisted confirmation probes written by an older build" + ); + } // Drop entries past the absolute lifetime cap before anything else: // the carrier-relative refresh below would otherwise re-grant them a diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 051a3a0e..dde49b28 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41046,6 +41046,36 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { assert!(!alice.outbox.contains_key(&orphan)); assert!(!alice.retry_queue.contains(&orphan.as_str())); assert!(!alice.ack_manager.is_waiting_for_ack(&orphan)); + + // 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. + for _ in 0..100_000 { + if alice + .ack_manager + .register_pending_ack_with_priority(MessageId::new(), None, MessagePriority::High) + .is_err() + { + break; + } + } + let probes_to_bob = |alice: &OfflineProtocol| { + alice + .outbox + .values() + .filter(|e| OfflineProtocol::is_confirmation_probe(&e.message)) + .count() + }; + alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); + alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); + assert_eq!( + probes_to_bob(&alice), + 0, + "a probe whose send failed is not left queued" + ); + + // The sender stamps the cadence, so the next scan does not supersede a + // fast-path probe before its ACK can arrive. + assert!(alice.confirmation_probe_due_at[&id("bob").to_string()] > Utc::now()); } /// A probe persisted before a restart is not in diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index 40a3e978..d902faa2 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -105,7 +105,8 @@ real messages. The supersede lives in the one function that sends a probe, because two paths call it: the periodic scan and the fast path taken when the transport confirms a Welcome went out. A supersede in the scan alone left the fast path's probe -with its full ladder. Superseding is not a delivery failure and is not counted +with its full ladder. The same function sets the next due time, so the scan +does not supersede a fast-path probe before its acknowledgement can arrive. Superseding is not a delivery failure and is not counted against the carrier. A probe is not persisted either, and one an older build persisted is dropped at restore: the new process does not know which probe was the last, so a restored one could never be superseded, and the next scan asks From e8a3b45cb042d4f68a5f435552e93d13f93a6e42 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 14:03:04 +0400 Subject: [PATCH 09/15] fix(protocol): a late relay verdict on a superseded probe still backs off Both verdict handlers find the peer through the outbox entry, which the supersede removes, so a verdict slower than the probe interval was dropped and the peer kept being probed every five seconds. Remember the superseded id per peer and escalate from it, once per probe. Drops the redundant outstanding-map check from has_pending_work. --- crates/offline-protocol/src/protocol/mod.rs | 8 +++++ crates/offline-protocol/src/protocol/send.rs | 2 ++ .../offline-protocol/src/protocol/session.rs | 31 ++++++++++++++++--- .../src/protocol/tests/mod.rs | 31 +++++++++++++++++++ docs/state-machines/session-lifecycle.md | 23 +++++++++----- 5 files changed, 82 insertions(+), 13 deletions(-) diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index dc8117f6..61928fad 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -718,6 +718,13 @@ pub struct OfflineProtocol { /// carrier. Capacity eviction then failed real messages. confirmation_probe_outstanding: HashMap, + /// The probe each pending peer's current one superseded. A relay verdict + /// finds its peer through the outbox entry, which the supersede removed, + /// so a verdict slower than `CONFIRMATION_PROBE_INTERVAL_SECS` would + /// otherwise be dropped and never back the schedule off, leaving the + /// peer probed every 5s against a relay that has said it is not there. + confirmation_probe_superseded: HashMap, + /// Token bucket that caps how fast the RETRY/PROBE path /// (`process_retry_queue`) re-sends, so a large backlog of resends to /// unreachable peers cannot burst past the relay's per-connection rate limit @@ -1186,6 +1193,7 @@ impl OfflineProtocol { confirmation_probe_due_at: HashMap::new(), confirmation_probe_unreachable_parks: HashMap::new(), confirmation_probe_outstanding: HashMap::new(), + confirmation_probe_superseded: HashMap::new(), retry_drain_tokens: 0.0, retry_drain_last: None, rekey_due_at: HashMap::new(), diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 0768e134..2da7ca6c 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -4140,6 +4140,7 @@ impl OfflineProtocol { message_id = %message_id, "Unreachable verdict for message without outbox entry, dropping" ); + self.note_superseded_probe_verdict(&parsed_id); return; } }, @@ -4295,6 +4296,7 @@ impl OfflineProtocol { return; }; if !self.is_parkable_plain_dm(&parsed_id) { + self.note_superseded_probe_verdict(&parsed_id); return; } let Some(entry) = self.outbox.get_mut(&parsed_id) else { diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 33c51f72..c5b6622b 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -16,7 +16,7 @@ use crate::mls_observability::MlsOperationContext; use crate::protocol::reachability::{Claim, FactSource}; use crate::{Error, EstablishmentState, Event, Result, SessionStateError}; use chrono::{Duration as ChronoDuration, Utc}; -use offline_protocol_core::{Message, MessagePriority}; +use offline_protocol_core::{Message, MessageId, MessagePriority}; use offline_protocol_mls::{MlsManager, WelcomeMessage}; use offline_protocol_transport::TransportType; use std::collections::HashSet; @@ -454,6 +454,24 @@ impl OfflineProtocol { if let Some(probe) = self.confirmation_probe_outstanding.remove(peer_id) { self.forget_outbound_message(&probe); } + self.confirmation_probe_superseded.remove(peer_id); + } + + /// Backs the probe schedule off for a relay verdict on a probe that has + /// since been superseded (`confirmation_probe_superseded`). Consumed, so + /// one probe's verdict escalates once, as it did while the probe was + /// still queued. A no-op for any other id. + pub(super) fn note_superseded_probe_verdict(&mut self, message_id: &MessageId) { + let Some(peer) = self + .confirmation_probe_superseded + .iter() + .find(|(_, probe)| *probe == message_id) + .map(|(peer, _)| peer.clone()) + else { + return; + }; + self.confirmation_probe_superseded.remove(&peer); + self.note_confirmation_probe_unreachable(&peer); } /// Fold a relay "recipient unreachable" verdict into the session-confirmation @@ -524,11 +542,13 @@ impl OfflineProtocol { // delivery failure is recorded against the carrier. Before the send, // not after: if the send errors the peer has no probe in flight // until the next scan, but a new probe can never evict a message at - // outbox capacity to make room beside the one it replaces. A relay - // verdict that arrives for a superseded probe finds no entry and is - // dropped; the current probe's own verdict backs the schedule off. + // outbox capacity to make room beside the one it replaces. The + // superseded id is kept so a relay verdict that arrives for it late + // still backs the schedule off (`note_superseded_probe_verdict`). if let Some(previous) = self.confirmation_probe_outstanding.remove(peer_id) { self.forget_outbound_message(&previous); + self.confirmation_probe_superseded + .insert(peer_id.to_string(), previous); } // The sender owns the cadence, so a fast-path probe is not superseded // by the next scan before its ACK can arrive, and a relay verdict for @@ -593,7 +613,6 @@ impl OfflineProtocol { // Fast path: nothing pending → skip entirely (no storage I/O) let has_pending_work = !self.pending_encrypted_messages.is_empty() || !self.confirmation_probe_due_at.is_empty() - || !self.confirmation_probe_outstanding.is_empty() || !self.confirmation_retry_due_at.is_empty(); if !has_pending_work { @@ -642,6 +661,8 @@ impl OfflineProtocol { .retain(|peer, _| pending_set.contains(peer)); self.confirmation_probe_unreachable_parks .retain(|peer, _| pending_set.contains(peer)); + self.confirmation_probe_superseded + .retain(|peer, _| pending_set.contains(peer)); // Not a plain `retain`: a probe whose peer left the pending set // without passing through `clear_confirmation_recovery_tracking` // would be forgotten while still queued. diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index dde49b28..26d4262b 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41015,6 +41015,37 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { "a superseded probe is withdrawn silently" ); + // A relay verdict slower than the probe interval names a probe already + // superseded. It still backs the schedule off, once per probe, for + // either verdict. + let bob = id("bob").to_string(); + for verdict in ["recipient_unreachable: User is offline", "relay_pushed"] { + alice.send_session_confirmation_probe(&bob, "test"); + let late = alice.confirmation_probe_superseded[&bob].clone(); + let parks = alice + .confirmation_probe_unreachable_parks + .get(&bob) + .copied() + .unwrap_or(0); + for _ in 0..2 { + alice + .on_transport_send_failed(&late.as_str(), Some(verdict.to_string())) + .unwrap(); + } + assert_eq!( + alice + .confirmation_probe_unreachable_parks + .get(&bob) + .copied(), + Some(parks + 1), + "a late {verdict} verdict escalates once" + ); + assert!( + alice.confirmation_probe_due_at[&bob] + > Utc::now() + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS) + ); + } + // A probe whose send failed waits in the retry queue; the next probe // takes it out of there too. handle.set_fail_next_sends(1); diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index d902faa2..c06bbcf2 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -106,14 +106,21 @@ The supersede lives in the one function that sends a probe, because two paths call it: the periodic scan and the fast path taken when the transport confirms a Welcome went out. A supersede in the scan alone left the fast path's probe with its full ladder. The same function sets the next due time, so the scan -does not supersede a fast-path probe before its acknowledgement can arrive. Superseding is not a delivery failure and is not counted -against the carrier. A probe is not persisted either, and one an older build -persisted is dropped at restore: the new process does not know which probe was -the last, so a restored one could never be superseded, and the next scan asks -the same question. For the same reason a probe is withdrawn with the rest of -the confirmation tracking when the session confirms or is torn down: a probe -the tracking no longer names could never be superseded, and would keep its -ladder. +does not supersede a fast-path probe before its acknowledgement can arrive. + +Superseding is not a delivery failure and is not counted against the carrier. +It does not discard the relay's answer either: a verdict finds its peer through +the outbox entry, so the superseded id is remembered, and a verdict slower than +the probe interval still backs the cadence off. Dropping it left a peer the +relay had refused probed every five seconds, which is the rate that flaps the +relay connection. + +A probe is not persisted, and one an older build persisted is dropped at +restore: the new process does not know which probe was the last, so a restored +one could never be superseded, and the next scan asks the same question. For +the same reason a probe is withdrawn with the rest of the confirmation tracking +when the session confirms or is torn down: a probe the tracking no longer names +could never be superseded, and would keep its ladder. ### Both-create convergence From e011573e070d3572d167641c08f2b63d51b0fc86 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 14:12:14 +0400 Subject: [PATCH 10/15] fix(protocol): a late verdict on a superseded probe records the fact too The superseded-probe path backed the schedule off but skipped the reachability fact the queued-probe path records, so the next DM still tried the carrier that had refused. A fast-path probe also keeps a backed-off due time instead of resetting it to five seconds, and the legacy-probe drop at restore logs at info. --- crates/offline-protocol/src/protocol/send.rs | 4 +- .../offline-protocol/src/protocol/session.rs | 40 ++++++++++++++----- .../offline-protocol/src/protocol/storage.rs | 6 ++- .../src/protocol/tests/mod.rs | 17 +++++++- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 2da7ca6c..0a18801b 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -4140,7 +4140,7 @@ impl OfflineProtocol { message_id = %message_id, "Unreachable verdict for message without outbox entry, dropping" ); - self.note_superseded_probe_verdict(&parsed_id); + self.note_superseded_probe_verdict(&parsed_id, carrier); return; } }, @@ -4296,7 +4296,7 @@ impl OfflineProtocol { return; }; if !self.is_parkable_plain_dm(&parsed_id) { - self.note_superseded_probe_verdict(&parsed_id); + self.note_superseded_probe_verdict(&parsed_id, carrier); return; } let Some(entry) = self.outbox.get_mut(&parsed_id) else { diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index c5b6622b..5c41a96c 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -457,11 +457,16 @@ impl OfflineProtocol { self.confirmation_probe_superseded.remove(peer_id); } - /// Backs the probe schedule off for a relay verdict on a probe that has - /// since been superseded (`confirmation_probe_superseded`). Consumed, so - /// one probe's verdict escalates once, as it did while the probe was - /// still queued. A no-op for any other id. - pub(super) fn note_superseded_probe_verdict(&mut self, message_id: &MessageId) { + /// Applies a relay verdict on a probe that has since been superseded + /// (`confirmation_probe_superseded`) the way the verdict handlers apply + /// one on a queued probe: the reachability fact, then the probe back-off. + /// Consumed, so one probe's verdict escalates once, as it did while the + /// probe was still queued. A no-op for any other id. + pub(super) fn note_superseded_probe_verdict( + &mut self, + message_id: &MessageId, + carrier: Option, + ) { let Some(peer) = self .confirmation_probe_superseded .iter() @@ -471,6 +476,15 @@ impl OfflineProtocol { return; }; self.confirmation_probe_superseded.remove(&peer); + if let Some(carrier) = carrier { + self.reachability.record( + &peer, + carrier, + Claim::Unreachable, + FactSource::GatewayVerdict, + Instant::now(), + ); + } self.note_confirmation_probe_unreachable(&peer); } @@ -480,7 +494,7 @@ impl OfflineProtocol { /// [`Self::apply_recipient_unreachable_failure`]). Without this, a peer that /// established an MLS session then vanished before confirming is re-probed /// every `CONFIRMATION_PROBE_INTERVAL_SECS` (5s) indefinitely, because - /// `kick_pending_session_reconciliation` re-arms it every scan and + /// `send_session_confirmation_probe` re-arms it on every probe and /// `on_transport_send_failed` otherwise never consults this scheduler. A /// handful of such peers pushes aggregate relay traffic past the /// per-connection rate limit, which disconnects the socket on a loop. @@ -555,10 +569,16 @@ impl OfflineProtocol { // it can back the schedule off (`note_confirmation_probe_unreachable` // only escalates a peer that has a due time). Stamped before the send // so a verdict delivered during it is not overwritten. - self.confirmation_probe_due_at.insert( - peer_id.to_string(), - Utc::now() + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS), - ); + // + // A later due time is kept: the scan only sends once the due time has + // passed, but the fast path can fire while the schedule is backed off, + // and resetting it would undo the ladder a relay verdict built. + let next = Utc::now() + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS); + let due_at = self + .confirmation_probe_due_at + .entry(peer_id.to_string()) + .or_insert(next); + *due_at = (*due_at).max(next); match self.send_internal_message( peer_id, internal_prefixes::SESSION_CONFIRM_PROBE.to_string(), diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index e4923a50..f340fd91 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -4016,9 +4016,11 @@ impl OfflineProtocol { restored.push(entry); } if legacy_probes_dropped > 0 { - warn!( + info!( + event = "outbox_entry_dropped", + repair_action = "legacy_confirmation_probe", count = legacy_probes_dropped, - "Dropped persisted confirmation probes written by an older build" + "outbox_entry_dropped" ); } diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 26d4262b..8eb0e557 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41029,9 +41029,20 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { .unwrap_or(0); for _ in 0..2 { alice - .on_transport_send_failed(&late.as_str(), Some(verdict.to_string())) + .on_transport_send_failed_via( + &late.as_str(), + Some(verdict.to_string()), + Some(TransportType::Internet), + ) .unwrap(); } + assert_eq!( + alice + .reachability + .claim_for(&bob, TransportType::Internet, std::time::Instant::now()), + Some(crate::protocol::reachability::Claim::Unreachable), + "a late {verdict} verdict is recorded as a fact too" + ); assert_eq!( alice .confirmation_probe_unreachable_parks @@ -41044,6 +41055,10 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { alice.confirmation_probe_due_at[&bob] > Utc::now() + ChronoDuration::seconds(CONFIRMATION_PROBE_INTERVAL_SECS) ); + // A fast-path probe while backed off keeps the back-off. + let backed_off = alice.confirmation_probe_due_at[&bob]; + alice.send_session_confirmation_probe(&bob, "transport_confirmed"); + assert_eq!(alice.confirmation_probe_due_at[&bob], backed_off); } // A probe whose send failed waits in the retry queue; the next probe From 1abf92d0303ac75fafac3d6407ab7a72e6736150 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 14:21:34 +0400 Subject: [PATCH 11/15] docs(protocol): bound the late-verdict window, and log a drop only when dropped The superseded slot holds one id per peer, so a verdict is honoured up to two probe intervals late; say so. The no-outbox-entry log now fires only when no superseded probe consumed the verdict. --- crates/offline-protocol/src/protocol/mod.rs | 3 +++ crates/offline-protocol/src/protocol/send.rs | 11 ++++++----- crates/offline-protocol/src/protocol/session.rs | 8 +++++--- docs/state-machines/session-lifecycle.md | 8 +++++--- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index 61928fad..07dd6e1d 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -723,6 +723,9 @@ pub struct OfflineProtocol { /// so a verdict slower than `CONFIRMATION_PROBE_INTERVAL_SECS` would /// otherwise be dropped and never back the schedule off, leaving the /// peer probed every 5s against a relay that has said it is not there. + /// One id per peer, so this covers a verdict up to two intervals late; + /// one later than that is dropped, and the next probe's verdict backs + /// the schedule off instead. confirmation_probe_superseded: HashMap, /// Token bucket that caps how fast the RETRY/PROBE path diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 0a18801b..75c06e39 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -4136,11 +4136,12 @@ impl OfflineProtocol { None => match self.media_outbox.get(&parsed_id) { Some(entry) => (entry, true), None => { - debug!( - message_id = %message_id, - "Unreachable verdict for message without outbox entry, dropping" - ); - self.note_superseded_probe_verdict(&parsed_id, carrier); + if !self.note_superseded_probe_verdict(&parsed_id, carrier) { + debug!( + message_id = %message_id, + "Unreachable verdict for message without outbox entry, dropping" + ); + } return; } }, diff --git a/crates/offline-protocol/src/protocol/session.rs b/crates/offline-protocol/src/protocol/session.rs index 5c41a96c..4b19ccd9 100644 --- a/crates/offline-protocol/src/protocol/session.rs +++ b/crates/offline-protocol/src/protocol/session.rs @@ -461,19 +461,20 @@ impl OfflineProtocol { /// (`confirmation_probe_superseded`) the way the verdict handlers apply /// one on a queued probe: the reachability fact, then the probe back-off. /// Consumed, so one probe's verdict escalates once, as it did while the - /// probe was still queued. A no-op for any other id. + /// probe was still queued. A no-op for any other id; returns whether + /// the verdict was applied. pub(super) fn note_superseded_probe_verdict( &mut self, message_id: &MessageId, carrier: Option, - ) { + ) -> bool { let Some(peer) = self .confirmation_probe_superseded .iter() .find(|(_, probe)| *probe == message_id) .map(|(peer, _)| peer.clone()) else { - return; + return false; }; self.confirmation_probe_superseded.remove(&peer); if let Some(carrier) = carrier { @@ -486,6 +487,7 @@ impl OfflineProtocol { ); } self.note_confirmation_probe_unreachable(&peer); + true } /// Fold a relay "recipient unreachable" verdict into the session-confirmation diff --git a/docs/state-machines/session-lifecycle.md b/docs/state-machines/session-lifecycle.md index c06bbcf2..f750e803 100644 --- a/docs/state-machines/session-lifecycle.md +++ b/docs/state-machines/session-lifecycle.md @@ -111,9 +111,11 @@ does not supersede a fast-path probe before its acknowledgement can arrive. Superseding is not a delivery failure and is not counted against the carrier. It does not discard the relay's answer either: a verdict finds its peer through the outbox entry, so the superseded id is remembered, and a verdict slower than -the probe interval still backs the cadence off. Dropping it left a peer the -relay had refused probed every five seconds, which is the rate that flaps the -relay connection. +one probe interval (but faster than two) still backs the cadence off. Dropping +it left a peer the relay had refused probed every five seconds, which is the +rate that flaps the relay connection. Only the last superseded id is kept: a +verdict that misses two intervals finds neither, and the cadence backs off on +the next probe's verdict instead. Relay verdicts arrive well inside one. A probe is not persisted, and one an older build persisted is dropped at restore: the new process does not know which probe was the last, so a restored From b3e87e8c8d09329af8f9bcc63ff90dcf62df8fc4 Mon Sep 17 00:00:00 2001 From: Mizan Ali Date: Tue, 6 Oct 2026 14:23:52 +0400 Subject: [PATCH 12/15] docs(protocol): name every penalty-free use of the outbound teardown --- docs/state-machines/outbox-and-retries.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/state-machines/outbox-and-retries.md b/docs/state-machines/outbox-and-retries.md index 294928ef..5f685266 100644 --- a/docs/state-machines/outbox-and-retries.md +++ b/docs/state-machines/outbox-and-retries.md @@ -93,9 +93,11 @@ the outbox together.** Capacity eviction, lifetime expiry, retry exhaustion and an acknowledgement timeout that finds no outbox entry all go through one function (`retire_undeliverable_message`) for that reason, and the event each one owes the application is emitted by the path itself. A session confirmation -probe superseded by a newer one leaves through the same three-way teardown -(`forget_outbound_message`), without the delivery failure a give-up records -against the carrier: it did not fail, it was replaced. +probe leaves through the same three-way teardown (`forget_outbound_message`) +when it is superseded by a newer one or withdrawn (its session confirmed or torn +down, or its send failed after it was queued), without the delivery failure a +give-up records against the carrier: it did not fail, it was replaced or is no +longer needed. Removing the outbox entry alone is not enough, because the retry and flush paths re-create a missing entry before they resend, with fresh timestamps. An id left From 712d5188b4af401da73b6d04e8ceb1ed37a06411 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Tue, 6 Oct 2026 18:37:35 +0530 Subject: [PATCH 13/15] docs(changelog): move the probe fix out of the shipped 0.28.0 notes This branch was cut before the 0.28.0 release. Its entry sat under the branch's [Unreleased] heading, which on main had since been renamed to [0.28.0]. The merge is textually clean, so git happily filed the bullet under 0.28.0's Fixed section. 0.28.0 did not ship this fix. Move it to the new [Unreleased] Fixed section, where it belongs. --- CHANGELOG.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c177fb5..9ff6f92d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,19 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **Confirmation probes no longer crowd real messages out of the outbox.** A + session the peer never confirms is probed every five seconds, and each probe + used to enter the outbox with its own retry ladder on top of the unanswered + ones before it. Only a relay verdict backed that off, so a mesh-only link + never did. An account with ten such contacts stacked about two probes a + second, and once the outbox hit its 500-entry cap, eviction failed the user's + own messages with `Outbox capacity exceeded`. Seen on a device test between + an Android and an iPhone, offline over BLE. Each new probe now supersedes + the last, quietly and without counting against the carrier, whether the + periodic scan or the Welcome fast path sent it. Probes are no longer written + to storage, and those an older build wrote are dropped at restore. A peer + holds one probe in the outbox and is still probed on the same cadence. + - **A message carried through a dense cluster no longer dies inside it** ([#510](https://github.com/Offline-Protocol/offline-protocol-sdk/issues/510)). A forwarder that received a second copy of a frame while its own forward @@ -469,19 +482,6 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed -- **Confirmation probes no longer crowd real messages out of the outbox.** A - session the peer never confirms is probed every five seconds, and each probe - used to enter the outbox with its own retry ladder on top of the unanswered - ones before it. Only a relay verdict backed that off, so a mesh-only link - never did. An account with ten such contacts stacked about two probes a - second, and once the outbox hit its 500-entry cap, eviction failed the user's - own messages with `Outbox capacity exceeded`. Seen on a device test between - an Android and an iPhone, offline over BLE. Each new probe now supersedes - the last, quietly and without counting against the carrier, whether the - periodic scan or the Welcome fast path sent it. Probes are no longer written - to storage, and those an older build wrote are dropped at restore. A peer - holds one probe in the outbox and is still probed on the same cadence. - - **A key package refused by this device's clock is reported.** A peer's key package is valid from an hour before it was minted until 30 days after, judged by the receiver's clock. A device more than an hour behind a peer From c1e25dbd505f34d482fa9994f9da3b0890855a3c Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Tue, 6 Oct 2026 18:37:42 +0530 Subject: [PATCH 14/15] test(protocol): split the probe supersede test, and pin what it missed The supersede test was one function walking eight phases over shared state. The first failure hid every phase after it, and the full ACK tracker phase left the fixture broken for anything added later. This is not great. Split it into three tests on a shared fixture: the supersede itself, late relay verdicts, and the withdrawals (confirm, session deleted under the engine, send failing after the probe was queued). While at it, assert two things the old test never checked. A verdict two intervals late finds no remembered probe, records no fact, and does not escalate. And the scan that withdraws a settled peer's probe also prunes the superseded id it kept for a late verdict. The second was checked to fail with that prune removed. --- .../src/protocol/tests/mod.rs | 160 +++++++++++------- 1 file changed, 99 insertions(+), 61 deletions(-) diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 8eb0e557..585e0bd1 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40904,15 +40904,9 @@ fn test_confirmation_probe_unreachable_escalation() { ); } -/// A peer whose session never confirms, and that no `unreachable` verdict -/// backs off (a relay that pushes to offline users, or a mesh-only link), -/// holds one probe in the outbox, however many intervals pass. Each due scan -/// still probes, so a lost probe is replaced on the next one, but the new -/// probe supersedes the last instead of stacking a fresh retry ladder on it: -/// stacking filled the outbox at one probe per such peer per 5s, and capacity -/// eviction failed real messages. -#[test] -fn an_unanswered_confirmation_probe_supersedes_the_last_one() { +/// Alice holds a session bob never confirms, running over a BLE mock. The +/// fixture for the confirmation-probe supersede tests below. +fn unconfirmed_probe_target() -> (OfflineProtocol, MockTransport) { let mut alice_config = create_test_config_for_user("alice"); alice_config.encryption.enabled = true; let mut alice = OfflineProtocol::new(alice_config).unwrap(); @@ -40923,7 +40917,6 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { bob.initialize_mls_for_test(Arc::new(InMemoryStorage::new())) .unwrap(); - // A session alice holds but bob never confirms. let bob_key_package = { let manager = bob.mls_manager.as_ref().unwrap().read().unwrap(); manager.get_or_create_key_package().unwrap() @@ -40946,24 +40939,33 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { .transport_manager_mut() .add_transport(TransportType::BLE, Box::new(transport)); alice.start().unwrap(); + handle.clear_sent_messages(); + (alice, handle) +} - let probes = |handle: &MockTransport| -> Vec { - handle - .sent_messages() - .into_iter() - .filter(|m| { - m.content - .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) - }) - .map(|m| m.id) - .collect() - }; - let make_due = |alice: &mut OfflineProtocol| { - alice.confirmation_probe_due_at.insert( - id("bob").to_string(), - Utc::now() - ChronoDuration::seconds(1), - ); - }; +fn sent_confirmation_probes(handle: &MockTransport) -> Vec { + handle + .sent_messages() + .into_iter() + .filter(|m| { + m.content + .starts_with(internal_prefixes::SESSION_CONFIRM_PROBE) + }) + .map(|m| m.id) + .collect() +} + +/// A peer whose session never confirms, and that no `unreachable` verdict +/// backs off (a relay that pushes to offline users, or a mesh-only link), +/// holds one probe in the outbox, however many intervals pass. Each due scan +/// still probes, so a lost probe is replaced on the next one, but the new +/// probe supersedes the last instead of stacking a fresh retry ladder on it: +/// stacking filled the outbox at one probe per such peer per 5s, and capacity +/// eviction failed real messages. +#[test] +fn an_unanswered_confirmation_probe_supersedes_the_last_one() { + let (mut alice, handle) = unconfirmed_probe_target(); + let bob = id("bob").to_string(); let events: Arc>> = Arc::new(Mutex::new(Vec::new())); let events_handle = Arc::clone(&events); @@ -40971,16 +40973,17 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { // The scan and the Welcome fast path (`on_transport_send_confirmed`) // both probe, interleaved: either may supersede the other's probe. - handle.clear_sent_messages(); for round in 0..6 { if round % 2 == 0 { - make_due(&mut alice); + alice + .confirmation_probe_due_at + .insert(bob.clone(), Utc::now() - ChronoDuration::seconds(1)); alice.kick_pending_session_reconciliation("test"); } else { - alice.send_session_confirmation_probe(&id("bob").to_string(), "transport_confirmed"); + alice.send_session_confirmation_probe(&bob, "transport_confirmed"); } } - let sent = probes(&handle); + let sent = sent_confirmation_probes(&handle); assert_eq!(sent.len(), 6, "every due scan and fast path still probes"); let outstanding: Vec<_> = alice.outbox.keys().filter(|id| sent.contains(id)).collect(); @@ -41015,10 +41018,47 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { "a superseded probe is withdrawn silently" ); - // A relay verdict slower than the probe interval names a probe already - // superseded. It still backs the schedule off, once per probe, for - // either verdict. + // The sender stamps the cadence, so the next scan does not supersede a + // fast-path probe before its ACK can arrive. + assert!(alice.confirmation_probe_due_at[&bob] > Utc::now()); + + // A probe whose send failed waits in the retry queue; the next probe + // takes it out of there too. + handle.set_fail_next_sends(1); + alice.send_session_confirmation_probe(&bob, "test"); + let deferred = alice.confirmation_probe_outstanding[&bob].clone(); + assert!(alice.retry_queue.contains(&deferred.as_str())); + alice.send_session_confirmation_probe(&bob, "test"); + assert!(!alice.retry_queue.contains(&deferred.as_str())); + assert!(!alice.outbox.contains_key(&deferred)); +} + +/// A relay verdict slower than the probe interval names a probe already +/// superseded. It still records the fact and backs the schedule off, once per +/// probe, for either verdict. One id per peer is kept, so a verdict two +/// intervals late finds nothing and changes nothing. +#[test] +fn a_late_verdict_on_a_superseded_probe_still_backs_off() { + let (mut alice, _handle) = unconfirmed_probe_target(); let bob = id("bob").to_string(); + + // Two intervals late: the oldest of three probes is no longer remembered. + alice.send_session_confirmation_probe(&bob, "test"); + let too_late = alice.confirmation_probe_outstanding[&bob].clone(); + alice.send_session_confirmation_probe(&bob, "test"); + alice.send_session_confirmation_probe(&bob, "test"); + assert!(!alice.note_superseded_probe_verdict(&too_late, Some(TransportType::Internet))); + assert_eq!( + alice + .reachability + .claim_for(&bob, TransportType::Internet, std::time::Instant::now()), + None, + "a verdict past the remembered probe records nothing" + ); + assert!(!alice + .confirmation_probe_unreachable_parks + .contains_key(&bob)); + for verdict in ["recipient_unreachable: User is offline", "relay_pushed"] { alice.send_session_confirmation_probe(&bob, "test"); let late = alice.confirmation_probe_superseded[&bob].clone(); @@ -41060,29 +41100,32 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { alice.send_session_confirmation_probe(&bob, "transport_confirmed"); assert_eq!(alice.confirmation_probe_due_at[&bob], backed_off); } +} - // A probe whose send failed waits in the retry queue; the next probe - // takes it out of there too. - handle.set_fail_next_sends(1); - alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); - let deferred = alice.confirmation_probe_outstanding[&id("bob").to_string()].clone(); - assert!(alice.retry_queue.contains(&deferred.as_str())); - alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); - assert!(!alice.retry_queue.contains(&deferred.as_str())); - assert!(!alice.outbox.contains_key(&deferred)); +/// A probe the tracking no longer names could never be superseded and would +/// keep its full retry ladder, so it is withdrawn whenever the tracking lets +/// go of it: the session confirms, the session is deleted under the engine, +/// or the send fails after the probe was queued. +#[test] +fn a_confirmation_probe_is_withdrawn_when_nothing_could_supersede_it() { + let (mut alice, handle) = unconfirmed_probe_target(); + let bob = id("bob").to_string(); - // Confirming the session withdraws the last probe too: with the - // tracking gone, nothing could ever supersede it. - let last = alice.confirmation_probe_outstanding[&id("bob").to_string()].clone(); - alice.clear_confirmation_recovery_tracking(&id("bob").to_string()); + // Confirming the session withdraws the last probe. + alice.send_session_confirmation_probe(&bob, "test"); + let last = alice.confirmation_probe_outstanding[&bob].clone(); + alice.clear_confirmation_recovery_tracking(&bob); assert!(!alice.outbox.contains_key(&last)); assert!(!alice.retry_queue.contains(&last.as_str())); assert!(!alice.ack_manager.is_waiting_for_ack(&last)); // A peer that leaves the pending set without that call (its session - // deleted under the engine) has its probe withdrawn by the next scan. - alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); - let orphan = probes(&handle).last().unwrap().clone(); + // deleted under the engine) has its probe withdrawn by the next scan, + // and the superseded id it kept for a late verdict is pruned too. + alice.send_session_confirmation_probe(&bob, "test"); + alice.send_session_confirmation_probe(&bob, "test"); + assert!(alice.confirmation_probe_superseded.contains_key(&bob)); + let orphan = sent_confirmation_probes(&handle).last().unwrap().clone(); assert!(alice.outbox.contains_key(&orphan)); { let manager = alice.mls_manager.as_ref().unwrap().read().unwrap(); @@ -41092,6 +41135,8 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { assert!(!alice.outbox.contains_key(&orphan)); assert!(!alice.retry_queue.contains(&orphan.as_str())); assert!(!alice.ack_manager.is_waiting_for_ack(&orphan)); + 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. @@ -41104,24 +41149,17 @@ fn an_unanswered_confirmation_probe_supersedes_the_last_one() { break; } } - let probes_to_bob = |alice: &OfflineProtocol| { + alice.send_session_confirmation_probe(&bob, "test"); + alice.send_session_confirmation_probe(&bob, "test"); + assert_eq!( alice .outbox .values() .filter(|e| OfflineProtocol::is_confirmation_probe(&e.message)) - .count() - }; - alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); - alice.send_session_confirmation_probe(&id("bob").to_string(), "test"); - assert_eq!( - probes_to_bob(&alice), + .count(), 0, "a probe whose send failed is not left queued" ); - - // The sender stamps the cadence, so the next scan does not supersede a - // fast-path probe before its ACK can arrive. - assert!(alice.confirmation_probe_due_at[&id("bob").to_string()] > Utc::now()); } /// A probe persisted before a restart is not in From d3caaa640d52648e3df486bea3b29a176d212f2a Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Tue, 6 Oct 2026 18:37:47 +0530 Subject: [PATCH 15/15] fix(protocol): pin that one restore can clear a full outbox of probes The upgrade path drops every confirmation probe an older build persisted, charging each delete to the outbox walk's prune pool. That only works out because the pool (512) happens to be bigger than the outbox cap (500). Nothing said so. If someone shrinks the pool, a device upgrading from the build that stacked probes spends the whole pool on them, the walk breaks, and the user's real messages behind them are not restored until a later launch. Not lost, since they stay on disk unsettled, but not delivered either. Silently. Pin it twice: a const assert next to the advisory-floor one, so the build refuses the change, and a restore test with a full outbox of legacy probes plus one real message, which must come back in the same launch. The test was checked to fail with the pool cut to 400. --- .../offline-protocol/src/protocol/storage.rs | 10 ++++ .../src/protocol/tests/mod.rs | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/crates/offline-protocol/src/protocol/storage.rs b/crates/offline-protocol/src/protocol/storage.rs index f340fd91..8509f5ae 100644 --- a/crates/offline-protocol/src/protocol/storage.rs +++ b/crates/offline-protocol/src/protocol/storage.rs @@ -882,6 +882,16 @@ const _: () = assert!( underflows to zero and it can never prune at all" ); +// The outbox walk charges a legacy confirmation probe's delete to its pool +// (`restore_outbox`). A build that stacked probes could leave a full outbox +// of them, and if the pool is smaller than that the walk breaks on probes and +// leaves real messages unrestored until a later launch. +const _: () = assert!( + MAX_RESTORE_PRUNE_DELETES >= MAX_OUTBOX_ENTRIES, + "an outbox full of legacy confirmation probes must clear in one launch, \ + or the walk breaks on them and defers restoring real messages" +); + /// A pool of durable restore-path deletes, drawn on by one or more walks. /// /// The unit the [`MAX_RESTORE_PRUNE_DELETES`] bound is actually about. A pool diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 585e0bd1..5ca4fcf4 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -41202,6 +41202,53 @@ fn a_persisted_confirmation_probe_is_not_restored() { assert_eq!(storage.list_keys(storage_keys::OUTBOX).unwrap().len(), 1); } +/// The first launch after upgrading from the build that stacked probes can +/// find an outbox full of them. Their deletes are charged to the walk's pool, +/// so the pool has to cover a full outbox, or the walk breaks on probes and a +/// real message behind them waits for a later launch to be restored. +#[test] +fn an_outbox_full_of_legacy_probes_clears_in_one_launch() { + let storage = Arc::new(InMemoryStorage::new()); + let entry = |message: Message| OutboxEntry { + message, + attempt_count: 1, + first_sent_at: chrono::Utc::now(), + last_sent_at: chrono::Utc::now(), + last_transport: None, + reseal: None, + relay_pushed: false, + }; + for _ in 0..crate::constants::MAX_OUTBOX_ENTRIES { + store_outbox_entry( + &storage, + &entry(test_message( + "bob", + internal_prefixes::SESSION_CONFIRM_PROBE, + )), + ); + } + let real = test_message("bob", "hello"); + let real_id = real.id.clone(); + store_outbox_entry(&storage, &entry(real)); + + let mut protocol = OfflineProtocol::new(create_test_config()).unwrap(); + protocol + .enable_message_persistence_for_test(storage.clone()) + .unwrap(); + + let restored: Vec<_> = protocol.outbox_messages().map(|m| m.id.clone()).collect(); + assert_eq!( + restored, + vec![real_id.clone()], + "the real message is restored" + ); + assert_eq!( + storage.list_keys(storage_keys::OUTBOX).unwrap(), + vec![real_id.as_str().to_string()], + "every legacy probe is deleted in the same launch" + ); +} + /// Pins component (D) of the flap fix: the opt-in edge-driven parking gate. /// With `edge_driven_unreachable_dm` on, a durably-unreachable DM keeps being /// timed-probed up to `DM_UNREACHABLE_PROBE_LIMIT` times, then stops (rests in