From 7971f4a2fca1ee3054fbc996e2bc041de63d3b89 Mon Sep 17 00:00:00 2001
From: Sam Kleinman <garen@tychoish.com>
Date: Tue, 14 Jun 2022 12:45:05 -0400
Subject: [PATCH] p2p: self-add node should not error (#8753)

---
 internal/p2p/peermanager.go      | 12 +++++++++++-
 internal/p2p/peermanager_test.go | 14 ++++++++------
 node/node.go                     |  2 +-
 node/seed.go                     |  2 +-
 node/setup.go                    |  2 ++
 5 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go
index 7391de4ea..65741f63f 100644
--- a/internal/p2p/peermanager.go
+++ b/internal/p2p/peermanager.go
@@ -15,6 +15,7 @@ import (
 	dbm "github.com/tendermint/tm-db"
 
 	tmsync "github.com/tendermint/tendermint/internal/libs/sync"
+	"github.com/tendermint/tendermint/libs/log"
 	p2pproto "github.com/tendermint/tendermint/proto/tendermint/p2p"
 	"github.com/tendermint/tendermint/types"
 )
@@ -145,6 +146,8 @@ type PeerManagerOptions struct {
 	// persistentPeers provides fast PersistentPeers lookups. It is built
 	// by optimize().
 	persistentPeers map[types.NodeID]bool
+
+	Logger log.Logger
 }
 
 // Validate validates the options.
@@ -264,6 +267,7 @@ type PeerManager struct {
 	rand       *rand.Rand
 	dialWaker  *tmsync.Waker // wakes up DialNext() on relevant peer changes
 	evictWaker *tmsync.Waker // wakes up EvictNext() on relevant peer changes
+	logger     log.Logger
 
 	mtx           sync.Mutex
 	store         *peerStore
@@ -298,6 +302,7 @@ func NewPeerManager(selfID types.NodeID, peerDB dbm.DB, options PeerManagerOptio
 		rand:       rand.New(rand.NewSource(time.Now().UnixNano())), // nolint:gosec
 		dialWaker:  tmsync.NewWaker(),
 		evictWaker: tmsync.NewWaker(),
+		logger:     log.NewNopLogger(),
 
 		store:         store,
 		dialing:       map[types.NodeID]bool{},
@@ -308,6 +313,11 @@ func NewPeerManager(selfID types.NodeID, peerDB dbm.DB, options PeerManagerOptio
 		evicting:      map[types.NodeID]bool{},
 		subscriptions: map[*PeerUpdates]*PeerUpdates{},
 	}
+
+	if options.Logger != nil {
+		peerManager.logger = options.Logger
+	}
+
 	if err = peerManager.configurePeers(); err != nil {
 		return nil, err
 	}
@@ -390,7 +400,7 @@ func (m *PeerManager) Add(address NodeAddress) (bool, error) {
 		return false, err
 	}
 	if address.NodeID == m.selfID {
-		return false, fmt.Errorf("can't add self (%v) to peer store", m.selfID)
+		return false, nil
 	}
 
 	m.mtx.Lock()
diff --git a/internal/p2p/peermanager_test.go b/internal/p2p/peermanager_test.go
index 47e8462a4..bb79fe771 100644
--- a/internal/p2p/peermanager_test.go
+++ b/internal/p2p/peermanager_test.go
@@ -265,8 +265,9 @@ func TestPeerManager_Add(t *testing.T) {
 	require.Error(t, err)
 
 	// Adding self should error
-	_, err = peerManager.Add(p2p.NodeAddress{Protocol: "memory", NodeID: selfID})
-	require.Error(t, err)
+	ok, err := peerManager.Add(p2p.NodeAddress{Protocol: "memory", NodeID: selfID})
+	require.False(t, ok)
+	require.NoError(t, err)
 }
 
 func TestPeerManager_DialNext(t *testing.T) {
@@ -841,13 +842,14 @@ func TestPeerManager_Dialed_Connected(t *testing.T) {
 	require.Error(t, peerManager.Dialed(b))
 }
 
-func TestPeerManager_Dialed_Self(t *testing.T) {
+func TestPeerManager_Adding_Self(t *testing.T) {
 	peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{})
 	require.NoError(t, err)
 
-	// Dialing self should error.
-	_, err = peerManager.Add(p2p.NodeAddress{Protocol: "memory", NodeID: selfID})
-	require.Error(t, err)
+	// Ingesting self should not error.
+	ok, err := peerManager.Add(p2p.NodeAddress{Protocol: "memory", NodeID: selfID})
+	require.False(t, ok)
+	require.NoError(t, err)
 }
 
 func TestPeerManager_Dialed_MaxConnected(t *testing.T) {
diff --git a/node/node.go b/node/node.go
index 1bda1f0f7..f2c4cd6a8 100644
--- a/node/node.go
+++ b/node/node.go
@@ -203,7 +203,7 @@ func makeNode(
 		}
 	}
 
-	peerManager, peerCloser, err := createPeerManager(cfg, dbProvider, nodeKey.ID)
+	peerManager, peerCloser, err := createPeerManager(logger, cfg, dbProvider, nodeKey.ID)
 	closers = append(closers, peerCloser)
 	if err != nil {
 		return nil, combineCloseError(
diff --git a/node/seed.go b/node/seed.go
index a0b71e411..3ba4d86d4 100644
--- a/node/seed.go
+++ b/node/seed.go
@@ -67,7 +67,7 @@ func makeSeedNode(
 	// Setup Transport and Switch.
 	p2pMetrics := p2p.PrometheusMetrics(cfg.Instrumentation.Namespace, "chain_id", genDoc.ChainID)
 
-	peerManager, closer, err := createPeerManager(cfg, dbProvider, nodeKey.ID)
+	peerManager, closer, err := createPeerManager(logger, cfg, dbProvider, nodeKey.ID)
 	if err != nil {
 		return nil, combineCloseError(
 			fmt.Errorf("failed to create peer manager: %w", err),
diff --git a/node/setup.go b/node/setup.go
index 51a048249..3240ba0e7 100644
--- a/node/setup.go
+++ b/node/setup.go
@@ -199,6 +199,7 @@ func createEvidenceReactor(
 }
 
 func createPeerManager(
+	logger log.Logger,
 	cfg *config.Config,
 	dbProvider config.DBProvider,
 	nodeID types.NodeID,
@@ -226,6 +227,7 @@ func createPeerManager(
 	maxUpgradeConns := uint16(4)
 
 	options := p2p.PeerManagerOptions{
+		Logger:                 logger.With("module", "peermanager"),
 		SelfAddress:            selfAddr,
 		MaxConnected:           maxConns,
 		MaxConnectedUpgrade:    maxUpgradeConns,