txorphange: move orphan workset to txorphanage

Author: ajtowns

src/net_processing.cpp

@@ -368,9 +368,6 @@ struct Peer {
     /** Total number of addresses that were processed (excludes rate-limited ones). */
     std::atomic<uint64_t> m_addr_processed{0};
 
-    /** Set of txids to reconsider once their parent transactions have been accepted **/
-    std::set<uint256> 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<uint256>& 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<std::chrono::seconds>();
 

src/test/fuzz/txorphan.cpp

@@ -36,7 +36,6 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage)
     SetMockTime(ConsumeTime(fuzzed_data_provider));
 
     TxOrphanage orphanage;
-    std::set<uint256> orphan_work_set;
     std::vector<COutPoint> 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()));

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<uint256, OrphanTx>::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<uint256>& 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<uint256>& 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()) {

src/txorphanage.h

@@ -43,9 +43,11 @@ class TxOrphanage {
     /** 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<uint256>& 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<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);
 
     /** Return how many entries exist in the orphange */
     size_t Size() LOCKS_EXCLUDED(::g_cs_orphans)