diff options
author | Anthony Towns <aj@erisian.com.au> | 2021-02-25 00:28:16 +1000 |
---|---|---|
committer | Anthony Towns <aj@erisian.com.au> | 2022-10-11 14:05:09 +1000 |
commit | a936f41a5d5f7bb97425f82ec64dfae62e840a56 (patch) | |
tree | a7d8e962139d566375a83ee191af3c204a7e8f0f /src | |
parent | 3614819864a84ac32f6d53c6ace79b5e71bc174a (diff) |
txorphanage: make m_peer_work_set private
Diffstat (limited to 'src')
-rw-r--r-- | src/net_processing.cpp | 18 | ||||
-rw-r--r-- | src/test/fuzz/txorphan.cpp | 12 | ||||
-rw-r--r-- | src/txorphanage.cpp | 22 | ||||
-rw-r--r-- | 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<uint256>& 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<CTransactionRef, NodeId> 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 <primitives/transaction.h> #include <sync.h> +#include <map> +#include <set> + /** 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<CTransactionRef, NodeId> 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<NodeId, std::set<uint256>> 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<uint256, OrphanTx> m_orphans GUARDED_BY(g_cs_orphans); + /** Which peer provided a parent tx of orphans that need to be reconsidered */ + std::map<NodeId, std::set<uint256>> m_peer_work_set GUARDED_BY(g_cs_orphans); + using OrphanMap = decltype(m_orphans); struct IteratorComparator |