Repository navigation
Conversation
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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 statesendingon the sender.Reproduction
Two nodes A and B can reach each other; B's public key is
$B_PK.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:
and
GET /api/v1/messageson B never returns the message. With a payload of 1550bytes (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_idxis 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 atAVERAGE_CHUNK_SIZE(1300 bytes), so the last chunk's index islen.div_ceil(1300) - 1. For 1529 bytes that's index 1, whilemax_chunk_idxis 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.
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.