Skip to content

Commit 289667d

Browse files
Add handshake retransmission on periodic announce
When an announce is received from a peer whose handshake is incomplete, retransmit the appropriate CONNECT packet: - IDLE/SYN_SENT: retransmit SYN - SYN_RECEIVED: retransmit SYN+ACK - CONFIRMED: retransmit ACK (helps peer stuck in SYN_RECEIVED) This handles all dropped packet scenarios by piggybacking on the ~500ms announce interval. No separate retransmission timer needed. Changes: - Discovery::process_announce now returns AnnounceResult with is_new and response_flags fields - NUClearNet uses the result to send CONNECT and force re-announce - Added unit tests for retransmission in each handshake state - Documented resilience behavior in nuclearnet.md
1 parent 3c475df commit 289667d

5 files changed

Lines changed: 169 additions & 15 deletions

File tree

docs/explanation/nuclearnet.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,50 @@ sequenceDiagram
336336
Duplicate or out-of-order handshake packets do not cause state regressions —
337337
once a peer reaches CONFIRMED, it stays there.
338338

339+
#### Handshake resilience
340+
341+
UDP packets can be dropped at any point in the handshake.
342+
Rather than adding a separate retransmission timer,
343+
the handshake piggybacks on the periodic announce cycle (~500ms):
344+
345+
Each time an announce is received from a peer whose handshake is incomplete,
346+
the appropriate CONNECT packet is retransmitted:
347+
348+
| Current state | Retransmit | Purpose |
349+
| ------------- | ---------- | ----------------------------------------------- |
350+
| IDLE | SYN | Initial SYN was never sent or was dropped |
351+
| SYN_SENT | SYN | Our SYN was dropped, retry |
352+
| SYN_RECEIVED | SYN+ACK | Our SYN+ACK was dropped, retry |
353+
| CONFIRMED | ACK | Our ACK was dropped, help peer finish handshake |
354+
355+
This provides automatic recovery for every drop scenario:
356+
357+
```mermaid
358+
sequenceDiagram
359+
participant A as Node A
360+
participant B as Node B
361+
362+
A->>B: ConnectPacket (SYN) [data port]
363+
Note over A: handshake = SYN_SENT
364+
Note over B: SYN dropped ✗
365+
366+
B-->>A: AnnouncePacket [multicast, periodic]
367+
Note over A: Peer not connected, retransmit
368+
A->>B: ConnectPacket (SYN) [data port, retransmit]
369+
370+
Note over B: Received SYN this time
371+
B->>A: ConnectPacket (SYN+ACK) [data port]
372+
Note over A: handshake = CONFIRMED ✓
373+
A->>B: ConnectPacket (ACK) [data port]
374+
Note over B: handshake = CONFIRMED ✓
375+
```
376+
377+
Since both peers announce periodically,
378+
a dropped packet is retried within at most one announce interval.
379+
If the data path is permanently broken in one direction,
380+
the handshake will never complete — which is correct,
381+
since bidirectional data connectivity is required for message exchange.
382+
339383
#### Connect packet
340384

341385
```mermaid

src/nuclearnet/Discovery.cpp

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,15 @@
9898
return packet;
9999
}
100100

101-
void Discovery::process_announce(const sock_t& source,
101+
Discovery::AnnounceResult Discovery::process_announce(const sock_t& source,
102102
const uint8_t* data,
103103
std::size_t length,
104104
std::chrono::steady_clock::time_point now) {
105+
AnnounceResult announce_result;
106+
105107
// Minimum size: header(5) + name_length(2) + num_subscriptions(2) = 9
106108
if (length < sizeof(PacketHeader) + sizeof(uint16_t) + sizeof(uint16_t)) {
107-
return;
109+
return announce_result;
108110
}
109111

110112
const uint8_t* ptr = data + sizeof(PacketHeader);
@@ -118,7 +120,7 @@
118120

119121
// Validate name fits
120122
if (remaining < name_len + sizeof(uint16_t)) {
121-
return;
123+
return announce_result;
122124
}
123125

124126
// Read name
@@ -128,7 +130,7 @@
128130

129131
// Ignore empty names
130132
if (name.empty()) {
131-
return;
133+
return announce_result;
132134
}
133135

134136
// Read subscription count
@@ -139,7 +141,7 @@
139141

140142
// Validate subscriptions fit
141143
if (remaining < num_subs * sizeof(uint64_t)) {
142-
return;
144+
return announce_result;
143145
}
144146

145147
// Read subscriptions
@@ -161,6 +163,7 @@
161163
auto it = peers.find(source);
162164
if (it == peers.end()) {
163165
// New peer — record with announce_heard = true
166+
announce_result.is_new = true;
164167
PeerInfo info;
165168
info.name = name;
166169
info.address = source;
@@ -193,6 +196,21 @@
193196
peer.subscriptions = std::move(subscriptions);
194197
subs_changed = true;
195198
}
199+
200+
// Determine retransmit flags based on handshake state
201+
switch (peer.handshake) {
202+
case HandshakeState::IDLE:
203+
case HandshakeState::SYN_SENT:
204+
announce_result.response_flags = SYN;
205+
break;
206+
case HandshakeState::SYN_RECEIVED:
207+
announce_result.response_flags = SYN | CON_ACK;
208+
break;
209+
case HandshakeState::CONFIRMED:
210+
// Send ACK to help the other side if they're stuck in SYN_RECEIVED
211+
announce_result.response_flags = CON_ACK;
212+
break;
213+
}
196214
}
197215
}
198216

@@ -216,6 +234,8 @@
216234
subscription_change_callback(info);
217235
}
218236
}
237+
238+
return announce_result;
219239
}
220240

221241
void Discovery::process_leave(const sock_t& source) {

src/nuclearnet/Discovery.hpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,23 @@ namespace network {
138138

139139
/**
140140
* Process a received announce packet from a peer.
141+
* Returns information about what action to take (send CONNECT, re-announce, etc.)
141142
*
142143
* @param source The UDP source address (IP + port) of the packet
143144
* @param data The raw packet data
144145
* @param length The length of the packet data
145146
* @param now The current time (defaults to steady_clock::now())
147+
*
148+
* @return Result indicating whether this is a new peer and what CONNECT flags to send
146149
*/
147-
void process_announce(const sock_t& source,
148-
const uint8_t* data,
149-
std::size_t length,
150-
std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now());
150+
struct AnnounceResult {
151+
bool is_new = false; ///< Whether this was a previously unknown peer
152+
uint8_t response_flags = 0; ///< CONNECT flags to send (0 = don't send)
153+
};
154+
AnnounceResult process_announce(const sock_t& source,
155+
const uint8_t* data,
156+
std::size_t length,
157+
std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now());
151158

152159
/**
153160
* Process a received leave packet from a peer.

src/nuclearnet/NUClearNet.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -436,18 +436,24 @@ namespace network {
436436
case ANNOUNCE: {
437437
// Check if this is a new peer before processing (which adds them)
438438
const bool is_new_peer = !discovery->has_peer(source);
439-
discovery->process_announce(source, data, length);
439+
auto announce_result = discovery->process_announce(source, data, length);
440440

441-
if (is_new_peer) {
441+
if (announce_result.is_new) {
442442
// Force an immediate announce to the multicast/broadcast group
443443
// so the new peer hears us on the announce channel (confirms our_d→their_a)
444444
announce();
445445
last_announce = std::chrono::steady_clock::now();
446+
}
447+
448+
// Send CONNECT packet if the handshake needs it (initial SYN or retransmit)
449+
if (announce_result.response_flags != 0) {
450+
auto pkt = Discovery::build_connect_packet(announce_result.response_flags);
451+
send_buf(data_fd, source, pkt.data(), pkt.size());
446452

447-
// Send SYN to the peer's data port to initiate the data path handshake
448-
auto syn = Discovery::build_connect_packet(SYN);
449-
send_buf(data_fd, source, syn.data(), syn.size());
450-
discovery->mark_syn_sent(source);
453+
// Mark SYN_SENT if we're sending a SYN (only advances from IDLE)
454+
if ((announce_result.response_flags & SYN) != 0) {
455+
discovery->mark_syn_sent(source);
456+
}
451457
}
452458
} break;
453459

tests/tests/nuclearnet/Discovery.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,3 +363,80 @@ SCENARIO("Discovery connection deferred until announce heard", "[nuclearnet][dis
363363
REQUIRE(join_called);
364364
REQUIRE(disc.is_connected(peer_addr));
365365
}
366+
367+
SCENARIO("Discovery retransmits SYN when announce received in SYN_SENT state", "[nuclearnet][discovery]") {
368+
Discovery disc(std::chrono::seconds(5));
369+
370+
sock_t peer_addr = make_addr(0x0A000001, 5000);
371+
372+
// First announce — new peer
373+
auto announce = Discovery::build_announce_packet("peer_a", {});
374+
auto result = disc.process_announce(peer_addr, announce.data(), announce.size());
375+
REQUIRE(result.is_new);
376+
377+
// We send SYN (externally) and mark state
378+
disc.mark_syn_sent(peer_addr);
379+
REQUIRE(disc.get_peer(peer_addr)->handshake == HandshakeState::SYN_SENT);
380+
381+
// SYN was dropped. Another announce arrives — should indicate SYN retransmit
382+
result = disc.process_announce(peer_addr, announce.data(), announce.size());
383+
REQUIRE_FALSE(result.is_new);
384+
REQUIRE(result.response_flags == SYN);
385+
}
386+
387+
SCENARIO("Discovery retransmits SYN+ACK when announce received in SYN_RECEIVED state", "[nuclearnet][discovery]") {
388+
Discovery disc(std::chrono::seconds(5));
389+
390+
sock_t peer_addr = make_addr(0x0A000001, 5000);
391+
392+
// Add peer via announce
393+
auto announce = Discovery::build_announce_packet("peer_a", {});
394+
disc.process_announce(peer_addr, announce.data(), announce.size());
395+
396+
// Peer sends SYN — we go to SYN_RECEIVED
397+
auto connect_result = disc.process_connect(peer_addr, SYN);
398+
REQUIRE(connect_result.response_flags == (SYN | CON_ACK));
399+
REQUIRE(disc.get_peer(peer_addr)->handshake == HandshakeState::SYN_RECEIVED);
400+
401+
// Our SYN+ACK was dropped. Another announce arrives — should indicate SYN+ACK retransmit
402+
auto result = disc.process_announce(peer_addr, announce.data(), announce.size());
403+
REQUIRE_FALSE(result.is_new);
404+
REQUIRE(result.response_flags == (SYN | CON_ACK));
405+
}
406+
407+
SCENARIO("Discovery retransmits ACK when announce received in CONFIRMED but peer not connected", "[nuclearnet][discovery]") {
408+
Discovery disc(std::chrono::seconds(5));
409+
410+
sock_t peer_addr = make_addr(0x0A000001, 5000);
411+
412+
// Add peer via announce and complete handshake
413+
auto announce = Discovery::build_announce_packet("peer_a", {});
414+
disc.process_announce(peer_addr, announce.data(), announce.size());
415+
disc.mark_syn_sent(peer_addr);
416+
auto connect_result = disc.process_connect(peer_addr, SYN | CON_ACK);
417+
REQUIRE(connect_result.just_connected);
418+
REQUIRE(disc.is_connected(peer_addr));
419+
420+
// Peer is fully connected — no retransmit needed
421+
auto result = disc.process_announce(peer_addr, announce.data(), announce.size());
422+
REQUIRE_FALSE(result.is_new);
423+
REQUIRE(result.response_flags == CON_ACK);
424+
}
425+
426+
SCENARIO("Discovery no retransmit for IDLE peer (not yet sent SYN)", "[nuclearnet][discovery]") {
427+
Discovery disc(std::chrono::seconds(5));
428+
429+
sock_t peer_addr = make_addr(0x0A000001, 5000);
430+
431+
// First announce — new peer, handshake IDLE
432+
auto announce = Discovery::build_announce_packet("peer_a", {});
433+
auto result = disc.process_announce(peer_addr, announce.data(), announce.size());
434+
REQUIRE(result.is_new);
435+
REQUIRE(disc.get_peer(peer_addr)->handshake == HandshakeState::IDLE);
436+
437+
// Second announce — peer still in IDLE (we haven't sent SYN yet)
438+
// Should indicate SYN needed
439+
result = disc.process_announce(peer_addr, announce.data(), announce.size());
440+
REQUIRE_FALSE(result.is_new);
441+
REQUIRE(result.response_flags == SYN);
442+
}

0 commit comments

Comments
 (0)