feat(modp2p): close peer connections after successful blocking (#3838)

nodebuilder/p2p/p2p.go

@@ -42,7 +42,8 @@ type Module interface {
 	// NATStatus returns the current NAT status.
 	NATStatus(context.Context) (network.Reachability, error)
 
-	// BlockPeer adds a peer to the set of blocked peers.
+	// BlockPeer adds a peer to the set of blocked peers and
+	// closes any existing connection to that peer.
 	BlockPeer(ctx context.Context, p peer.ID) error
 	// UnblockPeer removes a peer from the set of blocked peers.
 	UnblockPeer(ctx context.Context, p peer.ID) error
@@ -145,7 +146,13 @@ func (m *module) NATStatus(context.Context) (network.Reachability, error) {
 }
 
 func (m *module) BlockPeer(_ context.Context, p peer.ID) error {
-	return m.connGater.BlockPeer(p)
+	if err := m.connGater.BlockPeer(p); err != nil {
+		return err
+	}
+	if err := m.host.Network().ClosePeer(p); err != nil {
+		log.Warnf("failed to close connection to blocked peer %s: %v", p, err)
+	}
+	return nil
 }
 
 func (m *module) UnblockPeer(_ context.Context, p peer.ID) error {

nodebuilder/p2p/p2p_test.go

@@ -218,22 +218,36 @@ func TestP2PModule_Pubsub(t *testing.T) {
 // TestP2PModule_ConnGater tests P2P Module methods on
 // the instance of ConnectionGater.
 func TestP2PModule_ConnGater(t *testing.T) {
+	net, err := mocknet.FullMeshConnected(2)
+	require.NoError(t, err)
+	host, peer := net.Hosts()[0], net.Hosts()[1]
+
 	gater, err := connectionGater(datastore.NewMapDatastore())
 	require.NoError(t, err)
 
-	mgr := newModule(nil, nil, gater, nil, nil)
+	mgr := newModule(host, nil, gater, nil, nil)
 
 	ctx := context.Background()
 
-	assert.NoError(t, mgr.BlockPeer(ctx, "badpeer"))
+	assert.NoError(t, mgr.BlockPeer(ctx, peer.ID()))
 	blocked, err := mgr.ListBlockedPeers(ctx)
 	require.NoError(t, err)
-	assert.Len(t, blocked, 1)
+	assert.Contains(t, blocked, peer.ID())
+
+	// Check if the peer is disconnected (or remains disconnected) after blocking
+	connectedness, err := mgr.Connectedness(ctx, peer.ID())
+	require.NoError(t, err)
+	assert.Equal(t, network.NotConnected, connectedness)
 
-	assert.NoError(t, mgr.UnblockPeer(ctx, "badpeer"))
+	assert.NoError(t, mgr.UnblockPeer(ctx, peer.ID()))
 	blocked, err = mgr.ListBlockedPeers(ctx)
 	require.NoError(t, err)
-	assert.Len(t, blocked, 0)
+	assert.NotContains(t, blocked, peer.ID())
+
+	// Verify that unblocking doesn't automatically reconnect
+	connectedness, err = mgr.Connectedness(ctx, peer.ID())
+	require.NoError(t, err)
+	assert.Equal(t, network.NotConnected, connectedness)
 }
 
 // TestP2PModule_ResourceManager tests P2P Module methods on