Repository navigation
fix(protocol): a new confirmation probe supersedes the last - #515
Merged
Merged
Conversation
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.
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.
…racking 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.
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.
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.
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.
… 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.
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.
…en 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.
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.
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.
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.
bahdotsh
approved these changes
Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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 before it. Only a relay verdict (
unreachableorrelay_pushed) backed that cadence off, and a mesh-only carrier never produces one. 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 withOutbox capacity exceeded. Seen on an Android and iPhone device test, offline over BLE.A peer now holds at most one probe in the outbox. Each new probe supersedes the last, quietly and without counting a delivery failure against the carrier, and the cadence is unchanged, so a lost probe is replaced on the next scan.
send_session_confirmation_probe(protocol/session.rs), which both the periodic scan and the Welcome fast path (on_transport_send_confirmed) call. A fast-path probe keeps a due time a relay verdict has already backed off.forget_outbound_message(protocol/send.rs) takes a message out of the retry queue, the ACK tracker and the outbox together.retire_undeliverable_messagenow calls it and then records the carrier failure; the supersede calls it without one.clear_confirmation_recovery_tracking), when its peer leaves the pending set some other way (the scan's prune), and when the send fails after the probe entered the outbox (a full ACK tracker), where no id comes back to track.is_confirmation_probe), which removes a secure-storage write per probe and a delete per supersede under the protocol lock. Probes an older build persisted are dropped at restore, inside the existing delete budget.confirmation_probe_superseded). A verdict on it records the reachability fact and backs the schedule off, once per probe. This covers a verdict up to two probe intervals late.Type of change
fix: bug fixChecklist
cargo fmt --all -- --checkpassescargo clippy --workspace -- -D warningspassescargo test --workspacepasses (locally)RUSTDOCFLAGS="-D warnings" cargo docpasses foroffline-protocolcargo-denyis satisfied (no new dependencies)<type>(<scope>): <subject>)CHANGELOG.mdupdated where relevantunsafein core cratesTests
an_unanswered_confirmation_probe_supersedes_the_last_oneinterleaves scan and fast-path probes against a peer that never confirms, then asserts only the latest probe is in the outbox and none of the superseded ones is in the retry queue or ACK tracker. It also checks that nomessage_failedfires and no carrier failure is recorded, and covers each case above: a deferred probe, confirmation, a session deleted underneath the engine, a full ACK tracker, a laterecipient_unreachableandrelay_pushedverdict (escalating once, recording the fact), and a fast-path probe keeping a backed-off due time.a_persisted_confirmation_probe_is_not_restoredcovers the restore drop and the persist guard.Breaking changes
None. No API, wire, UDL or config change. Probe cadence is unchanged; only the number of probes waiting in the outbox per peer drops to one.
Notes for reviewers
docs/state-machines/session-lifecycle.mdhas a new section, "An unconfirmed session holds one probe", with the invariant and the failures each part prevents.outbox-and-retries.mdnames the probe paths (supersede and withdrawal) as the penalty-free uses of the three-way teardown, next to the give-up paths that record a carrier failure.