fix consumption of observed remote addrs

p2p/protocol/identify/id.go

@@ -190,7 +190,11 @@ func (ids *IDService) consumeMessage(mes *pb.Identify, c inet.Conn) {
 		lmaddrs = append(lmaddrs, maddr)
 	}
 
-	lmaddrs = append(lmaddrs, c.RemoteMultiaddr())
+	// if the address reported by the connection roughly matches their annoucned
+	// listener addresses, its likely to be an external NAT address
+	if HasConsistentTransport(c.RemoteMultiaddr(), lmaddrs) {
+		lmaddrs = append(lmaddrs, c.RemoteMultiaddr())
+	}
 
 	// update our peerstore with the addresses. here, we SET the addresses, clearing old ones.
 	// We are receiving from the peer itself. this is current address ground truth.
@@ -214,6 +218,35 @@ func (ids *IDService) consumeMessage(mes *pb.Identify, c inet.Conn) {
 	ids.Host.Peerstore().Put(p, "AgentVersion", av)
 }
 
+// HasConsistentTransport returns true if the address 'a' shares a
+// protocol set with any address in the green set. This is used
+// to check if a given address might be one of the addresses a peer is
+// listening on.
+func HasConsistentTransport(a ma.Multiaddr, green []ma.Multiaddr) bool {
+	protosMatch := func(a, b []ma.Protocol) bool {
+		if len(a) != len(b) {
+			return false
+		}
+
+		for i, p := range a {
+			if b[i].Code != p.Code {
+				return false
+			}
+		}
+		return true
+	}
+
+	protos := a.Protocols()
+
+	for _, ga := range green {
+		if protosMatch(protos, ga.Protocols()) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // IdentifyWait returns a channel which will be closed once
 // "ProtocolIdentify" (handshake3) finishes on given conn.
 // This happens async so the connection can start to be used

p2p/protocol/identify/id_test.go

@@ -111,3 +111,18 @@ func TestIDServiceNoWait(t *testing.T) {
 		subtestIDService(t, 0)
 	}
 }
+
+func TestProtoMatching(t *testing.T) {
+	tcp1, _ := ma.NewMultiaddr("/ip4/1.2.3.4/tcp/1234")
+	tcp2, _ := ma.NewMultiaddr("/ip4/1.2.3.4/tcp/2345")
+	tcp3, _ := ma.NewMultiaddr("/ip4/1.2.3.4/tcp/4567")
+	utp, _ := ma.NewMultiaddr("/ip4/1.2.3.4/udp/1234/utp")
+
+	if !identify.HasConsistentTransport(tcp1, []ma.Multiaddr{tcp2, tcp3, utp}) {
+		t.Fatal("expected match")
+	}
+
+	if identify.HasConsistentTransport(utp, []ma.Multiaddr{tcp2, tcp3}) {
+		t.Fatal("expected mismatch")
+	}
+}