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.

Sign up for GitHub

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(p2p)!: remove conngater dependency #107

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(p2p)!: remove conngater dependency #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions p2p/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/libp2p/go-libp2p/core/host"
"github.com/libp2p/go-libp2p/core/peer"
"github.com/libp2p/go-libp2p/core/protocol"
"github.com/libp2p/go-libp2p/p2p/net/conngater"

"github.com/celestiaorg/go-header"
p2p_pb "github.com/celestiaorg/go-header/p2p/pb"
Expand Down Expand Up @@ -50,7 +49,7 @@ type Exchange[H header.Header[H]] struct {
func NewExchange[H header.Header[H]](
host host.Host,
peers peer.IDSlice,
gater *conngater.BasicConnectionGater,
blacklistPeer func(id peer.ID) error,
Comment on lines -53 to +52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an interface https://github.com/libp2p/go-libp2p/blob/master/core/connmgr/gater.go

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is different one, used as dialer interceptor

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not have BlockPeer()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that's confusing

opts ...Option[ClientParameters],
) (*Exchange[H], error) {
params := DefaultClientParameters()
Expand All @@ -66,7 +65,7 @@ func NewExchange[H header.Header[H]](
ex := &Exchange[H]{
host: host,
protocolID: protocolID(params.networkID),
peerTracker: newPeerTracker(host, gater, params.pidstore),
peerTracker: newPeerTracker(host, blacklistPeer, params.pidstore),
Params: params,
}

Expand Down
24 changes: 11 additions & 13 deletions p2p/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ import (
"testing"
"time"

"github.com/ipfs/go-datastore"
"github.com/ipfs/go-datastore/sync"
libhost "github.com/libp2p/go-libp2p/core/host"
"github.com/libp2p/go-libp2p/core/network"
"github.com/libp2p/go-libp2p/core/peer"
"github.com/libp2p/go-libp2p/core/peerstore"
blankhost "github.com/libp2p/go-libp2p/p2p/host/blank"
"github.com/libp2p/go-libp2p/p2p/net/conngater"
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
swarm "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -148,6 +145,14 @@ func TestExchange_RequestVerifiedHeadersFails(t *testing.T) {
hosts := createMocknet(t, 2)
exchg, store := createP2PExAndServer(t, hosts[0], hosts[1])
store.Headers[2] = store.Headers[3]

// replace blacklistPeer with a function that records the peer that was blacklisted
var blacklisted peer.ID
exchg.peerTracker.blacklistPeer = func(pID peer.ID) error {
blacklisted = pID
return nil
}

// perform expected request
h := store.Headers[1]
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
Expand All @@ -156,9 +161,7 @@ func TestExchange_RequestVerifiedHeadersFails(t *testing.T) {
assert.Error(t, err)

// ensure that peer was added to the blacklist
peers := exchg.peerTracker.connGater.ListBlockedPeers()
require.Len(t, peers, 1)
require.True(t, hosts[1].ID() == peers[0])
require.Equal(t, hosts[1].ID(), blacklisted)
}

// TestExchange_RequestFullRangeHeaders requests max amount of headers
Expand All @@ -167,11 +170,9 @@ func TestExchange_RequestFullRangeHeaders(t *testing.T) {
// create mocknet with 5 peers
hosts := createMocknet(t, 5)
store := headertest.NewStore[*headertest.DummyHeader](t, headertest.NewTestSuite(t), int(header.MaxRangeRequestSize))
connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore()))
require.NoError(t, err)

// create new exchange
exchange, err := NewExchange[*headertest.DummyHeader](hosts[len(hosts)-1], []peer.ID{hosts[4].ID()}, connGater,
exchange, err := NewExchange[*headertest.DummyHeader](hosts[len(hosts)-1], []peer.ID{hosts[4].ID()}, nil,
WithNetworkID[ClientParameters](networkID),
WithChainID(networkID),
)
Expand Down Expand Up @@ -509,10 +510,7 @@ func createP2PExAndServer(
err = serverSideEx.Start(context.Background())
require.NoError(t, err)

connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore()))
require.NoError(t, err)

ex, err := NewExchange[*headertest.DummyHeader](host, []peer.ID{tpeer.ID()}, connGater,
ex, err := NewExchange[*headertest.DummyHeader](host, []peer.ID{tpeer.ID()}, nil,
WithNetworkID[ClientParameters](networkID),
WithChainID(networkID),
)
Expand Down
11 changes: 5 additions & 6 deletions p2p/peer_tracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/libp2p/go-libp2p/core/host"
"github.com/libp2p/go-libp2p/core/network"
libpeer "github.com/libp2p/go-libp2p/core/peer"
"github.com/libp2p/go-libp2p/p2p/net/conngater"
)

const (
Expand All @@ -28,8 +27,8 @@ var (
)

type peerTracker struct {
host host.Host
connGater *conngater.BasicConnectionGater
host host.Host
blacklistPeer func(id libpeer.ID) error

peerLk sync.RWMutex
// trackedPeers contains active peers that we can request to.
Expand All @@ -53,13 +52,13 @@ type peerTracker struct {

func newPeerTracker(
h host.Host,
connGater *conngater.BasicConnectionGater,
blacklistPeer func(id libpeer.ID) error,
pidstore PeerIDStore,
) *peerTracker {
ctx, cancel := context.WithCancel(context.Background())
return &peerTracker{
host: h,
connGater: connGater,
blacklistPeer: blacklistPeer,
trackedPeers: make(map[libpeer.ID]*peerStat),
disconnectedPeers: make(map[libpeer.ID]*peerStat),
pidstore: pidstore,
Expand Down Expand Up @@ -301,7 +300,7 @@ func (p *peerTracker) stop(ctx context.Context) error {
// blockPeer blocks a peer on the networking level and removes it from the local cache.
func (p *peerTracker) blockPeer(pID libpeer.ID, reason error) {
// add peer to the blacklist, so we can't connect to it in the future.
err := p.connGater.BlockPeer(pID)
err := p.blacklistPeer(pID)
if err != nil {
log.Errorw("header/p2p: blocking peer failed", "pID", pID, "err", err)
}
Expand Down
28 changes: 12 additions & 16 deletions p2p/peer_tracker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/ipfs/go-datastore/sync"
"github.com/libp2p/go-libp2p/core/peer"
testpeer "github.com/libp2p/go-libp2p/core/test"
"github.com/libp2p/go-libp2p/p2p/net/conngater"
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -25,11 +24,8 @@ func TestPeerTracker_GC(t *testing.T) {

gcCycle = time.Millisecond * 200

connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore()))
require.NoError(t, err)

pidstore := newDummyPIDStore()
p := newPeerTracker(h[0], connGater, pidstore)
p := newPeerTracker(h[0], nil, pidstore)

maxAwaitingTime = time.Millisecond

Expand All @@ -51,7 +47,7 @@ func TestPeerTracker_GC(t *testing.T) {

time.Sleep(time.Millisecond * 500)

err = p.stop(ctx)
err := p.stop(ctx)
require.NoError(t, err)

require.Nil(t, p.trackedPeers[pid1])
Expand All @@ -65,22 +61,22 @@ func TestPeerTracker_GC(t *testing.T) {

func TestPeerTracker_BlockPeer(t *testing.T) {
h := createMocknet(t, 2)
connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore()))
require.NoError(t, err)
p := newPeerTracker(h[0], connGater, nil)
maxAwaitingTime = time.Millisecond
// create blacklistPeer function that records the peer that was blacklisted
var blacklisted peer.ID
blacklistPeer := func(pID peer.ID) error {
blacklisted = pID
return nil
}
p := newPeerTracker(h[0], blacklistPeer, nil)
p.blockPeer(h[1].ID(), errors.New("test"))
require.Len(t, connGater.ListBlockedPeers(), 1)
require.True(t, connGater.ListBlockedPeers()[0] == h[1].ID())
require.Equal(t, h[1].ID(), blacklisted)
require.Len(t, h[0].Network().Conns(), 0)
}

func TestPeerTracker_Bootstrap(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore()))
require.NoError(t, err)

// mn := createMocknet(t, 10)
mn, err := mocknet.FullMeshConnected(10)
require.NoError(t, err)
Expand All @@ -100,7 +96,7 @@ func TestPeerTracker_Bootstrap(t *testing.T) {
err = pidstore.Put(ctx, prevSeen[2:])
require.NoError(t, err)

tracker := newPeerTracker(mn.Hosts()[0], connGater, pidstore)
tracker := newPeerTracker(mn.Hosts()[0], nil, pidstore)

go tracker.track()

Expand Down