diff --git a/p2p/host/basic/addrs_reachability_tracker.go b/p2p/host/basic/addrs_reachability_tracker.go index d2a370b90d..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 @@ -620,6 +616,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 +751,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}) }) }