Merge bitcoin#21845: net processing: Don't require locking cs_main be…

…fore calling RelayTransactions()

39e1971 [net processing] Add internal _RelayTransactions() (John Newbery)

Pull request description:

  As part of the general effort to reduce cs_main usage in net_processing, this removes the need to be holding `cs_main` when calling `RelayTransactions()` from outside net_processing. Internally, we lock `cs_main` and call an internal `_RelayTransactions()` function that _does_ require `cs_main`.

ACKs for top commit:
  MarcoFalke:
    re-unsigned-code-review ACK 39e1971
  promag:
    Code review ACK 39e1971, just included sync.h since last review.
  ajtowns:
    ACK 39e1971

Tree-SHA512: dc08441233adfb8eaac501cf497cb4bad029eb723bd3fa8a3d8b7e49cc984c98859b95780ad15f5701d62ac745a8223beb0df405e3d49d95a8c86c8be17c9543

src/net_processing.cpp

@@ -26,6 +26,7 @@
 #include <reverse_iterator.h>
 #include <scheduler.h>
 #include <streams.h>
+#include <sync.h>
 #include <tinyformat.h>
 #include <index/txindex.h>
 #include <txmempool.h>
@@ -407,6 +408,9 @@ class PeerManagerImpl final : public PeerManager
     /** Helper to process result of external handlers of message */
     void ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom);
 
+    void _RelayTransaction(const uint256& txid)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
     /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
     void ConsiderEviction(CNode& pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
@@ -1428,7 +1432,10 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
         CTransactionRef tx = m_mempool.get(txid);
 
         if (tx != nullptr) {
-            RelayTransaction(txid);
+            LOCK(cs_main);
+            {
+                _RelayTransaction(txid);
+            }
         } else {
             m_mempool.RemoveUnbroadcastTx(txid, true);
         }
@@ -2258,6 +2265,11 @@ void PeerManagerImpl::RelayInvFiltered(CInv &inv, const uint256& relatedTxHash,
 }
 
 void PeerManagerImpl::RelayTransaction(const uint256& txid)
+{
+    WITH_LOCK(cs_main, _RelayTransaction(txid););
+}
+
+void PeerManagerImpl::_RelayTransaction(const uint256& txid)
 {
     LOCK(m_peer_mutex);
     for(auto& it : m_peer_map) {
@@ -2937,7 +2949,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
 
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             LogPrint(BCLog::MEMPOOL, "   accepted orphan tx %s\n", orphanHash.ToString());
-            RelayTransaction(porphanTx->GetHash());
+            _RelayTransaction(porphanTx->GetHash());
             for (unsigned int i = 0; i < porphanTx->vout.size(); i++) {
                 auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(orphanHash, i));
                 if (it_by_prev != mapOrphanTransactionsByPrev.end()) {
@@ -4068,7 +4080,7 @@ void PeerManagerImpl::ProcessMessage(
                     LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId());
                 } else {
                     LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId());
-                    RelayTransaction(tx.GetHash());
+                    _RelayTransaction(tx.GetHash());
                 }
             }
             return;
@@ -4086,7 +4098,7 @@ void PeerManagerImpl::ProcessMessage(
             }
 
             m_mempool.check(m_chainman.ActiveChainstate());
-            RelayTransaction(tx.GetHash());
+            _RelayTransaction(tx.GetHash());
 
             for (unsigned int i = 0; i < tx.vout.size(); i++) {
                 auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(txid, i));

src/net_processing.h

@@ -7,7 +7,6 @@
 #define BITCOIN_NET_PROCESSING_H
 
 #include <net.h>
-#include <sync.h>
 #include <validationinterface.h>
 #include <version.h>
 
@@ -28,7 +27,6 @@ class CTransaction;
 struct CJContext;
 struct LLMQContext;
 
-extern RecursiveMutex cs_main;
 extern RecursiveMutex g_cs_orphans;
 
 /** Default for -maxorphantxsize, maximum size in megabytes the orphan map can grow before entries are removed */
@@ -94,8 +92,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
                                   const int minProtoVersion = MIN_PEER_PROTO_VERSION) = 0;
 
     /** Relay transaction to all peers. */
-    virtual void RelayTransaction(const uint256& txid)
-        EXCLUSIVE_LOCKS_REQUIRED(cs_main) = 0;
+    virtual void RelayTransaction(const uint256& txid) = 0;
 
     /** Set the best height */
     virtual void SetBestHeight(int height) = 0;

src/node/transaction.cpp

@@ -103,7 +103,6 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         // best-effort of initial broadcast
         node.mempool->AddUnbroadcastTx(hashTx);
 
-        LOCK(cs_main);
         node.peerman->RelayTransaction(hashTx);
     }