From 6ef5f5dacf07a722fce545321915fef029b02329 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 20 May 2020 11:35:49 -0700 Subject: [PATCH] fix: group observations by zeroing port In #917, we started dropping additional address observations if we had multiple for the same transport set. However, on further consideration, this isn't quite correct. We _want_ to keep additional observations for multiple IP addresses. The real issue is many observations for different ports. So this patch simply changes the key with which we group observations from "address protocols" to "address without the port" (well, with the port set to 0). --- p2p/protocol/identify/obsaddr.go | 23 ++++++++++++------ p2p/protocol/identify/obsaddr_test.go | 35 ++++++++++++++++++--------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go index 5d339a80..1fd8eb71 100644 --- a/p2p/protocol/identify/obsaddr.go +++ b/p2p/protocol/identify/obsaddr.go @@ -70,15 +70,24 @@ func (oa *ObservedAddr) numInbound() int { return count } +// GroupKey returns the group in which this observation belongs. Currently, an +// observed address's group is just the address with all ports set to 0. This +// means we can advertise the most commonly observed external ports without +// advertising _every_ observed port. func (oa *ObservedAddr) GroupKey() string { - key := "" - protos := oa.Addr.Protocols() + key := make([]byte, 0, len(oa.Addr.Bytes())) + ma.ForEach(oa.Addr, func(c ma.Component) bool { + switch proto := c.Protocol(); proto.Code { + case ma.P_TCP, ma.P_UDP: + key = append(key, proto.VCode...) + key = append(key, 0, 0) // zero in two bytes + default: + key = append(key, c.Bytes()...) + } + return true + }) - for i := range protos { - key = key + "/" + protos[i].Name - } - - return key + return string(key) } type newObservation struct { diff --git a/p2p/protocol/identify/obsaddr_test.go b/p2p/protocol/identify/obsaddr_test.go index 2beab1a6..a0f50cf9 100644 --- a/p2p/protocol/identify/obsaddr_test.go +++ b/p2p/protocol/identify/obsaddr_test.go @@ -314,17 +314,28 @@ func TestObservedAddrFiltering(t *testing.T) { } func TestObservedAddrGroupKey(t *testing.T) { - oa1 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/1231")} - require.Equal(t, "/ip4/tcp", oa1.GroupKey()) + oa1 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/2345")} + oa2 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/1231")} + oa3 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/tcp/1231")} + oa4 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1231")} + oa5 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1531")} + oa6 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1531/quic")} + oa7 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1111/quic")} + oa8 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/udp/1111/quic")} - oa2 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/tcp/2222")} - require.Equal(t, "/ip4/tcp", oa2.GroupKey()) - - oa3 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1231")} - require.Equal(t, "/ip4/udp", oa3.GroupKey()) - oa4 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.3.3.4/udp/1531")} - require.Equal(t, "/ip4/udp", oa4.GroupKey()) - - oa5 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.3.3.4/udp/1531/quic")} - require.Equal(t, "/ip4/udp/quic", oa5.GroupKey()) + // different ports, same IP => same key + require.Equal(t, oa1.GroupKey(), oa2.GroupKey()) + // different IPs => different key + require.NotEqual(t, oa2.GroupKey(), oa3.GroupKey()) + // same port, different protos => different keys + require.NotEqual(t, oa3.GroupKey(), oa4.GroupKey()) + // same port, same address, different protos => different keys + require.NotEqual(t, oa2.GroupKey(), oa4.GroupKey()) + // udp works as well + require.Equal(t, oa4.GroupKey(), oa5.GroupKey()) + // udp and quic are different + require.NotEqual(t, oa5.GroupKey(), oa6.GroupKey()) + // quic works as well + require.Equal(t, oa6.GroupKey(), oa7.GroupKey()) + require.NotEqual(t, oa7.GroupKey(), oa8.GroupKey()) }