From d1cb6b0eaa7f46796d6705f5a837d28e8a3c8dee Mon Sep 17 00:00:00 2001
From: Prem Chaitanya Prathi <chaitanyaprem@gmail.com>
Date: Tue, 28 May 2024 18:20:47 +0530
Subject: [PATCH] fix: crash noticed while adding existing peer that doesn't
 have an ENR (#1113)

---
 cmd/waku/node.go                    |  2 +-
 waku/v2/node/connectedness_test.go  |  2 +-
 waku/v2/node/wakunode2.go           |  3 ++-
 waku/v2/peermanager/peer_manager.go | 24 ++++++++++++++----------
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/cmd/waku/node.go b/cmd/waku/node.go
index 976d1024..8383c24d 100644
--- a/cmd/waku/node.go
+++ b/cmd/waku/node.go
@@ -331,7 +331,7 @@ func Execute(options NodeOptions) error {
 	}
 
 	for _, d := range discoveredNodes {
-		wakuNode.AddDiscoveredPeer(d.PeerID, d.PeerInfo.Addrs, wakupeerstore.DNSDiscovery, nil, true)
+		wakuNode.AddDiscoveredPeer(d.PeerID, d.PeerInfo.Addrs, wakupeerstore.DNSDiscovery, nil, d.ENR, true)
 	}
 
 	//For now assuming that static peers added support/listen on all topics specified via commandLine.
diff --git a/waku/v2/node/connectedness_test.go b/waku/v2/node/connectedness_test.go
index 49c7eb32..1b839cd1 100644
--- a/waku/v2/node/connectedness_test.go
+++ b/waku/v2/node/connectedness_test.go
@@ -74,7 +74,7 @@ func TestConnectionStatusChanges(t *testing.T) {
 
 	goCheckConnectedness(t, &wg, topicHealthStatusChan, peermanager.MinimallyHealthy)
 
-	node1.AddDiscoveredPeer(node2.host.ID(), node2.ListenAddresses(), peerstore.Static, []string{pubsubTopic}, true)
+	node1.AddDiscoveredPeer(node2.host.ID(), node2.ListenAddresses(), peerstore.Static, []string{pubsubTopic}, nil, true)
 
 	wg.Wait()
 
diff --git a/waku/v2/node/wakunode2.go b/waku/v2/node/wakunode2.go
index 323958e5..ca6a16bb 100644
--- a/waku/v2/node/wakunode2.go
+++ b/waku/v2/node/wakunode2.go
@@ -698,7 +698,7 @@ func (w *WakuNode) AddPeer(address ma.Multiaddr, origin wps.Origin, pubSubTopics
 }
 
 // AddDiscoveredPeer to add a discovered peer to the node peerStore
-func (w *WakuNode) AddDiscoveredPeer(ID peer.ID, addrs []ma.Multiaddr, origin wps.Origin, pubsubTopics []string, connectNow bool) {
+func (w *WakuNode) AddDiscoveredPeer(ID peer.ID, addrs []ma.Multiaddr, origin wps.Origin, pubsubTopics []string, enr *enode.Node, connectNow bool) {
 	p := service.PeerData{
 		Origin: origin,
 		AddrInfo: peer.AddrInfo{
@@ -706,6 +706,7 @@ func (w *WakuNode) AddDiscoveredPeer(ID peer.ID, addrs []ma.Multiaddr, origin wp
 			Addrs: addrs,
 		},
 		PubsubTopics: pubsubTopics,
+		ENR:          enr,
 	}
 	w.peermanager.AddDiscoveredPeer(p, connectNow)
 }
diff --git a/waku/v2/peermanager/peer_manager.go b/waku/v2/peermanager/peer_manager.go
index b4ec439d..e811632e 100644
--- a/waku/v2/peermanager/peer_manager.go
+++ b/waku/v2/peermanager/peer_manager.go
@@ -431,17 +431,21 @@ func (pm *PeerManager) AddDiscoveredPeer(p service.PeerData, connectNow bool) {
 	if err == nil {
 		enr, err := pm.host.Peerstore().(wps.WakuPeerstore).ENR(p.AddrInfo.ID)
 		// Verifying if the enr record is more recent (DiscV5 and peer exchange can return peers already seen)
-		if err == nil && enr.Record().Seq() >= p.ENR.Seq() {
-			return
-		}
-		if err != nil {
-			//Peer is already in peer-store but it doesn't have an enr, but discovered peer has ENR
-			pm.logger.Info("peer already found in peerstore, but doesn't have an ENR record, re-adding",
-				logging.HostID("peer", p.AddrInfo.ID), zap.Uint64("newENRSeq", p.ENR.Seq()))
+		if err == nil {
+			if p.ENR != nil {
+				if enr.Record().Seq() >= p.ENR.Seq() {
+					return
+				}
+				//Peer is already in peer-store but stored ENR is older than discovered one.
+				pm.logger.Info("peer already found in peerstore, but re-adding it as ENR sequence is higher than locally stored",
+					logging.HostID("peer", p.AddrInfo.ID), zap.Uint64("newENRSeq", p.ENR.Seq()), zap.Uint64("storedENRSeq", enr.Record().Seq()))
+			} else {
+				pm.logger.Info("peer already found in peerstore, but no new ENR", logging.HostID("peer", p.AddrInfo.ID))
+			}
 		} else {
-			//Peer is already in peer-store but stored ENR is older than discovered one.
-			pm.logger.Info("peer already found in peerstore, but re-adding it as ENR sequence is higher than locally stored",
-				logging.HostID("peer", p.AddrInfo.ID), zap.Uint64("newENRSeq", p.ENR.Seq()), zap.Uint64("storedENRSeq", enr.Record().Seq()))
+			//Peer is in peer-store but it doesn't have an enr
+			pm.logger.Info("peer already found in peerstore, but doesn't have an ENR record, re-adding",
+				logging.HostID("peer", p.AddrInfo.ID))
 		}
 	}