fix(p2p): ignore invalid host when loading peer db

internal/p2p/peermanager.go

@@ -338,7 +338,7 @@ func NewPeerManager(_ctx context.Context, selfID types.NodeID, peerDB dbm.DB, op
 
 	options.optimize()
 
-	store, err := newPeerStore(peerDB)
+	store, err := newPeerStore(peerDB, log.NewNopLogger().With("module", "peerstore"))
 	if err != nil {
 		return nil, err
 	}
@@ -379,6 +379,9 @@ func NewPeerManager(_ctx context.Context, selfID types.NodeID, peerDB dbm.DB, op
 // SetLogger sets a logger for the PeerManager
 func (m *PeerManager) SetLogger(logger log.Logger) {
 	m.logger = logger
+	if m.store != nil {
+		m.store.logger = logger.With("module", "peerstore")
+	}
 }
 
 // Close closes peer manager and frees up all resources
@@ -1371,15 +1374,22 @@ type peerStore struct {
 	peers  map[types.NodeID]*peerInfo
 	index  map[NodeAddress]types.NodeID
 	ranked []*peerInfo // cache for Ranked(), nil invalidates cache
+	logger log.Logger
 }
 
 // newPeerStore creates a new peer store, loading all persisted peers from the
 // database into memory.
-func newPeerStore(db dbm.DB) (*peerStore, error) {
+func newPeerStore(db dbm.DB, logger log.Logger) (*peerStore, error) {
 	if db == nil {
 		return nil, errors.New("no database provided")
 	}
-	store := &peerStore{db: db}
+	if logger == nil {
+		logger = log.NewNopLogger()
+	}
+	store := &peerStore{
+		db:     db,
+		logger: logger,
+	}
 	if err := store.loadPeers(); err != nil {
 		return nil, err
 	}
@@ -1398,15 +1408,15 @@ func (s *peerStore) loadPeers() error {
 	}
 	defer iter.Close()
 	for ; iter.Valid(); iter.Next() {
-		// FIXME: We may want to tolerate failures here, by simply logging
-		// the errors and ignoring the faulty peer entries.
 		msg := new(p2pproto.PeerInfo)
 		if err := proto.Unmarshal(iter.Value(), msg); err != nil {
-			return fmt.Errorf("invalid peer Protobuf data: %w", err)
+			s.logger.Error("invalid peer Protobuf data, skipping", "key", fmt.Sprintf("%x", iter.Key()), "error", err)
+			continue
 		}
 		peer, err := peerInfoFromProto(msg)
 		if err != nil {
-			return fmt.Errorf("invalid peer data: %w", err)
+			s.logger.Error("invalid peer data, skipping", "key", fmt.Sprintf("%x", iter.Key()), "error", err)
+			continue
 		}
 		peers[peer.ID] = peer
 		for addr := range peer.AddressInfo {

internal/p2p/peermanager_scoring_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/dashpay/tenderdash/crypto/ed25519"
+	"github.com/dashpay/tenderdash/libs/log"
 	"github.com/dashpay/tenderdash/types"
 )
 
@@ -100,7 +101,7 @@ func TestPeerScoring(t *testing.T) {
 
 func makeMockPeerStore(t *testing.T, peers ...peerInfo) *peerStore {
 	t.Helper()
-	s, err := newPeerStore(dbm.NewMemDB())
+	s, err := newPeerStore(dbm.NewMemDB(), log.NewNopLogger())
 	if err != nil {
 		t.Fatal(err)
 	}

internal/p2p/peermanager_test.go

@@ -10,11 +10,13 @@ import (
 
 	dbm "github.com/cometbft/cometbft-db"
 	"github.com/fortytw2/leaktest"
+	"github.com/google/orderedcode"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/dashpay/tenderdash/internal/p2p"
 	"github.com/dashpay/tenderdash/libs/log"
+	p2pproto "github.com/dashpay/tenderdash/proto/tendermint/p2p"
 	"github.com/dashpay/tenderdash/types"
 )
 
@@ -111,6 +113,56 @@ func TestPeerManagerOptions_Validate(t *testing.T) {
 	}
 }
 
+func TestPeerStoreLoadPeersSkipsInvalidAddress(t *testing.T) {
+	db := dbm.NewMemDB()
+	logger, err := log.NewDefaultLogger(log.LogFormatText, log.LogLevelInfo)
+	require.NoError(t, err)
+
+	invalidID := types.NodeID(strings.Repeat("a", 40))
+	invalidPeer := &p2pproto.PeerInfo{
+		ID: string(invalidID),
+		AddressInfo: []*p2pproto.PeerAddressInfo{{
+			Address: "mconn://" + string(invalidID) + "@[173.199.71.83:26656]:26656",
+		}},
+	}
+	invalidBz, err := invalidPeer.Marshal()
+	require.NoError(t, err)
+	require.NoError(t, db.Set(peerInfoKey(t, invalidID), invalidBz))
+
+	validID := types.NodeID(strings.Repeat("b", 40))
+	validAddr := p2p.NodeAddress{
+		Protocol: "mconn",
+		NodeID:   validID,
+		Hostname: "127.0.0.1",
+		Port:     26656,
+	}
+	validPeer := &p2pproto.PeerInfo{
+		ID: string(validID),
+		AddressInfo: []*p2pproto.PeerAddressInfo{{
+			Address: validAddr.String(),
+		}},
+	}
+	validBz, err := validPeer.Marshal()
+	require.NoError(t, err)
+	require.NoError(t, db.Set(peerInfoKey(t, validID), validBz))
+
+	selfID := types.NodeID(strings.Repeat("c", 40))
+	peerManager, err := p2p.NewPeerManager(context.Background(), selfID, db, p2p.PeerManagerOptions{})
+	require.NoError(t, err)
+	peerManager.SetLogger(logger)
+
+	peers := peerManager.Peers()
+	require.Contains(t, peers, validID)
+	require.NotContains(t, peers, invalidID)
+}
+
+func peerInfoKey(t *testing.T, id types.NodeID) []byte {
+	t.Helper()
+	key, err := orderedcode.Append(nil, int64(1), string(id))
+	require.NoError(t, err)
+	return key
+}
+
 func TestNewPeerManager(t *testing.T) {
 	ctx := context.TODO()
 	// Zero options should be valid.