Merge branch 'develop' into qbzzt/230325-devnode-l2-port

op-node/p2p/peer_gater.go

@@ -3,7 +3,6 @@ package p2p
 import (
 	log "github.com/ethereum/go-ethereum/log"
 	peer "github.com/libp2p/go-libp2p/core/peer"
-	slices "golang.org/x/exp/slices"
 )
 
 // ConnectionFactor is the factor by which we multiply the connection score.
@@ -15,6 +14,7 @@ const PeerScoreThreshold = -100
 // gater is an internal implementation of the [PeerGater] interface.
 type gater struct {
 	connGater  ConnectionGater
+	blockedMap map[peer.ID]bool
 	log        log.Logger
 	banEnabled bool
 }
@@ -25,33 +25,51 @@ type gater struct {
 type PeerGater interface {
 	// Update handles a peer score update and blocks/unblocks the peer if necessary.
 	Update(peer.ID, float64)
+	// IsBlocked returns true if the given [peer.ID] is blocked.
+	IsBlocked(peer.ID) bool
 }
 
 // NewPeerGater returns a new peer gater.
 func NewPeerGater(connGater ConnectionGater, log log.Logger, banEnabled bool) PeerGater {
 	return &gater{
 		connGater:  connGater,
+		blockedMap: make(map[peer.ID]bool),
 		log:        log,
 		banEnabled: banEnabled,
 	}
 }
 
+// IsBlocked returns true if the given [peer.ID] is blocked.
+func (s *gater) IsBlocked(peerID peer.ID) bool {
+	return s.blockedMap[peerID]
+}
+
+// setBlocked sets the blocked status of the given [peer.ID].
+func (s *gater) setBlocked(peerID peer.ID, blocked bool) {
+	s.blockedMap[peerID] = blocked
+}
+
 // Update handles a peer score update and blocks/unblocks the peer if necessary.
 func (s *gater) Update(id peer.ID, score float64) {
 	// Check if the peer score is below the threshold
 	// If so, we need to block the peer
-	if score < PeerScoreThreshold && s.banEnabled {
+	isAlreadyBlocked := s.IsBlocked(id)
+	if score < PeerScoreThreshold && s.banEnabled && !isAlreadyBlocked {
 		s.log.Warn("peer blocking enabled, blocking peer", "id", id.String(), "score", score)
 		err := s.connGater.BlockPeer(id)
 		if err != nil {
 			s.log.Warn("connection gater failed to block peer", "id", id.String(), "err", err)
 		}
+		// Set the peer as blocked in the blocked map
+		s.setBlocked(id, true)
 	}
 	// Unblock peers whose score has recovered to an acceptable level
-	if (score > PeerScoreThreshold) && slices.Contains(s.connGater.ListBlockedPeers(), id) {
+	if (score > PeerScoreThreshold) && isAlreadyBlocked {
 		err := s.connGater.UnblockPeer(id)
 		if err != nil {
 			s.log.Warn("connection gater failed to unblock peer", "id", id.String(), "err", err)
 		}
+		// Set the peer as unblocked in the blocked map
+		s.setBlocked(id, false)
 	}
 }

op-node/p2p/peer_gater_test.go

@@ -37,30 +37,59 @@ func (testSuite *PeerGaterTestSuite) TestPeerScoreConstants() {
 }
 
 // TestPeerGaterUpdate tests the peer gater update hook.
-func (testSuite *PeerGaterTestSuite) TestPeerGaterUpdate() {
+func (testSuite *PeerGaterTestSuite) TestPeerGater_UpdateBansPeers() {
 	gater := p2p.NewPeerGater(
 		testSuite.mockGater,
 		testSuite.logger,
 		true,
 	)
 
+	// Return an empty list of already blocked peers
+	testSuite.mockGater.On("ListBlockedPeers").Return([]peer.ID{}).Once()
+
 	// Mock a connection gater peer block call
 	// Since the peer score is below the [PeerScoreThreshold] of -100,
 	// the [BlockPeer] method should be called
-	testSuite.mockGater.On("BlockPeer", peer.ID("peer1")).Return(nil)
+	testSuite.mockGater.On("BlockPeer", peer.ID("peer1")).Return(nil).Once()
+
+	// The peer should initially be unblocked
+	testSuite.False(gater.IsBlocked(peer.ID("peer1")))
 
 	// Apply the peer gater update
-	gater.Update(peer.ID("peer1"), float64(-100))
+	gater.Update(peer.ID("peer1"), float64(-101))
+
+	// The peer should be considered blocked
+	testSuite.True(gater.IsBlocked(peer.ID("peer1")))
+
+	// Now let's unblock the peer
+	testSuite.mockGater.On("UnblockPeer", peer.ID("peer1")).Return(nil).Once()
+	gater.Update(peer.ID("peer1"), float64(0))
+
+	// The peer should be considered unblocked
+	testSuite.False(gater.IsBlocked(peer.ID("peer1")))
 }
 
 // TestPeerGaterUpdateNoBanning tests the peer gater update hook without banning set
-func (testSuite *PeerGaterTestSuite) TestPeerGaterUpdateNoBanning() {
+func (testSuite *PeerGaterTestSuite) TestPeerGater_UpdateNoBanning() {
 	gater := p2p.NewPeerGater(
 		testSuite.mockGater,
 		testSuite.logger,
 		false,
 	)
 
+	// Return an empty list of already blocked peers
+	testSuite.mockGater.On("ListBlockedPeers").Return([]peer.ID{})
+
 	// Notice: [BlockPeer] should not be called since banning is not enabled
+	// even though the peer score is way below the [PeerScoreThreshold] of -100
 	gater.Update(peer.ID("peer1"), float64(-100000))
+
+	// The peer should be unblocked
+	testSuite.False(gater.IsBlocked(peer.ID("peer1")))
+
+	// Make sure that if we then "unblock" the peer, nothing happens
+	gater.Update(peer.ID("peer1"), float64(0))
+
+	// The peer should still be unblocked
+	testSuite.False(gater.IsBlocked(peer.ID("peer1")))
 }