Skip to content

fix(dm): let a lost chunk be asked for again - #36

Merged
ForeverInLaw merged 1 commit into
mainfrom
fix/blob-chunk-rerequest
Jul 28, 2026
Merged

fix(dm): let a lost chunk be asked for again#36
ForeverInLaw merged 1 commit into
mainfrom
fix/blob-chunk-rerequest

Conversation

@ForeverInLaw

Copy link
Copy Markdown
Contributor

Problem

One lost chunk hung a transfer forever. Users saw 63%, 32% and 0%.

The receiver did ask again — next_chunk_request re-requests gaps below its cursor on every pump. But the repeat was byte-identical to the previous request, and has_seen_message dedups on channel + sha256(payload), so the sender dropped every repeat before handle_blob saw it and never re-served. Re-served chunk frames are byte-identical too, so they died on the way back.

Same trap that kept the MLS handshake silent: the recovery mechanism is re-sending identical bytes, and a payload-hash dedup makes recovery unreachable.

Fix

  • Exempt the blob channel from the dedup, like the control channel. Idempotent on both sides — serve_chunks re-encrypts, ingest_chunk answers Duplicate for a chunk already held.
  • Add an in-flight window to next_chunk_request: a chunk asked for less than CHUNK_REQUEST_TIMEOUT (10s) ago is not asked for again. This is what stops the once-a-second pump from re-requesting a batch that is still on the wire. The cursor still advances a fresh window every pump, so throughput is unchanged.
  • served_chunks becomes a count, which makes "the sender re-served it" observable.

Verification

  • a_repeated_chunk_request_still_reaches_the_sender drives handle_moss_message, not handle_blob — the existing attachment tests call handlers directly and so never crossed the layer the bug lived in. Confirmed red without the fix (served_count 1, want 2).
  • a_lost_chunk_is_asked_for_again_once_the_window_passes covers the pacing and the retry.
  • 186 lib tests pass, clippy -D warnings clean.
  • Heavy E2E private_dm_runtime_transfers_attachment_over_moss over real moss still completes in ~5s, so the window costs no throughput.

A transfer that lost a single chunk hung forever. The receiver did ask
again — `next_chunk_request` re-requests gaps below its cursor on every
pump — but the repeat request was byte-identical to the previous one, and
`has_seen_message` dedups on `channel + sha256(payload)`. So the sender
dropped every repeat before `handle_blob` ever saw it and never re-served.
Re-served chunk frames are byte-identical too, so they were dropped on the
way back. Users saw transfers stick at 63%, 32% and 0%.

This is the same trap that kept the MLS handshake silent: the recovery
mechanism is re-sending the identical bytes, and a payload-hash dedup makes
recovery unreachable. Exempt the blob channel like the control channel.
Both directions are idempotent — `serve_chunks` re-encrypts, `ingest_chunk`
answers Duplicate for a chunk already held.

What keeps the repeats from becoming a flood is now an in-flight window in
`next_chunk_request`: a chunk asked for less than CHUNK_REQUEST_TIMEOUT ago
is not asked for again, so the once-a-second pump stops re-requesting a
batch that is still on the wire. The cursor still advances a fresh window
every pump, so throughput is unchanged.

`served_chunks` becomes a count rather than a set, which is what makes "the
sender re-served it" observable.

The new DM test drives `handle_moss_message`, not `handle_blob`. The
existing attachment tests call the handlers directly and so never crossed
the layer the bug lived in — which is how it stayed hidden.
@ForeverInLaw
ForeverInLaw merged commit 925487e into main Jul 28, 2026
3 checks passed
@ForeverInLaw
ForeverInLaw deleted the fix/blob-chunk-rerequest branch July 28, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant