p2p/protocol/identify: dont double count observers
If the same peer observed the same address twice, it would be double counted as different observations. This change adds a map to make sure we're counting each observer once. This is easily extended to require more than two observations, but i have not yet encountered NATs for whom this is relevant.
Juan Batiz-Benet committed
Jun 7, 2015 at 16:47 UTC
96ed20bffe377e56c2193c41e86581d97470b729
3 files changed
+44
-19
p2p/protocol/identify/id.go
+1
-1
@@ -256,7 +256,7 @@ func (ids *IDService) consumeObservedAddress(observed []byte, c inet.Conn) {
256
257
// ok! we have the observed version of one of our ListenAddresses!
258
log.Debugf("added own observed listen addr: %s --> %s", c.LocalMultiaddr(), maddr)
259
- ids.observedAddrs.Add(maddr)
259
+ ids.observedAddrs.Add(maddr, c.RemoteMultiaddr())
260
}
261
262
func addrInAddrs(a ma.Multiaddr, as []ma.Multiaddr) bool {
p2p/protocol/identify/obsaddr.go
+22
-11
@@ -15,9 +15,9 @@ import (
15
// - have been observed recently (10min), because our position in the
16
// network, or network port mapppings, may have changed.
17
type ObservedAddr struct {
18
- Addr ma.Multiaddr
19
- LastSeen time.Time
20
- TimesSeen int
18
+ Addr ma.Multiaddr
19
+ SeenBy map[string]struct{}
20
+ LastSeen time.Time
21
}
22
23
// ObservedAddrSet keeps track of a set of ObservedAddrs
@@ -25,7 +25,7 @@ type ObservedAddr struct {
25
type ObservedAddrSet struct {
26
sync.Mutex // guards whole datastruct.
27
28
- addrs map[string]ObservedAddr
28
+ addrs map[string]*ObservedAddr
29
ttl time.Duration
30
}
31
@@ -54,29 +54,40 @@ func (oas *ObservedAddrSet) Addrs() []ma.Multiaddr {
54
// very useful. We make the assumption that if we've
55
// connected to two different peers, and they both have
56
// reported seeing the same address, it is probably useful.
57
- if a.TimesSeen > 1 {
57
+ //
58
+ // Note: make sure not to double count observers.
59
+ if len(a.SeenBy) > 1 {
60
addrs = append(addrs, a.Addr)
61
}
62
}
63
return addrs
64
}
65
64
-func (oas *ObservedAddrSet) Add(addr ma.Multiaddr) {
66
+func (oas *ObservedAddrSet) Add(addr ma.Multiaddr, observer ma.Multiaddr) {
67
oas.Lock()
68
defer oas.Unlock()
69
70
// for zero-value.
71
if oas.addrs == nil {
70
- oas.addrs = make(map[string]ObservedAddr)
72
+ oas.addrs = make(map[string]*ObservedAddr)
73
oas.ttl = peer.OwnObservedAddrTTL
74
}
75
76
s := addr.String()
75
- oas.addrs[s] = ObservedAddr{
76
- Addr: addr,
77
- TimesSeen: oas.addrs[s].TimesSeen + 1,
78
- LastSeen: time.Now(),
77
+ oa, found := oas.addrs[s]
78
+
79
+ // first time seeing address.
80
+ if !found {
81
+ oa = &ObservedAddr{
82
+ Addr: addr,
83
+ SeenBy: make(map[string]struct{}),
84
+ }
85
+ oas.addrs[s] = oa
86
}
87
+
88
+ // Add current observer.
89
+ oa.SeenBy[observer.String()] = struct{}{}
90
+ oa.LastSeen = time.Now()
91
}
92
93
func (oas *ObservedAddrSet) SetTTL(ttl time.Duration) {
p2p/protocol/identify/obsaddr_test.go
+21
-7
@@ -36,6 +36,9 @@ func TestObsAddrSet(t *testing.T) {
36
a1 := m("/ip4/1.2.3.4/tcp/1231")
37
a2 := m("/ip4/1.2.3.4/tcp/1232")
38
a3 := m("/ip4/1.2.3.4/tcp/1233")
39
+ a4 := m("/ip4/1.2.3.4/tcp/1234")
40
+ a5 := m("/ip4/1.2.3.4/tcp/1235")
41
+ a6 := m("/ip4/1.2.3.4/tcp/1236")
42
43
oas := ObservedAddrSet{}
44
@@ -43,23 +46,34 @@ func TestObsAddrSet(t *testing.T) {
46
t.Error("addrs should be empty")
47
}
48
46
- oas.Add(a1)
47
- oas.Add(a2)
48
- oas.Add(a3)
49
+ oas.Add(a1, a4)
50
+ oas.Add(a2, a4)
51
+ oas.Add(a3, a4)
52
53
// these are all different so we should not yet get them.
54
if !addrsMarch(oas.Addrs(), nil) {
55
t.Error("addrs should _still_ be empty (once)")
56
}
57
55
- oas.Add(a1)
58
+ // same observer, so should not yet get them.
59
+ oas.Add(a1, a4)
60
+ oas.Add(a2, a4)
61
+ oas.Add(a3, a4)
62
+ if !addrsMarch(oas.Addrs(), nil) {
63
+ t.Error("addrs should _still_ be empty (same obs)")
64
+ }
65
+
66
+ oas.Add(a1, a5)
67
if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1}) {
68
t.Error("addrs should only have a1")
69
}
70
60
- oas.Add(a2)
61
- oas.Add(a1)
62
- oas.Add(a1)
71
+ oas.Add(a2, a5)
72
+ oas.Add(a1, a5)
73
+ oas.Add(a1, a5)
74
+ oas.Add(a2, a6)
75
+ oas.Add(a1, a6)
76
+ oas.Add(a1, a6)
77
if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1, a2}) {
78
t.Error("addrs should only have a1, a2")
79
}