diff --git a/src/net.cpp b/src/net.cpp index 6b79e913e82..646fe9e9ea6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3139,12 +3139,12 @@ void CConnman::ThreadMessageHandler() continue; // Receive messages - bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, flagInterruptMsgProc); + bool fMoreNodeWork{m_msgproc->ProcessMessages(*pnode, flagInterruptMsgProc)}; fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend); if (flagInterruptMsgProc) return; // Send messages - m_msgproc->SendMessages(pnode); + m_msgproc->SendMessages(*pnode); if (flagInterruptMsgProc) return; diff --git a/src/net.h b/src/net.h index 1e4e9124e9f..1f7bbe4a500 100644 --- a/src/net.h +++ b/src/net.h @@ -1043,21 +1043,21 @@ public: virtual bool HasAllDesirableServiceFlags(ServiceFlags services) const = 0; /** - * Process protocol messages received from a given node - * - * @param[in] pnode The node which we have received messages from. - * @param[in] interrupt Interrupt condition for processing threads - * @return True if there is more work to be done - */ - virtual bool ProcessMessages(CNode* pnode, std::atomic& interrupt) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; + * Process protocol messages received from a given node + * + * @param[in] node The node which we have received messages from. + * @param[in] interrupt Interrupt condition for processing threads + * @return True if there is more work to be done + */ + virtual bool ProcessMessages(CNode& node, std::atomic& interrupt) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; /** - * Send queued protocol messages to a given node. - * - * @param[in] pnode The node which we are sending messages to. - * @return True if there is more work to be done - */ - virtual bool SendMessages(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; + * Send queued protocol messages to a given node. + * + * @param[in] node The node which we are sending messages to. + * @return True if there is more work to be done + */ + virtual bool SendMessages(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; protected: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 34425948876..3f8c221734a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -529,9 +529,9 @@ public: void InitializeNode(const CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_tx_download_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, !m_tx_download_mutex); bool HasAllDesirableServiceFlags(ServiceFlags services) const override; - bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override + bool ProcessMessages(CNode& node, std::atomic& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); - bool SendMessages(CNode* pto) override + bool SendMessages(CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, g_msgproc_mutex, !m_tx_download_mutex); /** Implement PeerManager */ @@ -5181,11 +5181,12 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer) return true; } -bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interruptMsgProc) +bool PeerManagerImpl::ProcessMessages(CNode& node, std::atomic& interruptMsgProc) { AssertLockNotHeld(m_tx_download_mutex); AssertLockHeld(g_msgproc_mutex); + CNode* pfrom{&node}; // alias removed in a later commit. PeerRef peer = GetPeerRef(pfrom->GetId()); if (peer == nullptr) return false; @@ -5683,11 +5684,12 @@ bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer) return true; } -bool PeerManagerImpl::SendMessages(CNode* pto) +bool PeerManagerImpl::SendMessages(CNode& node) { AssertLockNotHeld(m_tx_download_mutex); AssertLockHeld(g_msgproc_mutex); + CNode* pto{&node}; // alias removed in a later commit PeerRef peer = GetPeerRef(pto->GetId()); if (!peer) return false; const Consensus::Params& consensusParams = m_chainparams.GetConsensus(); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 0dc0323028e..776a18788e3 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -81,7 +81,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) } // Test starts here - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(dummyNode1)); // should result in getheaders { LOCK(dummyNode1.cs_vSend); @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) int64_t nStartTime = GetTime(); // Wait 21 minutes SetMockTime(nStartTime+21*60); - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(dummyNode1)); // should result in getheaders { LOCK(dummyNode1.cs_vSend); const auto& [to_send, _more, _msg_type] = dummyNode1.m_transport->GetBytesToSend(false); @@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) } // Wait 3 more minutes SetMockTime(nStartTime+24*60); - BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect + BOOST_CHECK(peerman.SendMessages(dummyNode1)); // should result in disconnect BOOST_CHECK(dummyNode1.fDisconnect == true); peerman.FinalizeNode(dummyNode1); @@ -336,7 +336,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) nodes[0]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[0]); peerLogic->UnitTestMisbehaving(nodes[0]->GetId()); // Should be discouraged - BOOST_CHECK(peerLogic->SendMessages(nodes[0])); + BOOST_CHECK(peerLogic->SendMessages(*nodes[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); @@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(*nodes[1], NODE_NETWORK); nodes[1]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[1]); - BOOST_CHECK(peerLogic->SendMessages(nodes[1])); + BOOST_CHECK(peerLogic->SendMessages(*nodes[1])); // [0] is still discouraged/disconnected. BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); @@ -364,7 +364,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(!banman->IsDiscouraged(addr[1])); BOOST_CHECK(!nodes[1]->fDisconnect); peerLogic->UnitTestMisbehaving(nodes[1]->GetId()); - BOOST_CHECK(peerLogic->SendMessages(nodes[1])); + BOOST_CHECK(peerLogic->SendMessages(*nodes[1])); // Expect both [0] and [1] to be discouraged/disconnected now. BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(nodes[0]->fDisconnect); @@ -388,7 +388,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) nodes[2]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[2]); peerLogic->UnitTestMisbehaving(nodes[2]->GetId()); - BOOST_CHECK(peerLogic->SendMessages(nodes[2])); + BOOST_CHECK(peerLogic->SendMessages(*nodes[2])); BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[1])); BOOST_CHECK(banman->IsDiscouraged(addr[2])); @@ -431,7 +431,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.fSuccessfullyConnected = true; peerLogic->UnitTestMisbehaving(dummyNode.GetId()); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); + BOOST_CHECK(peerLogic->SendMessages(dummyNode)); BOOST_CHECK(banman->IsDiscouraged(addr)); peerLogic->FinalizeNode(dummyNode); diff --git a/src/test/fuzz/p2p_handshake.cpp b/src/test/fuzz/p2p_handshake.cpp index aec7387eb2f..bf9b74450c1 100644 --- a/src/test/fuzz/p2p_handshake.cpp +++ b/src/test/fuzz/p2p_handshake.cpp @@ -100,7 +100,7 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize) more_work = connman.ProcessMessagesOnce(connection); } catch (const std::ios_base::failure&) { } - peerman->SendMessages(&connection); + peerman->SendMessages(connection); } } diff --git a/src/test/fuzz/p2p_headers_presync.cpp b/src/test/fuzz/p2p_headers_presync.cpp index d587137413c..4aa56686b53 100644 --- a/src/test/fuzz/p2p_headers_presync.cpp +++ b/src/test/fuzz/p2p_headers_presync.cpp @@ -97,7 +97,7 @@ void HeadersSyncSetup::SendMessage(FuzzedDataProvider& fuzzed_data_provider, CSe connman.ProcessMessagesOnce(connection); } catch (const std::ios_base::failure&) { } - m_node.peerman->SendMessages(&connection); + m_node.peerman->SendMessages(connection); } CBlockHeader ConsumeHeader(FuzzedDataProvider& fuzzed_data_provider, const uint256& prev_hash, uint32_t prev_nbits) diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index ef8cb686cee..c03dfbd68e7 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -121,7 +121,7 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) more_work = connman.ProcessMessagesOnce(p2p_node); } catch (const std::ios_base::failure&) { } - node.peerman->SendMessages(&p2p_node); + node.peerman->SendMessages(p2p_node); } node.validation_signals->SyncWithValidationInterfaceQueue(); node.connman->StopNodes(); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index f36f528b0e3..8997dbac46d 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -120,7 +120,7 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) more_work = connman.ProcessMessagesOnce(random_node); } catch (const std::ios_base::failure&) { } - node.peerman->SendMessages(&random_node); + node.peerman->SendMessages(random_node); } } node.validation_signals->SyncWithValidationInterfaceQueue(); diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index 90c018a4275..d19c4cbf2f3 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -150,9 +150,9 @@ public: virtual bool HasAllDesirableServiceFlags(ServiceFlags) const override { return m_fdp.ConsumeBool(); } - virtual bool ProcessMessages(CNode*, std::atomic&) override { return m_fdp.ConsumeBool(); } + virtual bool ProcessMessages(CNode&, std::atomic&) override { return m_fdp.ConsumeBool(); } - virtual bool SendMessages(CNode*) override { return m_fdp.ConsumeBool(); } + virtual bool SendMessages(CNode&) override { return m_fdp.ConsumeBool(); } private: FuzzedDataProvider& m_fdp; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index ff2083dee0b..72a06a68df8 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -847,7 +847,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) m_node.peerman->InitializeNode(peer, NODE_NETWORK); - m_node.peerman->SendMessages(&peer); + m_node.peerman->SendMessages(peer); connman.FlushSendBuffer(peer); // Drop sent version message auto msg_version_receive = @@ -857,7 +857,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) bool more_work{connman.ProcessMessagesOnce(peer)}; Assert(!more_work); - m_node.peerman->SendMessages(&peer); + m_node.peerman->SendMessages(peer); connman.FlushSendBuffer(peer); // Drop sent verack message Assert(connman.ReceiveMsgFrom(peer, NetMsg::Make(NetMsgType::VERACK))); @@ -890,7 +890,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) } }; - m_node.peerman->SendMessages(&peer); + m_node.peerman->SendMessages(peer); BOOST_CHECK(sent); diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 0f59e0d6c84..0979e2ae45d 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -31,7 +31,7 @@ void ConnmanTestMsg::Handshake(CNode& node, auto& connman{*this}; peerman.InitializeNode(node, local_services); - peerman.SendMessages(&node); + peerman.SendMessages(node); FlushSendBuffer(node); // Drop the version message added by SendMessages. CSerializedNetMsg msg_version{ @@ -52,7 +52,7 @@ void ConnmanTestMsg::Handshake(CNode& node, (void)connman.ReceiveMsgFrom(node, std::move(msg_version)); node.fPauseSend = false; connman.ProcessMessagesOnce(node); - peerman.SendMessages(&node); + peerman.SendMessages(node); FlushSendBuffer(node); // Drop the verack message added by SendMessages. if (node.fDisconnect) return; assert(node.nVersion == version); @@ -66,7 +66,7 @@ void ConnmanTestMsg::Handshake(CNode& node, (void)connman.ReceiveMsgFrom(node, std::move(msg_verack)); node.fPauseSend = false; connman.ProcessMessagesOnce(node); - peerman.SendMessages(&node); + peerman.SendMessages(node); assert(node.fSuccessfullyConnected == true); } } diff --git a/src/test/util/net.h b/src/test/util/net.h index ee02d404ec0..732aefb3742 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -101,7 +101,7 @@ struct ConnmanTestMsg : public CConnman { bool ProcessMessagesOnce(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) { - return m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); + return m_msgproc->ProcessMessages(node, flagInterruptMsgProc); } void NodeReceiveMsgBytes(CNode& node, std::span msg_bytes, bool& complete) const;