net_processing: only process orphans before messages

ajtowns · ajtowns · commit c5837757068b · 2023-01-25T18:13:42.000+10:00
Previously, when we processed a new tx we would attempt to ATMP any
orphans that considered the new tx a parent immediately, but would only
accept at most one such tx, leaving any others to be considered on a
future run of ProcessMessages(). With this patch, we don't attempt any
orphan processing immediately after receiving a tx, instead deferring
all of them until the next call to ProcessMessages().
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -591,7 +591,7 @@ class PeerManagerImpl final : public PeerManager
      * @return             True if there are still orphans in this peer's work set.
      */
     bool ProcessOrphanTx(Peer& peer)
-        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex);
     /** Process a single headers message from a peer.
      *
      * @param[in]   pfrom     CNode of the peer
@@ -2889,7 +2889,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer,
 bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
 {
     AssertLockHeld(g_msgproc_mutex);
-    AssertLockHeld(cs_main);
+    LOCK(cs_main);
 
     CTransactionRef porphanTx = nullptr;
     bool more = false;
@@ -4041,9 +4041,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
             }
-
-            // Recursively process any orphan transactions that depended on this one
-            ProcessOrphanTx(*peer);
         }
         else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS)
         {
@@ -4852,11 +4849,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
         }
     }
 
-    bool has_more_orphans;
-    {
-        LOCK(cs_main);
-        has_more_orphans = ProcessOrphanTx(*peer);
-    }
+    const bool has_more_orphans = ProcessOrphanTx(*peer);
 
     if (pfrom->fDisconnect)
         return false;

Original file line number	Diff line number	Diff line change
`@@ -591,7 +591,7 @@ class PeerManagerImpl final : public PeerManager`
`591`	`591`	`* @return True if there are still orphans in this peer's work set.`
`592`	`592`	`*/`
`593`	`593`	`bool ProcessOrphanTx(Peer& peer)`
`594`		`- EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);`
	`594`	`+ EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex);`
`595`	`595`	`/** Process a single headers message from a peer.`
`596`	`596`	`*`
`597`	`597`	`* @param[in] pfrom CNode of the peer`
`@@ -2889,7 +2889,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer,`
`2889`	`2889`	`bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)`
`2890`	`2890`	`{`
`2891`	`2891`	`AssertLockHeld(g_msgproc_mutex);`
`2892`		`- AssertLockHeld(cs_main);`
	`2892`	`+ LOCK(cs_main);`
`2893`	`2893`
`2894`	`2894`	`CTransactionRef porphanTx = nullptr;`
`2895`	`2895`	`bool more = false;`
`@@ -4041,9 +4041,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,`
`4041`	`4041`	`for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {`
`4042`	`4042`	`AddToCompactExtraTransactions(removedTx);`
`4043`	`4043`	`}`
`4044`		`-`
`4045`		`- // Recursively process any orphan transactions that depended on this one`
`4046`		`- ProcessOrphanTx(*peer);`
`4047`	`4044`	`}`
`4048`	`4045`	`else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS)`
`4049`	`4046`	`{`
`@@ -4852,11 +4849,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt`
`4852`	`4849`	`}`
`4853`	`4850`	`}`
`4854`	`4851`
`4855`		`- bool has_more_orphans;`
`4856`		`- {`
`4857`		`- LOCK(cs_main);`
`4858`		`- has_more_orphans = ProcessOrphanTx(*peer);`
`4859`		`- }`
	`4852`	`+ const bool has_more_orphans = ProcessOrphanTx(*peer);`
`4860`	`4853`
`4861`	`4854`	`if (pfrom->fDisconnect)`
`4862`	`4855`	`return false;`