From 37e0779d6d9bde45e3322922d993c37c91fbd846 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed, 10 Nov 2021 11:10:06 -0500
Subject: [PATCH] p2p: reduce peer score for dial failures (backport #7265)
 (#7271)

* p2p: reduce peer score for dial failures (#7265)

When dialing fails to succeed we should reduce the score of the peer,
which puts the peer at (potentially) greater chances of being removed
from the peer manager, and reduces the chance of the peer being
gossiped by the PEX reactor.

(cherry picked from commit 27560cf7a4dc5c8a1fed98f6bbe86898c1ca790f)

Co-authored-by: Sam Kleinman <garen@tychoish.com>
---
 CHANGELOG_PENDING.md        |  2 ++
 internal/p2p/peermanager.go | 21 ++++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index 56efd763b..01b331e56 100644
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@@ -16,6 +16,8 @@ Special thanks to external contributors on this release:
 
 - P2P Protocol
 
+  - [p2p] \#7265 Peer manager reduces peer score for each failed dial attempts for peers that have not successfully dialed. (@tychoish)
+
 - Go API
 
 - Blockchain Protocol
diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go
index b89b5c8d0..397b16941 100644
--- a/internal/p2p/peermanager.go
+++ b/internal/p2p/peermanager.go
@@ -534,6 +534,7 @@ func (m *PeerManager) DialFailed(address NodeAddress) error {
 	if !ok {
 		return nil // Assume the address has been removed, ignore.
 	}
+
 	addressInfo.LastDialFailure = time.Now().UTC()
 	addressInfo.DialFailures++
 	if err := m.store.Set(peer); err != nil {
@@ -604,6 +605,7 @@ func (m *PeerManager) Dialed(address NodeAddress) error {
 		addressInfo.LastDialSuccess = now
 		// If not found, assume address has been removed.
 	}
+
 	if err := m.store.Set(peer); err != nil {
 		return err
 	}
@@ -662,6 +664,11 @@ func (m *PeerManager) Accepted(peerID types.NodeID) error {
 		peer = m.newPeerInfo(peerID)
 	}
 
+	// reset this to avoid penalizing peers for their past transgressions
+	for _, addr := range peer.AddressInfo {
+		addr.DialFailures = 0
+	}
+
 	// If all connections slots are full, but we allow upgrades (and we checked
 	// above that we have upgrade capacity), then we can look for a lower-scored
 	// peer to replace and if found accept the connection anyway and evict it.
@@ -1289,15 +1296,23 @@ func (p *peerInfo) Score() PeerScore {
 		return PeerScorePersistent
 	}
 
-	if p.MutableScore <= 0 {
+	score := p.MutableScore
+
+	for _, addr := range p.AddressInfo {
+		// DialFailures is reset when dials succeed, so this
+		// is either the number of dial failures or 0.
+		score -= int64(addr.DialFailures)
+	}
+
+	if score <= 0 {
 		return 0
 	}
 
-	if p.MutableScore >= math.MaxUint8 {
+	if score >= math.MaxUint8 {
 		return PeerScore(math.MaxUint8)
 	}
 
-	return PeerScore(p.MutableScore)
+	return PeerScore(score)
 }
 
 // Validate validates the peer info.