From ff8d44d1967d8ff9fb9b9ea0b87c0470d8cc2550 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 5 Oct 2022 18:50:33 +1000 Subject: Remove unnecessary includes of txorphange.h --- src/init.cpp | 1 - src/test/fuzz/process_message.cpp | 1 - src/test/fuzz/process_messages.cpp | 1 - 3 files changed, 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 25b40c6c6e..9a53003168 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -65,7 +65,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index f6a35da4fc..b5efa94f1e 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 1df1717ec3..3c8d662f74 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include -- cgit v1.2.3 From 89e2e0da0bcd0b0757d7b42907e2d2214da9f68b Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 5 Oct 2022 18:54:14 +1000 Subject: net_processing: move extra transactions to msgproc mutex Previously vExtraTxnForCompact and vExtraTxnForCompactIt were protected by g_cs_orphans; protect them by g_msgproc_mutex instead, as they are only used during message processing. --- src/net_processing.cpp | 9 +++++---- src/txorphanage.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index eca6263392..80a5098f61 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -590,7 +590,7 @@ private: bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); void ProcessOrphanTx(std::set& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); /** Process a single headers message from a peer. * * @param[in] pfrom CNode of the peer @@ -924,14 +924,14 @@ private: /** Storage for orphan information */ TxOrphanage m_orphanage; - void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction. * The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of * these are kept in a ring buffer */ - std::vector> vExtraTxnForCompact GUARDED_BY(g_cs_orphans); + std::vector> vExtraTxnForCompact GUARDED_BY(g_msgproc_mutex); /** Offset into vExtraTxnForCompact to insert the next tx */ - size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0; + size_t vExtraTxnForCompactIt GUARDED_BY(g_msgproc_mutex) = 0; /** Check whether the last unknown block a peer advertised is not yet known. */ void ProcessBlockAvailability(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -2885,6 +2885,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, */ void PeerManagerImpl::ProcessOrphanTx(std::set& orphan_work_set) { + AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); diff --git a/src/txorphanage.h b/src/txorphanage.h index 9363e6f733..92f4319d7d 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -10,7 +10,7 @@ #include #include -/** Guards orphan transactions and extra txs for compact blocks */ +/** Guards orphan transactions */ extern RecursiveMutex g_cs_orphans; /** A class to track orphan transactions (failed on TX_MISSING_INPUTS) -- cgit v1.2.3 From 9910ed755c3dfd7669707b44d993a20030dd7477 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 5 Oct 2022 18:58:53 +1000 Subject: net_processing: Pass a Peer& to ProcessOrphanTx --- src/net_processing.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 80a5098f61..7e309d8771 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -589,7 +589,7 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); - void ProcessOrphanTx(std::set& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) + void ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); /** Process a single headers message from a peer. * @@ -2878,20 +2878,20 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, /** * Reconsider orphan transactions after a parent has been accepted to the mempool. * - * @param[in,out] orphan_work_set The set of orphan transactions to reconsider. Generally only one + * @peer[in] peer The peer whose orphan transactions we will reconsider. Generally only one * orphan will be reconsidered on each call of this function. This set * may be added to if accepting an orphan causes its children to be * reconsidered. */ -void PeerManagerImpl::ProcessOrphanTx(std::set& orphan_work_set) +void PeerManagerImpl::ProcessOrphanTx(Peer& peer) { AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); - while (!orphan_work_set.empty()) { - const uint256 orphanHash = *orphan_work_set.begin(); - orphan_work_set.erase(orphan_work_set.begin()); + while (!peer.m_orphan_work_set.empty()) { + const uint256 orphanHash = *peer.m_orphan_work_set.begin(); + peer.m_orphan_work_set.erase(peer.m_orphan_work_set.begin()); const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash); if (porphanTx == nullptr) continue; @@ -2902,7 +2902,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set& orphan_work_set) if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanHash, porphanTx->GetWitnessHash()); - m_orphanage.AddChildrenToWorkSet(*porphanTx, orphan_work_set); + m_orphanage.AddChildrenToWorkSet(*porphanTx, peer.m_orphan_work_set); m_orphanage.EraseTx(orphanHash); for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) { AddToCompactExtraTransactions(removedTx); @@ -3959,7 +3959,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } // Recursively process any orphan transactions that depended on this one - ProcessOrphanTx(peer->m_orphan_work_set); + ProcessOrphanTx(*peer); } else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { @@ -4771,7 +4771,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt { LOCK2(cs_main, g_cs_orphans); if (!peer->m_orphan_work_set.empty()) { - ProcessOrphanTx(peer->m_orphan_work_set); + ProcessOrphanTx(*peer); } } -- cgit v1.2.3 From 0027174b396cacbaac5fd52f13be3dca9fcbbfb8 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 5 Oct 2022 19:02:55 +1000 Subject: net_processing: move ProcessOrphanTx docs to declaration --- src/net_processing.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7e309d8771..11d1f29792 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -589,6 +589,14 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); + /** + * Reconsider orphan transactions after a parent has been accepted to the mempool. + * + * @peer[in] peer The peer whose orphan transactions we will reconsider. Generally only one + * orphan will be reconsidered on each call of this function. This set + * may be added to if accepting an orphan causes its children to be + * reconsidered. + */ void ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); /** Process a single headers message from a peer. @@ -2875,14 +2883,6 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } -/** - * Reconsider orphan transactions after a parent has been accepted to the mempool. - * - * @peer[in] peer The peer whose orphan transactions we will reconsider. Generally only one - * orphan will be reconsidered on each call of this function. This set - * may be added to if accepting an orphan causes its children to be - * reconsidered. - */ void PeerManagerImpl::ProcessOrphanTx(Peer& peer) { AssertLockHeld(g_msgproc_mutex); -- cgit v1.2.3 From 6f8e442ba61378489a6e2e2ab5bcfbccca1a29ec Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 5 Oct 2022 19:12:14 +1000 Subject: net_processing: Localise orphan_work_set handling to ProcessOrphanTx --- src/net_processing.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 11d1f29792..3f384b921f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -596,8 +596,9 @@ private: * orphan will be reconsidered on each call of this function. This set * may be added to if accepting an orphan causes its children to be * reconsidered. + * @return True if there are still orphans in this peer's work set. */ - void ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) + bool ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); /** Process a single headers message from a peer. * @@ -2883,12 +2884,14 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } -void PeerManagerImpl::ProcessOrphanTx(Peer& peer) +bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) { AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); + if (peer.m_orphan_work_set.empty()) return false; + while (!peer.m_orphan_work_set.empty()) { const uint256 orphanHash = *peer.m_orphan_work_set.begin(); peer.m_orphan_work_set.erase(peer.m_orphan_work_set.begin()); @@ -2953,6 +2956,8 @@ void PeerManagerImpl::ProcessOrphanTx(Peer& peer) break; } } + + return !peer.m_orphan_work_set.empty(); } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, @@ -4768,16 +4773,17 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt } } + bool has_more_orphans; { LOCK2(cs_main, g_cs_orphans); - if (!peer->m_orphan_work_set.empty()) { - ProcessOrphanTx(*peer); - } + has_more_orphans = ProcessOrphanTx(*peer); } if (pfrom->fDisconnect) return false; + if (has_more_orphans) return true; + // this maintains the order of responses // and prevents m_getdata_requests to grow unbounded { @@ -4785,11 +4791,6 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt if (!peer->m_getdata_requests.empty()) return true; } - { - LOCK(g_cs_orphans); - if (!peer->m_orphan_work_set.empty()) return true; - } - // Don't bother if send buffer is too full to respond anyway if (pfrom->fPauseSend) return false; -- cgit v1.2.3 From 3614819864a84ac32f6d53c6ace79b5e71bc174a Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 25 Feb 2021 00:03:23 +1000 Subject: txorphange: move orphan workset to txorphanage --- src/net_processing.cpp | 20 ++++++++++---------- src/test/fuzz/txorphan.cpp | 3 +-- src/txorphanage.cpp | 8 +++++++- src/txorphanage.h | 8 +++++--- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3f384b921f..931a2a8100 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -368,9 +368,6 @@ struct Peer { /** Total number of addresses that were processed (excludes rate-limited ones). */ std::atomic m_addr_processed{0}; - /** Set of txids to reconsider once their parent transactions have been accepted **/ - std::set m_orphan_work_set GUARDED_BY(g_cs_orphans); - /** Whether we've sent this peer a getheaders in response to an inv prior to initial-headers-sync completing */ bool m_inv_triggered_getheaders_before_sync GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; @@ -2890,11 +2887,14 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); - if (peer.m_orphan_work_set.empty()) return false; + auto work_set_it = m_orphanage.m_peer_work_set.find(peer.m_id); + if (work_set_it == m_orphanage.m_peer_work_set.end()) return false; + + std::set& orphan_work_set = work_set_it->second; - while (!peer.m_orphan_work_set.empty()) { - const uint256 orphanHash = *peer.m_orphan_work_set.begin(); - peer.m_orphan_work_set.erase(peer.m_orphan_work_set.begin()); + while (!orphan_work_set.empty()) { + const uint256 orphanHash = *orphan_work_set.begin(); + orphan_work_set.erase(orphan_work_set.begin()); const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash); if (porphanTx == nullptr) continue; @@ -2905,7 +2905,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanHash, porphanTx->GetWitnessHash()); - m_orphanage.AddChildrenToWorkSet(*porphanTx, peer.m_orphan_work_set); + m_orphanage.AddChildrenToWorkSet(*porphanTx, peer.m_id); m_orphanage.EraseTx(orphanHash); for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) { AddToCompactExtraTransactions(removedTx); @@ -2957,7 +2957,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) } } - return !peer.m_orphan_work_set.empty(); + return !orphan_work_set.empty(); } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, @@ -3950,7 +3950,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_txrequest.ForgetTxHash(tx.GetHash()); m_txrequest.ForgetTxHash(tx.GetWitnessHash()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); - m_orphanage.AddChildrenToWorkSet(tx, peer->m_orphan_work_set); + m_orphanage.AddChildrenToWorkSet(tx, peer->m_id); pfrom.m_last_tx_time = GetTime(); diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 55060f31cf..730e814e1d 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -36,7 +36,6 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) SetMockTime(ConsumeTime(fuzzed_data_provider)); TxOrphanage orphanage; - std::set orphan_work_set; std::vector outpoints; // initial outpoints used to construct transactions later for (uint8_t i = 0; i < 4; i++) { @@ -87,7 +86,7 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) fuzzed_data_provider, [&] { LOCK(g_cs_orphans); - orphanage.AddChildrenToWorkSet(*tx, orphan_work_set); + orphanage.AddChildrenToWorkSet(*tx, peer_id); }, [&] { bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 69ae8ea582..d26a84b0ef 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -89,6 +89,8 @@ void TxOrphanage::EraseForPeer(NodeId peer) { AssertLockHeld(g_cs_orphans); + m_peer_work_set.erase(peer); + int nErased = 0; std::map::iterator iter = m_orphans.begin(); while (iter != m_orphans.end()) @@ -138,9 +140,13 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); } -void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set& orphan_work_set) const +void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) { AssertLockHeld(g_cs_orphans); + + // Get this peer's work set, emplacing an empty set it didn't exist + std::set& orphan_work_set = m_peer_work_set.try_emplace(peer).first->second; + for (unsigned int i = 0; i < tx.vout.size(); i++) { const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i)); if (it_by_prev != m_outpoint_to_orphan_it.end()) { diff --git a/src/txorphanage.h b/src/txorphanage.h index 92f4319d7d..aff57e84f7 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -43,9 +43,11 @@ public: /** Limit the orphanage to the given maximum */ void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); - /** Add any orphans that list a particular tx as a parent into a peer's work set - * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */ - void AddChildrenToWorkSet(const CTransaction& tx, std::set& orphan_work_set) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + /** Which peer provided a parent tx of orphans that need to be reconsidered */ + std::map> m_peer_work_set GUARDED_BY(g_cs_orphans); + + /** Add any orphans that list a particular tx as a parent into a peer's work set */ + void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** Return how many entries exist in the orphange */ size_t Size() LOCKS_EXCLUDED(::g_cs_orphans) -- cgit v1.2.3 From a936f41a5d5f7bb97425f82ec64dfae62e840a56 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 25 Feb 2021 00:28:16 +1000 Subject: txorphanage: make m_peer_work_set private --- src/net_processing.cpp | 18 ++++++------------ src/test/fuzz/txorphan.cpp | 12 +++++++++--- src/txorphanage.cpp | 22 ++++++++++++++++++---- src/txorphanage.h | 19 +++++++++++++------ 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 931a2a8100..f4b2d2e38f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2887,20 +2887,14 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); - auto work_set_it = m_orphanage.m_peer_work_set.find(peer.m_id); - if (work_set_it == m_orphanage.m_peer_work_set.end()) return false; - - std::set& orphan_work_set = work_set_it->second; - - while (!orphan_work_set.empty()) { - const uint256 orphanHash = *orphan_work_set.begin(); - orphan_work_set.erase(orphan_work_set.begin()); - - const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash); - if (porphanTx == nullptr) continue; + CTransactionRef porphanTx = nullptr; + NodeId from_peer = -1; + bool more = false; + while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id, from_peer, more)) { const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx); const TxValidationState& state = result.m_state; + const uint256& orphanHash = porphanTx->GetHash(); if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); @@ -2957,7 +2951,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) } } - return !orphan_work_set.empty(); + return more; } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 730e814e1d..b200f94144 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -89,11 +89,17 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) orphanage.AddChildrenToWorkSet(*tx, peer_id); }, [&] { - bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); { LOCK(g_cs_orphans); - bool get_tx = orphanage.GetTx(tx->GetHash()).first != nullptr; - Assert(have_tx == get_tx); + NodeId originator; + bool more = true; + CTransactionRef ref = orphanage.GetTxToReconsider(peer_id, originator, more); + if (!ref) { + Assert(!more); + } else { + bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetHash())); + Assert(have_tx); + } } }, [&] { diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index d26a84b0ef..4cccb1affb 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -167,13 +167,27 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const } } -std::pair TxOrphanage::GetTx(const uint256& txid) const +CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) { AssertLockHeld(g_cs_orphans); - const auto it = m_orphans.find(txid); - if (it == m_orphans.end()) return {nullptr, -1}; - return {it->second.tx, it->second.fromPeer}; + auto work_set_it = m_peer_work_set.find(peer); + if (work_set_it != m_peer_work_set.end()) { + auto& work_set = work_set_it->second; + while (!work_set.empty()) { + uint256 txid = *work_set.begin(); + work_set.erase(work_set.begin()); + + const auto orphan_it = m_orphans.find(txid); + if (orphan_it != m_orphans.end()) { + more = !work_set.empty(); + originator = orphan_it->second.fromPeer; + return orphan_it->second.tx; + } + } + } + more = false; + return nullptr; } void TxOrphanage::EraseForBlock(const CBlock& block) diff --git a/src/txorphanage.h b/src/txorphanage.h index aff57e84f7..5af1f2f0e0 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -10,6 +10,9 @@ #include #include +#include +#include + /** Guards orphan transactions */ extern RecursiveMutex g_cs_orphans; @@ -26,10 +29,14 @@ public: /** Check if we already have an orphan transaction (by txid or wtxid) */ bool HaveTx(const GenTxid& gtxid) const LOCKS_EXCLUDED(::g_cs_orphans); - /** Get an orphan transaction and its originating peer - * (Transaction ref will be nullptr if not found) + /** Extract a transaction from a peer's work set + * Returns nullptr and sets more to false if there are no transactions + * to work on. Otherwise returns the transaction reference, removes + * the transaction from the work set, and populates its arguments with + * the originating peer, and whether there are more orphans for this peer + * to work on after this tx. */ - std::pair GetTx(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** Erase an orphan by txid */ int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); @@ -43,9 +50,6 @@ public: /** Limit the orphanage to the given maximum */ void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); - /** Which peer provided a parent tx of orphans that need to be reconsidered */ - std::map> m_peer_work_set GUARDED_BY(g_cs_orphans); - /** Add any orphans that list a particular tx as a parent into a peer's work set */ void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); @@ -68,6 +72,9 @@ protected: * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ std::map m_orphans GUARDED_BY(g_cs_orphans); + /** Which peer provided a parent tx of orphans that need to be reconsidered */ + std::map> m_peer_work_set GUARDED_BY(g_cs_orphans); + using OrphanMap = decltype(m_orphans); struct IteratorComparator -- cgit v1.2.3 From 733d85f79cde353d8c9b54370f296b1031fa33d9 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 7 Oct 2022 14:25:47 +1000 Subject: Move all g_cs_orphans locking to txorphanage --- src/net_processing.cpp | 13 ++++++------- src/test/fuzz/txorphan.cpp | 9 +-------- src/test/orphanage_tests.cpp | 8 ++++---- src/txorphanage.cpp | 26 ++++++++++++++++---------- src/txorphanage.h | 29 ++++++++++++++++------------- 5 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f4b2d2e38f..dc839cd883 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -595,8 +595,8 @@ private: * reconsidered. * @return True if there are still orphans in this peer's work set. */ - bool ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); + bool ProcessOrphanTx(Peer& peer) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); /** Process a single headers message from a peer. * * @param[in] pfrom CNode of the peer @@ -1501,7 +1501,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) for (const QueuedBlock& entry : state->vBlocksInFlight) { mapBlocksInFlight.erase(entry.pindex->GetBlockHash()); } - WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid)); + m_orphanage.EraseForPeer(nodeid); m_txrequest.DisconnectedPeer(nodeid); m_num_preferred_download_peers -= state->fPreferredDownload; m_peers_downloading_from -= (state->nBlocksInFlight != 0); @@ -2885,7 +2885,6 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) { AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); - AssertLockHeld(g_cs_orphans); CTransactionRef porphanTx = nullptr; NodeId from_peer = -1; @@ -3903,7 +3902,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, AddKnownTx(*peer, txid); } - LOCK2(cs_main, g_cs_orphans); + LOCK(cs_main); m_txrequest.ReceivedResponse(pfrom.GetId(), txid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); @@ -4139,7 +4138,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool fBlockReconstructed = false; { - LOCK2(cs_main, g_cs_orphans); + LOCK(cs_main); // If AcceptBlockHeader returned true, it set pindex assert(pindex); UpdateBlockAvailability(pfrom.GetId(), pindex->GetBlockHash()); @@ -4769,7 +4768,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt bool has_more_orphans; { - LOCK2(cs_main, g_cs_orphans); + LOCK(cs_main); has_more_orphans = ProcessOrphanTx(*peer); } diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index b200f94144..02743051e8 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -85,12 +85,10 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) CallOneOf( fuzzed_data_provider, [&] { - LOCK(g_cs_orphans); orphanage.AddChildrenToWorkSet(*tx, peer_id); }, [&] { { - LOCK(g_cs_orphans); NodeId originator; bool more = true; CTransactionRef ref = orphanage.GetTxToReconsider(peer_id, originator, more); @@ -107,14 +105,12 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) // AddTx should return false if tx is too big or already have it // tx weight is unknown, we only check when tx is already in orphanage { - LOCK(g_cs_orphans); bool add_tx = orphanage.AddTx(tx, peer_id); // have_tx == true -> add_tx == false Assert(!have_tx || !add_tx); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); { - LOCK(g_cs_orphans); bool add_tx = orphanage.AddTx(tx, peer_id); // if have_tx is still false, it must be too big Assert(!have_tx == (GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT)); @@ -125,25 +121,22 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // EraseTx should return 0 if m_orphans doesn't have the tx { - LOCK(g_cs_orphans); Assert(have_tx == orphanage.EraseTx(tx->GetHash())); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // have_tx should be false and EraseTx should fail { - LOCK(g_cs_orphans); Assert(!have_tx && !orphanage.EraseTx(tx->GetHash())); } }, [&] { - LOCK(g_cs_orphans); orphanage.EraseForPeer(peer_id); }, [&] { // test mocktime and expiry SetMockTime(ConsumeTime(fuzzed_data_provider)); auto limit = fuzzed_data_provider.ConsumeIntegral(); - WITH_LOCK(g_cs_orphans, orphanage.LimitOrphans(limit)); + orphanage.LimitOrphans(limit); Assert(orphanage.Size() <= limit); }); } diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index 842daa8bd4..84609f453c 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -20,13 +20,15 @@ BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup) class TxOrphanageTest : public TxOrphanage { public: - inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) + inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) { + LOCK(g_cs_orphans); return m_orphans.size(); } - CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) + CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) { + LOCK(g_cs_orphans); std::map::iterator it; it = m_orphans.lower_bound(InsecureRand256()); if (it == m_orphans.end()) @@ -59,8 +61,6 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) FillableSigningProvider keystore; BOOST_CHECK(keystore.AddKey(key)); - LOCK(g_cs_orphans); - // 50 orphan transactions: for (int i = 0; i < 50; i++) { diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 4cccb1affb..bdebfc161f 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -15,11 +15,11 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; -RecursiveMutex g_cs_orphans; +RecursiveMutex TxOrphanage::g_cs_orphans; bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); const uint256& hash = tx->GetHash(); if (m_orphans.count(hash)) @@ -54,6 +54,12 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) } int TxOrphanage::EraseTx(const uint256& txid) +{ + LOCK(g_cs_orphans); + return _EraseTx(txid); +} + +int TxOrphanage::_EraseTx(const uint256& txid) { AssertLockHeld(g_cs_orphans); std::map::iterator it = m_orphans.find(txid); @@ -87,7 +93,7 @@ int TxOrphanage::EraseTx(const uint256& txid) void TxOrphanage::EraseForPeer(NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); m_peer_work_set.erase(peer); @@ -98,7 +104,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) std::map::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { - nErased += EraseTx(maybeErase->second.tx->GetHash()); + nErased += _EraseTx(maybeErase->second.tx->GetHash()); } } if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer); @@ -106,7 +112,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) void TxOrphanage::LimitOrphans(unsigned int max_orphans) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); unsigned int nEvicted = 0; static int64_t nNextSweep; @@ -120,7 +126,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) { std::map::iterator maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { - nErased += EraseTx(maybeErase->second.tx->GetHash()); + nErased += _EraseTx(maybeErase->second.tx->GetHash()); } else { nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime); } @@ -134,7 +140,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) { // Evict a random orphan: size_t randompos = rng.randrange(m_orphan_list.size()); - EraseTx(m_orphan_list[randompos]->first); + _EraseTx(m_orphan_list[randompos]->first); ++nEvicted; } if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); @@ -142,7 +148,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); // Get this peer's work set, emplacing an empty set it didn't exist std::set& orphan_work_set = m_peer_work_set.try_emplace(peer).first->second; @@ -169,7 +175,7 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); auto work_set_it = m_peer_work_set.find(peer); if (work_set_it != m_peer_work_set.end()) { @@ -215,7 +221,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) if (vOrphanErase.size()) { int nErased = 0; for (const uint256& orphanHash : vOrphanErase) { - nErased += EraseTx(orphanHash); + nErased += _EraseTx(orphanHash); } LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); } diff --git a/src/txorphanage.h b/src/txorphanage.h index 5af1f2f0e0..91cfb7786d 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -13,9 +13,6 @@ #include #include -/** Guards orphan transactions */ -extern RecursiveMutex g_cs_orphans; - /** A class to track orphan transactions (failed on TX_MISSING_INPUTS) * Since we cannot distinguish orphans from bad transactions with * non-existent inputs, we heavily limit the number of orphans @@ -24,10 +21,10 @@ extern RecursiveMutex g_cs_orphans; class TxOrphanage { public: /** Add a new orphan transaction */ - bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Check if we already have an orphan transaction (by txid or wtxid) */ - bool HaveTx(const GenTxid& gtxid) const LOCKS_EXCLUDED(::g_cs_orphans); + bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Extract a transaction from a peer's work set * Returns nullptr and sets more to false if there are no transactions @@ -36,31 +33,34 @@ public: * the originating peer, and whether there are more orphans for this peer * to work on after this tx. */ - CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Erase an orphan by txid */ - int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ - void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Erase all orphans included in or invalidated by a new block */ - void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans); + void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Limit the orphanage to the given maximum */ - void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Add any orphans that list a particular tx as a parent into a peer's work set */ - void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); /** Return how many entries exist in the orphange */ - size_t Size() LOCKS_EXCLUDED(::g_cs_orphans) + size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) { - LOCK(::g_cs_orphans); + LOCK(g_cs_orphans); return m_orphans.size(); } protected: + /** Guards orphan transactions */ + static RecursiveMutex g_cs_orphans; + struct OrphanTx { CTransactionRef tx; NodeId fromPeer; @@ -96,6 +96,9 @@ protected: /** Index from wtxid into the m_orphans to lookup orphan * transactions using their witness ids. */ std::map m_wtxid_to_orphan_it GUARDED_BY(g_cs_orphans); + + /** Erase an orphan by txid */ + int _EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); }; #endif // BITCOIN_TXORPHANAGE_H -- cgit v1.2.3 From 7082ce3e88d77456d60a5a66bd38807fbd66f311 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 7 Oct 2022 14:32:19 +1000 Subject: scripted-diff: rename and de-globalise g_cs_orphans -BEGIN VERIFY SCRIPT- sed -i -e 's/static RecursiveMutex/mutable Mutex/' src/txorphanage.h sed -i -e '/RecursiveMutex/d' src/txorphanage.cpp sed -i -e 's/g_cs_orphans/m_mutex/g' $(git grep -l g_cs_orphans src/) -END VERIFY SCRIPT- --- src/test/orphanage_tests.cpp | 8 ++++---- src/txorphanage.cpp | 19 +++++++++---------- src/txorphanage.h | 34 +++++++++++++++++----------------- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index 84609f453c..a55b0bbcd0 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -20,15 +20,15 @@ BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup) class TxOrphanageTest : public TxOrphanage { public: - inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) + inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { - LOCK(g_cs_orphans); + LOCK(m_mutex); return m_orphans.size(); } - CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) + CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { - LOCK(g_cs_orphans); + LOCK(m_mutex); std::map::iterator it; it = m_orphans.lower_bound(InsecureRand256()); if (it == m_orphans.end()) diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index bdebfc161f..b0b71e135c 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -15,11 +15,10 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; -RecursiveMutex TxOrphanage::g_cs_orphans; bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { - LOCK(g_cs_orphans); + LOCK(m_mutex); const uint256& hash = tx->GetHash(); if (m_orphans.count(hash)) @@ -55,13 +54,13 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) int TxOrphanage::EraseTx(const uint256& txid) { - LOCK(g_cs_orphans); + LOCK(m_mutex); return _EraseTx(txid); } int TxOrphanage::_EraseTx(const uint256& txid) { - AssertLockHeld(g_cs_orphans); + AssertLockHeld(m_mutex); std::map::iterator it = m_orphans.find(txid); if (it == m_orphans.end()) return 0; @@ -93,7 +92,7 @@ int TxOrphanage::_EraseTx(const uint256& txid) void TxOrphanage::EraseForPeer(NodeId peer) { - LOCK(g_cs_orphans); + LOCK(m_mutex); m_peer_work_set.erase(peer); @@ -112,7 +111,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) void TxOrphanage::LimitOrphans(unsigned int max_orphans) { - LOCK(g_cs_orphans); + LOCK(m_mutex); unsigned int nEvicted = 0; static int64_t nNextSweep; @@ -148,7 +147,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) { - LOCK(g_cs_orphans); + LOCK(m_mutex); // Get this peer's work set, emplacing an empty set it didn't exist std::set& orphan_work_set = m_peer_work_set.try_emplace(peer).first->second; @@ -165,7 +164,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) bool TxOrphanage::HaveTx(const GenTxid& gtxid) const { - LOCK(g_cs_orphans); + LOCK(m_mutex); if (gtxid.IsWtxid()) { return m_wtxid_to_orphan_it.count(gtxid.GetHash()); } else { @@ -175,7 +174,7 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) { - LOCK(g_cs_orphans); + LOCK(m_mutex); auto work_set_it = m_peer_work_set.find(peer); if (work_set_it != m_peer_work_set.end()) { @@ -198,7 +197,7 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, NodeId& originator, void TxOrphanage::EraseForBlock(const CBlock& block) { - LOCK(g_cs_orphans); + LOCK(m_mutex); std::vector vOrphanErase; diff --git a/src/txorphanage.h b/src/txorphanage.h index 91cfb7786d..551502d325 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -21,10 +21,10 @@ class TxOrphanage { public: /** Add a new orphan transaction */ - bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Check if we already have an orphan transaction (by txid or wtxid) */ - bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Extract a transaction from a peer's work set * Returns nullptr and sets more to false if there are no transactions @@ -33,33 +33,33 @@ public: * the originating peer, and whether there are more orphans for this peer * to work on after this tx. */ - CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + CTransactionRef GetTxToReconsider(NodeId peer, NodeId& originator, bool& more) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase an orphan by txid */ - int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ - void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans included in or invalidated by a new block */ - void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Limit the orphanage to the given maximum */ - void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Add any orphans that list a particular tx as a parent into a peer's work set */ - void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans); + void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Return how many entries exist in the orphange */ - size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans) + size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { - LOCK(g_cs_orphans); + LOCK(m_mutex); return m_orphans.size(); } protected: /** Guards orphan transactions */ - static RecursiveMutex g_cs_orphans; + mutable Mutex m_mutex; struct OrphanTx { CTransactionRef tx; @@ -70,10 +70,10 @@ protected: /** Map from txid to orphan transaction record. Limited by * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ - std::map m_orphans GUARDED_BY(g_cs_orphans); + std::map m_orphans GUARDED_BY(m_mutex); /** Which peer provided a parent tx of orphans that need to be reconsidered */ - std::map> m_peer_work_set GUARDED_BY(g_cs_orphans); + std::map> m_peer_work_set GUARDED_BY(m_mutex); using OrphanMap = decltype(m_orphans); @@ -88,17 +88,17 @@ protected: /** Index from the parents' COutPoint into the m_orphans. Used * to remove orphan transactions from the m_orphans */ - std::map> m_outpoint_to_orphan_it GUARDED_BY(g_cs_orphans); + std::map> m_outpoint_to_orphan_it GUARDED_BY(m_mutex); /** Orphan transactions in vector for quick random eviction */ - std::vector m_orphan_list GUARDED_BY(g_cs_orphans); + std::vector m_orphan_list GUARDED_BY(m_mutex); /** Index from wtxid into the m_orphans to lookup orphan * transactions using their witness ids. */ - std::map m_wtxid_to_orphan_it GUARDED_BY(g_cs_orphans); + std::map m_wtxid_to_orphan_it GUARDED_BY(m_mutex); /** Erase an orphan by txid */ - int _EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + int _EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); }; #endif // BITCOIN_TXORPHANAGE_H -- cgit v1.2.3