From daf55531260833d597ee599e2d289ea1be0b1d9c Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Sat, 24 Oct 2020 03:16:04 -0400
Subject: [PATCH] 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.
---
 src/net.cpp                        |  2 +-
 src/net.h                          |  2 +-
 src/net_processing.cpp             |  6 ++++--
 src/net_processing.h               |  2 +-
 src/test/denialofservice_tests.cpp | 10 +++++-----
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index e8a27c35305..9680e9947d6 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 2652d82ea0b..fc01f44dd2d 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 94d4052fa12..a4dfab62e2c 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 578660355af..87eee566deb 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 6743dc00709..c399da900f6 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()