Merge pull request #120 from libp2p/fix/useful-peers

Replacing UsefulPeer() with UsefulNewPeer()

go.mod

@@ -6,11 +6,11 @@ require (
 	github.com/ipfs/boxo v0.8.1
 	github.com/ipfs/go-log v1.0.5
 	github.com/libp2p/go-cidranger v1.1.0
-	github.com/libp2p/go-libp2p v0.27.5
+	github.com/libp2p/go-libp2p v0.27.6
 	github.com/libp2p/go-libp2p-asn-util v0.3.0
 	github.com/minio/sha256-simd v1.0.1
 	github.com/multiformats/go-multiaddr v0.9.0
-	github.com/multiformats/go-multihash v0.2.2
+	github.com/multiformats/go-multihash v0.2.3
 	github.com/stretchr/testify v1.8.4
 )
 

go.sum

@@ -42,8 +42,8 @@ github.com/libp2p/go-buffer-pool v0.1.0 h1:oK4mSFcQz7cTQIfqbe4MIj9gLW+mnanjyFtc6
 github.com/libp2p/go-buffer-pool v0.1.0/go.mod h1:N+vh8gMqimBzdKkSMVuydVDq+UV5QTWy5HSiZacSbPg=
 github.com/libp2p/go-cidranger v1.1.0 h1:ewPN8EZ0dd1LSnrtuwd4709PXVcITVeuwbag38yPW7c=
 github.com/libp2p/go-cidranger v1.1.0/go.mod h1:KWZTfSr+r9qEo9OkI9/SIEeAtw+NNoU0dXIXt15Okic=
-github.com/libp2p/go-libp2p v0.27.5 h1:KwA7pXKXpz8hG6Cr1fMA7UkgleogcwQj0sxl5qquWRg=
-github.com/libp2p/go-libp2p v0.27.5/go.mod h1:oMfQGTb9CHnrOuSM6yMmyK2lXz3qIhnkn2+oK3B1Y2g=
+github.com/libp2p/go-libp2p v0.27.6 h1:KmGU5kskCaaerm53heqzfGOlrW2z8icZ+fnyqgrZs38=
+github.com/libp2p/go-libp2p v0.27.6/go.mod h1:oMfQGTb9CHnrOuSM6yMmyK2lXz3qIhnkn2+oK3B1Y2g=
 github.com/libp2p/go-libp2p-asn-util v0.3.0 h1:gMDcMyYiZKkocGXDQ5nsUQyquC9+H+iLEQHwOCZ7s8s=
 github.com/libp2p/go-libp2p-asn-util v0.3.0/go.mod h1:B1mcOrKUE35Xq/ASTmQ4tN3LNzVVaMNmq2NACuqyB9w=
 github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
@@ -63,8 +63,8 @@ github.com/multiformats/go-multibase v0.2.0 h1:isdYCVLvksgWlMW9OZRYJEa9pZETFivnc
 github.com/multiformats/go-multibase v0.2.0/go.mod h1:bFBZX4lKCA/2lyOFSAoKH5SS6oPyjtnzK/XTFDPkNuk=
 github.com/multiformats/go-multicodec v0.9.0 h1:pb/dlPnzee/Sxv/j4PmkDRxCOi3hXTz3IbPKOXWJkmg=
 github.com/multiformats/go-multicodec v0.9.0/go.mod h1:L3QTQvMIaVBkXOXXtVmYE+LI16i14xuaojr/H7Ai54k=
-github.com/multiformats/go-multihash v0.2.2 h1:Uu7LWs/PmWby1gkj1S1DXx3zyd3aVabA4FiMKn/2tAc=
-github.com/multiformats/go-multihash v0.2.2/go.mod h1:dXgKXCXjBzdscBLk9JkjINiEsCKRVch90MdaGiKsvSM=
+github.com/multiformats/go-multihash v0.2.3 h1:7Lyc8XfX/IY2jWb/gI7JP+o7JEq9hOa7BFvVU9RSh+U=
+github.com/multiformats/go-multihash v0.2.3/go.mod h1:dXgKXCXjBzdscBLk9JkjINiEsCKRVch90MdaGiKsvSM=
 github.com/multiformats/go-multistream v0.4.1 h1:rFy0Iiyn3YT0asivDUIR05leAdwZq3de4741sbiSdfo=
 github.com/multiformats/go-multistream v0.4.1/go.mod h1:Mz5eykRVAjJWckE2U78c6xqdtyNUEhKSM0Lwar2p77Q=
 github.com/multiformats/go-varint v0.0.7 h1:sWSGR+f/eu5ABZA2ZpYKBILXTTs9JWpdEM/nEGOHFS8=

table.go

@@ -112,13 +112,19 @@ func (rt *RoutingTable) NPeersForCpl(cpl uint) int {
 	}
 }
 
-// UsefulPeer verifies whether the given peer.ID would be a good fit for the routing table
-// It returns true if the bucket corresponding to peer.ID isn't full, if it contains
-// replaceable peers or if it is the last bucket and adding a peer would unfold it.
-func (rt *RoutingTable) UsefulPeer(p peer.ID) bool {
+// UsefulNewPeer verifies whether the given peer.ID would be a good fit for the
+// routing table. It returns true if the peer isn't in the routing table yet, if
+// the bucket corresponding to peer.ID isn't full, if it contains replaceable
+// peers or if it is the last bucket and adding a peer would unfold it.
+func (rt *RoutingTable) UsefulNewPeer(p peer.ID) bool {
 	rt.tabLock.RLock()
 	defer rt.tabLock.RUnlock()
 
+	if rt.Find(p) != "" {
+		// peer already exists in the routing table, so it isn't useful
+		return false
+	}
+
 	// bucket corresponding to p
 	bucketID := rt.bucketIdForPeer(p)
 	bucket := rt.buckets[bucketID]

table_test.go

@@ -126,7 +126,7 @@ func TestNPeersForCpl(t *testing.T) {
 	require.Equal(t, 2, rt.NPeersForCpl(0))
 }
 
-func TestUsefulPeer(t *testing.T) {
+func TestUsefulNewPeer(t *testing.T) {
 	t.Parallel()
 	local := test.RandPeerIDFatal(t)
 	m := pstore.NewMetrics()
@@ -135,58 +135,60 @@ func TestUsefulPeer(t *testing.T) {
 
 	// add first peer to bucket 0
 	p, _ := rt.GenRandPeerID(0)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
+	// first peer shouldn't be useful, as it is already in the rt
+	require.False(t, rt.UsefulNewPeer(p))
 
 	// add second peer to bucket 0
 	p, _ = rt.GenRandPeerID(0)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 
 	// bucket 0 (also last bucket) full with non replaceable peers
 	p, _ = rt.GenRandPeerID(0)
-	require.False(t, rt.UsefulPeer(p))
+	require.False(t, rt.UsefulNewPeer(p))
 
 	// bucket 0 is full, unfolding it
 	// add first peer to bucket 1
 	p, _ = rt.GenRandPeerID(1)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 
 	// add second peer to bucket 1
 	// cpl is 2, but bucket 1 is last bucket
 	p, _ = rt.GenRandPeerID(2)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 
 	// unfolding bucket 1
 	// adding second peer to bucket 2
 	p, _ = rt.GenRandPeerID(2)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 
 	// adding replaceable peer to bucket 1
 	// bucket 1 size: 1 -> 2
 	p, _ = rt.GenRandPeerID(1)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, true)
 
 	// adding replaceable peer to bucket 1
 	// bucket 1 size: 2 -> 2
 	p, _ = rt.GenRandPeerID(1)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, true)
 
 	// adding non replaceable peer to bucket 1
 	// bucket 1 size: 2 -> 2
 	p, _ = rt.GenRandPeerID(1)
-	require.True(t, rt.UsefulPeer(p))
+	require.True(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 
 	// adding non replaceable peer to bucket 1
 	// bucket 1 size: 2 -> 2
 	p, _ = rt.GenRandPeerID(1)
-	require.False(t, rt.UsefulPeer(p))
+	require.False(t, rt.UsefulNewPeer(p))
 	rt.TryAddPeer(p, true, false)
 }