Merge bitcoin/bitcoin#27379: net processing: #26140 follow-ups

3fa4c54 [net processing] Pass TxRelay to FindTxForGetData instead of Peer (dergoegge)
c85ee76 [net processin] Don't take cs_main in FindTxForGetData (dergoegge)

Pull request description:

  Addresses left over feedback from #26140.

  * bitcoin/bitcoin#26140 (comment)
  * bitcoin/bitcoin#26140 (comment)

  `mapRelay` is only accessed from the message processing thread and does not need to be kept in sync with anything validation specific, it is therfore perfectly fine to have it guarded by `g_msgproc_mutex`.

ACKs for top commit:
  jnewbery:
    utACK 3fa4c54
  hebasto:
    ACK 3fa4c54, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 3ef84bfe4abfa8d991a7e65d9184221294d80e0df0bbb47f0270ab6ca1593266c98abf83c610f9f86b4d16c7a4b62bcf83f8856c68d3c2e10894bff6ed3e88cd

Author: fanquake

src/net_processing.cpp

@@ -317,10 +317,6 @@ struct Peer {
     {
         return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get());
     };
-    const TxRelay* GetTxRelay() const EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex)
-    {
-        return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get());
-    };
 
     /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */
     std::vector<CAddress> m_addrs_to_send GUARDED_BY(NetEventsInterface::g_msgproc_mutex);
@@ -908,8 +904,8 @@ class PeerManagerImpl final : public PeerManager
     std::atomic<std::chrono::seconds> m_last_tip_update{0s};
 
     /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */
-    CTransactionRef FindTxForGetData(const Peer& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
-        LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
+    CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
+        EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 
     void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc)
         EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex, NetEventsInterface::g_msgproc_mutex)
@@ -920,9 +916,9 @@ class PeerManagerImpl final : public PeerManager
 
     /** Relay map (txid or wtxid -> CTransactionRef) */
     typedef std::map<uint256, CTransactionRef> MapRelay;
-    MapRelay mapRelay GUARDED_BY(cs_main);
+    MapRelay mapRelay GUARDED_BY(NetEventsInterface::g_msgproc_mutex);
     /** Expiration-time ordered list of (expire time, relay map entry) pairs. */
-    std::deque<std::pair<std::chrono::microseconds, MapRelay::iterator>> g_relay_expiration GUARDED_BY(cs_main);
+    std::deque<std::pair<std::chrono::microseconds, MapRelay::iterator>> g_relay_expiration GUARDED_BY(NetEventsInterface::g_msgproc_mutex);
 
     /**
      * When a peer sends us a valid block, instruct it to announce blocks to us
@@ -2258,7 +2254,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
     }
 }
 
-CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
+CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
 {
     auto txinfo = m_mempool.info(gtxid);
     if (txinfo.tx) {
@@ -2270,16 +2266,13 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer& peer, const GenTxi
         }
     }
 
-    {
-        LOCK(cs_main);
-        // Otherwise, the transaction must have been announced recently.
-        if (Assume(peer.GetTxRelay())->m_recently_announced_invs.contains(gtxid.GetHash())) {
-            // If it was, it can be relayed from either the mempool...
-            if (txinfo.tx) return std::move(txinfo.tx);
-            // ... or the relay pool.
-            auto mi = mapRelay.find(gtxid.GetHash());
-            if (mi != mapRelay.end()) return mi->second;
-        }
+    // Otherwise, the transaction must have been announced recently.
+    if (tx_relay.m_recently_announced_invs.contains(gtxid.GetHash())) {
+        // If it was, it can be relayed from either the mempool...
+        if (txinfo.tx) return std::move(txinfo.tx);
+        // ... or the relay pool.
+        auto mi = mapRelay.find(gtxid.GetHash());
+        if (mi != mapRelay.end()) return mi->second;
     }
 
     return {};
@@ -2316,7 +2309,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
             continue;
         }
 
-        CTransactionRef tx = FindTxForGetData(peer, ToGenTxid(inv), mempool_req, now);
+        CTransactionRef tx = FindTxForGetData(*tx_relay, ToGenTxid(inv), mempool_req, now);
         if (tx) {
             // WTX and WITNESS_TX imply we serialize with witness
             int nSendFlags = (inv.IsMsgTx() ? SERIALIZE_TRANSACTION_NO_WITNESS : 0);