Fix downscore by peers when a node gracefully stops. (#15505)

* Log when downscoring a peer.

* `validateSequenceNumber`: Downscore peer in function, clarify and add logs

* `AddConnectionHandler`: Send majority code to the outer scope (no funtional change).

* `disconnectBadPeer`: Improve log.

* `sendRPCStatusRequest`: Improve log.

* `findPeersWithSubnets`: Add preventive peer filtering.
(As done in `s.findPeers`.)

* `Stop`: Use one `defer` for the whole function.
Reminder: `defer`s are executed backwards.

* `Stop`: Send a goodbye message to all connected peers when stopping the service.

Before this commit, stopping the service did not send any goodbye message to all connected peers. The issue with this approach is that the peer still thinks we are alive, and behaves so by trying to communicate with us. Unfortunatly, because we are offline, we cannot respond. Because of that, the peer starts to downscore us, and then bans us. As a consequence, when we restart, the peer refuses our connection request.

By sending a goodbye message when stopping the service, we ensure the peer stops to expect anything from us. When restarting, everything is allright.

* `ConnectedF` and `DisconnectedF`: Workaround very probable libp2p bug by preventing outbound connection to very recently disconnected peers.

* Fix James' comment.

* Fix James' comment.

* Fix James' comment.

* Fix James' comment.

* Fix James' comment.

* `AddDisconnectionHandler`: Handle multiple close calls to `DisconnectedF` for the same peer.

beacon-chain/p2p/BUILD.bazel

@@ -104,6 +104,7 @@ go_library(
         "@com_github_libp2p_go_mplex//:go_default_library",
         "@com_github_multiformats_go_multiaddr//:go_default_library",
         "@com_github_multiformats_go_multiaddr//net:go_default_library",
+        "@com_github_patrickmn_go_cache//:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
         "@com_github_prometheus_client_golang//prometheus:go_default_library",
         "@com_github_prometheus_client_golang//prometheus/promauto:go_default_library",

beacon-chain/p2p/handshake.go

@@ -13,6 +13,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/host"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/patrickmn/go-cache"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -79,8 +80,8 @@ func (s *Service) disconnectFromPeerOnError(
 // and validating the response from the peer.
 func (s *Service) AddConnectionHandler(reqFunc, goodByeFunc func(ctx context.Context, id peer.ID) error) {
 	// Peer map and lock to keep track of current connection attempts.
+	var peerLock sync.Mutex
 	peerMap := make(map[peer.ID]bool)
-	peerLock := new(sync.Mutex)
 
 	// This is run at the start of each connection attempt, to ensure
 	// that there aren't multiple inflight connection requests for the
@@ -108,6 +109,19 @@ func (s *Service) AddConnectionHandler(reqFunc, goodByeFunc func(ctx context.Con
 	s.host.Network().Notify(&network.NotifyBundle{
 		ConnectedF: func(_ network.Network, conn network.Conn) {
 			remotePeer := conn.RemotePeer()
+			log := log.WithField("peer", remotePeer)
+			direction := conn.Stat().Direction
+
+			// For some reason, right after a disconnection, this `ConnectedF` callback
+			// is called. We want to avoid processing this connection if the peer was
+			// disconnected too recently and if we are at the initiative of this connection.
+			// This is very probably a bug in libp2p.
+			if direction == network.DirOutbound {
+				if err := s.wasDisconnectedTooRecently(remotePeer); err != nil {
+					log.WithError(err).Debug("Skipping connection handler")
+					return
+				}
+			}
 
 			// Connection handler must be non-blocking as part of libp2p design.
 			go func() {
@@ -133,53 +147,56 @@ func (s *Service) AddConnectionHandler(reqFunc, goodByeFunc func(ctx context.Con
 					return
 				}
 
-				// Do not perform handshake on inbound dials.
-				if conn.Stat().Direction == network.DirInbound {
-					_, err := s.peers.ChainState(remotePeer)
-					peerExists := err == nil
-					currentTime := prysmTime.Now()
-
-					// Wait for peer to initiate handshake
-					time.Sleep(timeForStatus)
-
-					// Exit if we are disconnected with the peer.
-					if s.host.Network().Connectedness(remotePeer) != network.Connected {
+				if direction != network.DirInbound {
+					s.peers.SetConnectionState(conn.RemotePeer(), peers.Connecting)
+					if err := reqFunc(context.TODO(), conn.RemotePeer()); err != nil && !errors.Is(err, io.EOF) {
+						s.disconnectFromPeerOnError(conn, goodByeFunc, err)
 						return
 					}
 
-					// If peer hasn't sent a status request, we disconnect with them
-					if _, err := s.peers.ChainState(remotePeer); errors.Is(err, peerdata.ErrPeerUnknown) || errors.Is(err, peerdata.ErrNoPeerStatus) {
-						statusMessageMissing.Inc()
-						s.disconnectFromPeerOnError(conn, goodByeFunc, errors.Wrap(err, "chain state"))
-						return
-					}
-
-					if peerExists {
-						updated, err := s.peers.ChainStateLastUpdated(remotePeer)
-						if err != nil {
-							s.disconnectFromPeerOnError(conn, goodByeFunc, errors.Wrap(err, "chain state last updated"))
-							return
-						}
-
-						// Exit if we don't receive any current status messages from peer.
-						if updated.IsZero() {
-							s.disconnectFromPeerOnError(conn, goodByeFunc, errors.New("is zero"))
-							return
-						}
-
-						if updated.Before(currentTime) {
-							s.disconnectFromPeerOnError(conn, goodByeFunc, errors.New("did not update"))
-							return
-						}
-					}
-
 					s.connectToPeer(conn)
 					return
 				}
 
-				s.peers.SetConnectionState(conn.RemotePeer(), peers.Connecting)
-				if err := reqFunc(context.TODO(), conn.RemotePeer()); err != nil && !errors.Is(err, io.EOF) {
-					s.disconnectFromPeerOnError(conn, goodByeFunc, err)
+				// The connection is inbound.
+				_, err = s.peers.ChainState(remotePeer)
+				peerExists := err == nil
+				currentTime := prysmTime.Now()
+
+				// Wait for peer to initiate handshake
+				time.Sleep(timeForStatus)
+
+				// Exit if we are disconnected with the peer.
+				if s.host.Network().Connectedness(remotePeer) != network.Connected {
+					return
+				}
+
+				// If peer hasn't sent a status request, we disconnect with them
+				if _, err := s.peers.ChainState(remotePeer); errors.Is(err, peerdata.ErrPeerUnknown) || errors.Is(err, peerdata.ErrNoPeerStatus) {
+					statusMessageMissing.Inc()
+					s.disconnectFromPeerOnError(conn, goodByeFunc, errors.Wrap(err, "chain state"))
+					return
+				}
+
+				if !peerExists {
+					s.connectToPeer(conn)
+					return
+				}
+
+				updated, err := s.peers.ChainStateLastUpdated(remotePeer)
+				if err != nil {
+					s.disconnectFromPeerOnError(conn, goodByeFunc, errors.Wrap(err, "chain state last updated"))
+					return
+				}
+
+				// Exit if we don't receive any current status messages from peer.
+				if updated.IsZero() {
+					s.disconnectFromPeerOnError(conn, goodByeFunc, errors.New("is zero"))
+					return
+				}
+
+				if updated.Before(currentTime) {
+					s.disconnectFromPeerOnError(conn, goodByeFunc, errors.New("did not update"))
 					return
 				}
 
@@ -220,6 +237,12 @@ func (s *Service) AddDisconnectionHandler(handler func(ctx context.Context, id p
 				}
 
 				s.peers.SetConnectionState(peerID, peers.Disconnected)
+				if err := s.peerDisconnectionTime.Add(peerID.String(), time.Now(), cache.DefaultExpiration); err != nil {
+					// The `DisconnectedF` funcition already called for this peer less than `cache.DefaultExpiration` ago. Skip.
+					// (Very probably a bug in libp2p.)
+					log.WithError(err).Trace("Failed to set peer disconnection time")
+					return
+				}
 
 				// Only log disconnections if we were fully connected.
 				if priorState == peers.Connected {
@@ -231,6 +254,28 @@ func (s *Service) AddDisconnectionHandler(handler func(ctx context.Context, id p
 	})
 }
 
+// wasDisconnectedTooRecently checks if the peer was disconnected within the last second.
+func (s *Service) wasDisconnectedTooRecently(peerID peer.ID) error {
+	const disconnectionDurationThreshold = 1 * time.Second
+
+	peerDisconnectionTimeObj, ok := s.peerDisconnectionTime.Get(peerID.String())
+	if !ok {
+		return nil
+	}
+
+	peerDisconnectionTime, ok := peerDisconnectionTimeObj.(time.Time)
+	if !ok {
+		return errors.New("invalid peer disconnection time type")
+	}
+
+	timeSinceDisconnection := time.Since(peerDisconnectionTime)
+	if timeSinceDisconnection < disconnectionDurationThreshold {
+		return errors.Errorf("peer %s was disconnected too recently: %s", peerID, timeSinceDisconnection)
+	}
+
+	return nil
+}
+
 func agentString(pid peer.ID, hst host.Host) string {
 	rawVersion, storeErr := hst.Peerstore().Get(pid, agentVersionKey)
 

beacon-chain/p2p/peers/scorers/bad_responses.go

@@ -101,21 +101,24 @@ func (s *BadResponsesScorer) countNoLock(pid peer.ID) (int, error) {
 
 // Increment increments the number of bad responses we have received from the given remote peer.
 // If peer doesn't exist this method is no-op.
-func (s *BadResponsesScorer) Increment(pid peer.ID) {
+func (s *BadResponsesScorer) Increment(pid peer.ID) int {
 	if pid == "" {
-		return
+		return 0
 	}
+
 	s.store.Lock()
 	defer s.store.Unlock()
 
 	peerData, ok := s.store.PeerData(pid)
-	if !ok {
-		s.store.SetPeerData(pid, &peerdata.PeerData{
-			BadResponses: 1,
-		})
-		return
+	if ok {
+		peerData.BadResponses++
+		return peerData.BadResponses
 	}
-	peerData.BadResponses++
+
+	const badResponses = 1
+	peerData = &peerdata.PeerData{BadResponses: badResponses}
+	s.store.SetPeerData(pid, peerData)
+	return badResponses
 }
 
 // IsBadPeer states if the peer is to be considered bad.

beacon-chain/p2p/service.go

@@ -31,6 +31,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/protocol"
 	"github.com/multiformats/go-multiaddr"
+	"github.com/patrickmn/go-cache"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -86,6 +87,7 @@ type Service struct {
 	genesisTime           time.Time
 	genesisValidatorsRoot []byte
 	activeValidatorCount  uint64
+	peerDisconnectionTime *cache.Cache
 }
 
 // NewService initializes a new p2p service compatible with shared.Service interface. No
@@ -115,16 +117,17 @@ func NewService(ctx context.Context, cfg *Config) (*Service, error) {
 	ipLimiter := leakybucket.NewCollector(ipLimit, ipBurst, 30*time.Second, true /* deleteEmptyBuckets */)
 
 	s := &Service{
-		ctx:          ctx,
-		cancel:       cancel,
-		cfg:          cfg,
-		addrFilter:   addrFilter,
-		ipLimiter:    ipLimiter,
-		privKey:      privKey,
-		metaData:     metaData,
-		isPreGenesis: true,
-		joinedTopics: make(map[string]*pubsub.Topic, len(gossipTopicMappings)),
-		subnetsLock:  make(map[uint64]*sync.RWMutex),
+		ctx:                   ctx,
+		cancel:                cancel,
+		cfg:                   cfg,
+		addrFilter:            addrFilter,
+		ipLimiter:             ipLimiter,
+		privKey:               privKey,
+		metaData:              metaData,
+		isPreGenesis:          true,
+		joinedTopics:          make(map[string]*pubsub.Topic, len(gossipTopicMappings)),
+		subnetsLock:           make(map[uint64]*sync.RWMutex),
+		peerDisconnectionTime: cache.New(1*time.Second, 1*time.Minute),
 	}
 
 	ipAddr := prysmnetwork.IPAddr()
@@ -486,14 +489,17 @@ func (s *Service) connectWithPeer(ctx context.Context, info peer.AddrInfo) error
 	if info.ID == s.host.ID() {
 		return nil
 	}
+
 	if err := s.Peers().IsBad(info.ID); err != nil {
-		return errors.Wrap(err, "refused to connect to bad peer")
+		return errors.Wrap(err, "bad peer")
 	}
+
 	ctx, cancel := context.WithTimeout(ctx, maxDialTimeout)
 	defer cancel()
+
 	if err := s.host.Connect(ctx, info); err != nil {
-		s.Peers().Scorers().BadResponsesScorer().Increment(info.ID)
-		return err
+		s.downscorePeer(info.ID, "connectionError")
+		return errors.Wrap(err, "peer connect")
 	}
 	return nil
 }
@@ -524,3 +530,8 @@ func (s *Service) connectToBootnodes() error {
 func (s *Service) isInitialized() bool {
 	return !s.genesisTime.IsZero() && len(s.genesisValidatorsRoot) == 32
 }
+
+func (s *Service) downscorePeer(peerID peer.ID, reason string) {
+	newScore := s.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{"peerID": peerID, "reason": reason, "newScore": newScore}).Debug("Downscore peer")
+}

beacon-chain/p2p/service_test.go

@@ -403,7 +403,7 @@ func TestService_connectWithPeer(t *testing.T) {
 				return ps
 			}(),
 			info:    peer.AddrInfo{ID: "bad"},
-			wantErr: "refused to connect to bad peer",
+			wantErr: "bad peer",
 		},
 	}
 	for _, tt := range tests {

beacon-chain/p2p/subnets.go

@@ -181,6 +181,11 @@ func (s *Service) findPeersWithSubnets(
 		// Get all needed subnets that the node is subscribed to.
 		// Skip nodes that are not subscribed to any of the defective subnets.
 		node := iterator.Node()
+
+		if !s.filterPeer(node) {
+			continue
+		}
+
 		nodeSubnets, err := filter(node)
 		if err != nil {
 			return nil, errors.Wrap(err, "filter node")

beacon-chain/sync/backfill/service.go

@@ -17,6 +17,7 @@ import (
 	"github.com/OffchainLabs/prysm/v6/time/slots"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 type Service struct {
@@ -211,7 +212,7 @@ func (s *Service) importBatches(ctx context.Context) {
 		_, err := s.batchImporter(ctx, current, ib, s.store)
 		if err != nil {
 			log.WithError(err).WithFields(ib.logFields()).Debug("Backfill batch failed to import")
-			s.downscore(ib)
+			s.downscorePeer(ib.blockPid, "backfillBatchImportError")
 			s.batchSeq.update(ib.withState(batchErrRetryable))
 			// If a batch fails, the subsequent batches are no longer considered importable.
 			break
@@ -336,10 +337,6 @@ func (s *Service) initBatches() error {
 	return nil
 }
 
-func (s *Service) downscore(b batch) {
-	s.p2p.Peers().Scorers().BadResponsesScorer().Increment(b.blockPid)
-}
-
 func (*Service) Stop() error {
 	return nil
 }
@@ -383,3 +380,8 @@ func (s *Service) WaitForCompletion() error {
 		return nil
 	}
 }
+
+func (s *Service) downscorePeer(peerID peer.ID, reason string) {
+	newScore := s.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{"peerID": peerID, "reason": reason, "newScore": newScore}).Debug("Downscore peer")
+}

beacon-chain/sync/initial-sync/blocks_queue.go

@@ -337,14 +337,15 @@ func (q *blocksQueue) onDataReceivedEvent(ctx context.Context) eventHandlerFn {
 					}
 				}
 			}
+
 			if errors.Is(response.err, beaconsync.ErrInvalidFetchedData) {
-				// Peer returned invalid data, penalize.
-				q.blocksFetcher.p2p.Peers().Scorers().BadResponsesScorer().Increment(response.blocksFrom)
-				log.WithField("pid", response.blocksFrom).Debug("Peer is penalized for invalid blocks")
-			} else if errors.Is(response.err, verification.ErrBlobInvalid) {
-				q.blocksFetcher.p2p.Peers().Scorers().BadResponsesScorer().Increment(response.blobsFrom)
-				log.WithField("pid", response.blobsFrom).Debug("Peer is penalized for invalid blob response")
+				q.downscorePeer(response.blocksFrom, "invalidBlocks")
 			}
+
+			if errors.Is(response.err, verification.ErrBlobInvalid) {
+				q.downscorePeer(response.blobsFrom, "invalidBlobs")
+			}
+
 			return m.state, response.err
 		}
 		m.fetched = *response
@@ -455,6 +456,11 @@ func (q *blocksQueue) onProcessSkippedEvent(ctx context.Context) eventHandlerFn
 	}
 }
 
+func (q *blocksQueue) downscorePeer(peerID peer.ID, reason string) {
+	newScore := q.blocksFetcher.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{"peerID": peerID, "reason": reason, "newScore": newScore}).Debug("Downscore peer")
+}
+
 // onCheckStaleEvent is an event that allows to mark stale epochs,
 // so that they can be re-processed.
 func onCheckStaleEvent(ctx context.Context) eventHandlerFn {

beacon-chain/sync/initial-sync/blocks_queue_test.go

@@ -522,7 +522,7 @@ func TestBlocksQueue_onDataReceivedEvent(t *testing.T) {
 		})
 		assert.ErrorContains(t, beaconsync.ErrInvalidFetchedData.Error(), err)
 		assert.Equal(t, stateScheduled, updatedState)
-		assert.LogsContain(t, hook, "msg=\"Peer is penalized for invalid blocks\" pid=ZiCa")
+		assert.LogsContain(t, hook, "Downscore peer")
 	})
 
 	t.Run("transition ok", func(t *testing.T) {

beacon-chain/sync/initial-sync/round_robin.go

@@ -14,6 +14,7 @@ import (
 	"github.com/OffchainLabs/prysm/v6/consensus-types/interfaces"
 	"github.com/OffchainLabs/prysm/v6/consensus-types/primitives"
 	"github.com/OffchainLabs/prysm/v6/time/slots"
+	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/paulbellamy/ratecounter"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -345,12 +346,11 @@ func isPunishableError(err error) bool {
 func (s *Service) updatePeerScorerStats(data *blocksQueueFetchedData, count uint64, err error) {
 	if isPunishableError(err) {
 		if verification.IsBlobValidationFailure(err) {
-			log.WithError(err).WithField("peer_id", data.blobsFrom).Warn("Downscoring peer for invalid blobs")
-			s.cfg.P2P.Peers().Scorers().BadResponsesScorer().Increment(data.blobsFrom)
+			s.downscorePeer(data.blobsFrom, "invalidBlobs")
 		} else {
-			log.WithError(err).WithField("peer_id", data.blocksFrom).Warn("Downscoring peer for invalid blocks")
-			s.cfg.P2P.Peers().Scorers().BadResponsesScorer().Increment(data.blocksFrom)
+			s.downscorePeer(data.blocksFrom, "invalidBlocks")
 		}
+
 		// If the error is punishable, exit here so that we don't give them credit for providing bad blocks.
 		return
 	}
@@ -376,3 +376,8 @@ func (s *Service) isProcessedBlock(ctx context.Context, blk blocks.ROBlock) bool
 	}
 	return false
 }
+
+func (s *Service) downscorePeer(peerID peer.ID, reason string) {
+	newScore := s.cfg.P2P.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{"peerID": peerID, "reason": reason, "newScore": newScore}).Debug("Downscore peer")
+}

beacon-chain/sync/rate_limiter.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/libp2p/go-libp2p/core/network"
+	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/trailofbits/go-mutexasserts"
@@ -122,7 +123,7 @@ func (l *limiter) validateRequest(stream network.Stream, amt uint64) error {
 
 	collector, err := l.retrieveCollector(topic)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "retrieve collector")
 	}
 
 	remaining := collector.Remaining(remotePeer.String())
@@ -131,7 +132,7 @@ func (l *limiter) validateRequest(stream network.Stream, amt uint64) error {
 		amt = 1
 	}
 	if amt > uint64(remaining) {
-		l.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeer)
+		l.downscorePeer(remotePeer, topic, "rateLimitExceeded")
 		writeErrorResponseToStream(responseCodeInvalidRequest, p2ptypes.ErrRateLimited.Error(), stream, l.p2p)
 		return p2ptypes.ErrRateLimited
 	}
@@ -139,22 +140,20 @@ func (l *limiter) validateRequest(stream network.Stream, amt uint64) error {
 }
 
 // This is used to validate all incoming rpc streams from external peers.
-func (l *limiter) validateRawRpcRequest(stream network.Stream) error {
+func (l *limiter) validateRawRpcRequest(stream network.Stream, amt uint64) error {
 	l.RLock()
 	defer l.RUnlock()
 
-	topic := rpcLimiterTopic
-
-	collector, err := l.retrieveCollector(topic)
+	remotePeer := stream.Conn().RemotePeer()
+	collector, err := l.retrieveCollector(rpcLimiterTopic)
 	if err != nil {
 		return err
 	}
 	key := stream.Conn().RemotePeer().String()
 	remaining := collector.Remaining(key)
-	// Treat each request as a minimum of 1.
-	amt := int64(1)
-	if amt > remaining {
-		l.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+
+	if amt > uint64(remaining) {
+		l.downscorePeer(remotePeer, rpcLimiterTopic, "rawRateLimitExceeded")
 		writeErrorResponseToStream(responseCodeInvalidRequest, p2ptypes.ErrRateLimited.Error(), stream, l.p2p)
 		return p2ptypes.ErrRateLimited
 	}
@@ -233,3 +232,13 @@ func (l *limiter) retrieveCollector(topic string) (*leakybucket.Collector, error
 func (_ *limiter) topicLogger(topic string) *logrus.Entry {
 	return log.WithField("rateLimiter", topic)
 }
+
+func (l *limiter) downscorePeer(peerID peer.ID, topic, reason string) {
+	newScore := l.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{
+		"peerID":   peerID.String(),
+		"reason":   reason,
+		"newScore": newScore,
+		"topic":    topic,
+	}).Debug("Downscore peer")
+}

beacon-chain/sync/rate_limiter_test.go

@@ -85,16 +85,16 @@ func TestRateLimiter_ExceedRawCapacity(t *testing.T) {
 	require.NoError(t, err, "could not create stream")
 
 	for i := 0; i < 2*defaultBurstLimit; i++ {
-		err = rlimiter.validateRawRpcRequest(stream)
+		err = rlimiter.validateRawRpcRequest(stream, 1)
 		rlimiter.addRawStream(stream)
 		require.NoError(t, err, "could not validate incoming request")
 	}
 	// Triggers rate limit error on burst.
-	assert.ErrorContains(t, p2ptypes.ErrRateLimited.Error(), rlimiter.validateRawRpcRequest(stream))
+	assert.ErrorContains(t, p2ptypes.ErrRateLimited.Error(), rlimiter.validateRawRpcRequest(stream, 1))
 
 	// Make Peer bad.
 	for i := 0; i < defaultBurstLimit; i++ {
-		assert.ErrorContains(t, p2ptypes.ErrRateLimited.Error(), rlimiter.validateRawRpcRequest(stream))
+		assert.ErrorContains(t, p2ptypes.ErrRateLimited.Error(), rlimiter.validateRawRpcRequest(stream, 1))
 	}
 	assert.NotNil(t, p1.Peers().IsBad(p2.PeerID()), "peer is not marked as a bad peer")
 	require.NoError(t, stream.Close(), "could not close stream")

beacon-chain/sync/rpc.go

@@ -20,6 +20,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/pkg/errors"
 	ssz "github.com/prysmaticlabs/fastssz"
+	"github.com/sirupsen/logrus"
 )
 
 var (
@@ -238,7 +239,7 @@ func (s *Service) registerRPC(baseTopic string, handle rpcHandler) {
 		defer span.End()
 		span.SetAttributes(trace.StringAttribute("topic", topic))
 		span.SetAttributes(trace.StringAttribute("peer", remotePeer.String()))
-		log := log.WithField("peer", stream.Conn().RemotePeer().String()).WithField("topic", string(stream.Protocol()))
+		log := log.WithFields(logrus.Fields{"peer": remotePeer.String(), "topic": string(stream.Protocol())})
 
 		// Check before hand that peer is valid.
 		if err := s.cfg.p2p.Peers().IsBad(remotePeer); err != nil {
@@ -248,7 +249,7 @@ func (s *Service) registerRPC(baseTopic string, handle rpcHandler) {
 			return
 		}
 		// Validate request according to peer limits.
-		if err := s.rateLimiter.validateRawRpcRequest(stream); err != nil {
+		if err := s.rateLimiter.validateRawRpcRequest(stream, 1); err != nil {
 			log.WithError(err).Debug("Could not validate rpc request from peer")
 			return
 		}
@@ -304,7 +305,7 @@ func (s *Service) registerRPC(baseTopic string, handle rpcHandler) {
 			if err := s.cfg.p2p.Encoding().DecodeWithMaxLength(stream, msg); err != nil {
 				logStreamErrors(err, topic)
 				tracing.AnnotateError(span, err)
-				s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+				s.downscorePeer(remotePeer, "registerRpcError")
 				return
 			}
 			if err := handle(ctx, msg, stream); err != nil {
@@ -324,7 +325,7 @@ func (s *Service) registerRPC(baseTopic string, handle rpcHandler) {
 			if err := s.cfg.p2p.Encoding().DecodeWithMaxLength(stream, msg); err != nil {
 				logStreamErrors(err, topic)
 				tracing.AnnotateError(span, err)
-				s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+				s.downscorePeer(remotePeer, "registerRpcError")
 				return
 			}
 			if err := handle(ctx, nTyp.Elem().Interface(), stream); err != nil {

beacon-chain/sync/rpc_beacon_blocks_by_range.go

@@ -15,6 +15,7 @@ import (
 	pb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1"
 	"github.com/OffchainLabs/prysm/v6/time/slots"
 	libp2pcore "github.com/libp2p/go-libp2p/core"
+	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -43,7 +44,7 @@ func (s *Service) beaconBlocksByRangeRPCHandler(ctx context.Context, msg interfa
 	rp, err := validateRangeRequest(m, s.cfg.clock.CurrentSlot())
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeer)
+		s.downscorePeer(remotePeer, "beaconBlocksByRangeRPCHandlerValidationError")
 		tracing.AnnotateError(span, err)
 		return err
 	}
@@ -201,3 +202,13 @@ func (s *Service) writeBlockBatchToStream(ctx context.Context, batch blockBatch,
 
 	return nil
 }
+
+func (s *Service) downscorePeer(peerID peer.ID, reason string, fields ...logrus.Fields) {
+	log := log
+	for _, field := range fields {
+		log = log.WithFields(field)
+	}
+
+	newScore := s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+	log.WithFields(logrus.Fields{"peerID": peerID, "reason": reason, "newScore": newScore}).Debug("Downscore peer")
+}

beacon-chain/sync/rpc_beacon_blocks_by_root.go

@@ -92,9 +92,11 @@ func (s *Service) beaconBlocksRootRPCHandler(ctx context.Context, msg interface{
 		return errors.New("no block roots provided")
 	}
 
+	remotePeer := stream.Conn().RemotePeer()
+
 	currentEpoch := slots.ToEpoch(s.cfg.clock.CurrentSlot())
 	if uint64(len(blockRoots)) > params.MaxRequestBlock(currentEpoch) {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+		s.downscorePeer(remotePeer, "beaconBlocksRootRPCHandlerTooManyRoots")
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, "requested more than the max block limit", stream)
 		return errors.New("requested more than the max block limit")
 	}

beacon-chain/sync/rpc_blob_sidecars_by_range.go

@@ -74,10 +74,13 @@ func (s *Service) blobSidecarsByRangeRPCHandler(ctx context.Context, msg interfa
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
 		return err
 	}
+
+	remotePeer := stream.Conn().RemotePeer()
+
 	rp, err := validateBlobsByRange(r, s.cfg.chain.CurrentSlot())
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+		s.downscorePeer(remotePeer, "blobSidecarsByRangeRpcHandlerValidationError")
 		tracing.AnnotateError(span, err)
 		return err
 	}
@@ -87,7 +90,7 @@ func (s *Service) blobSidecarsByRangeRPCHandler(ctx context.Context, msg interfa
 	defer ticker.Stop()
 	batcher, err := newBlockRangeBatcher(rp, s.cfg.beaconDB, s.rateLimiter, s.cfg.chain.IsCanonical, ticker)
 	if err != nil {
-		log.WithError(err).Info("error in BlobSidecarsByRange batch")
+		log.WithError(err).Error("Cannot create new block range batcher")
 		s.writeErrorResponseToStream(responseCodeServerError, p2ptypes.ErrGeneric.Error(), stream)
 		tracing.AnnotateError(span, err)
 		return err
@@ -112,7 +115,7 @@ func (s *Service) blobSidecarsByRangeRPCHandler(ctx context.Context, msg interfa
 		}
 	}
 	if err := batch.error(); err != nil {
-		log.WithError(err).Debug("error in BlobSidecarsByRange batch")
+		log.WithError(err).Debug("Error in BlobSidecarsByRange batch")
 
 		// If a rate limit is hit, it means an error response has already been sent and the stream has been closed.
 		if !errors.Is(err, p2ptypes.ErrRateLimited) {

beacon-chain/sync/rpc_blob_sidecars_by_root.go

@@ -39,7 +39,7 @@ func (s *Service) blobSidecarByRootRPCHandler(ctx context.Context, msg interface
 	cs := s.cfg.clock.CurrentSlot()
 	remotePeer := stream.Conn().RemotePeer()
 	if err := validateBlobByRootRequest(blobIdents, cs); err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeer)
+		s.downscorePeer(remotePeer, "blobSidecarsByRootRpcHandlerValidationError")
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
 		return err
 	}

beacon-chain/sync/rpc_data_column_sidecars_by_range.go

@@ -65,7 +65,7 @@ func (s *Service) dataColumnSidecarsByRangeRPCHandler(ctx context.Context, msg i
 	rangeParameters, err := validateDataColumnsByRange(request, s.cfg.chain.CurrentSlot())
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeer)
+		s.downscorePeer(remotePeer, "dataColumnSidecarsByRangeRpcHandlerValidationError")
 		tracing.AnnotateError(span, err)
 		return errors.Wrap(err, "validate data columns by range")
 	}

beacon-chain/sync/rpc_data_column_sidecars_by_root.go

@@ -42,7 +42,7 @@ func (s *Service) dataColumnSidecarByRootRPCHandler(ctx context.Context, msg int
 	}
 
 	requestedColumnIdents := *ref
-	remotePeerId := stream.Conn().RemotePeer()
+	remotePeer := stream.Conn().RemotePeer()
 
 	ctx, cancel := context.WithTimeout(ctx, ttfbTimeout)
 	defer cancel()
@@ -51,7 +51,7 @@ func (s *Service) dataColumnSidecarByRootRPCHandler(ctx context.Context, msg int
 
 	// Penalize peers that send invalid requests.
 	if err := validateDataColumnsByRootRequest(requestedColumnIdents); err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeerId)
+		s.downscorePeer(remotePeer, "dataColumnSidecarByRootRPCHandlerValidationError")
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
 		return errors.Wrap(err, "validate data columns by root request")
 	}
@@ -85,7 +85,7 @@ func (s *Service) dataColumnSidecarByRootRPCHandler(ctx context.Context, msg int
 	}
 
 	log := log.WithFields(logrus.Fields{
-		"peer":    remotePeerId,
+		"peer":    remotePeer,
 		"columns": requestedColumnsByRootLog,
 	})
 

beacon-chain/sync/rpc_goodbye.go

@@ -13,6 +13,7 @@ import (
 	libp2pcore "github.com/libp2p/go-libp2p/core"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -35,22 +36,40 @@ var backOffTime = map[primitives.SSZUint64]time.Duration{
 
 // goodbyeRPCHandler reads the incoming goodbye rpc message from the peer.
 func (s *Service) goodbyeRPCHandler(_ context.Context, msg interface{}, stream libp2pcore.Stream) error {
+	const amount = 1
 	SetRPCStreamDeadlines(stream)
+	peerID := stream.Conn().RemotePeer()
 
 	m, ok := msg.(*primitives.SSZUint64)
 	if !ok {
 		return fmt.Errorf("wrong message type for goodbye, got %T, wanted *uint64", msg)
 	}
-	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
-		log.WithError(err).Debug("Goodbye message from rate-limited peer")
-	} else {
+
+	isRateLimitedPeer := false
+	if err := s.rateLimiter.validateRequest(stream, amount); err != nil {
+		if !errors.Is(err, p2ptypes.ErrRateLimited) {
+			return errors.Wrap(err, "validate request")
+		}
+		isRateLimitedPeer = true
+	}
+
+	if !isRateLimitedPeer {
 		s.rateLimiter.add(stream, 1)
 	}
-	log := log.WithField("Reason", goodbyeMessage(*m))
-	log.WithField("peer", stream.Conn().RemotePeer()).Trace("Peer has sent a goodbye message")
-	s.cfg.p2p.Peers().SetNextValidTime(stream.Conn().RemotePeer(), goodByeBackoff(*m))
-	// closes all streams with the peer
-	return s.cfg.p2p.Disconnect(stream.Conn().RemotePeer())
+
+	log.WithFields(logrus.Fields{
+		"peer":          peerID,
+		"reason":        goodbyeMessage(*m),
+		"isRateLimited": isRateLimitedPeer,
+	}).Debug("Received a goodbye message")
+
+	s.cfg.p2p.Peers().SetNextValidTime(peerID, goodByeBackoff(*m))
+
+	if err := s.cfg.p2p.Disconnect(peerID); err != nil {
+		return errors.Wrap(err, "disconnect")
+	}
+
+	return nil
 }
 
 // disconnectBadPeer checks whether peer is considered bad by some scorer, and tries to disconnect
@@ -70,7 +89,7 @@ func (s *Service) disconnectBadPeer(ctx context.Context, id peer.ID, badPeerErr
 			"peerID": id,
 			"agent":  agentString(id, s.cfg.p2p.Host()),
 		}).
-		Debug("Sent peer disconnection")
+		Debug("Sent bad peer disconnection")
 }
 
 // A custom goodbye method that is used by our connection handler, in the

beacon-chain/sync/rpc_light_client.go

@@ -26,7 +26,7 @@ func (s *Service) lightClientBootstrapRPCHandler(ctx context.Context, msg interf
 
 	SetRPCStreamDeadlines(stream)
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
-		logger.WithError(err).Error("s.rateLimiter.validateRequest")
+		logger.WithError(err).Error("Cannot validate request")
 		return err
 	}
 	s.rateLimiter.add(stream, 1)
@@ -42,7 +42,7 @@ func (s *Service) lightClientBootstrapRPCHandler(ctx context.Context, msg interf
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeServerError, types.ErrGeneric.Error(), stream)
 		tracing.AnnotateError(span, err)
-		logger.WithError(err).Error("s.cfg.beaconDB.LightClientBootstrap")
+		logger.WithError(err).Error("Cannot bootstrap light client")
 		return err
 	}
 	if bootstrap == nil {
@@ -74,10 +74,11 @@ func (s *Service) lightClientUpdatesByRangeRPCHandler(ctx context.Context, msg i
 	defer cancel()
 
 	logger := log.WithField("handler", p2p.LightClientUpdatesByRangeName[1:])
+	remotePeer := stream.Conn().RemotePeer()
 
 	SetRPCStreamDeadlines(stream)
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
-		logger.WithError(err).Error("s.rateLimiter.validateRequest")
+		logger.WithError(err).Error("Cannot validate request")
 		return err
 	}
 	s.rateLimiter.add(stream, 1)
@@ -90,7 +91,8 @@ func (s *Service) lightClientUpdatesByRangeRPCHandler(ctx context.Context, msg i
 
 	if r.Count == 0 {
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, "count is 0", stream)
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+		s.downscorePeer(remotePeer, "lightClientUpdatesByRangeRPCHandlerCount0")
+
 		logger.Error("Count is 0")
 		return nil
 	}
@@ -102,7 +104,7 @@ func (s *Service) lightClientUpdatesByRangeRPCHandler(ctx context.Context, msg i
 	endPeriod, err := math.Add64(r.StartPeriod, r.Count-1)
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeInvalidRequest, err.Error(), stream)
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
+		s.downscorePeer(remotePeer, "lightClientUpdatesByRangeRPCHandlerEndPeriodOverflow")
 		tracing.AnnotateError(span, err)
 		logger.WithError(err).Error("End period overflows")
 		return err
@@ -114,7 +116,7 @@ func (s *Service) lightClientUpdatesByRangeRPCHandler(ctx context.Context, msg i
 	if err != nil {
 		s.writeErrorResponseToStream(responseCodeServerError, types.ErrGeneric.Error(), stream)
 		tracing.AnnotateError(span, err)
-		logger.WithError(err).Error("s.cfg.beaconDB.LightClientUpdates")
+		logger.WithError(err).Error("Cannot retrieve light client updates")
 		return err
 	}
 
@@ -153,7 +155,7 @@ func (s *Service) lightClientFinalityUpdateRPCHandler(ctx context.Context, _ int
 
 	SetRPCStreamDeadlines(stream)
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
-		logger.WithError(err).Error("s.rateLimiter.validateRequest")
+		logger.WithError(err).Error("Cannot validate request")
 		return err
 	}
 	s.rateLimiter.add(stream, 1)
@@ -191,7 +193,7 @@ func (s *Service) lightClientOptimisticUpdateRPCHandler(ctx context.Context, _ i
 
 	SetRPCStreamDeadlines(stream)
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
-		logger.WithError(err).Error("s.rateLimiter.validateRequest")
+		logger.WithError(err).Error("Cannot validate request")
 		return err
 	}
 	s.rateLimiter.add(stream, 1)

beacon-chain/sync/rpc_metadata.go

@@ -172,12 +172,12 @@ func (s *Service) sendMetaDataRequest(ctx context.Context, peerID peer.ID) (meta
 	// Read the METADATA response from the peer.
 	code, errMsg, err := ReadStatusCode(stream, s.cfg.p2p.Encoding())
 	if err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+		s.downscorePeer(peerID, "MetadataReadStatusCodeError")
 		return nil, errors.Wrap(err, "read status code")
 	}
 
 	if code != 0 {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+		s.downscorePeer(peerID, "NonNullMetadataReadStatusCode")
 		return nil, errors.New(errMsg)
 	}
 
@@ -214,8 +214,8 @@ func (s *Service) sendMetaDataRequest(ctx context.Context, peerID peer.ID) (meta
 
 	// Decode the metadata from the peer.
 	if err := s.cfg.p2p.Encoding().DecodeWithMaxLength(stream, msg); err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
-		return nil, err
+		s.downscorePeer(peerID, "MetadataDecodeError")
+		return nil, errors.Wrap(err, "decode with max length")
 	}
 
 	return msg, nil

beacon-chain/sync/rpc_ping.go

@@ -13,6 +13,7 @@ import (
 	libp2pcore "github.com/libp2p/go-libp2p/core"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // pingHandler reads the incoming ping rpc message from the peer.
@@ -27,6 +28,7 @@ func (s *Service) pingHandler(_ context.Context, msg interface{}, stream libp2pc
 	if !ok {
 		return fmt.Errorf("wrong message type for ping, got %T, wanted *uint64", msg)
 	}
+	sequenceNumber := uint64(*m)
 
 	// Validate the incoming request regarding rate limiting.
 	if err := s.rateLimiter.validateRequest(stream, 1); err != nil {
@@ -39,14 +41,9 @@ func (s *Service) pingHandler(_ context.Context, msg interface{}, stream libp2pc
 	peerID := stream.Conn().RemotePeer()
 
 	// Check if the peer sequence number is higher than the one we have in our store.
-	valid, err := s.validateSequenceNum(*m, peerID)
+	valid, err := s.isSequenceNumberUpToDate(sequenceNumber, peerID)
 	if err != nil {
-		// Descore peer for giving us a bad sequence number.
-		if errors.Is(err, p2ptypes.ErrInvalidSequenceNum) {
-			s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
-			s.writeErrorResponseToStream(responseCodeInvalidRequest, p2ptypes.ErrInvalidSequenceNum.Error(), stream)
-		}
-
+		s.writeErrorResponseToStream(responseCodeInvalidRequest, p2ptypes.ErrInvalidSequenceNum.Error(), stream)
 		return errors.Wrap(err, "validate sequence number")
 	}
 
@@ -141,7 +138,7 @@ func (s *Service) sendPingRequest(ctx context.Context, peerID peer.ID) error {
 
 	// If the peer responded with an error, increment the bad responses scorer.
 	if code != 0 {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
+		s.downscorePeer(peerID, "NotNullPingReadStatusCode")
 		return errors.Errorf("code: %d - %s", code, errMsg)
 	}
 
@@ -150,15 +147,11 @@ func (s *Service) sendPingRequest(ctx context.Context, peerID peer.ID) error {
 	if err := s.cfg.p2p.Encoding().DecodeWithMaxLength(stream, msg); err != nil {
 		return errors.Wrap(err, "decode sequence number")
 	}
+	sequenceNumber := uint64(*msg)
 
 	// Determine if the peer's sequence number returned by the peer is higher than the one we have in our store.
-	valid, err := s.validateSequenceNum(*msg, peerID)
+	valid, err := s.isSequenceNumberUpToDate(sequenceNumber, peerID)
 	if err != nil {
-		// Descore peer for giving us a bad sequence number.
-		if errors.Is(err, p2ptypes.ErrInvalidSequenceNum) {
-			s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(peerID)
-		}
-
 		return errors.Wrap(err, "validate sequence number")
 	}
 
@@ -180,27 +173,36 @@ func (s *Service) sendPingRequest(ctx context.Context, peerID peer.ID) error {
 	return nil
 }
 
-// validateSequenceNum validates the peer's sequence number.
-// - If the peer's sequence number is greater than the sequence number we have in our store for the peer, return false.
-// - If the peer's sequence number is equal to the sequence number we have in our store for the peer, return true.
-// - If the peer's sequence number is less than the sequence number we have in our store for the peer, return an error.
-func (s *Service) validateSequenceNum(seq primitives.SSZUint64, id peer.ID) (bool, error) {
+// isSequenceNumberUpToDate check if our internal sequence number for the peer is up to date wrt. the incoming one.
+// - If the incoming sequence number is greater than the sequence number we have in our store for the peer, return false.
+// - If the incoming sequence number is equal to the sequence number we have in our store for the peer, return true.
+// - If the incoming sequence number is less than the sequence number we have in our store for the peer, return an error.
+func (s *Service) isSequenceNumberUpToDate(incomingSequenceNumber uint64, peerID peer.ID) (bool, error) {
 	// Retrieve the metadata for the peer we got in our store.
-	md, err := s.cfg.p2p.Peers().Metadata(id)
+	storedMetadata, err := s.cfg.p2p.Peers().Metadata(peerID)
 	if err != nil {
-		return false, errors.Wrap(err, "get metadata")
+		return false, errors.Wrap(err, "peers metadata")
 	}
 
 	// If we have no metadata for the peer, return false.
-	if md == nil || md.IsNil() {
+	if storedMetadata == nil || storedMetadata.IsNil() {
 		return false, nil
 	}
 
 	// The peer's sequence number must be less than or equal to the sequence number we have in our store.
-	if md.SequenceNumber() > uint64(seq) {
+	storedSequenceNumber := storedMetadata.SequenceNumber()
+	if storedSequenceNumber > incomingSequenceNumber {
+		s.downscorePeer(peerID, "pingInvalidSequenceNumber", logrus.Fields{
+			"storedSequenceNumber":   storedSequenceNumber,
+			"incomingSequenceNumber": incomingSequenceNumber,
+		})
 		return false, p2ptypes.ErrInvalidSequenceNum
 	}
 
-	// Return true if the peer's sequence number is equal to the sequence number we have in our store.
-	return md.SequenceNumber() == uint64(seq), nil
+	// If this is the case, our information about the peer is outdated.
+	if storedSequenceNumber < incomingSequenceNumber {
+		return false, nil
+	}
+
+	return true, nil
 }

beacon-chain/sync/rpc_status.go

@@ -35,6 +35,9 @@ func (s *Service) maintainPeerStatuses() {
 			wg.Add(1)
 			go func(id peer.ID) {
 				defer wg.Done()
+
+				log := log.WithField("peer", id)
+
 				// If our peer status has not been updated correctly we disconnect over here
 				// and set the connection state over here instead.
 				if s.cfg.p2p.Host().Network().Connectedness(id) != network.Connected {
@@ -43,27 +46,26 @@ func (s *Service) maintainPeerStatuses() {
 						log.WithError(err).Debug("Error when disconnecting with peer")
 					}
 					s.cfg.p2p.Peers().SetConnectionState(id, peers.Disconnected)
-					log.WithFields(logrus.Fields{
-						"peer":   id,
-						"reason": "maintain peer statuses - peer is not connected",
-					}).Debug("Initiate peer disconnection")
+					log.WithField("reason", "maintainPeerStatusesNotConnectedPeer").Debug("Initiate peer disconnection")
 					return
 				}
+
 				// Disconnect from peers that are considered bad by any of the registered scorers.
 				if err := s.cfg.p2p.Peers().IsBad(id); err != nil {
 					s.disconnectBadPeer(s.ctx, id, err)
 					return
 				}
+
 				// If the status hasn't been updated in the recent interval time.
 				lastUpdated, err := s.cfg.p2p.Peers().ChainStateLastUpdated(id)
 				if err != nil {
 					// Peer has vanished; nothing to do.
 					return
 				}
+
 				if prysmTime.Now().After(lastUpdated.Add(interval)) {
 					if err := s.reValidatePeer(s.ctx, id); err != nil {
-						log.WithField("peer", id).WithError(err).Debug("Could not revalidate peer")
-						s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(id)
+						log.WithError(err).Debug("Cannot re-validate peer")
 					}
 				}
 			}(pid)
@@ -128,19 +130,20 @@ func (s *Service) shouldReSync() bool {
 }
 
 // sendRPCStatusRequest for a given topic with an expected protobuf message type.
-func (s *Service) sendRPCStatusRequest(ctx context.Context, id peer.ID) error {
+func (s *Service) sendRPCStatusRequest(ctx context.Context, peer peer.ID) error {
 	ctx, cancel := context.WithTimeout(ctx, respTimeout)
 	defer cancel()
 
 	headRoot, err := s.cfg.chain.HeadRoot(ctx)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "head root")
 	}
 
 	forkDigest, err := s.currentForkDigest()
 	if err != nil {
-		return err
+		return errors.Wrap(err, "current fork digest")
 	}
+
 	cp := s.cfg.chain.FinalizedCheckpt()
 	resp := &pb.Status{
 		ForkDigest:     forkDigest[:],
@@ -149,37 +152,42 @@ func (s *Service) sendRPCStatusRequest(ctx context.Context, id peer.ID) error {
 		HeadRoot:       headRoot,
 		HeadSlot:       s.cfg.chain.HeadSlot(),
 	}
+
+	log := log.WithField("peer", peer)
+
 	topic, err := p2p.TopicFromMessage(p2p.StatusMessageName, slots.ToEpoch(s.cfg.clock.CurrentSlot()))
 	if err != nil {
-		return err
+		return errors.Wrap(err, "topic from message")
 	}
-	stream, err := s.cfg.p2p.Send(ctx, resp, topic, id)
+
+	stream, err := s.cfg.p2p.Send(ctx, resp, topic, peer)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "send p2p message")
 	}
 	defer closeStream(stream, log)
 
 	code, errMsg, err := ReadStatusCode(stream, s.cfg.p2p.Encoding())
 	if err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
-		return err
+		s.downscorePeer(peer, "statusRequestReadStatusCodeError")
+		return errors.Wrap(err, "read status code")
 	}
 
 	if code != 0 {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(id)
+		s.downscorePeer(peer, "statusRequestNonNullStatusCode")
 		return errors.New(errMsg)
 	}
+
 	msg := &pb.Status{}
 	if err := s.cfg.p2p.Encoding().DecodeWithMaxLength(stream, msg); err != nil {
-		s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(stream.Conn().RemotePeer())
-		return err
+		s.downscorePeer(peer, "statusRequestDecodeError")
+		return errors.Wrap(err, "decode status message")
 	}
 
 	// If validation fails, validation error is logged, and peer status scorer will mark peer as bad.
 	err = s.validateStatusMessage(ctx, msg)
-	s.cfg.p2p.Peers().Scorers().PeerStatusScorer().SetPeerStatus(id, msg, err)
-	if err := s.cfg.p2p.Peers().IsBad(id); err != nil {
-		s.disconnectBadPeer(s.ctx, id, err)
+	s.cfg.p2p.Peers().Scorers().PeerStatusScorer().SetPeerStatus(peer, msg, err)
+	if err := s.cfg.p2p.Peers().IsBad(peer); err != nil {
+		s.disconnectBadPeer(s.ctx, peer, err)
 	}
 	return err
 }
@@ -238,7 +246,7 @@ func (s *Service) statusRPCHandler(ctx context.Context, msg interface{}, stream
 			return nil
 		default:
 			respCode = responseCodeInvalidRequest
-			s.cfg.p2p.Peers().Scorers().BadResponsesScorer().Increment(remotePeer)
+			s.downscorePeer(remotePeer, "statusRpcHandlerInvalidMessage")
 		}
 
 		originalErr := err

beacon-chain/sync/service.go

@@ -11,10 +11,12 @@ import (
 
 	lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client"
 	"github.com/OffchainLabs/prysm/v6/beacon-chain/core/peerdas"
+	p2ptypes "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p/types"
 	"github.com/OffchainLabs/prysm/v6/crypto/rand"
 	lru "github.com/hashicorp/golang-lru"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	libp2pcore "github.com/libp2p/go-libp2p/core"
+	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
 	gcache "github.com/patrickmn/go-cache"
 	"github.com/pkg/errors"
@@ -278,19 +280,32 @@ func (s *Service) Start() {
 // Stop the regular sync service.
 func (s *Service) Stop() error {
 	defer func() {
+		s.cancel()
+
 		if s.rateLimiter != nil {
 			s.rateLimiter.free()
 		}
 	}()
+
+	// Say goodbye to all peers.
+	for _, peerID := range s.cfg.p2p.Peers().Connected() {
+		if s.cfg.p2p.Host().Network().Connectedness(peerID) == network.Connected {
+			if err := s.sendGoodByeAndDisconnect(s.ctx, p2ptypes.GoodbyeCodeClientShutdown, peerID); err != nil {
+				log.WithError(err).WithField("peerID", peerID).Error("Failed to send goodbye message")
+			}
+		}
+	}
+
 	// Removing RPC Stream handlers.
 	for _, p := range s.cfg.p2p.Host().Mux().Protocols() {
 		s.cfg.p2p.Host().RemoveStreamHandler(p)
 	}
+
 	// Deregister Topic Subscribers.
 	for _, t := range s.cfg.p2p.PubSub().GetTopics() {
 		s.unSubscribeFromTopic(t)
 	}
-	defer s.cancel()
+
 	return nil
 }
 

changelog/manu-peer-ban-at-restart.md

@@ -0,0 +1,2 @@
+### Fixed 
+- Fixed various reasons why a node is banned by its peers when it stops.