Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Oct 18, 2024
Merged
11 changes: 9 additions & 2 deletions nodebuilder/p2p/p2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 19 additions & 5 deletions nodebuilder/p2p/p2p_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down