feat(op-node): gater unblock (#9763)

* feat(op-node): gater unblock

* --amend

op-node/p2p/gating/expiry.go

@@ -45,22 +45,35 @@ func AddBanExpiry(gater BlockingConnectionGater, store ExpiryStore, log log.Logg
 	}
 }
 
+func (g *ExpiryConnectionGater) UnblockPeer(p peer.ID) error {
+	if err := g.BlockingConnectionGater.UnblockPeer(p); err != nil {
+		log.Warn("failed to unblock peer from underlying gater", "method", "UnblockPeer", "peer_id", p, "err", err)
+		return err
+	}
+	if err := g.store.SetPeerBanExpiration(p, time.Time{}); err != nil {
+		log.Warn("failed to unblock peer from expiry gater", "method", "UnblockPeer", "peer_id", p, "err", err)
+		return err
+	}
+	g.m.RecordPeerUnban()
+	return nil
+}
+
 func (g *ExpiryConnectionGater) peerBanExpiryCheck(p peer.ID) (allow bool) {
 	// if the peer is blocked, check if it's time to unblock
 	expiry, err := g.store.GetPeerBanExpiration(p)
 	if errors.Is(err, store.UnknownBanErr) {
 		return true // peer is allowed if it has not been banned
 	}
 	if err != nil {
-		g.log.Warn("failed to load peer-ban expiry time", "peer_id", p, "err", err)
+		g.log.Warn("failed to load peer-ban expiry time", "method", "peerBanExpiryCheck", "peer_id", p, "err", err)
 		return false
 	}
 	if g.clock.Now().Before(expiry) {
 		return false
 	}
 	g.log.Info("peer-ban expired, unbanning peer", "peer_id", p, "expiry", expiry)
 	if err := g.store.SetPeerBanExpiration(p, time.Time{}); err != nil {
-		g.log.Warn("failed to unban peer", "peer_id", p, "err", err)
+		g.log.Warn("failed to unban peer", "method", "peerBanExpiryCheck", "peer_id", p, "err", err)
 		return false // if we ignored the error, then the inner connection-gater would drop them
 	}
 	g.m.RecordPeerUnban()
@@ -70,7 +83,7 @@ func (g *ExpiryConnectionGater) peerBanExpiryCheck(p peer.ID) (allow bool) {
 func (g *ExpiryConnectionGater) addrBanExpiryCheck(ma multiaddr.Multiaddr) (allow bool) {
 	ip, err := manet.ToIP(ma)
 	if err != nil {
-		g.log.Error("tried to check multi-addr with bad IP", "addr", ma)
+		g.log.Error("tried to check multi-addr with bad IP", "method", "addrBanExpiryCheck", "addr", ma)
 		return false
 	}
 	// if just the IP is blocked, check if it's time to unblock
@@ -79,15 +92,15 @@ func (g *ExpiryConnectionGater) addrBanExpiryCheck(ma multiaddr.Multiaddr) (allo
 		return true // IP is allowed if it has not been banned
 	}
 	if err != nil {
-		g.log.Warn("failed to load IP-ban expiry time", "ip", ip, "err", err)
+		g.log.Warn("failed to load IP-ban expiry time", "method", "addrBanExpiryCheck", "ip", ip, "err", err)
 		return false
 	}
 	if g.clock.Now().Before(expiry) {
 		return false
 	}
-	g.log.Info("IP-ban expired, unbanning IP", "ip", ip, "expiry", expiry)
+	g.log.Info("IP-ban expired, unbanning IP", "method", "addrBanExpiryCheck", "ip", ip, "expiry", expiry)
 	if err := g.store.SetIPBanExpiration(ip, time.Time{}); err != nil {
-		g.log.Warn("failed to unban IP", "ip", ip, "err", err)
+		g.log.Warn("failed to unban IP", "method", "addrBanExpiryCheck", "ip", ip, "err", err)
 		return false // if we ignored the error, then the inner connection-gater would drop them
 	}
 	g.m.RecordIPUnban()
@@ -100,7 +113,7 @@ func (g *ExpiryConnectionGater) InterceptPeerDial(p peer.ID) (allow bool) {
 	}
 	peerBan := g.peerBanExpiryCheck(p)
 	if !peerBan {
-		log.Warn("peer is temporarily banned", "peer_id", p)
+		log.Warn("peer is temporarily banned", "method", "InterceptPeerDial", "peer_id", p)
 	}
 	return peerBan
 }
@@ -111,12 +124,12 @@ func (g *ExpiryConnectionGater) InterceptAddrDial(id peer.ID, ma multiaddr.Multi
 	}
 	peerBan := g.peerBanExpiryCheck(id)
 	if !peerBan {
-		log.Warn("peer id is temporarily banned", "peer_id", id, "multi_addr", ma)
+		log.Warn("peer id is temporarily banned", "method", "InterceptAddrDial", "peer_id", id, "multi_addr", ma)
 		return false
 	}
 	addrBan := g.addrBanExpiryCheck(ma)
 	if !addrBan {
-		log.Warn("peer address is temporarily banned", "peer_id", id, "multi_addr", ma)
+		log.Warn("peer address is temporarily banned", "method", "InterceptAddrDial", "peer_id", id, "multi_addr", ma)
 		return false
 	}
 	return true
@@ -128,7 +141,7 @@ func (g *ExpiryConnectionGater) InterceptAccept(mas network.ConnMultiaddrs) (all
 	}
 	addrBan := g.addrBanExpiryCheck(mas.RemoteMultiaddr())
 	if !addrBan {
-		log.Warn("peer address is temporarily banned", "multi_addr", mas.RemoteMultiaddr())
+		log.Warn("peer address is temporarily banned", "method", "InterceptAccept", "multi_addr", mas.RemoteMultiaddr())
 	}
 	return addrBan
 }
@@ -145,7 +158,7 @@ func (g *ExpiryConnectionGater) InterceptSecured(direction network.Direction, id
 	// This leaves just the peer-ID expiry to check on inbound connections.
 	peerBan := g.peerBanExpiryCheck(id)
 	if !peerBan {
-		log.Warn("peer id is temporarily banned", "peer_id", id, "multi_addr", mas.RemoteMultiaddr())
+		log.Warn("peer id is temporarily banned", "method", "InterceptSecured", "peer_id", id, "multi_addr", mas.RemoteMultiaddr())
 	}
 	return peerBan
 }

op-node/p2p/host_test.go

@@ -159,7 +159,7 @@ func TestP2PFull(t *testing.T) {
 	require.False(t, nodeB.IsStatic(hostB.ID()), "node B must not be static peer of node B itself")
 
 	select {
-	case <-time.After(time.Second):
+	case <-time.After(30 * time.Second):
 		t.Fatal("failed to connect new host")
 	case c := <-conns:
 		require.Equal(t, hostB.ID(), c.RemotePeer())

op-node/p2p/rpc_server.go

@@ -379,8 +379,15 @@ func (s *APIBackend) DisconnectPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_disconnectPeer")
 	defer recordDur()
 	err := s.node.Host().Network().ClosePeer(id)
+	if err != nil {
+		return err
+	}
 	ps := s.node.Host().Peerstore()
 	ps.RemovePeer(id)
 	ps.ClearAddrs(id)
-	return err
+	err = s.node.ConnectionGater().UnblockPeer(id)
+	if err != nil {
+		return fmt.Errorf("closed peer but failed to unblock: %w", err)
+	}
+	return nil
 }