From 904390af2cd91acfad6e5f8fab58d72f1b003f52 Mon Sep 17 00:00:00 2001 From: tycho garen Date: Mon, 13 Jun 2022 10:47:25 -0400 Subject: [PATCH] cleanup peer manager --- internal/p2p/peermanager.go | 7 +-- internal/p2p/peermanager_scoring_test.go | 4 +- internal/p2p/peermanager_test.go | 56 ++++++++++++------------ 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go index a1ad0222c..2d74c761d 100644 --- a/internal/p2p/peermanager.go +++ b/internal/p2p/peermanager.go @@ -44,7 +44,6 @@ type PeerScore int16 const ( PeerScorePersistent PeerScore = math.MaxInt16 // persistent peers MaxPeerScoreNotPersistent PeerScore = PeerScorePersistent - 1 - DefaultMutablePeerScore = 256 ) // PeerUpdate is a peer update event sent via PeerUpdates. @@ -410,10 +409,6 @@ func (m *PeerManager) Add(address NodeAddress) (bool, error) { return false, nil } - // set the peer's mutable score to something non-zero so that - // peer's we've never seen aren't very low at start. - peer.MutableScore = DefaultMutablePeerScore - // else add the new address peer.AddressInfo[address] = &peerAddressInfo{Address: address} if err := m.store.Set(peer); err != nil { @@ -1032,6 +1027,8 @@ func (m *PeerManager) findUpgradeCandidate(id types.NodeID, score PeerScore) typ for i := len(ranked) - 1; i >= 0; i-- { candidate := ranked[i] switch { + case candidate.ID == id: + continue case candidate.Score() >= score: return "" // no further peers can be scored lower, due to sorting case !m.connected[candidate.ID]: diff --git a/internal/p2p/peermanager_scoring_test.go b/internal/p2p/peermanager_scoring_test.go index 7086985d1..36352beaf 100644 --- a/internal/p2p/peermanager_scoring_test.go +++ b/internal/p2p/peermanager_scoring_test.go @@ -34,7 +34,7 @@ func TestPeerScoring(t *testing.T) { t.Run("Synchronous", func(t *testing.T) { // update the manager and make sure it's correct - require.EqualValues(t, DefaultMutablePeerScore, peerManager.Scores()[id]) + require.Zero(t, peerManager.Scores()[id]) // add a bunch of good status updates and watch things increase. for i := 1; i < 10; i++ { @@ -42,7 +42,7 @@ func TestPeerScoring(t *testing.T) { NodeID: id, Status: PeerStatusGood, }) - require.EqualValues(t, int(DefaultMutablePeerScore)+i, peerManager.Scores()[id]) + require.EqualValues(t, i, peerManager.Scores()[id]) } // watch the corresponding decreases respond to update diff --git a/internal/p2p/peermanager_test.go b/internal/p2p/peermanager_test.go index 373a0abf0..bc505a78e 100644 --- a/internal/p2p/peermanager_test.go +++ b/internal/p2p/peermanager_test.go @@ -169,7 +169,7 @@ func TestNewPeerManager_Persistence(t *testing.T) { require.Equal(t, map[types.NodeID]p2p.PeerScore{ aID: p2p.PeerScorePersistent, bID: 1, - cID: p2p.PeerScore(p2p.DefaultMutablePeerScore), + cID: 0, }, peerManager.Scores()) // Creating a new peer manager with the same database should retain the @@ -524,11 +524,11 @@ func TestPeerManager_TryDialNext_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: p2p.PeerScore(0 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), - e.NodeID: p2p.PeerScore(0 + p2p.DefaultMutablePeerScore), + a.NodeID: p2p.PeerScore(0), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(2), + d.NodeID: p2p.PeerScore(3), + e.NodeID: p2p.PeerScore(0), }, PersistentPeers: []types.NodeID{c.NodeID, d.NodeID}, MaxConnected: 2, @@ -581,10 +581,8 @@ func TestPeerManager_TryDialNext_MaxConnectedUpgrade(t *testing.T) { // Now, if we disconnect a, we should be allowed to dial d because we have a // free upgrade slot. + require.Error(t, peerManager.Dialed(d)) peerManager.Disconnected(ctx, a.NodeID) - dial, err = peerManager.TryDialNext() - require.NoError(t, err) - require.Equal(t, d, dial) require.NoError(t, peerManager.Dialed(d)) // However, if we disconnect b (such that only c and d are connected), we @@ -605,7 +603,7 @@ func TestPeerManager_TryDialNext_UpgradeReservesPeer(t *testing.T) { c := p2p.NodeAddress{Protocol: "memory", NodeID: types.NodeID(strings.Repeat("c", 40))} peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), c.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, MaxConnected: 1, MaxConnectedUpgrade: 2, }) @@ -772,8 +770,8 @@ func TestPeerManager_DialFailed_UnreservePeer(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(2), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -890,7 +888,7 @@ func TestPeerManager_Dialed_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 2, MaxConnectedUpgrade: 1, - PeerScores: map[types.NodeID]p2p.PeerScore{c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), d.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{c.NodeID: p2p.PeerScore(1), d.NodeID: 1}, }) require.NoError(t, err) @@ -940,7 +938,7 @@ func TestPeerManager_Dialed_Upgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 1, MaxConnectedUpgrade: 2, - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), c.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, }) require.NoError(t, err) @@ -987,10 +985,10 @@ func TestPeerManager_Dialed_UpgradeEvenLower(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(10 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + a.NodeID: p2p.PeerScore(3), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(10), + d.NodeID: p2p.PeerScore(1), }, }) require.NoError(t, err) @@ -1043,9 +1041,9 @@ func TestPeerManager_Dialed_UpgradeNoEvict(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), + a.NodeID: p2p.PeerScore(1), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(3), }, }) require.NoError(t, err) @@ -1164,8 +1162,8 @@ func TestPeerManager_Accepted_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), + c.NodeID: p2p.PeerScore(1), + d.NodeID: p2p.PeerScore(2), }, MaxConnected: 1, MaxConnectedUpgrade: 1, @@ -1212,8 +1210,8 @@ func TestPeerManager_Accepted_Upgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1255,8 +1253,8 @@ func TestPeerManager_Accepted_UpgradeDialing(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1431,7 +1429,7 @@ func TestPeerManager_EvictNext_WakeOnUpgradeDialed(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 1, MaxConnectedUpgrade: 1, - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(p2p.DefaultMutablePeerScore + 1)}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1)}, }) require.NoError(t, err) @@ -1473,7 +1471,7 @@ func TestPeerManager_EvictNext_WakeOnUpgradeAccepted(t *testing.T) { MaxConnected: 1, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), }, }) require.NoError(t, err)