Merge bitcoin/bitcoin#24692: refactoring: [Net Processing] Follow-ups to #21160

a40978dcbd6608695bc7f5191c4d0a3e48cbca0b [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly (John Newbery)
0bca5f2b465616d7ac915ddcc1acd9a021e40abb [net processing] PushNodeVersion() takes a const Peer& (John Newbery)
21154ff9270e4bbae02d9012a4a73cce86a00334 net_processing: move CNode data access out of lock (John Newbery)

Pull request description:

  #21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments.

ACKs for top commit:
  MarcoFalke:
    review ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b
  dergoegge:
    ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b
  ajtowns:
    ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b

Tree-SHA512: 46624e275f918c5f32d0adab0766e9b3ef8ebdbc74a3c8886d8a2e2ff1079029dcc371b40ef0d787609e9c05219b7456f3e2dfe4fb0cb7bf23ef966769aef1a1

2 files changed, 22 insertions, 12 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 1df47fe3dd..fb585937ee 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -426,7 +426,7 @@ private:
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /** Send a version message to a peer */
-    void PushNodeVersion(CNode& pnode, Peer& peer);
+    void PushNodeVersion(CNode& pnode, const Peer& peer);
 
     /** Send a ping message every PING_INTERVAL or if requested via RPC. May
      *  mark the peer to be disconnected if a ping has timed out.
@@ -1158,7 +1158,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count
 
 } // namespace
 
-void PeerManagerImpl::PushNodeVersion(CNode& pnode, Peer& peer)
+void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer)
 {
     // Note that pnode->GetLocalServices() is a reflection of the local
     // services we were offering when the CNode object was created for this
@@ -2715,8 +2715,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED));
 
         if (peer->m_tx_relay != nullptr) {
-            LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
-            peer->m_tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message
+            {
+                LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
+                peer->m_tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message
+            }
             if (fRelay) pfrom.m_relays_txs = true;
         }
 
@@ -4030,10 +4032,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         }
         else if (peer->m_tx_relay != nullptr)
         {
-            LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
-            peer->m_tx_relay->m_bloom_filter.reset(new CBloomFilter(filter));
+            {
+                LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
+                peer->m_tx_relay->m_bloom_filter.reset(new CBloomFilter(filter));
+                peer->m_tx_relay->m_relay_txs = true;
+            }
             pfrom.m_bloom_filter_loaded = true;
-            peer->m_tx_relay->m_relay_txs = true;
             pfrom.m_relays_txs = true;
         }
         return;
@@ -4076,10 +4080,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         if (peer->m_tx_relay == nullptr) {
             return;
         }
-        LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
-        peer->m_tx_relay->m_bloom_filter = nullptr;
+
+        {
+            LOCK(peer->m_tx_relay->m_bloom_filter_mutex);
+            peer->m_tx_relay->m_bloom_filter = nullptr;
+            peer->m_tx_relay->m_relay_txs = true;
+        }
         pfrom.m_bloom_filter_loaded = false;
-        peer->m_tx_relay->m_relay_txs = true;
         pfrom.m_relays_txs = true;
         return;
     }
diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp
index d57c0081db..6766fbf2d9 100644
--- a/src/test/fuzz/util.cpp
+++ b/src/test/fuzz/util.cpp
@@ -225,7 +225,7 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman,
     const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
     const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
     const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max());
-    const bool filter_txs = fuzzed_data_provider.ConsumeBool();
+    const bool relay_txs{fuzzed_data_provider.ConsumeBool()};
 
     const CNetMsgMaker mm{0};
 
@@ -241,7 +241,7 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman,
                 uint64_t{1},                                    // dummy nonce
                 std::string{},                                  // dummy subver
                 int32_t{},                                      // dummy starting_height
-                filter_txs),
+                relay_txs),
     };
 
     (void)connman.ReceiveMsgFrom(node, msg_version);
@@ -255,6 +255,9 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman,
     assert(node.nVersion == version);
     assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION));
     assert(node.nServices == remote_services);
+    CNodeStateStats statestats;
+    assert(peerman.GetNodeStateStats(node.GetId(), statestats));
+    assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn()));
     node.m_permissionFlags = permission_flags;
     if (successfully_connected) {
         CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)};