diff options
| -rw-r--r-- | src/net_processing.cpp | 26 | ||||
| -rw-r--r-- | src/test/fuzz/txorphan.cpp | 7 | ||||
| -rw-r--r-- | src/txorphanage.cpp | 4 | ||||
| -rw-r--r-- | src/txorphanage.h | 10 |
4 files changed, 21 insertions, 26 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 25d9e75425..13c412842d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -584,14 +584,17 @@ private: /** * 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. - * @return True if there are still orphans in this peer's work set. + * @peer[in] peer The peer whose orphan transactions we will reconsider. Generally only + * one orphan will be reconsidered on each call of this function. If an + * accepted orphan has orphaned children, those will need to be + * reconsidered, creating more work, possibly for other peers. + * @return True if meaningful work was done (an orphan was accepted/rejected). + * If no meaningful work was done, then the work set for this peer + * will be empty. */ bool ProcessOrphanTx(Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex); + /** Process a single headers message from a peer. * * @param[in] pfrom CNode of the peer @@ -2892,9 +2895,8 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) LOCK(cs_main); CTransactionRef porphanTx = nullptr; - bool more = false; - while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id, more)) { + while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id)) { const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx); const TxValidationState& state = result.m_state; const uint256& orphanHash = porphanTx->GetHash(); @@ -2907,7 +2909,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) { AddToCompactExtraTransactions(removedTx); } - break; + return true; } else if (state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) { if (state.IsInvalid()) { LogPrint(BCLog::MEMPOOL, " invalid orphan tx %s from peer=%d. %s\n", @@ -2950,11 +2952,11 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) } } m_orphanage.EraseTx(orphanHash); - break; + return true; } } - return more; + return false; } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, @@ -4849,12 +4851,12 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt } } - const bool has_more_orphans = ProcessOrphanTx(*peer); + const bool processed_orphan = ProcessOrphanTx(*peer); if (pfrom->fDisconnect) return false; - if (has_more_orphans) return true; + if (processed_orphan) return true; // this maintains the order of responses // and prevents m_getdata_requests to grow unbounded diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 4673b884dc..f9d1bb85cb 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -89,11 +89,8 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) }, [&] { { - bool more = true; - CTransactionRef ref = orphanage.GetTxToReconsider(peer_id, more); - if (!ref) { - Assert(!more); - } else { + CTransactionRef ref = orphanage.GetTxToReconsider(peer_id); + if (ref) { 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 31c6ff7106..f82cd886f9 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -174,7 +174,7 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const } } -CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, bool& more) +CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer) { LOCK(m_mutex); @@ -187,12 +187,10 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer, bool& more) const auto orphan_it = m_orphans.find(txid); if (orphan_it != m_orphans.end()) { - more = !work_set.empty(); return orphan_it->second.tx; } } } - more = false; return nullptr; } diff --git a/src/txorphanage.h b/src/txorphanage.h index 55c02976dd..f886e07b12 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -27,13 +27,11 @@ public: 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 - * to work on. Otherwise returns the transaction reference, removes - * the transaction from the work set, and sets "more" to indicate - * if there are more orphans for this peer - * to work on after this tx. + * 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, bool& more) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase an orphan by txid */ int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); |