fix: NAT traversal hardening and 1000-node scale fixes

[mickvandijke]

Summary

Bundles NAT-traversal correctness fixes, a new observed-address cache, and two perf fixes targeting 1000-node routing contention, all against the rc-2026.4.1 release branch.

NAT traversal & dialable addresses

Root cause

DhtNetworkManager::local_dht_node() built its self-entry from the static NodeConfig::listen_addrs() derivation, which returns wildcard IPs (0.0.0.0 / ::) and the configured port (0 for ephemeral binds). Receivers' dialable_addresses() filter then rejected the entry, so DHT-based peer discovery returned zero contactable addresses for any node. Connections only succeeded via side channels (static bootstrap, in-memory cache, relay fallback) — exactly the sporadic-failure pattern reported by saorsa-node-lmdb.

Fix

local_dht_node() is now an async fn that sources its self-entry addresses from TransportHandle::observed_external_address() — the OBSERVED_ADDRESS reflexive address reported by peers — with the new observed-address cache as a fallback for the window immediately after a connection drop. The earlier iteration added a UdpSocket::connect-then-getsockname helper (primary_local_ip) to substitute wildcard IPs, but that was dropped in a follow-up commit because it ran blocking syscalls inside an async context; the OBSERVED_ADDRESS-only flow supersedes it and needs no interface probing.

Other NAT fixes bundled

• Bootstrap referrer threading — bootstrap_from_peers now records the queried bootstrap peer as the hole-punch coordinator referrer for every node it returned, completing the dial_candidate → set_hole_punch_preferred_coordinator chain the iterative-lookup path already used.
• Event-driven relay/address updates — Relay-established and peer-address-update events drain via a tokio::select! over TransportHandle::recv_relay_established() / recv_peer_address_update() instead of the previous 1-second polling loop, eliminating the race window where outbound DHT queries advertised no relay address for up to a second after relay setup.
• Identity-exchange timeout parity — IDENTITY_EXCHANGE_TIMEOUT and BOOTSTRAP_IDENTITY_TIMEOUT_SECS are both now 15 s (previously 5 s). Identity exchange (two RTTs + ML-DSA-65 signature, ~3.3 kB) blew through 5 s sporadically on congested cellular and cross-region links. Review noted the initial commit only bumped the bootstrap path; the non-bootstrap send_dht_request path is now on the same 15 s budget.
• Multi-address dial helper — dial_addresses() loops through every dialable address until one succeeds. Applied at 5 call sites (bootstrap_from_peers, spawn_bucket_refresh_task, trigger_self_lookup, find_closest_nodes_network parallel-query block, send_dht_request fallback) so a stale NAT binding on entry style: Fix formatting issues to resolve CI failures #1 no longer kills the dial when entry Comparison between saorsa and libp2p or iroh #2 would have worked.

Observed-address cache (new)

src/transport/observed_address_cache.rs (new, 372 lines) records OBSERVED_ADDRESS reflexive addresses against the local socket they were reported on, with a TTL and recency-based eviction. The cache is consulted by TransportHandle::observed_external_address() when no live connection currently carries a fresh reflexive address, so a node whose upstream connection just dropped can still publish a dialable self-entry for the few seconds before a new connection re-reports the address. Without this fallback the fix above degrades to "works only while you already have at least one live peer with recent OBSERVED_ADDRESS traffic."

1000-node scaling

Two independent contention points identified in 1000-node testnet traces where responses were missing the 25 s caller timeout ([STEP 6 FAILED] / Response channel closed (receiver timed out) cascade):

• perf(dht): atomic last_seen under a read lock — routing-table touch was called on every inbound DHT message and always took a RwLock::write(), serialising every read behind each update. NodeInfo.last_seen is now an AtomicInstant and there is a fast path that atomically bumps the timestamp under a read lock when the address merge would be a no-op (address absent, already present, or skipped by the loopback-injection rule). The write-lock slow path is still used when a real address merge is needed.
• perf(transport): shard the inbound message dispatcher — the previous single consumer task took three async write locks plus an awaited peer-id registration per message, so messages queued behind each other instead of being processed in parallel. The receive loop is now a light dispatcher that hashes the source IpAddr and hands off to one of 8 parallel shard consumers, each running the full parse/register/broadcast pipeline independently. Hashing by IpAddr (not full SocketAddr) keeps a peer pinned to the same shard across ephemeral-port reconnects so per-peer message ordering is preserved.

Regression tests

tests/dht_self_advertisement.rs (new, 649 lines) pins the published self-entry behaviour with three cases covering public-mode wildcard, port match, and local-mode ephemeral bind. All three FAILED on rc-2026.4.1 with concrete evidence (e.g. [Quic(0.0.0.0:0)] published as the only address; transport bound to port 55886 but DHT advertised port 0). All pass after this branch. They use only public APIs (DhtNetworkManager::find_closest_nodes_local_with_self) so no new test surface area was added to the library.

Out of scope

• The dead hole-punch chain in saorsa-transport's nat_traversal_api.rs is fixed in a separate commit in that crate.
• The --port 0 default in ant-node remains a deliberate operator-config decision.

Commits

• 08febcc fix: advertise dialable addresses to DHT and harden NAT traversal
• 4aeecf6 chore: drop needless struct update in IPDiversityConfig test
• a528dec fix: bump IDENTITY_EXCHANGE_TIMEOUT to 15s for slow-link parity
• 26eb128 fix: drop primary_local_ip and rely on OBSERVED_ADDRESS for self-entry
• 7086ccb feat: cache observed external addresses as fallback for dropped connections
• 0a854cd perf(dht): atomic last_seen to run touch under a read lock
• cf1f32d perf(transport): shard inbound message dispatcher by source IP

Test plan

• cargo test --lib
• cargo test --test dht_self_advertisement — 3/3 passing (new regression suite)
• cargo test --test two_node_messaging
• cargo test --test node_lifecycle
• cargo clippy --lib -- -D warnings — clean
• Re-run cargo test --lib and clippy after the two perf: commits
• Smoke test against saorsa-2.saorsalabs.com:10000 / saorsa-3.saorsalabs.com:10000 bootstrap nodes
• Confirm DHT-published self-entries contain dialable addresses on the testnet
• Validate 1000-node testnet run no longer hits the [STEP 6 FAILED] / Response channel closed cascade

Greptile Summary

This PR bundles a well-scoped set of fixes and performance improvements targeting DHT self-advertisement correctness, NAT traversal reliability, and routing-table write-lock contention at 1000-node scale.

Core changes:

• local_dht_node() is now async and sources self-entry addresses exclusively from TransportHandle::observed_external_addresses() (live QUIC OBSERVED_ADDRESS frames) and the runtime-bound listen address set, discarding the prior NodeConfig::listen_addrs() call that returned un-dialable wildcards/port-0 values
• New ObservedAddressCache (src/transport/observed_address_cache.rs) fills the "invisible window" between connection drop and reconnection with frequency+recency-aware address selection; multi-homed partitioned by (local_bind, observed) key
• Bootstrap referrer threading: bootstrap_from_peers now passes the bootstrap peer's socket address as hole-punch coordinator referrer to every dial_addresses call
• IDENTITY_EXCHANGE_TIMEOUT and BOOTSTRAP_IDENTITY_TIMEOUT_SECS unified at 15 s (was 5 s) to handle congested cellular / cross-region links
• NodeInfo.last_seen promoted to AtomicInstant (parking_lot::Mutex<Instant>) so the hot touch path can run under a read lock on the routing table instead of taking a write lock on every inbound DHT message
• Inbound message dispatcher sharded across 8 parallel consumer tasks keyed by source-IP hash, eliminating head-of-line blocking under 1000-node load
• Relay/address update consumption moved from a 1-second polling loop to an event-driven tokio::select! over recv_relay_established / recv_peer_address_update
• New regression suite tests/dht_self_advertisement.rs (649 lines) that confirmed the exact failure modes on the base branch and passes cleanly on this branch

Confidence Score: 5/5

Safe to merge — all remaining findings are P2 style suggestions with no production correctness impact

No P0 or P1 issues found. The two P2 comments flag a write-lock optimization opportunity in the shard consumer and the use of DefaultHasher (which is deterministic within a run and carries no correctness risk for a load-balancing shard key). The root-cause fixes are correct, the new ObservedAddressCache is well-designed and thoroughly tested, the regression suite concretely demonstrates the pre-fix failures, and the atomic touch path is a valid read-lock optimization with proper fallback. Prior concern about blocking I/O in local_dht_node() is fully resolved by the OBSERVED_ADDRESS-only approach.

src/transport_handle.rs — the shard consumer write-lock pattern is worth a follow-up optimization once the scalability impact is measured in production

Important Files Changed

Filename	Overview
src/transport/observed_address_cache.rs	New module: bounded (16-entry), frequency+recency-aware cache for QUIC OBSERVED_ADDRESS fallback; well-designed with per-local-bind partitioning for multi-homed safety; 11 deterministic unit tests cover all selection paths and eviction
src/dht_network_manager.rs	Core fix: local_dht_node() is now async and sources self-entry addresses from observed_external_addresses() and runtime listen_addrs; adds dial_addresses() multi-address retry helper (applied at 5 call sites); bootstrap_referrer threading corrected
src/transport_handle.rs	Single-consumer recv loop replaced by 8-shard parallel dispatcher keyed by source-IP hash; observed_address_cache field + accessor methods added; recv_relay_established / recv_peer_address_update event channels; write lock held across await in shard consumer (P2 style note)
src/transport/saorsa_transport_adapter.rs	Adds ADDRESS_EVENT_CHANNEL_CAPACITY bounded channel (256), drop-logging helper with coalesced warnings, and supporting constants for the sharded dispatcher infrastructure
src/network.rs	BOOTSTRAP_IDENTITY_TIMEOUT_SECS bumped to 15s matching IDENTITY_EXCHANGE_TIMEOUT; polling relay/address loop replaced with event-driven tokio::select! on recv_relay_established / recv_peer_address_update, eliminating 1-second race window
src/dht/core_engine.rs	AtomicInstant (parking_lot::Mutex) replaces plain Instant in NodeInfo.last_seen; touch_node_typed fast-path atomically bumps timestamp under read lock with no bucket mutation; escalates to write lock only when address merge is needed
tests/dht_self_advertisement.rs	New 649-line regression suite: 3 tests covering wildcard/public-mode, port-match, and local-mode ephemeral-bind self-advertisement contracts; all 3 failed on base branch (concrete evidence of the bug) and pass on this branch
src/dht/security_tests.rs	Minor chore: struct update syntax in IPDiversityConfig tests; no logic changes
src/transport.rs	Adds pub(crate) mod observed_address_cache module declaration; no other changes

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Inbound QUIC message] --> B[upstream_rx channel]
    B --> C{shard_index_for_addr\nhash source IP}
    C -->|shard 0| D0[Shard Consumer 0]
    C -->|shard 1| D1[Shard Consumer 1]
    C -->|...| D2[...]
    C -->|shard 7| D7[Shard Consumer 7]
    D0 & D1 & D2 & D7 --> E[parse_protocol_message]
    E --> F{authenticated?}
    F -->|yes| G[peer_to_channel.write\nchannel_to_peers.write\nregister_connection_peer_id]
    F -->|no| H[broadcast P2PEvent]
    G --> I{is /rr/ response?}
    I -->|yes| J[active_requests.write\ndeliver to oneshot]
    I -->|no| H

    subgraph NAT [NAT Self-Entry Fix]
        K[local_dht_node async] --> L{observed_external\naddresses?}
        L -->|yes| M[publish observed addr\nper interface]
        L -->|no| N[ObservedAddressCache\nfallback]
        N --> O{specific IP\nlisten addr?}
        O -->|yes| P[publish listen addr]
        O -->|no| Q[publish nothing —\nhonest empty entry]
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/transport_handle.rs
Line: 1701-1723

Comment:
**Write lock held across two await points**

`peer_to_channel.write()` is acquired on line 1701 and held across both `channel_to_peers.write().await` (line 1706) and the async `dual_node_for_peer_reg.register_connection_peer_id(...).await` call (line 1717). All 8 shards contend on this single global write lock for every authenticated message — not just on first peer registration — partially re-introducing the serialization the sharding was meant to remove.

A read-first check avoids the write lock on the hot path (already-known peer + already-known channel):

```rust
// Fast path: read lock sufficient when peer+channel already known.
{
    let p2c = peer_to_channel.read().await;
    if p2c.get(app_id).is_some_and(|chs| chs.contains(&channel_id)) {
        // Nothing to register; fall through to event dispatch.
        drop(p2c);
    } else {
        drop(p2c);
        // Slow path: new peer or new channel — take write lock.
        let mut p2c = peer_to_channel.write().await;
        // ... existing write-lock body ...
    }
}
```

The correctness argument in the existing comment (consistency between the app map and the transport's internal registration) still holds on the slow path only; the fast path is safe because there is nothing to register.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/transport_handle.rs
Line: 1847-1851

Comment:
**`DefaultHasher` is not guaranteed stable across Rust versions**

`std::collections::hash_map::DefaultHasher::new()` uses SipHash-1-3 with a zeroed key (deterministic within a run), but the Rust reference explicitly states the algorithm is "subject to change at any point in the future." A Rust upgrade that changes the algorithm would silently redistribute all peers across shards, causing a brief message-ordering hiccup while in-flight messages drain from the old shard assignments.

For a load-balancing shard key, correctness is maintained even across a reshuffle (no persistent state is keyed by shard), but replacing with a stable, purpose-built hash (e.g. `rustc-hash::FxHasher` or a simple FNV) would make the choice explicit and immune to compiler updates:

```rust
fn shard_index_for_addr(addr: &SocketAddr) -> usize {
    use std::hash::{Hash, Hasher};
    // FNV-1a — fast, stable, no crate dependency beyond std.
    let mut h: u64 = 14695981039346656037;
    for byte in addr.ip().to_string().bytes() {
        h ^= u64::from(byte);
        h = h.wrapping_mul(1099511628211);
    }
    (h as usize) % MESSAGE_DISPATCH_SHARDS
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.
Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.}

_{Reviews (2): Last reviewed commit: "fix: address deep-review findings on PR ..." | Re-trigger Greptile}