Repository navigation
feat(control): verified application transfers over 0x50, and non-blocking UDP sockets - #136
Merged
Merged
Conversation
Event loops drain the gossip socket until recvFrom returns null, as Linux sockets (SOCK_NONBLOCK) do. On macOS/BSD the socket was blocking, so once any peer was live the loop parked in recvfrom after the last datagram and the control socket (status, APPSEND, APPRECV) stopped answering, in --gossip-only and macOS TUN modes alike.
Add MGXF1 transfer frames inside the authenticated 0x50 plaintext, so files follow the same direct, hole-punched, and relayed paths as application messages and work in --gossip-only mode. - OFFER/DATA/ACK/DONE/CANCEL with 960-byte chunks, cumulative plus 512-chunk selective acknowledgements, a congestion window (slow start, additive increase, halving on loss), Karn RTT estimation, RTO and fast retransmit. - The receiver verifies SHA-256 before DONE; the sender verifies staged bytes against the declared hash before sending. - Receivers accept only channels registered with XFERLISTEN, within per-file, slot, and memory limits (32 MiB, 8+8, 96 MiB per direction). A receiver that lost state answers CANCEL(unknown) and the sender re-offers once for all in-flight chunks. - Control commands XFERINFO/LISTEN/OFFER/PUT/START/STATUS/RECV/GET/ DONE/CANCEL; owner uid only on Unix, since the socket is 0666. APPINFO advertises "transfers":1. - 0x50 replay state is updated only after authentication; transfer frames stay out of the 128-entry nonce ring. - Gossip-only and macOS loops wake on the control socket (and TUN), so commands no longer wait out the 200 ms gossip poll. Two gossip-only daemons on Apple Silicon moved 10 MiB in 369 ms over loopback with the hash verified.
Replace the macOS-only fcntl change with the pilot's reviewed control fix (patch c2362fe0 against a0fa2f3): Winsock sockets use FIONBIO, macOS/BSD use F_GETFL/F_SETFL preserving flags, Linux keeps atomic SOCK_NONBLOCK, setup covers IPv4, IPv6, and connected sockets and closes the socket on failure, and IPv4/IPv6 empty-drain regression tests guard against the event loop parking in recvfrom again.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
Windows validation (x86_64, Windows 11 Pro)
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical replay, transfer-state, arithmetic, and lifecycle issues block approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 5
Open (5)
What changed in this PR
Adds verified bulk application transfers over authenticated 0x50 datagrams and improves cross-platform UDP responsiveness.
Changes:
- Adds
MGXF1framing, selective ACKs, congestion control, integrity verification, and bounded transfer state. - Adds owner-authorized
XFER*control commands and documentation. - Makes UDP sockets non-blocking across platforms and improves event-loop wakeups.
| File | Summary |
|---|---|
src/services/transfers.zig |
Transfer lifecycle, reliability, verification, and tests |
src/services/control.zig |
Transfer commands and authorization |
src/protocol/transfer.zig |
Binary transfer framing |
src/net/udp.zig |
Cross-platform non-blocking UDP |
src/main.zig |
Transfer dispatch, replay handling, and event loops |
src/lib.zig |
Public module exports |
src/discovery/swim.zig |
Configurable polling timeout |
docs/reference/app-transfers.md |
Transfer API and protocol documentation |
docs/reference/app-channels.md |
Transfer documentation link |
CHANGELOG.md |
Release notes |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // rules; keeping them out of this small ring stops bulk traffic from flushing | ||
| // the nonces that protect application messages. | ||
| const plaintext_slice = plaintext[0..payload_len]; | ||
| if (!lib.protocol.Transfer.looksLikeTransferFrame(plaintext_slice) and isDuplicateAppNonce(nonce)) return; |
Comment on lines
+262
to
+269
| if (offset + bytes.len <= out.staged) { | ||
| if (!std.mem.eql(u8, out.data[@intCast(offset)..][0..bytes.len], bytes)) return error.InvalidOffset; | ||
| return out.staged; | ||
| } | ||
| if (offset != out.staged or offset + bytes.len > out.data.len) return error.InvalidOffset; | ||
| @memcpy(out.data[@intCast(offset)..][0..bytes.len], bytes); | ||
| out.staged += bytes.len; | ||
| return out.staged; |
Comment on lines
+374
to
+377
| if (out.state != .sending and out.state != .staging and now_ns - out.finished_at > self.limits.retention_ns) self.freeOutgoing(slot, out); | ||
| if (out.state == .staging) { | ||
| // Staging has no clock of its own; start one at the first tick and drop abandoned uploads. | ||
| if (out.last_progress == 0) out.last_progress = now_ns else if (now_ns - out.last_progress > self.limits.retention_ns) self.freeOutgoing(slot, out); |
| return; | ||
| } | ||
| const reason: ?wire.Reason = blk: { | ||
| if (now_unix - o.created_unix > self.limits.max_offer_age_secs) break :blk .expired; |
| return; | ||
| } | ||
| // Acknowledge promptly on gaps and periodically otherwise. | ||
| if (index != in.cumulative - 1 or in.unacked >= ACK_EVERY) { |
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.
What
Verified application transfers. Files that don't fit one application message (screenshots, logs, patches) can now be sent as a verified bulk transfer. Frames are
MGXF1binary plaintext inside the authenticated0x50envelope, so they take the same direct, hole-punched, and relayed paths asAPPSENDand work in--gossip-onlymode without TUN or admin rights.OFFER/DATA(960-byte chunks) /ACK/DONE/CANCEL. AnACKcarries a cumulative point plus a selective bitmap covering the next 512 chunks.DONE. The sender checks its staged bytes against the same hash before sending anything.XFERLISTEN. Limits: 32 MiB per file, 8 transfers each way, 96 MiB of memory each way. Offers older than one hour are refused, and a re-sent offer for a recently finished transfer is answered withDONErather than delivered twice.CANCEL(unknown)and the sender offers the file again once, not once per in-flight chunk.XFERINFO,XFERLISTEN,XFEROFFER,XFERPUT,XFERSTART,XFERSTATUS,XFERRECV,XFERGET,XFERDONE,XFERCANCEL. On Unix they require the daemon owner (same uid or root), because the control socket is world-writable and these commands carry file contents.APPINFOnow reports"transfers":1.docs/reference/app-transfers.md.Fixes
c2362fe0againsta0fa2f3). On macOS/BSD and Windows the gossip socket was blocking, and SWIM empties it by callingrecvFromuntil it returns null. So once any peer was live, the event loop blocked insiderecvfromafter the last datagram and the control socket stopped answering (status,APPSEND,APPRECV). Linux keepsSOCK_NONBLOCK; macOS/BSD usefcntland Windows usesFIONBIO. IPv4 and IPv6 regression tests cover the empty queue.0x50replay state is now updated only after a packet authenticates, so forged datagrams can't push real nonces out of the 128-entry ring. Transfer frames stay out of that ring, so bulk traffic can't flush it either.Validation
zig build test -Dno-sodium=true: macOS arm64 382/384 (2 skipped); Windows 190/191 (1 skipped).ControlSockets over real0x50encryption and checks that non-owners are refused.Remaining limits / follow-up
XFERPUT/XFERGETwasn't measured separately; the Windows run above used the pipe and was fine.0x50path, which doesn't know about transfers.