Skip to content

Fix issue where messages whose last chunk is smaller than 250 bytes are never delivered - #866

Open
4z3 wants to merge 3 commits into
threefoldtech:mainfrom
4z3:fix-message-chunk-reception
Open

4z3 wants to merge 3 commits into
threefoldtech:mainfrom
4z3:fix-message-chunk-reception

Conversation

@4z3

@4z3 4z3 commented Oct 21, 2026

Copy link
Copy Markdown

This PR fixes an issue where a message longer than one chunk (1300 bytes) is never delivered if its last chunk is smaller than MINIMUM_CHUNK_SIZE (250 bytes). The receiver drops that chunk as "too small", the sender retransmits it until the message's deadline passes, and the message stays in state sending on the sender.

Reproduction

Two nodes A and B can reach each other; B's public key is $B_PK.

# On A: send a 1529 byte message (1300 + 229 bytes)
payload=$(head -c 1529 /dev/urandom | base64 -w0)
topic=$(printf 'repro' | base64 -w0)
curl -Ss http://127.0.0.1:8989/api/v1/messages -H content-type:application/json \
    -d "{\"dst\":{\"pk\":\"$B_PK\"},\"topic\":\"$topic\",\"payload\":\"$payload\"}"
# -> {"id":"<id>"}
curl -s http://127.0.0.1:8989/api/v1/messages/status/<id>

The status never leaves sending:

{"dst":"$B_PK","state":{"sending":{"pending":0,"sent":1,"acked":1}},"created":1792593683,"deadline":1792593983,"msgLen":1529}

On B, with debug logging, every retransmission of the second chunk is dropped:

Dropping CHUNK 1/6 which is too small (229 bytes / 250 bytes)

and GET /api/v1/messages on B never returns the message. With a payload of 1550
bytes (last chunk 250 bytes), or anything up to 1300 bytes, the same steps deliver the
message at once.

Cause

In mycelium/src/message.rs, MessageStack::handle_message, chunk branch:
max_chunk_idx is the highest index possible if every chunk had the minimum size, which is a good bound against a flood of tiny chunks. But it is also used to recognize the last chunk, and senders split messages at AVERAGE_CHUNK_SIZE (1300 bytes), so the last chunk's index is len.div_ceil(1300) - 1. For 1529 bytes that's index 1, while max_chunk_idx is 6, so the 229 byte last chunk doesn't get the exception.

Fix

REcognize the last chunk by where it ends, not by its index: a chunk is the last one if
chunk_offset + chunk_size == message.len. The index bound stays as it is.

The fix has been implemented with three commits.

  1. e51c6d9 adds only a test case showing the issue
  2. 24e0056 adds the minimal fix
  3. b848b71 refactors the check so the test added in (1) can be simplified. It also prevents a potential overflow when computering the chunk end and fixes the misleading debug message Dropping CHUNK 1/6 which is too small (229 bytes / 250 bytes) when there really where just two chunks in total.

If you prefer a single commit instead of this staggering, we can squash these commits.

@4z3
4z3 requested a review from LeeSmet as a code owner October 21, 2026 15:33
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