Repository navigation
Conversation
… rns-rs (leviculum#70)
Two nodes whose link runs through a transport node can now move that link
onto a direct UDP path through both NATs. Each side learns its reflexive
address from a facilitator, the pair swap addresses as channel signals on
the link they already share, and both punch at once. On success the link
moves onto a new point-to-point interface, and that interface announces
the node's destinations, so the path drops to one hop. On failure
(symmetric NAT, common on cellular) nothing changes. If the direct
interface dies later, the link goes back to the interface it had.
The wire protocol is rns-rs's DirectLink, implemented independently:
channel signals 0xFE00..=0xFE04 with the same msgpack bodies, RNSP and
STUN probes, the RNSH/RNSA punch frames, and the HKDF punch token. rns-rs
is under the Reticulum License, whose extra conditions AGPL code cannot
take on, so none of its source is copied. Its encoder supplied golden
vectors, which match byte for byte. Interop was verified against rns-rs
0.7.3 (98205ea) in all three combinations: rns-rs rnsd as relay and
facilitator, an rns-rs peer accepting a leviculum proposal, and an rns-rs
peer proposing to leviculum. Each time, data crossed the punched path
after the relay was killed.
- core: direct_link::{wire, session} (pure codecs and a sans-I/O state
machine) and node::direct_link (policy, signals taken off the channel
before the application sees them, a job queue for the driver, link
re-attachment and fallback). New NodeEvents DirectLinkEstablished,
DirectLinkFailed and DirectLinkLost.
- std: probe client, punch loop, a facilitator answering RNSP and STUN,
and interfaces::direct_udp. The direct interface is leaf-only, sends a
keepalive every 30 s, goes down after 120 s of silence, and answers a
late punch so a peer whose last ack was lost still completes.
- Off by default: policy `reject`, no facilitator. Config keys probe_port,
probe_addr, probe_protocol and direct_connect_policy, or
ReticulumNodeBuilder::direct_link. ReticulumNode::propose_direct_link
starts an upgrade. An upgrade is never automatic: a Python RNS peer
proves the signal and then stalls its channel on the unknown MSGTYPE.
Gate bookkeeping:
- scripts/ignored-counts.txt pins 3 ignored tests in direct_link_upgrade.
They are the rns-rs interop arms. No tier can run them, because each
needs an rns-rs process started beside it. They are run by name with
`-- --ignored` and the LEVICULUM_INTEROP_* variables their doc comments
give. The leviculum-only arms in that file run in every tier.
- scripts/core-lock-census.txt adds propose_direct_link and
direct_link_interface, plus known_destination_evictions (leviculum#49),
which should have been added when that method landed.
- Fork additions sit at the ends of upstream files where they can, so few
upstream lines move. The doc citations they did move (config.rs,
builder.rs, driver/mod.rs) are corrected. The doc-citation guard reports
no drift beyond what main already has.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bf7bf88fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if crate::direct_link::wire::is_signal_msgtype(envelope.msgtype) { | ||
| direct_link_signals.push((envelope.msgtype, envelope.data)); |
There was a problem hiding this comment.
This classification occurs only after handle_channel_packet has returned early when channel_delivery_budget == Some(0), and a successfully classified signal still decrements that application-event budget below. Consequently, when the data event queue is full, REQUEST/ACCEPT/READY signals—which never enter that queue—remain unprocessed and unproved; the proposal times out and retransmissions can exhaust the existing channel. Direct-link system messages need to be admitted without consuming application delivery capacity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. When the sink is full, the packet is decrypted and its envelope header checked. If it's a direct-link signal and the next message in sequence, it's admitted; anything else is still refused unproved. Signals are no longer charged to the delivery budget in either the in-order or the drain path. Test: a_full_application_sink_does_not_hold_back_signals.
| let Some(attachment) = self.direct_links.attached.remove(&index) else { | ||
| return; | ||
| }; |
There was a problem hiding this comment.
When the original relayed interface goes down while the upgraded direct interface is still active, attached is keyed by the direct interface, so this lookup returns and leaves Attachment::previous pointing at the removed relay index. If the direct interface later dies, direct_link_interface_down restores that stale index and reports DirectLinkLost as though fallback succeeded, causing subsequent link packets to target an unregistered interface instead of closing the link or selecting a viable route.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. When an interface goes down, it's removed as any attachment's fallback. If the direct path later dies with no fallback, the link closes as Stale (DirectLinkLost, then LinkClosed) instead of being pinned to a removed index. Test: a_direct_link_whose_fallback_died_closes_when_the_direct_path_dies.
| pub(crate) async fn resolve_facilitator(addr: &str) -> io::Result<SocketAddr> { | ||
| tokio::net::lookup_host(addr) | ||
| .await? | ||
| .find(SocketAddr::is_ipv4) |
There was a problem hiding this comment.
For an IPv6 literal or IPv6-only facilitator hostname, this filter discards every resolved address and propose_direct_link always returns NotFound. The rest of the new implementation explicitly supports IPv6 endpoints, IPv6 probe sockets, STUN/RNSP responses, and punch traffic, so selecting only IPv4 unnecessarily makes that supported path unusable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. IPv4 is preferred; with no IPv4 address, the first resolved address (IPv6) is used.
| /// who learns its public address. Anything else stays tolerated and ignored. | ||
| pub(crate) fn apply_ini_key(config: &mut crate::config::ReticulumConfig, key: &str, value: &str) { | ||
| match key { | ||
| "probe_port" => config.direct_link.probe_port = value.trim().parse().ok(), |
There was a problem hiding this comment.
For an INI configuration such as probe_port = 434x or an out-of-range port, .parse().ok() silently converts the configured value to None; DirectLinkSettings::from_config then accepts it and the event loop treats the facilitator as disabled. This contradicts the stated fail-fast behavior for these keys and leaves an operator with a successfully started node that never listens for probes, so the parse failure needs to reach configuration validation rather than becoming an absent optional value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. A non-parsing INI probe_port is kept and refused by DirectLinkSettings::from_config as a config error. Test: a_malformed_probe_port_is_a_config_error (covers 434x and 70000).
| // Signals no MTU, like UDPInterface (Codeberg #357). The link | ||
| // moved onto this interface keeps the MTU it negotiated. | ||
| hw_mtu: None, |
There was a problem hiding this comment.
A link established over the documented TCP-relay topology commonly negotiates TCP's 16,384-byte hardware MTU, and this code deliberately preserves that MTU after moving the link. The direct receiver, however, has only a 1,064-byte datagram buffer, so channel messages and resource parts valid under the negotiated link MDU are sent as large UDP datagrams and then truncated before parsing (or fail to send above UDP's limit). The loopback test only sends tiny strings; normal large messages and resources over a TCP-origin link will fail after a successful upgrade unless the link is constrained to the direct interface's actual datagram ceiling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. On attach, both ends lower the link MTU to DIRECT_LINK_MTU (1400, the same value rns-rs uses, so a mixed pair agrees on the MDU). The direct interface now reads datagrams up to 64 KiB, so a part already in flight at the old size isn't truncated. Test: moving_onto_a_direct_interface_lowers_a_large_link_mtu_on_both_ends.
| impl Drop for DirectLinkRuntime { | ||
| fn drop(&mut self) { | ||
| for task in self.tasks.values().chain(self.interfaces.values()) { | ||
| task.abort(); | ||
| } |
There was a problem hiding this comment.
On ReticulumNode::stop, dropping this runtime aborts the direct interface tasks but never reports those interfaces down or abandons in-flight sessions in NodeCore. Starting the same node again therefore leaves established links attached to an interface that is not recreated (and whose numeric ID may be reused by an unrelated dynamic interface), while in-flight sessions retain jobs whose sockets were destroyed. Shutdown must reconcile the core's direct-link attachments and sessions before these tasks are aborted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. New NodeCore::abandon_direct_links, called at event-loop start next to abandon_resource_assemblies. It returns attached links to their fallback (or closes them if there is none), fails in-flight sessions, and drops stale jobs. Test: a_restarted_driver_abandons_every_direct_link_and_session.
| last_heard = Instant::now(); | ||
| let frame = &buf[..len]; | ||
| if wire::punch_frame_kind(frame).is_some() { | ||
| if let Some((PunchKind::Punch, seq)) = | ||
| wire::parse_punch_frame(frame, &session, &token) |
There was a problem hiding this comment.
The inactivity clock is refreshed before the frame's session and token are authenticated. Thus any punch-shaped datagram from the expected socket address—even one from a stale session with the wrong token, or one with a spoofed source tuple—is discarded below but still prevents INACTIVITY_TIMEOUT from firing. This can leave the link pinned to a dead direct interface instead of falling back; punch control traffic should count as liveness only after parse_punch_frame succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9481753. A punch-shaped frame refreshes liveness only after parse_punch_frame validates its session and token.
…estart, liveness, config - P1: a link that came up over TCP may have negotiated far more than one UDP datagram carries. Both ends lower the link MTU to DIRECT_LINK_MTU (1400, as rns-rs does) when the link moves onto the direct interface, and the interface reads datagrams up to 64 KiB, so a part already in flight at the old size is not truncated. - A full application sink no longer holds back signals. If the next in-order channel message is a direct-link signal it is admitted, and signals stop being charged to the delivery budget. - When a link's relayed fallback interface goes down while the link is direct, the fallback is forgotten. If the direct path then dies too, the link closes as Stale instead of being pinned to a removed index. - NodeCore::abandon_direct_links, called when the event loop starts, drops sessions and direct attachments left from a previous run of the node (stop, then start). - The direct interface counts a punch frame as liveness only once its session and token check out. - An IPv6-only facilitator resolves (IPv4 still preferred). A malformed INI probe_port is a config error instead of quietly running no facilitator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9481753e3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !self.send_direct_link_signal(&link_id, &signal, now_ms) { | ||
| self.end_direct_link_session(link_id, Some(Failure::Timeout)); | ||
| return; |
There was a problem hiding this comment.
When the responder receives a REQUEST shortly after sending any application message, send_system_raw can return the normal transient PacingDelay (and similarly Busy when the window is full), but this branch immediately destroys the upgrade session. Thus upgrades on an active bidirectional channel fail depending on message timing, while the initiator waits for its timeout and then enters the 60-second proposal cooldown. Preserve the session and retry the ACCEPT/READY/REJECT signal when the channel becomes writable instead of treating transient channel backpressure as a terminal timeout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43d3fb4. A Busy or PacingDelay from the channel now holds the signal in a per-link outbox (in order, at most 8 signals), and every tick flushes it. The session's phase timer still bounds the wait; only an inactive link or a hard channel error ends the session. The test a_busy_channel_delays_signals_instead_of_failing_the_upgrade saturates B's window, asserts the ACCEPT is held, then asserts it is flushed and both sides reach the punch.
| match attachment.previous { | ||
| Some(previous) => link.set_attached_interface(previous), |
There was a problem hiding this comment.
When only one direction of the UDP path fails, the endpoint that stops receiving eventually executes this fallback while its peer remains attached to the direct interface. Packets sent over the restored relay are accepted by that peer, but every reply is still routed onto its dead direct path; the fallback endpoint therefore receives nothing and its ordinary link stale timer can close the link well before the peer's 120-second direct-interface inactivity timer expires. Detect traffic arriving on the saved fallback (or otherwise notify the peer) so both endpoints switch paths together instead of turning a recoverable one-way failure into link closure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 43d3fb4. Your scenario turned out to be worse than described. On a fast link the link goes stale (keepalive x2, about 20 s) long before the direct interface's 120 s silence timer, so the fallback mostly never ran. Two parts to the fix. (1) A link that goes stale while it is on a direct interface falls back immediately: the direct interface is retired and a keepalive goes out on the relay. (2) A link packet that arrives over the relay the link left, more than FALLBACK_GRACE_MS (10 s) after the upgrade, means the peer has fallen back. The end that is still direct then follows it: it retires the interface and answers on the relay. The grace keeps relayed stragglers from the moment of the upgrade from triggering it. Both ends converge on the relay and the stale link recovers. Tests: a_link_gone_stale_on_its_direct_path_falls_back_and_nudges_the_peer and traffic_over_the_relay_after_the_grace_brings_a_direct_end_back.
…re, recover from one-way failure
- P2: a full channel window or channel pacing no longer fails the upgrade.
The signal waits in a per-link outbox (in order, at most 8) and is
flushed on every tick. The session's own phase timer still bounds the
wait. Only a dead link or a hard channel error ends it.
- P1: one-way failure of the direct path. Two things were wrong. The two
ends could end up on different paths. And on a fast link the link went
stale (about 20 s) long before the direct interface's 120 s silence
timer, so the fallback never got to run. Now:
- a link that goes stale while on a direct interface falls back at
once: the direct interface is retired, and a keepalive goes out over
the relay;
- a link packet arriving over the relay the link left, more than
FALLBACK_GRACE_MS (10 s) after the upgrade, means the peer has fallen
back, so this end follows (interface retired, keepalive sent on the
relay).
Both ends converge on the relay, and the stale link recovers instead of
closing.
Tests: the backpressure test saturates the channel and asserts the
ACCEPT was held and then flushed. The stale-fallback test checks the
nudge goes out on the relay. The return test checks a packet inside the
grace is ignored and one after it brings the end back.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43d3fb4e64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.direct_link_inbound( | ||
| &LinkId::new(packet.destination_hash), | ||
| interface_index, | ||
| now_ms, | ||
| ); |
There was a problem hiding this comment.
When any non-link-request packet naming an attached link arrives on its saved relay interface after the grace period, this invokes direct_link_inbound before the proof or encrypted payload is validated by the context-specific handler. An invalid or forged packet from a party able to inject traffic onto that relay can therefore make the node close a healthy direct interface and emit DirectLinkLost, even though the packet is subsequently rejected. Trigger fallback only after the packet has authenticated successfully.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cae1431. This went one layer deeper than the call order. Most link handlers record liveness before they decrypt, so neither "the handler ran" nor "last_inbound moved" is evidence the packet came from the peer. The fallback trigger is now direct_link_authenticated, called from the channel path only after the packet has decrypted under the link key. process_link_packet only records which interface the packet arrived on. Keepalives, which aren't encrypted, never count. The fallback nudge is therefore authenticated too: alongside the keepalive it now sends COMPLETE for the session that made the link direct. The peer ignores it as a protocol message and acts on where it arrived from. Test: the_stale_ends_nudge_alone_brings_the_peer_back_and_a_forged_keepalive_does_not shows that a forged keepalive and a tampered channel packet on the relay leave the link direct, while the stale end's nudge alone brings the peer back.
| protocol, | ||
| now_ms, | ||
| ); | ||
| self.direct_links.sessions.insert(link_id, session); |
There was a problem hiding this comment.
When two accepted peers deliberately submit the same peer-controlled session ID on different links, this per-link insertion allows both sessions to coexist. The std runtime keys sockets, tokens, and tasks only by SessionId, so starting the second probe overwrites the first socket and aborts its task, while link_of_session later routes outcomes to whichever matching link is encountered first. This lets one concurrent request corrupt or fail another; reject an ID already used anywhere in the node or include the link ID in the runtime key and outcomes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cae1431. A REQUEST whose session ID is already held by any session on the node is now refused with REJECT_BUSY, so a reused ID can no longer collide in the driver's session-keyed sockets and tasks. Test: a_session_id_in_use_on_another_link_is_refused (two links, the same ID, one probe).
| // leviculum#70: a punched socket's interface is live; move | ||
| // its link onto it. | ||
| if let Some(session) = direct_links.registered(InterfaceId(iface_idx)) { | ||
| let output = inner | ||
| .lock_recover() |
There was a problem hiding this comment.
When this endpoint finishes punching before its peer, the registration branch calls handle_interface_up and sends all cached destination announces before direct_link_punched attaches the link. The peer can still be inside punch(), where every non-punch datagram is discarded, so these one-shot announces are lost; in particular, if the initiator finishes first, the later responder has no path request or other mechanism that restores routes to the initiator's destinations. Synchronize readiness or repeat the announces after both endpoints can process Reticulum traffic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cae1431. A new direct interface now repeats its announces 1 s and 4 s after registration, as long as it's still live (DirectLinkRuntime::take_due_reannounces, run from the timer branch). The peer finishes punching within about a round trip of us, and our direct interface acks any late punch, so the first repeat normally lands; the second covers a slow peer. Test: a_new_direct_interface_announces_again_while_it_lives.
…wide session ids, repeated announces - Fallback now triggers only on authenticated traffic. Leaving the direct path because of a packet over the relay was decided before the packet was validated, so anyone able to inject onto the relay could pull a node off a working direct link. Most link handlers record liveness before they decrypt, so liveness is no evidence. The trigger is now a channel packet that has decrypted under the link key (direct_link_authenticated, fed by the receiving interface that process_link_packet notes). The fallback nudge is therefore authenticated too: besides the keepalive, which helps a stale link recover but proves nothing, it sends COMPLETE for the session that made the link direct. The peer ignores COMPLETE as a protocol message (it no longer holds that session) and acts on where it arrived from. Signals may go out on a Stale link. - A REQUEST whose session id is already in use anywhere on the node is refused as BUSY, not just one on the same link. The driver keys sockets and tasks by session id, so a reused id could take over another link's probe. - A new direct interface repeats its announces 1 s and 4 s after registration. The first can go out while the peer is still inside its punch loop, which discards everything but punch frames. That would leave the peer without a route to this node's destinations. Tests: a forged keepalive and a tampered channel packet on the relay leave the link direct, while the stale end's nudge alone brings the peer back; a reused session id starts no second probe; the repeat schedule fires for live interfaces only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cae14313f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1662,15 +1693,25 @@ impl<R: CryptoRngCore, C: Clock, S: Storage> NodeCore<R, C, S> { | |||
| .map(|ch| ch.drain_received_limit(drain_limit)) | |||
There was a problem hiding this comment.
When the delivery budget is zero and an in-order signal closes a sequence gap, drain_received_limit(0) leaves any following ACCEPT or READY that was already buffered untouched. Its retransmissions remain duplicates, and every subsequent drain still has a zero limit, so the signal is neither handled nor proved until application capacity returns and the upgrade can time out despite protocol messages requiring no sink capacity. Fresh evidence beyond the earlier backpressure report is this buffered-signal path: the new direct-delivery exception applies only before receive, not while draining the receive ring.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 06d7697. The drain now uses Channel::drain_received_budgeted(limit, is_signal_msgtype): signals behind a gap drain without counting against the host limit. Draining still stops at the first application message once the limit is spent, so order holds. Test: a_buffered_signal_drains_past_a_spent_limit_and_order_holds.
| fn end_direct_link_session(&mut self, link_id: LinkId, failure: Option<Failure>) { | ||
| let Some(session) = self.direct_links.sessions.remove(&link_id) else { | ||
| return; |
There was a problem hiding this comment.
If a REQUEST remains queued under Busy or PacingDelay for longer than the proposal timeout, polling ends the session here but leaves its outbox entry intact. Once the channel becomes writable, flush_direct_link_outbox sends that expired REQUEST anyway; the peer then accepts it and starts probing/punching for a session the initiator has already released, wasting the peer's socket and holding its link busy until another timeout. Failed sessions should cancel their queued handshake signals rather than allowing them to outlive the session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 06d7697. end_direct_link_session now drops that session's signals from the link's outbox, so a REQUEST that outlived its proposal timeout is never sent. Test: an_expired_sessions_held_request_is_never_sent.
| None => { | ||
| let is_initiator = link.is_initiator(); | ||
| let destination = *link.destination_hash(); | ||
| link.close(); | ||
| self.remove_link(&attachment.link_id); |
There was a problem hiding this comment.
When the relay interface has already gone down, Attachment::previous is None; if the link later goes stale while the direct interface task is still running, this branch removes the link without enqueueing CloseInterface. Because direct_link_link_stale removed the attachment before calling this function, remove_link cannot discover and close that interface either. The punched socket and registered interface can therefore remain alive indefinitely while the peer keeps sending authenticated keepalives, even though their link no longer exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 06d7697. The no-fallback branch of release_direct_attachment now queues CloseInterface for the direct interface before closing the link. Test: a_link_with_no_fallback_that_goes_stale_retires_its_direct_interface.
| // The direct path just proved itself both ways; do not let a stale | ||
| // timer that was running on the relayed path fire against it. | ||
| link.record_inbound(now_ms / MS_PER_SECOND); |
There was a problem hiding this comment.
If the relayed link becomes Stale while the asynchronous punch is still running, a later successful punch reaches this attachment path, but record_inbound only updates the timestamp and does not change LinkState::Stale back to Active. An otherwise idle upgraded link therefore emits DirectLinkEstablished while remaining stale, sends no normal link keepalives, and is closed by the stale watchdog after its refreshed grace period unless another valid link packet happens to arrive. The authenticated bidirectional punch should recover the state just as valid inbound link traffic does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 06d7697. On attach, a Stale link is set back to Active and LinkRecovered is emitted alongside DirectLinkEstablished, since a punch that went through both ways is the peer answering. Test: a_link_that_went_stale_during_the_punch_is_live_again_when_it_lands.
…box, orphaned interface, stale recovery - A signal buffered behind a sequence gap now drains even when the host sink is full (Channel::drain_received_budgeted, in a child module so channel/mod.rs moves no cited line). Before, only an in-order signal got past the full-sink refusal, so an ACCEPT or READY behind a gap sat unhandled and unproved until the application drained, and the upgrade timed out. Order still holds: draining stops at the first application message once the limit is spent. - When a session ends, its signals still waiting in the outbox go with it. Before, a REQUEST that waited out its proposal timeout was sent once the channel opened, and the peer started on a session nobody held. - A link that closes with no fallback left also retires its direct interface. The attachment was already removed, so remove_link could not find the interface, and the punched socket lived on while the peer kept it alive. - A punch that lands on a link that went stale meanwhile makes it Active again and emits LinkRecovered. Before, it stayed Stale and the stale watchdog closed a working link. Tests: the budgeted drain past a spent limit, with order preserved; an expired session's held REQUEST is dropped; an orphaned direct interface is retired; a stale link recovers on attach. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d7697502
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A link that came up over TCP may have negotiated far more than a | ||
| // UDP datagram on the open internet carries. Both ends lower it to | ||
| // the same ceiling here, as rns-rs does, so they keep agreeing on the | ||
| // link MDU (leviculum#70 review). Never raised. | ||
| link.lower_mtu(crate::direct_link::DIRECT_LINK_MTU); |
There was a problem hiding this comment.
When an upgrade completes during an outgoing resource created under a larger negotiated MTU, this only lowers the Link's value; the resource has already stored its old sdu and pre-cut parts in OutgoingResource::new_with_flags (resource/outgoing.rs:760-761). Fresh evidence beyond the earlier MTU report is this persistent resource state: a valid 262,144-byte local-link resource continues emitting roughly 262 KB parts after attachment, while direct_io_task passes each part to one UDP send_to, which must fail above UDP's payload limit. Small punch/link keepalives can keep the direct path alive, so the resource repeatedly loses its parts instead of triggering fallback. Delay attachment until such transfers finish, or restart/rechunk them at DIRECT_LINK_MTU.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ed7e5d0 with the first option. When a punch lands while a resource is in flight on the link (outgoing, incoming, or in off-lock assembly), the link stays on the relay and is recorded as deferred. The tick moves it once the transfer is finished, and DirectLinkEstablished is emitted then. If the interface dies, the link closes, or the node restarts first, the upgrade ends as DirectLinkFailed and the interface is closed. Test: a_punched_link_with_a_transfer_under_way_moves_once_it_is_done.
| plaintext[0], | ||
| plaintext[1], | ||
| ])) | ||
| && u16::from_be_bytes([plaintext[2], plaintext[3]]) == next_rx; |
There was a problem hiding this comment.
When a direct-link signal was delivered but its proof was lost, a retransmission has a sequence behind next_rx; if the application sink is then full, this equality rejects the duplicate before Channel::receive can return AlreadyDelivered and rebuild its proof. Fresh evidence beyond the earlier backpressure fixes is this lost-proof retransmission path: the signal itself costs no sink capacity, but every retry remains unproved until the sender exhausts its channel retries and can close the link. Admit already-delivered direct-link signal sequences as well as the next sequence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ed7e5d0. The full-sink check now admits any direct-link signal, whatever its sequence. The next one is delivered, one ahead is buffered and later drained past the limit, and a duplicate gets AlreadyDelivered and is re-proved. Application messages are still refused. Test: a_full_sink_still_reproves_a_duplicate_signal.
| // REQUEST sent after the initiator gave up starts the peer on a | ||
| // session nobody holds (Codex review on #74). | ||
| if let Some(queue) = self.direct_links.outbox.get_mut(&link_id) { | ||
| queue.retain(|signal| signal.session() != session.id()); |
There was a problem hiding this comment.
When a responder's facilitator probe fails while the channel returns Busy or PacingDelay, Session::probe_failed produces Send(REJECT_UNSUPPORTED) followed by Failed(ProbeFailed): the first step queues the rejection, then end_direct_link_session immediately removes it here because it belongs to the ending session. Fresh evidence beyond the expired-request cleanup is this terminal-signal path, where the queued message is not stale handshake initiation; deleting it forces the initiator to wait for its READY timeout instead of receiving the explicit failure the state machine promises.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ed7e5d0. When a session ends, the outbox cleanup now keeps a queued Reject (the session's last word) and drops only stale handshake signals. Test: a_responders_queued_reject_survives_its_session (with B's channel saturated and its probe failed, the REJECT stays queued and the stale ACCEPT doesn't).
| let Some(attachment) = self.direct_links.attached.remove(&index) else { | ||
| return; | ||
| }; | ||
| self.release_direct_attachment(index, attachment, "direct interface lost", false); |
There was a problem hiding this comment.
On a one-way failure when the inherited link stale interval exceeds the direct interface's 120-second inactivity timeout—for example, a relayed RTT around 300 ms or a keepalive override above 60 seconds—the UDP task disconnects first and reaches this path with retire = false. This moves only the local endpoint back to the relay and sends no authenticated COMPLETE nudge; the peer remains attached to its working transmit direction, while unauthenticated link keepalives arriving over the relay refresh its link liveness without triggering direct_link_inbound. Fresh evidence beyond the earlier coordinated-fallback report is this timeout ordering through direct_link_interface_down: the local endpoint can go stale and close before the peer's UDP interface times out, so an otherwise recoverable link is lost. Send the authenticated fallback notification on this path too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ed7e5d0. Every fallback that has a relay to return to now sends the authenticated nudge (keepalive plus COMPLETE). That includes direct_link_interface_down, where the direct interface's own inactivity timer fires first. retire now decides only whether the interface also gets closed. Test: a_dead_direct_interface_nudges_the_peer_over_the_relay.
…sfers finish first, signals past a full sink, terminal REJECT kept - P1: every fallback to the relay now sends the peer the authenticated nudge (keepalive plus COMPLETE), including when the direct interface's own inactivity timer fires first. Before, that path moved only this end back. The peer stayed on its working direction, kept alive by unauthenticated keepalives over the relay, while this end's link went stale and closed. - A punched link with a resource transfer under way, in either direction or in off-lock assembly, stays on the relay until the transfer is done, then moves on the next tick. A transfer cut for the link's earlier, larger MTU can carry parts no UDP datagram can. If the interface dies or the node restarts while the link waits, the upgrade is reported failed. - A full host sink no longer refuses any direct-link signal, whatever its sequence. A duplicate of a delivered one is re-proved, so a lost proof no longer costs the sender its channel retries. Application messages are still refused. - When a session ends, a REJECT queued in the outbox stays. It is the session's last word, and the initiator should hear it rather than time out. Tests: a dead direct interface nudges over the relay; a punched link waits out a transfer and then moves; a full sink re-proves a duplicate REQUEST; a responder's queued REJECT outlives its session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…estart, liveness, config - P1: a link that came up over TCP may have negotiated far more than one UDP datagram carries. Both ends lower the link MTU to DIRECT_LINK_MTU (1400, as rns-rs does) when the link moves onto the direct interface, and the interface reads datagrams up to 64 KiB, so a part already in flight at the old size is not truncated. - A full application sink no longer holds back signals. If the next in-order channel message is a direct-link signal it is admitted, and signals stop being charged to the delivery budget. - When a link's relayed fallback interface goes down while the link is direct, the fallback is forgotten. If the direct path then dies too, the link closes as Stale instead of being pinned to a removed index. - NodeCore::abandon_direct_links, called when the event loop starts, drops sessions and direct attachments left from a previous run of the node (stop, then start). - The direct interface counts a punch frame as liveness only once its session and token check out. - An IPv6-only facilitator resolves (IPv4 still preferred). A malformed INI probe_port is a config error instead of quietly running no facilitator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
…re, recover from one-way failure
- P2: a full channel window or channel pacing no longer fails the upgrade.
The signal waits in a per-link outbox (in order, at most 8) and is
flushed on every tick. The session's own phase timer still bounds the
wait. Only a dead link or a hard channel error ends it.
- P1: one-way failure of the direct path. Two things were wrong. The two
ends could end up on different paths. And on a fast link the link went
stale (about 20 s) long before the direct interface's 120 s silence
timer, so the fallback never got to run. Now:
- a link that goes stale while on a direct interface falls back at
once: the direct interface is retired, and a keepalive goes out over
the relay;
- a link packet arriving over the relay the link left, more than
FALLBACK_GRACE_MS (10 s) after the upgrade, means the peer has fallen
back, so this end follows (interface retired, keepalive sent on the
relay).
Both ends converge on the relay, and the stale link recovers instead of
closing.
Tests: the backpressure test saturates the channel and asserts the
ACCEPT was held and then flushed. The stale-fallback test checks the
nudge goes out on the relay. The return test checks a packet inside the
grace is ignored and one after it brings the end back.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
…wide session ids, repeated announces - Fallback now triggers only on authenticated traffic. Leaving the direct path because of a packet over the relay was decided before the packet was validated, so anyone able to inject onto the relay could pull a node off a working direct link. Most link handlers record liveness before they decrypt, so liveness is no evidence. The trigger is now a channel packet that has decrypted under the link key (direct_link_authenticated, fed by the receiving interface that process_link_packet notes). The fallback nudge is therefore authenticated too: besides the keepalive, which helps a stale link recover but proves nothing, it sends COMPLETE for the session that made the link direct. The peer ignores COMPLETE as a protocol message (it no longer holds that session) and acts on where it arrived from. Signals may go out on a Stale link. - A REQUEST whose session id is already in use anywhere on the node is refused as BUSY, not just one on the same link. The driver keys sockets and tasks by session id, so a reused id could take over another link's probe. - A new direct interface repeats its announces 1 s and 4 s after registration. The first can go out while the peer is still inside its punch loop, which discards everything but punch frames. That would leave the peer without a route to this node's destinations. Tests: a forged keepalive and a tampered channel packet on the relay leave the link direct, while the stale end's nudge alone brings the peer back; a reused session id starts no second probe; the repeat schedule fires for live interfaces only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
…box, orphaned interface, stale recovery - A signal buffered behind a sequence gap now drains even when the host sink is full (Channel::drain_received_budgeted, in a child module so channel/mod.rs moves no cited line). Before, only an in-order signal got past the full-sink refusal, so an ACCEPT or READY behind a gap sat unhandled and unproved until the application drained, and the upgrade timed out. Order still holds: draining stops at the first application message once the limit is spent. - When a session ends, its signals still waiting in the outbox go with it. Before, a REQUEST that waited out its proposal timeout was sent once the channel opened, and the peer started on a session nobody held. - A link that closes with no fallback left also retires its direct interface. The attachment was already removed, so remove_link could not find the interface, and the punched socket lived on while the peer kept it alive. - A punch that lands on a link that went stale meanwhile makes it Active again and emits LinkRecovered. Before, it stayed Stale and the stale watchdog closed a working link. Tests: the budgeted drain past a spent limit, with order preserved; an expired session's held REQUEST is dropped; an orphaned direct interface is retired; a stale link recovers on attach. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
…sfers finish first, signals past a full sink, terminal REJECT kept - P1: every fallback to the relay now sends the peer the authenticated nudge (keepalive plus COMPLETE), including when the direct interface's own inactivity timer fires first. Before, that path moved only this end back. The peer stayed on its working direction, kept alive by unauthenticated keepalives over the relay, while this end's link went stale and closed. - A punched link with a resource transfer under way, in either direction or in off-lock assembly, stays on the relay until the transfer is done, then moves on the next tick. A transfer cut for the link's earlier, larger MTU can carry parts no UDP datagram can. If the interface dies or the node restarts while the link waits, the upgrade is reported failed. - A full host sink no longer refuses any direct-link signal, whatever its sequence. A duplicate of a delivered one is re-proved, so a lost proof no longer costs the sender its channel retries. Application messages are still refused. - When a session ends, a REJECT queued in the outbox stays. It is the session's last word, and the initiator should hear it rather than time out. Tests: a dead direct interface nudges over the relay; a punched link waits out a transfer and then moves; a full sink re-proves a duplicate REQUEST; a responder's queued REJECT outlives its session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5
… telemetry (#77), the close-during-assembly pin Lands PRs #74, #75 and #76. - #74: direct-link upgrade, wire-compatible with rns-rs. Five Codex rounds. - #75: a test pinning the close-during-assembly invariant CIRISEdge relies on. - #76: link census, link list, lifecycle counters, the mirror-divergence alarm and the metrics facade. Four Codex rounds. The final fixes on #74 (round 5) and #76 (reject_link) answer Codex's own findings. Codex then hit its usage limit and could not review them; each has a test. Releasing on green CI was the maintainer's call. Integration: #74 and #76 overlapped only where both appended to the same places (NodeCore fields, ReticulumNode fields, lib.rs re-exports, the event loop's prologue and timer branch, the lock census). Each commit builds on its own. The apply_inbound citation in event_log.rs follows the combined driver. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ye3GBY8MB38EsYH5eaZKq
Closes #70.
What
Two nodes whose link runs through a transport node can move that link onto a direct UDP path punched through both NATs:
On success the link moves onto a new point-to-point interface. That interface announces each node's destinations, so the path drops from 2 hops to 1. On failure (symmetric NAT) nothing changes. If the direct path dies later, the link goes back to the relay.
Wire compatibility with rns-rs
This implements rns-rs's DirectLink protocol, independently:
0xFE00..=0xFE04with the same msgpack bodies.RNSH/RNSApunch frames.rns-rs is under the Reticulum License, whose extra conditions AGPL code can't take on, so none of its source is copied. Golden vectors from its encoder match ours byte for byte.
Interop verified against rns-rs 0.7.3 (
98205ea). In every case data crossed the punched path after the relay process was killed:rnsdlnsdlnsdShape
direct_link::{wire, session}: pure codecs and a sans-I/O state machine.node::direct_link: the policy, signals taken off the channel before the app sees them, a job queue for the driver, and moving the link onto the direct interface and back.DirectLinkEstablished,DirectLinkFailedandDirectLinkLost.interfaces::direct_udp: leaf-only, 30 s keepalive, 120 s dead timer. It answers a late punch, so a peer whose last ack was lost still completes.probe_port,probe_addr,probe_protocolanddirect_connect_policy(defaultreject), orReticulumNodeBuilder::direct_link.ReticulumNode::propose_direct_linkstarts an upgrade.Tests
#[ignore]d and pinned inscripts/ignored-counts.txt: they need an rns-rs process beside them.-D warnings. thumbv6m and thumbv7em builds pass.Pre-existing on main, not touched here
cargo clippy -p leviculum-ffi --all-targetsdoesn't compile. TheInterfaceStatusSnapshotinitializer inleviculum-ffi/src/diagnostics.rstests is missing two fields since3b940e73.scripts/check-core-lock-census.pycan't resolve theifac_*rotation methods. This PR adds its own methods, plusknown_destination_evictionsfrom Known-destinations capacity backpressure: retain-aware LRU eviction, merge must not resurrect (persist-pattern) #49.scripts/check-ignored-counts-source.py:transport_fanout_benchholds 8 ignored tests with no pin.🤖 Generated with Claude Code
https://claude.ai/code/session_01BXj7C4ZeFuczxxDVadAHZ5