fix(autonatv2): secondary addrs inherit reachability from primary (#3435)

Author: lidel

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

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})
 	})
 }