diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index 450bf6a4fc733..1582c2424ad9c 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -21,15 +21,13 @@ BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup) class TxOrphanageTest : public TxOrphanage { public: - inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + inline size_t CountOrphans() const { - LOCK(m_mutex); return m_orphans.size(); } - CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + CTransactionRef RandomOrphan() { - LOCK(m_mutex); std::map<Wtxid, OrphanTx>::iterator it; it = m_orphans.lower_bound(Wtxid::FromUint256(InsecureRand256())); if (it == m_orphans.end()) diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 13ef96f3be29f..1263985068de2 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -19,8 +19,6 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { - LOCK(m_mutex); - const Txid& hash = tx->GetHash(); const Wtxid& wtxid = tx->GetWitnessHash(); if (m_orphans.count(wtxid)) @@ -54,13 +52,11 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) int TxOrphanage::EraseTx(const Wtxid& wtxid) { - LOCK(m_mutex); return EraseTxNoLock(wtxid); } int TxOrphanage::EraseTxNoLock(const Wtxid& wtxid) { - AssertLockHeld(m_mutex); std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid); if (it == m_orphans.end()) return 0; @@ -96,8 +92,6 @@ int TxOrphanage::EraseTxNoLock(const Wtxid& wtxid) void TxOrphanage::EraseForPeer(NodeId peer) { - LOCK(m_mutex); - m_peer_work_set.erase(peer); int nErased = 0; @@ -115,8 +109,6 @@ void TxOrphanage::EraseForPeer(NodeId peer) void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng) { - LOCK(m_mutex); - unsigned int nEvicted = 0; static int64_t nNextSweep; int64_t nNow = GetTime(); @@ -150,9 +142,6 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng) void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx) { - LOCK(m_mutex); - - 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()) { @@ -171,14 +160,11 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx) bool TxOrphanage::HaveTx(const Wtxid& wtxid) const { - LOCK(m_mutex); return m_orphans.count(wtxid); } CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer) { - LOCK(m_mutex); - 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; @@ -197,8 +183,6 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer) bool TxOrphanage::HaveTxToReconsider(NodeId peer) { - LOCK(m_mutex); - 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; @@ -209,8 +193,6 @@ bool TxOrphanage::HaveTxToReconsider(NodeId peer) void TxOrphanage::EraseForBlock(const CBlock& block) { - LOCK(m_mutex); - std::vector<Wtxid> vOrphanErase; for (const CTransactionRef& ptx : block.vtx) { @@ -239,8 +221,6 @@ void TxOrphanage::EraseForBlock(const CBlock& block) std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const { - LOCK(m_mutex); - // First construct a vector of iterators to ensure we do not return duplicates of the same tx // and so we can sort by nTimeExpire. std::vector<OrphanMap::iterator> iters; @@ -281,8 +261,6 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const { - LOCK(m_mutex); - // First construct vector of iterators to ensure we do not return duplicates of the same tx. std::vector<OrphanMap::iterator> iters; diff --git a/src/txorphanage.h b/src/txorphanage.h index 5f9888c969e07..7b2de7d420d19 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -21,55 +21,51 @@ class TxOrphanage { public: /** Add a new orphan transaction */ - bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + bool AddTx(const CTransactionRef& tx, NodeId peer); /** Check if we already have an orphan transaction (by wtxid only) */ - bool HaveTx(const Wtxid& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + bool HaveTx(const Wtxid& wtxid) const; /** Extract a transaction from a peer's work set * Returns nullptr if there are no transactions to work on. * Otherwise returns the transaction reference, and removes * it from the work set. */ - CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + CTransactionRef GetTxToReconsider(NodeId peer); /** Erase an orphan by wtxid */ - int EraseTx(const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + int EraseTx(const Wtxid& wtxid); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ - void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + void EraseForPeer(NodeId peer); /** Erase all orphans included in or invalidated by a new block */ - void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + void EraseForBlock(const CBlock& block); /** Limit the orphanage to the given maximum */ - void LimitOrphans(unsigned int max_orphans, FastRandomContext& rng) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + void LimitOrphans(unsigned int max_orphans, FastRandomContext& rng); /** Add any orphans that list a particular tx as a parent into the from peer's work set */ - void AddChildrenToWorkSet(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);; + void AddChildrenToWorkSet(const CTransaction& tx); /** Does this peer have any work to do? */ - bool HaveTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);; + bool HaveTxToReconsider(NodeId peer); /** Get all children that spend from this tx and were received from nodeid. Sorted from most * recent to least recent. */ - std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const; /** Get all children that spend from this tx but were not received from nodeid. Also return * which peer provided each tx. */ - std::vector<std::pair<CTransactionRef, NodeId>> GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + std::vector<std::pair<CTransactionRef, NodeId>> GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const; /** Return how many entries exist in the orphange */ - size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + size_t Size() { - LOCK(m_mutex); return m_orphans.size(); } protected: - /** Guards orphan transactions */ - mutable Mutex m_mutex; - struct OrphanTx { CTransactionRef tx; NodeId fromPeer; @@ -79,10 +75,10 @@ class TxOrphanage { /** Map from wtxid to orphan transaction record. Limited by * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ - std::map<Wtxid, OrphanTx> m_orphans GUARDED_BY(m_mutex); + std::map<Wtxid, OrphanTx> m_orphans; /** Which peer provided the orphans that need to be reconsidered */ - std::map<NodeId, std::set<Wtxid>> m_peer_work_set GUARDED_BY(m_mutex); + std::map<NodeId, std::set<Wtxid>> m_peer_work_set; using OrphanMap = decltype(m_orphans); @@ -97,13 +93,13 @@ class TxOrphanage { /** Index from the parents' COutPoint into the m_orphans. Used * to remove orphan transactions from the m_orphans */ - std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> m_outpoint_to_orphan_it GUARDED_BY(m_mutex); + std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> m_outpoint_to_orphan_it; /** Orphan transactions in vector for quick random eviction */ - std::vector<OrphanMap::iterator> m_orphan_list GUARDED_BY(m_mutex); + std::vector<OrphanMap::iterator> m_orphan_list; /** Erase an orphan by wtxid */ - int EraseTxNoLock(const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + int EraseTxNoLock(const Wtxid& wtxid); }; #endif // BITCOIN_TXORPHANAGE_H