From 61745c7451ec64b26c74f672c688e82efb3b96aa Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 16 Apr 2024 14:58:42 +0100 Subject: [PATCH] lock m_recent_confirmed_transactions using m_tx_download_mutex --- src/net_processing.cpp | 43 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e7012650165..c2419947631 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -490,11 +490,11 @@ public: /** Overridden from CValidationInterface. */ void ActiveTipChange(const CBlockIndex* new_tip, bool) override - EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); void BlockConnected(ChainstateRole role, const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) override - EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); void BlockDisconnected(const std::shared_ptr &block, const CBlockIndex* pindex) override - EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void BlockChecked(const CBlock& block, const BlockValidationState& state) override @@ -507,9 +507,9 @@ public: 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 - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); + 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 - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, g_msgproc_mutex, !m_tx_download_mutex); /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; @@ -528,7 +528,7 @@ public: void UnitTestMisbehaving(NodeId peer_id) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const override; @@ -863,7 +863,7 @@ private: * - m_recent_confirmed_transactions * */ bool AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable) - EXCLUSIVE_LOCKS_REQUIRED(!m_recent_confirmed_transactions_mutex, m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(m_tx_download_mutex); /** * Filter for transactions that were recently rejected by the mempool. @@ -938,8 +938,7 @@ private: * transaction per day that would be inadvertently ignored (which is the * same probability that we have in the reject filter). */ - Mutex m_recent_confirmed_transactions_mutex; - CRollingBloomFilter m_recent_confirmed_transactions GUARDED_BY(m_recent_confirmed_transactions_mutex){48'000, 0.000'001}; + CRollingBloomFilter m_recent_confirmed_transactions GUARDED_BY(m_tx_download_mutex){48'000, 0.000'001}; /** * For sending `inv`s to inbound peers, we use a single (exponentially @@ -2119,20 +2118,15 @@ void PeerManagerImpl::BlockConnected( LOCK(m_tx_download_mutex); m_orphanage.EraseForBlock(*pblock); - { - LOCK(m_recent_confirmed_transactions_mutex); - for (const auto& ptx : pblock->vtx) { - m_recent_confirmed_transactions.insert(ptx->GetHash().ToUint256()); - if (ptx->HasWitness()) { - m_recent_confirmed_transactions.insert(ptx->GetWitnessHash().ToUint256()); - } + for (const auto& ptx : pblock->vtx) { + m_recent_confirmed_transactions.insert(ptx->GetHash().ToUint256()); + if (ptx->HasWitness()) { + m_recent_confirmed_transactions.insert(ptx->GetWitnessHash().ToUint256()); } } - { - for (const auto& ptx : pblock->vtx) { - m_txrequest.ForgetTxHash(ptx->GetHash()); - m_txrequest.ForgetTxHash(ptx->GetWitnessHash()); - } + for (const auto& ptx : pblock->vtx) { + m_txrequest.ForgetTxHash(ptx->GetHash()); + m_txrequest.ForgetTxHash(ptx->GetWitnessHash()); } } @@ -2146,7 +2140,7 @@ void PeerManagerImpl::BlockDisconnected(const std::shared_ptr &blo // block's worth of transactions in it, but that should be fine, since // presumably the most common case of relaying a confirmed transaction // should be just after a new block containing it is found. - LOCK(m_recent_confirmed_transactions_mutex); + LOCK(m_tx_download_mutex); m_recent_confirmed_transactions.reset(); } @@ -2310,10 +2304,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconside if (include_reconsiderable && m_recent_rejects_reconsiderable.contains(hash)) return true; - { - LOCK(m_recent_confirmed_transactions_mutex); - if (m_recent_confirmed_transactions.contains(hash)) return true; - } + if (m_recent_confirmed_transactions.contains(hash)) return true; return m_recent_rejects.contains(hash) || m_mempool.exists(gtxid); }