diff options
author | John Newbery <john@johnnewbery.com> | 2020-10-23 10:28:33 +0100 |
---|---|---|
committer | John Newbery <john@johnnewbery.com> | 2021-03-22 10:25:39 +0000 |
commit | 3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 (patch) | |
tree | 8fb6ee74b7dafe3c7a333c8804bfa4f9d7a03937 | |
parent | 7c4cc67c0c3c50df004ee53cac5b2884b7fbab29 (diff) | |
download | bitcoin-3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682.tar.xz |
[net] remove fUpdateConnectionTime from FinalizeNode
PeerManager can just call directly into CAddrMan::Connected() now.
-rw-r--r-- | src/net.cpp | 6 | ||||
-rw-r--r-- | src/net.h | 2 | ||||
-rw-r--r-- | src/net_processing.cpp | 22 | ||||
-rw-r--r-- | src/test/denialofservice_tests.cpp | 14 |
4 files changed, 19 insertions, 25 deletions
diff --git a/src/net.cpp b/src/net.cpp index 9553bbddfa..f9ae67b75c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2621,11 +2621,7 @@ void CConnman::StopNodes() void CConnman::DeleteNode(CNode* pnode) { assert(pnode); - bool fUpdateConnectionTime = false; - m_msgproc->FinalizeNode(*pnode, fUpdateConnectionTime); - if (fUpdateConnectionTime) { - addrman.Connected(pnode->addr); - } + m_msgproc->FinalizeNode(*pnode); delete pnode; } @@ -770,7 +770,7 @@ public: virtual void InitializeNode(CNode* pnode) = 0; /** Handle removal of a peer (clear state) */ - virtual void FinalizeNode(const CNode& node, bool& update_connection_time) = 0; + virtual void FinalizeNode(const CNode& node) = 0; /** * Process protocol messages received from a given node diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4b91d86cfa..68be99c3ff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -238,7 +238,7 @@ public: /** Implement NetEventsInterface */ void InitializeNode(CNode* pnode) override; - void FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) override; + void FinalizeNode(const CNode& node) override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override; bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); @@ -969,13 +969,13 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta); } -void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) +void PeerManagerImpl::FinalizeNode(const CNode& node) { NodeId nodeid = node.GetId(); - fUpdateConnectionTime = false; - LOCK(cs_main); int misbehavior{0}; { + LOCK(cs_main); + { // We remove the PeerRef from g_peer_map here, but we don't always // destruct the Peer. Sometimes another thread is still holding a // PeerRef, so the refcount is >= 1. Be careful not to do any @@ -991,12 +991,6 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim if (state->fSyncStarted) nSyncStarted--; - if (node.fSuccessfullyConnected && misbehavior == 0 && - !node.IsBlockOnlyConn() && !node.IsInboundConn()) { - // Only change visible addrman state for outbound, full-relay peers - fUpdateConnectionTime = true; - } - for (const QueuedBlock& entry : state->vBlocksInFlight) { mapBlocksInFlight.erase(entry.hash); } @@ -1021,6 +1015,14 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim assert(m_wtxid_relay_peers == 0); assert(m_txrequest.Size() == 0); } + } // cs_main + if (node.fSuccessfullyConnected && misbehavior == 0 && + !node.IsBlockOnlyConn() && !node.IsInboundConn()) { + // Only change visible addrman state for full outbound peers. We don't + // call Connected() for feeler connections since they don't have + // fSuccessfullyConnected set. + m_addrman.Connected(node.addr); + } LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index a4ad7ed7cf..7557d4618a 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -117,8 +117,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) BOOST_CHECK(dummyNode1.fDisconnect == true); SetMockTime(0); - bool dummy; - peerLogic->FinalizeNode(dummyNode1, dummy); + peerLogic->FinalizeNode(dummyNode1); } static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman) @@ -199,9 +198,8 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); - bool dummy; for (const CNode *node : vNodes) { - peerLogic->FinalizeNode(*node, dummy); + peerLogic->FinalizeNode(*node); } connman->ClearNodes(); @@ -249,9 +247,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(banman->IsDiscouraged(addr1)); // Expect both 1 and 2 BOOST_CHECK(banman->IsDiscouraged(addr2)); // to be discouraged now - bool dummy; - peerLogic->FinalizeNode(dummyNode1, dummy); - peerLogic->FinalizeNode(dummyNode2, dummy); + peerLogic->FinalizeNode(dummyNode1); + peerLogic->FinalizeNode(dummyNode2); } BOOST_AUTO_TEST_CASE(DoS_bantime) @@ -279,8 +276,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) } BOOST_CHECK(banman->IsDiscouraged(addr)); - bool dummy; - peerLogic->FinalizeNode(dummyNode, dummy); + peerLogic->FinalizeNode(dummyNode); } class TxOrphanageTest : public TxOrphanage |