From b63259a462fe5430659ee82dcdd814a8046b31b9 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Tue, 30 May 2023 12:24:10 +0200 Subject: [PATCH] refactor(p2p): Use PeerIDStore iface instead of direct impol --- p2p/exchange_test.go | 1 + p2p/peer_tracker.go | 14 ++-- p2p/peer_tracker_test.go | 78 +++++++++++++---- .../persisted_peerstore.go | 83 ------------------- .../persisted_peerstore_test.go | 46 ---------- p2p/persisted_peerstore/testing.go | 39 --------- 6 files changed, 71 insertions(+), 190 deletions(-) delete mode 100644 p2p/persisted_peerstore/persisted_peerstore.go delete mode 100644 p2p/persisted_peerstore/persisted_peerstore_test.go delete mode 100644 p2p/persisted_peerstore/testing.go diff --git a/p2p/exchange_test.go b/p2p/exchange_test.go index bdfcbf21..a6fd7e42 100644 --- a/p2p/exchange_test.go +++ b/p2p/exchange_test.go @@ -20,6 +20,7 @@ import ( "github.com/celestiaorg/go-libp2p-messenger/serde" + "github.com/celestiaorg/go-header" "github.com/celestiaorg/go-header/headertest" p2p_pb "github.com/celestiaorg/go-header/p2p/pb" ) diff --git a/p2p/peer_tracker.go b/p2p/peer_tracker.go index ef154e56..8f9f0779 100644 --- a/p2p/peer_tracker.go +++ b/p2p/peer_tracker.go @@ -10,8 +10,6 @@ import ( "github.com/libp2p/go-libp2p/core/network" "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/p2p/net/conngater" - - "github.com/celestiaorg/go-header/p2p/persisted_peerstore" ) const ( @@ -42,8 +40,8 @@ type PeerTracker struct { // online until pruneDeadline, it will be removed and its score will be lost. disconnectedPeers map[peer.ID]*peerStat - // peerstore is used to store peers periodically. - peerstore *persisted_peerstore.PersistedPeerstore + // pidstore is used to store peers periodically. + pidstore PeerIDStore ctx context.Context cancel context.CancelFunc @@ -55,7 +53,7 @@ type PeerTracker struct { func NewPeerTracker( h host.Host, connGater *conngater.BasicConnectionGater, - peerstore *persisted_peerstore.PersistedPeerstore, + pidstore PeerIDStore, ) *PeerTracker { ctx, cancel := context.WithCancel(context.Background()) return &PeerTracker{ @@ -63,7 +61,7 @@ func NewPeerTracker( connGater: connGater, disconnectedPeers: make(map[peer.ID]*peerStat), trackedPeers: make(map[peer.ID]*peerStat), - peerstore: peerstore, + pidstore: pidstore, ctx: ctx, cancel: cancel, done: make(chan struct{}, 2), @@ -211,11 +209,11 @@ func (p *PeerTracker) gc() { } func (p *PeerTracker) persistPeers(trackedPeers []peer.ID) { - if p.peerstore == nil { + if p.pidstore == nil { return } - err := p.peerstore.Put(p.ctx, trackedPeers) + err := p.pidstore.Put(p.ctx, trackedPeers) if err != nil { log.Errorw("persisting updated peer list", "err", err) } diff --git a/p2p/peer_tracker_test.go b/p2p/peer_tracker_test.go index 24155556..aa83dd8e 100644 --- a/p2p/peer_tracker_test.go +++ b/p2p/peer_tracker_test.go @@ -2,18 +2,20 @@ package p2p import ( "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" "errors" + "github.com/libp2p/go-libp2p/core/crypto" + "github.com/libp2p/go-libp2p/core/peer" "testing" "time" "github.com/ipfs/go-datastore" "github.com/ipfs/go-datastore/sync" - "github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoremem" "github.com/libp2p/go-libp2p/p2p/net/conngater" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "github.com/celestiaorg/go-header/p2p/persisted_peerstore" ) func TestPeerTracker_GC(t *testing.T) { @@ -27,20 +29,16 @@ func TestPeerTracker_GC(t *testing.T) { connGater, err := conngater.NewBasicConnectionGater(sync.MutexWrap(datastore.NewMapDatastore())) require.NoError(t, err) - ds := sync.MutexWrap(datastore.NewMapDatastore()) - addrBook, err := pstoremem.NewPeerstore() - require.NoError(t, err) - - mockPeerStore := persisted_peerstore.NewPersistedPeerstore(ds, addrBook) + mockPeerStore := newMockPIDStore() p := NewPeerTracker(h[0], connGater, mockPeerStore) - peerlist, err := persisted_peerstore.GenerateRandomPeerlist(4) + peerlist, err := generateRandomPeerlist(4) require.NoError(t, err) - pid1 := peerlist[0].ID - pid2 := peerlist[1].ID - pid3 := peerlist[2].ID - pid4 := peerlist[3].ID + pid1 := peerlist[0] + pid2 := peerlist[1] + pid3 := peerlist[2] + pid4 := peerlist[3] // Add peer with low score to test if it will be GC'ed (it should) p.trackedPeers[pid1] = &peerStat{peerID: pid1, peerScore: 0.5} @@ -73,7 +71,7 @@ func TestPeerTracker_GC(t *testing.T) { peers, err := mockPeerStore.Load(ctx) require.NoError(t, err) - assert.Equal(t, peers[0].ID, p.trackedPeers[pid2].peerID) + assert.Equal(t, peers[0], p.trackedPeers[pid2].peerID) assert.Equal(t, 1, len(p.trackedPeers)) } @@ -87,3 +85,55 @@ func TestPeerTracker_BlockPeer(t *testing.T) { require.Len(t, connGater.ListBlockedPeers(), 1) require.True(t, connGater.ListBlockedPeers()[0] == h[1].ID()) } + +type mockPIDStore struct { + ds datastore.Datastore +} + +func (m mockPIDStore) Put(ctx context.Context, peers []peer.ID) error { + bin, err := json.Marshal(peers) + if err != nil { + return err + } + + return m.ds.Put(ctx, datastore.NewKey("peers"), bin) +} + +func (m mockPIDStore) Load(ctx context.Context) ([]peer.ID, error) { + bin, err := m.ds.Get(ctx, datastore.NewKey("peers")) + if err != nil { + return nil, err + } + + var peers []peer.ID + err = json.Unmarshal(bin, &peers) + return peers, err +} + +func newMockPIDStore() PeerIDStore { + return &mockPIDStore{ds: sync.MutexWrap(datastore.NewMapDatastore())} +} + +func generateRandomPeerlist(length int) ([]peer.ID, error) { + peerlist := make([]peer.ID, length) + for i := range peerlist { + key, err := rsa.GenerateKey(rand.Reader, 2096) + if err != nil { + return nil, err + } + + _, pubkey, err := crypto.KeyPairFromStdKey(key) + if err != nil { + return nil, err + } + + peerID, err := peer.IDFromPublicKey(pubkey) + if err != nil { + return nil, err + } + + peerlist[i] = peerID + } + + return peerlist, nil +} diff --git a/p2p/persisted_peerstore/persisted_peerstore.go b/p2p/persisted_peerstore/persisted_peerstore.go deleted file mode 100644 index 3e75d8b5..00000000 --- a/p2p/persisted_peerstore/persisted_peerstore.go +++ /dev/null @@ -1,83 +0,0 @@ -package persisted_peerstore - -import ( - "context" - "encoding/json" - "fmt" - - "github.com/ipfs/go-datastore" - "github.com/ipfs/go-datastore/namespace" - logging "github.com/ipfs/go-log/v2" - "github.com/libp2p/go-libp2p/core/peer" - "github.com/libp2p/go-libp2p/core/peerstore" -) - -var ( - storePrefix = datastore.NewKey("persisted_peerstore") - peersKey = datastore.NewKey("peers") - - log = logging.Logger("persisted_peerstore") -) - -// PersistedPeerstore is used to store/load peers to/from disk. -type PersistedPeerstore struct { - ds datastore.Datastore - addrBook peerstore.AddrBook -} - -// NewPersistedPeerstore creates a new peerstore backed by the given datastore which is -// wrapped with `persisted_peerstore` prefix. It also takes an AddrBook so that it can -// look up a peer by its ID to get AddrInfo. -func NewPersistedPeerstore(ds datastore.Datastore, addrBook peerstore.AddrBook) *PersistedPeerstore { - return &PersistedPeerstore{ - ds: namespace.Wrap(ds, storePrefix), - addrBook: addrBook, - } -} - -// Load loads the peers from datastore and returns their AddrInfo. -func (pp *PersistedPeerstore) Load(ctx context.Context) ([]peer.AddrInfo, error) { - log.Debug("Loading peers") - - bin, err := pp.ds.Get(ctx, peersKey) - if err != nil { - return make([]peer.AddrInfo, 0), fmt.Errorf("peerstore: loading peers from datastore: %w", err) - } - - var peers []peer.ID - err = json.Unmarshal(bin, &peers) - if err != nil { - return make([]peer.AddrInfo, 0), fmt.Errorf("peerstore: unmarshalling peer IDs: %w", err) - } - - log.Info("Loaded peers from disk", "amount", len(peers)) - - // lookup and collect their AddrInfo - addrInfos := make([]peer.AddrInfo, len(peers)) - for i, peerID := range peers { - addrs := pp.addrBook.Addrs(peerID) - addrInfos[i] = peer.AddrInfo{ - ID: peerID, - Addrs: addrs, - } - } - - return addrInfos, nil -} - -// Put persists the given peer IDs to the datastore. -func (pp *PersistedPeerstore) Put(ctx context.Context, peers []peer.ID) error { - log.Debug("Persisting peers to disk", "amount", len(peers)) - - bin, err := json.Marshal(peers) - if err != nil { - return fmt.Errorf("peerstore: marshal peerlist: %w", err) - } - - if err = pp.ds.Put(ctx, peersKey, bin); err != nil { - return fmt.Errorf("peerstore: error writing to datastore: %w", err) - } - - log.Info("Persisted peers successfully", "amount", len(peers)) - return nil -} diff --git a/p2p/persisted_peerstore/persisted_peerstore_test.go b/p2p/persisted_peerstore/persisted_peerstore_test.go deleted file mode 100644 index 25748707..00000000 --- a/p2p/persisted_peerstore/persisted_peerstore_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package persisted_peerstore - -import ( - "context" - "testing" - "time" - - "github.com/ipfs/go-datastore" - "github.com/ipfs/go-datastore/sync" - "github.com/libp2p/go-libp2p/core/peer" - "github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoreds" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestPutLoad(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer t.Cleanup(cancel) - - ds := sync.MutexWrap(datastore.NewMapDatastore()) - // using an on-disk addrbook (with underlying in-mem ds) here as persisted - // peerstore is intended to be used in tandem with an on-disk AddrBook // TODO does this really have to be the case? - addrbook, err := pstoreds.NewPeerstore(ctx, ds, pstoreds.DefaultOpts()) - require.NoError(t, err) - - peerstore := NewPersistedPeerstore(ds, addrbook) - - peerlist, err := GenerateRandomPeerlist(10) - require.NoError(t, err) - - // add the generated multiaddrs to the addrbook - ids := make([]peer.ID, len(peerlist)) - for i, peer := range peerlist { - addrbook.AddAddrs(peer.ID, peer.Addrs, time.Hour) - ids[i] = peer.ID - } - - err = peerstore.Put(ctx, ids) - require.NoError(t, err) - - retrievedPeerlist, err := peerstore.Load(ctx) - require.NoError(t, err) - - assert.Equal(t, len(peerlist), len(retrievedPeerlist)) - assert.Equal(t, peerlist, retrievedPeerlist) -} diff --git a/p2p/persisted_peerstore/testing.go b/p2p/persisted_peerstore/testing.go deleted file mode 100644 index e87e6c24..00000000 --- a/p2p/persisted_peerstore/testing.go +++ /dev/null @@ -1,39 +0,0 @@ -package persisted_peerstore - -import ( - "crypto/rand" - "crypto/rsa" - - "github.com/libp2p/go-libp2p/core/crypto" - "github.com/libp2p/go-libp2p/core/peer" - ma "github.com/multiformats/go-multiaddr" -) - -func GenerateRandomPeerlist(length int) ([]peer.AddrInfo, error) { - peerlist := make([]peer.AddrInfo, 0) - for i := 0; i < length; i++ { - key, err := rsa.GenerateKey(rand.Reader, 2096) - if err != nil { - return nil, err - } - - _, pubkey, err := crypto.KeyPairFromStdKey(key) - if err != nil { - return nil, err - } - - peerID, err := peer.IDFromPublicKey(pubkey) - if err != nil { - return nil, err - } - - peerlist = append(peerlist, peer.AddrInfo{ - ID: peerID, - Addrs: []ma.Multiaddr{ - ma.StringCast("/ip4/0.0.0.0/tcp/2121"), - }, - }) - } - - return peerlist, nil -}