txorphanage: index workset by originating peer

ajtowns · ajtowns · commit a4fe09973aa8 · 2022-11-29T09:03:57.000+10:00
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -2903,7 +2903,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_id);
+            m_orphanage.AddChildrenToWorkSet(*porphanTx);
             m_orphanage.EraseTx(orphanHash);
             for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
@@ -4030,7 +4030,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_id);
+            m_orphanage.AddChildrenToWorkSet(tx);
 
             pfrom.m_last_tx_time = GetTime<std::chrono::seconds>();
 
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
@@ -85,7 +85,7 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage)
             CallOneOf(
                 fuzzed_data_provider,
                 [&] {
-                    orphanage.AddChildrenToWorkSet(*tx, peer_id);
+                    orphanage.AddChildrenToWorkSet(*tx);
                 },
                 [&] {
                     {
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
@@ -145,17 +145,19 @@ 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, NodeId peer)
+void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
 {
     LOCK(m_mutex);
 
-    // 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()) {
             for (const auto& elem : it_by_prev->second) {
+                // Get this source peer's work set, emplacing an empty set if it didn't exist
+                // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
+                std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
+                // Add this tx to the work set
                 orphan_work_set.insert(elem->first);
             }
         }
diff --git a/src/txorphanage.h b/src/txorphanage.h
@@ -47,8 +47,8 @@ class TxOrphanage {
     /** Limit the orphanage to the given maximum */
     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(!m_mutex);
+    /** 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);;
 
     /** Return how many entries exist in the orphange */
     size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
@@ -72,7 +72,7 @@ class TxOrphanage {
      *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
     std::map<uint256, OrphanTx> m_orphans GUARDED_BY(m_mutex);
 
-    /** Which peer provided a parent tx of orphans that need to be reconsidered */
+    /** Which peer provided the orphans that need to be reconsidered */
     std::map<NodeId, std::set<uint256>> m_peer_work_set GUARDED_BY(m_mutex);
 
     using OrphanMap = decltype(m_orphans);

Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage)`
`85`	`85`	`CallOneOf(`
`86`	`86`	`fuzzed_data_provider,`
`87`	`87`	`[&] {`
`88`		`- orphanage.AddChildrenToWorkSet(*tx, peer_id);`
	`88`	`+ orphanage.AddChildrenToWorkSet(*tx);`
`89`	`89`	`},`
`90`	`90`	`[&] {`
`91`	`91`	`{`
Original file line number	Diff line number	Diff line change
`@@ -145,17 +145,19 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans)`
`145`	`145`	`if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted);`
`146`	`146`	`}`
`147`	`147`
`148`		`-void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, NodeId peer)`
	`148`	`+void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)`
`149`	`149`	`{`
`150`	`150`	`LOCK(m_mutex);`
`151`	`151`
`152`		`- // Get this peer's work set, emplacing an empty set it didn't exist`
`153`		`- std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(peer).first->second;`
`154`	`152`
`155`	`153`	`for (unsigned int i = 0; i < tx.vout.size(); i++) {`
`156`	`154`	`const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));`
`157`	`155`	`if (it_by_prev != m_outpoint_to_orphan_it.end()) {`
`158`	`156`	`for (const auto& elem : it_by_prev->second) {`
	`157`	`+ // Get this source peer's work set, emplacing an empty set if it didn't exist`
	`158`	`+ // (note: if this peer wasn't still connected, we would have removed the orphan tx already)`
	`159`	`+ std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;`
	`160`	`+ // Add this tx to the work set`
`159`	`161`	`orphan_work_set.insert(elem->first);`
`160`	`162`	`}`
`161`	`163`	`}`