Commit Graph

2 Commits

Author SHA1 Message Date
Siavash Sameni
00deb97a5d fix(reflect): drop LAN/private reflex addrs from NAT classification
Real-world report: a user with one LAN relay + one internet relay
got "Multiple IPs — treating as symmetric" because the LAN relay
saw the client's LAN IP (172.16.81.172) while the internet relay
saw the WAN IP (150.228.49.65). Two observations of "different
public IPs" from the classifier's perspective, but semantically
they describe two different network paths and shouldn't be
compared.

The LAN relay's reflection is always true, just not useful for
public NAT classification: there's no NAT between the client and
the LAN relay, so that path's reflex addr is always the LAN
interface IP regardless of what the public-facing NAT beyond it
looks like.

Fix: new `is_private_or_loopback` helper filters the probe set
before classification. Drops:
 - 127.0.0.0/8 loopback
 - 10/8, 172.16/12, 192.168/16 RFC1918 private
 - 169.254/16 link-local
 - 100.64/10 CGNAT shared-transition (same reasoning: a relay
   that sees the client with a CGNAT addr is on the same carrier
   network and can't describe public NAT state)
 - IPv6 loopback, unspecified, fe80::/10 link-local

Failed probes still filtered out of classification (they were
already) but now dimmed in the UI list instead of highlighted
amber. Same rationale: a momentarily-offline probe target isn't
a warning-worthy state, it's just a fact about the probe run.

UI palette rebalance: only Cone gets green, everything else
neutral text-dim. Wording changed from warning-tone
"⚠ must use relay" to informational "ℹ P2P falls back to relay,
calls still work" — symmetric NAT isn't broken state, it just
means media takes the relay path.

Tests added (4 new in wzp_client::reflect):
- classify_drops_private_ip_probes — LAN + public → Unknown
- classify_drops_loopback_probes — loopback + 2 public → Cone
- classify_drops_cgnat_probes — CGNAT + 2 public same-IP-
  diff-port → SymmetricPort
- classify_two_lan_probes_is_unknown_not_cone — all LAN → Unknown

Existing multi_reflect integration test updated: two loopback
relays now correctly classify as Unknown (because loopback reflex
addrs are filtered) with the plumbing-works invariant preserved.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 18:29:09 +04:00
Siavash Sameni
8d903f16c6 feat(reflect): multi-relay NAT type detection — Phase 2
Builds on Phase 1's SignalMessage::Reflect to probe N relays in
parallel through transient QUIC connections and classify the
client's NAT type for the future P2P hole-punching path. No wire
protocol changes — Phase 1's Reflect/ReflectResponse pair is
reused unchanged.

New client-side module (crates/wzp-client/src/reflect.rs):
- probe_reflect_addr(relay, timeout_ms): opens a throwaway
  quinn::Endpoint (fresh ephemeral source port per probe,
  essential for NAT-type detection — sharing one endpoint would
  make a symmetric NAT look like a cone NAT), connects to _signal,
  sends RegisterPresence with zero identity, consumes the Ack,
  sends Reflect, awaits ReflectResponse, cleanly closes.
- detect_nat_type(relays, timeout_ms): parallel probes via
  tokio::task::JoinSet (bounded by slowest probe not sum) and
  returns a NatDetection with per-probe results + aggregate
  classification.
- classify_nat(probes): pure-function classifier split out for
  network-free unit tests. Rules:
    * 0-1 successful probes              → Unknown
    * 2+ successes, same ip same port    → Cone (P2P viable)
    * 2+ successes, same ip diff ports   → SymmetricPort (relay)
    * 2+ successes, different ips        → Multiple (treat as
                                             symmetric)

Tauri command (desktop/src-tauri/src/lib.rs):
- detect_nat_type({ relays: [{ name, address }] }) -> NatDetection
  as JSON. Takes the relay list from JS because localStorage
  owns the config. Parse-up-front so a malformed entry fails
  clean instead of as a probe error. 1500ms per-probe timeout.

UI (desktop/index.html + src/main.ts):
- New "NAT type" row + "Detect NAT" button in the Network
  settings section. Renders per-probe status (name, address,
  observed addr, latency, or error) plus the colored verdict:
    * green  Cone — shows consensus addr
    * amber  SymmetricPort / Multiple — must relay
    * gray   Unknown — not enough data

Tests:
- 7 unit tests in wzp-client/src/reflect.rs covering every
  classifier branch (empty, 1 success, 2 identical, 2 diff ports,
  2 diff ips, success+failure mix, pure-failure).
- 3 integration tests in crates/wzp-relay/tests/multi_reflect.rs:
    * probe_reflect_addr_happy_path — single mock relay end-to-end
    * detect_nat_type_two_loopback_relays_is_cone — two concurrent
      relays, asserts both see 127.0.0.1 and classifier returns
      Cone or SymmetricPort (accepted because the test harness
      uses fresh ephemeral ports per probe which look like
      SymmetricPort on single-host loopback)
    * detect_nat_type_dead_relay_is_unknown — alive + dead port
      mix, asserts the dead probe surfaces an error string and
      the aggregator returns Unknown (only 1 success)

Full workspace test goes from 386 → 396 passing.

PRD: .taskmaster/docs/prd_multi_relay_reflect.txt
Tasks: 47-52 all completed

Next up: hole-punching (Phase 3) — use the reflected address in
DirectCallOffer/Answer and CallSetup so peers attempt a direct
QUIC handshake to each other, with relay fallback on timeout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:47:12 +04:00