From fa014144a00047a28588f95473c88d74826545a2 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 5 Dec 2025 20:54:22 +0100 Subject: [PATCH 1/2] fix(basichost): secondary addrs inherit reachability from primary secondary transports (WSS, WebTransport, WebRTC) now inherit ReachabilityPublic from their primary transport (TCP, QUIC) when they share the same network socket. rationale: as the Amino DHT gained more diverse implementations (2025 Q4), we observed false negatives where AutoNAT v2 probes failed for secondary protocols not because the port was blocked, but because the probing peer simply didn't support the protocol. there is also a chicken-and-egg problem with AutoTLS (p2p-forge): WSS probes fail during initial setup because the TLS certificate is not provisioned yet. without inheritance, WSS would be marked unreachable and excluded from announcements. waiting for the next probe cycle means ~5 minutes without WSS on initial start or when cert needs renewal. when the primary confirms the port is network-reachable, secondary addresses sharing that socket are also reachable to peers that support those protocols. inheriting Public from a confirmed primary avoids incorrect Private status from protocol-level probe failures. inheritance is conservative: only Public propagates. Private doesn't propagate because it could indicate protocol-specific issues rather than port unreachability, so secondaries still get probed. --- p2p/host/basic/addrs_reachability_tracker.go | 60 ++++++++++++-- .../basic/addrs_reachability_tracker_test.go | 83 +++++++++++-------- 2 files changed, 105 insertions(+), 38 deletions(-) diff --git a/p2p/host/basic/addrs_reachability_tracker.go b/p2p/host/basic/addrs_reachability_tracker.go index d2a370b90d..25cf0a175f 100644 --- a/p2p/host/basic/addrs_reachability_tracker.go +++ b/p2p/host/basic/addrs_reachability_tracker.go @@ -620,6 +620,19 @@ func (s *addrStatus) Reachability() network.Reachability { } func (s *addrStatus) RequiredProbeCount(now time.Time) int { + // Secondary addresses inherit reachability from their confirmed-public primary. + // If the primary is ReachabilityPublic, the port is confirmed open at the + // network level, so the secondary is also reachable (they share the socket). + // + // If the primary is ReachabilityPrivate, we still probe the secondary because + // Private is a weaker signal - it could indicate: + // - Port genuinely blocked (secondary will also fail) + // - Protocol-specific issues with the primary (secondary might work) + // The cost of extra probes when truly firewalled is low (quick failures). + if s.primary != nil && s.primary.Reachability() == network.ReachabilityPublic { + return 0 + } + if s.consecutiveRefusals >= maxConsecutiveRefusals { if now.Sub(s.lastRefusalTime) < recentProbeInterval { return 0 @@ -742,12 +755,49 @@ func (s *addrStatus) reachabilityAndCounts() (rch network.Reachability, successe } if s.primary != nil { prch, _, _ := s.primary.reachabilityAndCounts() - switch prch { - case network.ReachabilityPublic: - successes *= secondaryAddrsScalingFactor - case network.ReachabilityPrivate: - failures *= secondaryAddrsScalingFactor + if prch == network.ReachabilityPublic { + // Secondary transports inherit Public reachability from their primary. + // + // This is important because not all AutoNAT v2 server implementations + // support all secondary transports. As the Amino DHT gained a more + // diverse set of node implementations (2025 Q4), we observed false + // negatives: secondary addresses being marked unreachable when probing + // peers simply didn't support the protocol, not because the port was + // actually blocked. + // + // This handles shared-listener configurations where multiple + // protocols share the same network socket: + // + // TCP-based (libp2p.ShareTCPListener): + // Primary: /ip4/.../tcp/port + // Secondary: /ip4/.../tcp/port/tls/sni/*.libp2p.direct/ws + // TCP and Secure WebSocket share the same TCP listener. + // + // UDP/QUIC-based (quicreuse.ConnManager): + // Primary: /ip4/.../udp/port/quic-v1 + // Secondary: /ip4/.../udp/port/quic-v1/webtransport + // Secondary: /ip4/.../udp/port/webrtc-direct + // QUIC, WebTransport, and WebRTC share the same UDP socket. + // + // AutoNAT v2 probe failures for secondary protocols typically + // indicate protocol incompatibility at the probing peer, not + // port unreachability: + // + // - Secure WebSocket: Probing peer may not support WebSockets, + // or TLS handshake fails because the certificate isn't + // provisioned yet (AutoTLS still obtaining cert). + // - WebTransport: Probing peer supports QUIC but not HTTP/3. + // - WebRTC: Probing peer supports QUIC but not DTLS-SRTP. + // + // Since the primary confirms the port is network-reachable, we + // inherit that status. Protocol-level failures don't indicate + // the address is unreachable to peers that DO support the protocol. + return network.ReachabilityPublic, successes, failures } + // If primary is Private or Unknown, we don't inherit - the secondary + // builds its own status through probing. This is more conservative: + // Private could indicate protocol-specific issues rather than port + // unreachability, so we give the secondary a chance to prove itself. } if successes-failures >= minConfidence { return network.ReachabilityPublic, successes, failures diff --git a/p2p/host/basic/addrs_reachability_tracker_test.go b/p2p/host/basic/addrs_reachability_tracker_test.go index f1c35d4ba8..c6a69f0797 100644 --- a/p2p/host/basic/addrs_reachability_tracker_test.go +++ b/p2p/host/basic/addrs_reachability_tracker_test.go @@ -209,71 +209,88 @@ func TestProbeManager(t *testing.T) { } return res } - // tcp private - for range targetConfidence { + + // Conservative inheritance: secondaries only inherit Public from primary. + // If primary is Private, secondaries still get probed (Private could be + // protocol-specific, not port-level). + // + // TCP gets Public results - websocket inherits Public (skips probing) + // This is the AutoTLS use case: TCP works, WSS inherits reachability. + for i := range targetConfidence { reqs := nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic, websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) + if i < minConfidence { + // TCP not yet confirmed Public, all 4 addresses need probing + matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic, websocket, webrtc}, extractAddrs(reqs)) + } else { + // TCP confirmed Public, websocket inherits - only 3 addresses + matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic, webrtc}, extractAddrs(reqs)) + } + pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPublic}, nil) } - // quic public + + // QUIC gets Private results - webrtc still needs probing (no inheritance) + // This tests the conservative behavior: Private doesn't propagate. for range targetConfidence { reqs := nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{quic, websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPublic}, nil) + // websocket already inherited from tcp, but webrtc doesn't inherit Private + matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{quic, webrtc}, extractAddrs(reqs)) + pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) } - // only 1 check now required for websocket - for range 1 { - reqs := nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: websocket, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) - } - // 3 checks required for webrtc to make its reachability different from quic. + + // webrtc still needs probing (doesn't inherit Private from quic) + // Give webrtc its own Private status through probing for range targetConfidence { reqs := nextProbe(pm) matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{webrtc}, extractAddrs(reqs)) pm.CompleteProbe(reqs, autonatv2.Result{Addr: webrtc, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) } + reqs := nextProbe(pm) + require.Empty(t, reqs) + reachable, unreachable, _ := pm.AppendConfirmedAddrs(nil, nil, nil) - matest.AssertMultiaddrsMatch(t, []ma.Multiaddr{quic}, reachable) - matest.AssertMultiaddrsMatch(t, []ma.Multiaddr{tcp, websocket, webrtc}, unreachable) + // websocket inherits Public from tcp (confirmed reachable port) - AutoTLS case + // webrtc has its own Private status (probed independently) + matest.AssertMultiaddrsMatch(t, []ma.Multiaddr{tcp, websocket}, reachable) + matest.AssertMultiaddrsMatch(t, []ma.Multiaddr{quic, webrtc}, unreachable) - // Every `highConfidenceAddrsProbeInterval` we refresh the primary addr binding + // After highConfidenceAddrProbeInterval (1h), only primaries need refresh. + // websocket inherits from tcp (Public), webrtc has longer refresh interval (3h). for range 2 { cl.Add(highConfidenceAddrProbeInterval + 1*time.Millisecond) reqs := nextProbe(pm) + // Only tcp and quic need refresh; websocket inherits, webrtc has 3h interval matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) + pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPublic}, nil) reqs = nextProbe(pm) matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{quic}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPublic}, nil) + pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) reqs = nextProbe(pm) require.Empty(t, reqs) } - cl.Add(highConfidenceAddrProbeInterval + 1*time.Millisecond) - reqs := nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic, websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) - reqs = nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{quic, websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPublic}, nil) + reachable, unreachable, _ = pm.AppendConfirmedAddrs(nil, nil, nil) + matest.AssertMultiaddrsMatch(t, reachable, []ma.Multiaddr{tcp, websocket}) + matest.AssertMultiaddrsMatch(t, unreachable, []ma.Multiaddr{quic, webrtc}) - // secondary addrs refreshed at 3*highConfidenceProbeInterval + // After highConfidenceSecondaryAddrProbeInterval (3h), webrtc needs refresh too. + // We've advanced 2h+2ms, need to reach 3h+ for webrtc's refresh. + // Also need to exceed 1h since last tcp/quic refresh for them to need refresh. + cl.Add(highConfidenceSecondaryAddrProbeInterval - 2*highConfidenceAddrProbeInterval + 1*time.Millisecond) + reqs = nextProbe(pm) + // tcp, quic, and webrtc need refresh; websocket still inherits from tcp + matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{tcp, quic, webrtc}, extractAddrs(reqs)) + pm.CompleteProbe(reqs, autonatv2.Result{Addr: tcp, Idx: 0, Reachability: network.ReachabilityPublic}, nil) reqs = nextProbe(pm) - matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{websocket, webrtc}, extractAddrs(reqs)) - pm.CompleteProbe(reqs, autonatv2.Result{Addr: websocket, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) + matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{quic, webrtc}, extractAddrs(reqs)) + pm.CompleteProbe(reqs, autonatv2.Result{Addr: quic, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) reqs = nextProbe(pm) matest.AssertEqualMultiaddrs(t, []ma.Multiaddr{webrtc}, extractAddrs(reqs)) pm.CompleteProbe(reqs, autonatv2.Result{Addr: webrtc, Idx: 0, Reachability: network.ReachabilityPrivate}, nil) reqs = nextProbe(pm) require.Empty(t, reqs) - - reachable, unreachable, _ = pm.AppendConfirmedAddrs(nil, nil, nil) - matest.AssertMultiaddrsMatch(t, reachable, []ma.Multiaddr{quic}) - matest.AssertMultiaddrsMatch(t, unreachable, []ma.Multiaddr{tcp, websocket, webrtc}) }) } From b576d4ef626ad1f39f09b1ce7861489d6c72d854 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Sat, 20 Dec 2025 01:05:07 +0100 Subject: [PATCH 2/2] fix(basichost): remove unused secondaryAddrsScalingFactor const fixes staticcheck U1000 error in CI --- p2p/host/basic/addrs_reachability_tracker.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/p2p/host/basic/addrs_reachability_tracker.go b/p2p/host/basic/addrs_reachability_tracker.go index 25cf0a175f..0f802977df 100644 --- a/p2p/host/basic/addrs_reachability_tracker.go +++ b/p2p/host/basic/addrs_reachability_tracker.go @@ -361,10 +361,6 @@ const ( // and then a success(...S S S S F S). The confidence in the targetConfidence window will be equal to // targetConfidence, the last F and S cancel each other, and we won't probe again for maxProbeInterval. maxRecentDialsWindow = targetConfidence + 2 - // secondaryAddrsScalingFactor is the multiplier applied to secondary address dial outcomes. For secondary - // addr, if the primary addr is reachable, a single successful dial is enough to consider the secondary addr - // reachable. - secondaryAddrsScalingFactor = targetConfidence // highConfidenceAddrProbeInterval is the maximum interval between probes for an address highConfidenceAddrProbeInterval = 1 * time.Hour // highConfidenceSecondaryAddrProbeInterval is the maximum interval between probes for an address