diff --git a/src/net.h b/src/net.h index 421e3a85a1..c5980ad88b 100644 --- a/src/net.h +++ b/src/net.h @@ -727,7 +727,7 @@ class NetEventsInterface { public: /** Initialize a peer (setup state, queue any initial messages) */ - virtual void InitializeNode(CNode* pnode) = 0; + virtual void InitializeNode(const CNode* pnode) = 0; /** Handle removal of a peer (clear state) */ virtual void FinalizeNode(const CNode& node) = 0; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 760fe37e73..016db49f24 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -222,6 +222,10 @@ struct Peer { * Most peers use headers-first syncing, which doesn't use this mechanism */ uint256 m_continuation_block GUARDED_BY(m_block_inv_mutex) {}; + Mutex m_msgproc_mutex; + /** Set to true once initial VERSION message was sent (only relevant for outbound peers). */ + bool m_outbound_version_message_sent GUARDED_BY(m_msgproc_mutex){false}; + /** This peer's reported block height when we connected */ std::atomic m_starting_height{-1}; @@ -312,7 +316,7 @@ class PeerManagerImpl final : public PeerManager void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) override; /** Implement NetEventsInterface */ - void InitializeNode(CNode* pnode) override; + void InitializeNode(const CNode* pnode) override; void FinalizeNode(const CNode& node) override; bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override; bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); @@ -1189,7 +1193,7 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) if (state) state->m_last_block_announcement = time_in_seconds; } -void PeerManagerImpl::InitializeNode(CNode *pnode) +void PeerManagerImpl::InitializeNode(const CNode *pnode) { NodeId nodeid = pnode->GetId(); { @@ -1202,9 +1206,6 @@ void PeerManagerImpl::InitializeNode(CNode *pnode) LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer)); } - if (!pnode->IsInboundConn()) { - PushNodeVersion(*pnode); - } } void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) @@ -4201,6 +4202,10 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt PeerRef peer = GetPeerRef(pfrom->GetId()); if (peer == nullptr) return false; + // For outbound connections, ensure that the initial VERSION message + // has been sent first before processing any incoming messages + if (!pfrom->IsInboundConn() && WITH_LOCK(peer->m_msgproc_mutex, return !peer->m_outbound_version_message_sent)) return false; + { LOCK(peer->m_getdata_requests_mutex); if (!peer->m_getdata_requests.empty()) { @@ -4660,6 +4665,13 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // disconnect misbehaving peers even before the version handshake is complete. if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true; + // Initiate version handshake for outbound connections + if (!pto->IsInboundConn() && WITH_LOCK(peer->m_msgproc_mutex, return !peer->m_outbound_version_message_sent)) { + LOCK(peer->m_msgproc_mutex); + PushNodeVersion(*pto); + peer->m_outbound_version_message_sent = true; + } + // Don't send anything until the version handshake is complete if (!pto->fSuccessfullyConnected || pto->fDisconnect) return true; diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index db5580e10f..9c52de7955 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -244,13 +244,13 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, filter_txs), }; - (void)connman.ReceiveMsgFrom(node, msg_version); - node.fPauseSend = false; - connman.ProcessMessagesOnce(node); { LOCK(node.cs_sendProcessing); peerman.SendMessages(&node); } + (void)connman.ReceiveMsgFrom(node, msg_version); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); if (node.fDisconnect) return; assert(node.nVersion == version); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION));