From c85ee76a3647a42d3a59c28444c5ca52e7db7c5e Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 31 Mar 2023 12:53:46 +0200 Subject: [PATCH 1/2] [net processin] Don't take cs_main in FindTxForGetData Taking cs_main is no longer necessary since we moved `m_recently_announced_invs` to `Peer` and `mapRelay` is actually only accessed from the message processing thread. --- src/net_processing.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2348f6c0597..29d123e16bf 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -909,7 +909,7 @@ private: /** 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); + EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex, NetEventsInterface::g_msgproc_mutex) @@ -920,9 +920,9 @@ private: /** Relay map (txid or wtxid -> CTransactionRef) */ typedef std::map 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> g_relay_expiration GUARDED_BY(cs_main); + std::deque> g_relay_expiration GUARDED_BY(NetEventsInterface::g_msgproc_mutex); /** * When a peer sends us a valid block, instruct it to announce blocks to us @@ -2270,16 +2270,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 (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; } return {}; From 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 31 Mar 2023 13:15:22 +0200 Subject: [PATCH 2/2] [net processing] Pass TxRelay to FindTxForGetData instead of Peer --- src/net_processing.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 29d123e16bf..276295b7d03 100644 --- a/src/net_processing.cpp +++ b/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 m_addrs_to_send GUARDED_BY(NetEventsInterface::g_msgproc_mutex); @@ -908,7 +904,7 @@ private: std::atomic 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) + 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& interruptMsgProc) @@ -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) { @@ -2271,7 +2267,7 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer& peer, const GenTxi } // Otherwise, the transaction must have been announced recently. - if (Assume(peer.GetTxRelay())->m_recently_announced_invs.contains(gtxid.GetHash())) { + 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. @@ -2313,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);