Avoid calling CAddrMan::Connected() on block-relay-only peer addresses

Connected() updates the time we serve in addr messages, so avoid leaking
block-relay-only peer connections by avoiding these calls.

5 files changed, 12 insertions, 10 deletions

diff --git a/src/net.cpp b/src/net.cpp
index e8a27c3530..9680e9947d 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2624,7 +2624,7 @@ void CConnman::DeleteNode(CNode* pnode)
 {
     assert(pnode);
     bool fUpdateConnectionTime = false;
-    m_msgproc->FinalizeNode(pnode->GetId(), fUpdateConnectionTime);
+    m_msgproc->FinalizeNode(*pnode, fUpdateConnectionTime);
     if (fUpdateConnectionTime) {
         addrman.Connected(pnode->addr);
     }
diff --git a/src/net.h b/src/net.h
index 2652d82ea0..fc01f44dd2 100644
--- a/src/net.h
+++ b/src/net.h
@@ -618,7 +618,7 @@ public:
     virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
     virtual bool SendMessages(CNode* pnode) = 0;
     virtual void InitializeNode(CNode* pnode) = 0;
-    virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0;
+    virtual void FinalizeNode(const CNode& node, bool& update_connection_time) = 0;
 
 protected:
     /**
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 94d4052fa1..a4dfab62e2 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -837,7 +837,8 @@ void PeerManager::ReattemptInitialBroadcast(CScheduler& scheduler) const
     scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta);
 }
 
-void PeerManager::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) {
+void PeerManager::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) {
+    NodeId nodeid = node.GetId();
     fUpdateConnectionTime = false;
     LOCK(cs_main);
     int misbehavior{0};
@@ -854,7 +855,8 @@ void PeerManager::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) {
     if (state->fSyncStarted)
         nSyncStarted--;
 
-    if (misbehavior == 0 && state->fCurrentlyConnected) {
+    if (misbehavior == 0 && state->fCurrentlyConnected && !node.IsBlockOnlyConn()) {
+        // Note: we avoid changing visible addrman state for block-relay-only peers
         fUpdateConnectionTime = true;
     }
 
diff --git a/src/net_processing.h b/src/net_processing.h
index 578660355a..87eee566de 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -58,7 +58,7 @@ public:
     /** Initialize a peer by adding it to mapNodeState and pushing a message requesting its version */
     void InitializeNode(CNode* pnode) override;
     /** Handle removal of a peer by updating various state and removing it from mapNodeState */
-    void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) override;
+    void FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) override;
     /**
     * Process protocol messages received from a given node
     *
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 6743dc0070..c399da900f 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -129,7 +129,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
     SetMockTime(0);
 
     bool dummy;
-    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
+    peerLogic->FinalizeNode(dummyNode1, dummy);
 }
 
 static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman)
@@ -211,7 +211,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 
     bool dummy;
     for (const CNode *node : vNodes) {
-        peerLogic->FinalizeNode(node->GetId(), dummy);
+        peerLogic->FinalizeNode(*node, dummy);
     }
 
     connman->ClearNodes();
@@ -259,8 +259,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
     BOOST_CHECK(banman->IsDiscouraged(addr2));  // to be discouraged now
 
     bool dummy;
-    peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
-    peerLogic->FinalizeNode(dummyNode2.GetId(), dummy);
+    peerLogic->FinalizeNode(dummyNode1, dummy);
+    peerLogic->FinalizeNode(dummyNode2, dummy);
 }
 
 BOOST_AUTO_TEST_CASE(DoS_bantime)
@@ -288,7 +288,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     BOOST_CHECK(banman->IsDiscouraged(addr));
 
     bool dummy;
-    peerLogic->FinalizeNode(dummyNode.GetId(), dummy);
+    peerLogic->FinalizeNode(dummyNode, dummy);
 }
 
 static CTransactionRef RandomOrphan()