diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go index 56dedf655..d9cc0b1e0 100644 --- a/p2p/protocol/identify/id.go +++ b/p2p/protocol/identify/id.go @@ -256,7 +256,7 @@ func (ids *IDService) consumeObservedAddress(observed []byte, c inet.Conn) { // ok! we have the observed version of one of our ListenAddresses! log.Debugf("added own observed listen addr: %s --> %s", c.LocalMultiaddr(), maddr) - ids.observedAddrs.Add(maddr) + ids.observedAddrs.Add(maddr, c.RemoteMultiaddr()) } func addrInAddrs(a ma.Multiaddr, as []ma.Multiaddr) bool { diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go index b2169057a..c55757df3 100644 --- a/p2p/protocol/identify/obsaddr.go +++ b/p2p/protocol/identify/obsaddr.go @@ -15,9 +15,9 @@ import ( // - have been observed recently (10min), because our position in the // network, or network port mapppings, may have changed. type ObservedAddr struct { - Addr ma.Multiaddr - LastSeen time.Time - TimesSeen int + Addr ma.Multiaddr + SeenBy map[string]struct{} + LastSeen time.Time } // ObservedAddrSet keeps track of a set of ObservedAddrs @@ -25,7 +25,7 @@ type ObservedAddr struct { type ObservedAddrSet struct { sync.Mutex // guards whole datastruct. - addrs map[string]ObservedAddr + addrs map[string]*ObservedAddr ttl time.Duration } @@ -54,29 +54,40 @@ func (oas *ObservedAddrSet) Addrs() []ma.Multiaddr { // very useful. We make the assumption that if we've // connected to two different peers, and they both have // reported seeing the same address, it is probably useful. - if a.TimesSeen > 1 { + // + // Note: make sure not to double count observers. + if len(a.SeenBy) > 1 { addrs = append(addrs, a.Addr) } } return addrs } -func (oas *ObservedAddrSet) Add(addr ma.Multiaddr) { +func (oas *ObservedAddrSet) Add(addr ma.Multiaddr, observer ma.Multiaddr) { oas.Lock() defer oas.Unlock() // for zero-value. if oas.addrs == nil { - oas.addrs = make(map[string]ObservedAddr) + oas.addrs = make(map[string]*ObservedAddr) oas.ttl = peer.OwnObservedAddrTTL } s := addr.String() - oas.addrs[s] = ObservedAddr{ - Addr: addr, - TimesSeen: oas.addrs[s].TimesSeen + 1, - LastSeen: time.Now(), + oa, found := oas.addrs[s] + + // first time seeing address. + if !found { + oa = &ObservedAddr{ + Addr: addr, + SeenBy: make(map[string]struct{}), + } + oas.addrs[s] = oa } + + // Add current observer. + oa.SeenBy[observer.String()] = struct{}{} + oa.LastSeen = time.Now() } func (oas *ObservedAddrSet) SetTTL(ttl time.Duration) { diff --git a/p2p/protocol/identify/obsaddr_test.go b/p2p/protocol/identify/obsaddr_test.go index 8a5ef80fe..416e8f681 100644 --- a/p2p/protocol/identify/obsaddr_test.go +++ b/p2p/protocol/identify/obsaddr_test.go @@ -36,6 +36,9 @@ func TestObsAddrSet(t *testing.T) { a1 := m("/ip4/1.2.3.4/tcp/1231") a2 := m("/ip4/1.2.3.4/tcp/1232") a3 := m("/ip4/1.2.3.4/tcp/1233") + a4 := m("/ip4/1.2.3.4/tcp/1234") + a5 := m("/ip4/1.2.3.4/tcp/1235") + a6 := m("/ip4/1.2.3.4/tcp/1236") oas := ObservedAddrSet{} @@ -43,23 +46,34 @@ func TestObsAddrSet(t *testing.T) { t.Error("addrs should be empty") } - oas.Add(a1) - oas.Add(a2) - oas.Add(a3) + oas.Add(a1, a4) + oas.Add(a2, a4) + oas.Add(a3, a4) // these are all different so we should not yet get them. if !addrsMarch(oas.Addrs(), nil) { t.Error("addrs should _still_ be empty (once)") } - oas.Add(a1) + // same observer, so should not yet get them. + oas.Add(a1, a4) + oas.Add(a2, a4) + oas.Add(a3, a4) + if !addrsMarch(oas.Addrs(), nil) { + t.Error("addrs should _still_ be empty (same obs)") + } + + oas.Add(a1, a5) if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1}) { t.Error("addrs should only have a1") } - oas.Add(a2) - oas.Add(a1) - oas.Add(a1) + oas.Add(a2, a5) + oas.Add(a1, a5) + oas.Add(a1, a5) + oas.Add(a2, a6) + oas.Add(a1, a6) + oas.Add(a1, a6) if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1, a2}) { t.Error("addrs should only have a1, a2") }