net: Remove g_relay_txes

Summary:
```
g_relay_txes is only required inside net_processing and is set only once at startup. Instead of having a global, move it to be a const member of PeerManager.

This requires moving PushNodeVersion() into PeerManager, which also allows us to remove the connman argument.
```

Backport of [[bitcoin/bitcoin#20217 | core#20217]].

Ref T1696.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Maniphest Tasks: T1696

Differential Revision: https://reviews.bitcoinabc.org/D10884

src/avalanche/test/processor_tests.cpp

@@ -85,7 +85,7 @@ struct AvalancheTestingSetup : public TestChain100Setup {
         m_node.connman = std::move(connman);
         m_node.peerman = std::make_unique<::PeerManager>(
             config.GetChainParams(), *m_connman, m_node.banman.get(),
-            *m_node.scheduler, *m_node.chainman, *m_node.mempool);
+            *m_node.scheduler, *m_node.chainman, *m_node.mempool, false);
         m_node.chain = interfaces::MakeChain(m_node, config.GetChainParams());
 
         // Get the processor ready.

src/init.cpp

@@ -2355,9 +2355,11 @@ bool AppInitMain(Config &config, RPCServer &rpcServer,
     node.chainman = &g_chainman;
     ChainstateManager &chainman = *Assert(node.chainman);
 
-    node.peerman.reset(new PeerManager(chainparams, *node.connman,
-                                       node.banman.get(), *node.scheduler,
-                                       chainman, *node.mempool));
+    assert(!node.peerman);
+    node.peerman = std::make_unique<PeerManager>(
+        chainparams, *node.connman, node.banman.get(), *node.scheduler,
+        chainman, *node.mempool,
+        args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY));
     RegisterValidationInterface(node.peerman.get());
 
     // sanitize comments per BIP-0014, format user agent and check total size
@@ -2467,10 +2469,8 @@ bool AppInitMain(Config &config, RPCServer &rpcServer,
         }
     }
 
-    // see Step 2: parameter interactions for more information about these
     fListen = args.GetBoolArg("-listen", DEFAULT_LISTEN);
     fDiscover = args.GetBoolArg("-discover", true);
-    g_relay_txes = !args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY);
 
     for (const std::string &strAddr : args.GetArgs("-externalip")) {
         CService addrLocal;

src/net.cpp

@@ -130,7 +130,6 @@ static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL;
 //
 bool fDiscover = true;
 bool fListen = true;
-bool g_relay_txes = !DEFAULT_BLOCKSONLY;
 RecursiveMutex cs_mapLocalHost;
 std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
 static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {};

src/net.h

@@ -258,7 +258,6 @@ CAddress GetLocalAddress(const CNetAddr *paddrPeer,
 
 extern bool fDiscover;
 extern bool fListen;
-extern bool g_relay_txes;
 
 struct LocalServiceInfo {
     int nScore;

src/net_processing.cpp

@@ -599,47 +599,6 @@ static void UpdatePreferredDownload(const CNode &node, CNodeState *state)
     nPreferredDownload += state->fPreferredDownload;
 }
 
-static void PushNodeVersion(const Config &config, CNode &pnode,
-                            CConnman &connman, int64_t nTime) {
-    // Note that pnode.GetLocalServices() is a reflection of the local
-    // services we were offering when the CNode object was created for this
-    // peer.
-    ServiceFlags nLocalNodeServices = pnode.GetLocalServices();
-    uint64_t nonce = pnode.GetLocalNonce();
-    int nNodeStartingHeight = pnode.GetMyStartingHeight();
-    NodeId nodeid = pnode.GetId();
-    CAddress addr = pnode.addr;
-    uint64_t extraEntropy = pnode.GetLocalExtraEntropy();
-
-    CAddress addrYou =
-        addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible()
-            ? addr
-            : CAddress(CService(), addr.nServices);
-    CAddress addrMe = CAddress(CService(), nLocalNodeServices);
-
-    connman.PushMessage(
-        &pnode,
-        CNetMsgMaker(INIT_PROTO_VERSION)
-            .Make(NetMsgType::VERSION, PROTOCOL_VERSION,
-                  uint64_t(nLocalNodeServices), nTime, addrYou, addrMe, nonce,
-                  userAgent(config), nNodeStartingHeight,
-                  ::g_relay_txes && pnode.m_tx_relay != nullptr, extraEntropy));
-
-    if (fLogIPs) {
-        LogPrint(BCLog::NET,
-                 "send version message: version %d, blocks=%d, us=%s, them=%s, "
-                 "peer=%d\n",
-                 PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(),
-                 addrYou.ToString(), nodeid);
-    } else {
-        LogPrint(
-            BCLog::NET,
-            "send version message: version %d, blocks=%d, us=%s, peer=%d\n",
-            PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), nodeid);
-    }
-    LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid);
-}
-
 // Returns a bool indicating whether we requested this block.
 // Also used if a block was /not/ received and timed out or started with another
 // peer.
@@ -1000,6 +959,47 @@ ComputeRequestTime(const CNode &node,
     return current_time + delay;
 }
 
+void PeerManager::PushNodeVersion(const Config &config, CNode &pnode,
+                                  int64_t nTime) {
+    // Note that pnode.GetLocalServices() is a reflection of the local
+    // services we were offering when the CNode object was created for this
+    // peer.
+    ServiceFlags nLocalNodeServices = pnode.GetLocalServices();
+    uint64_t nonce = pnode.GetLocalNonce();
+    int nNodeStartingHeight = pnode.GetMyStartingHeight();
+    NodeId nodeid = pnode.GetId();
+    CAddress addr = pnode.addr;
+    uint64_t extraEntropy = pnode.GetLocalExtraEntropy();
+
+    CAddress addrYou =
+        addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible()
+            ? addr
+            : CAddress(CService(), addr.nServices);
+    CAddress addrMe = CAddress(CService(), nLocalNodeServices);
+
+    m_connman.PushMessage(
+        &pnode, CNetMsgMaker(INIT_PROTO_VERSION)
+                    .Make(NetMsgType::VERSION, PROTOCOL_VERSION,
+                          uint64_t(nLocalNodeServices), nTime, addrYou, addrMe,
+                          nonce, userAgent(config), nNodeStartingHeight,
+                          !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr,
+                          extraEntropy));
+
+    if (fLogIPs) {
+        LogPrint(BCLog::NET,
+                 "send version message: version %d, blocks=%d, us=%s, them=%s, "
+                 "peer=%d\n",
+                 PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(),
+                 addrYou.ToString(), nodeid);
+    } else {
+        LogPrint(
+            BCLog::NET,
+            "send version message: version %d, blocks=%d, us=%s, peer=%d\n",
+            PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), nodeid);
+    }
+    LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid);
+}
+
 void PeerManager::AddTxAnnouncement(const CNode &node, const TxId &txid,
                                     std::chrono::microseconds current_time) {
     // For m_txrequest and state
@@ -1062,7 +1062,7 @@ void PeerManager::InitializeNode(const Config &config, CNode *pnode) {
         m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer));
     }
     if (!pnode->IsInboundConn()) {
-        PushNodeVersion(config, *pnode, m_connman, GetTime());
+        PushNodeVersion(config, *pnode, GetTime());
     }
 }
 
@@ -1506,9 +1506,11 @@ static bool BlockRequestAllowed(const CBlockIndex *pindex,
 
 PeerManager::PeerManager(const CChainParams &chainparams, CConnman &connman,
                          BanMan *banman, CScheduler &scheduler,
-                         ChainstateManager &chainman, CTxMemPool &pool)
+                         ChainstateManager &chainman, CTxMemPool &pool,
+                         bool ignore_incoming_txs)
     : m_chainparams(chainparams), m_connman(connman), m_banman(banman),
-      m_chainman(chainman), m_mempool(pool), m_stale_tip_check_time(0) {
+      m_chainman(chainman), m_mempool(pool), m_stale_tip_check_time(0),
+      m_ignore_incoming_txs(ignore_incoming_txs) {
     // Initialize global variables that cannot be constructed at startup.
     recentRejects.reset(new CRollingBloomFilter(120000, 0.000001));
 
@@ -3004,9 +3006,10 @@ void PeerManager::ProcessMessage(const Config &config, CNode &pfrom,
             SeenLocal(addrMe);
         }
 
-        // Be shy and don't send version until we hear
+        // Inbound peers send us their version message when they connect.
+        // We send our version message in response.
         if (pfrom.IsInboundConn()) {
-            PushNodeVersion(config, pfrom, m_connman, GetAdjustedTime());
+            PushNodeVersion(config, pfrom, GetAdjustedTime());
         }
 
         // Change version
@@ -3330,7 +3333,8 @@ void PeerManager::ProcessMessage(const Config &config, CNode &pfrom,
 
         // We won't accept tx inv's if we're in blocks-only mode, or this is a
         // block-relay-only peer
-        bool fBlocksOnly = !g_relay_txes || (pfrom.m_tx_relay == nullptr);
+        bool fBlocksOnly =
+            m_ignore_incoming_txs || (pfrom.m_tx_relay == nullptr);
 
         // Allow peers with relay permission to send data other than blocks
         // in blocks only mode
@@ -3688,7 +3692,7 @@ void PeerManager::ProcessMessage(const Config &config, CNode &pfrom,
         // Stop processing the transaction early if
         // 1) We are in blocks only mode and peer has no relay permission
         // 2) This peer is a block-relay-only peer
-        if ((!g_relay_txes && !pfrom.HasPermission(PF_RELAY)) ||
+        if ((m_ignore_incoming_txs && !pfrom.HasPermission(PF_RELAY)) ||
             (pfrom.m_tx_relay == nullptr)) {
             LogPrint(BCLog::NET,
                      "transaction sent in violation of protocol peer=%d\n",

src/net_processing.h

@@ -123,7 +123,8 @@ class PeerManager final : public CValidationInterface,
 public:
     PeerManager(const CChainParams &chainparams, CConnman &connman,
                 BanMan *banman, CScheduler &scheduler,
-                ChainstateManager &chainman, CTxMemPool &pool);
+                ChainstateManager &chainman, CTxMemPool &pool,
+                bool ignore_incoming_txs);
 
     /**
      * Overridden from CValidationInterface.
@@ -204,6 +205,9 @@ class PeerManager final : public CValidationInterface,
     /** Get statistics from node state */
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats);
 
+    /** Whether this node ignores txs received over p2p. */
+    bool IgnoresIncomingTxs() { return m_ignore_incoming_txs; };
+
 private:
     /**
      * Get a shared pointer to the Peer object.
@@ -289,6 +293,9 @@ class PeerManager final : public CValidationInterface,
                          std::chrono::microseconds current_time, bool preferred)
         EXCLUSIVE_LOCKS_REQUIRED(cs_proofrequest);
 
+    /** Send a version message to a peer */
+    void PushNodeVersion(const Config &config, CNode &pnode, int64_t nTime);
+
     const CChainParams &m_chainparams;
     CConnman &m_connman;
     /**
@@ -307,6 +314,9 @@ class PeerManager final : public CValidationInterface,
     //! Next time to check for stale tip
     int64_t m_stale_tip_check_time;
 
+    /** Whether this node is running in blocks only mode */
+    const bool m_ignore_incoming_txs;
+
     /**
      * Protects m_peer_map. This mutex must not be locked while holding a lock
      * on any of the mutexes inside a Peer object.

src/rpc/net.cpp

@@ -788,7 +788,9 @@ static RPCHelpMan getnetworkinfo() {
                 obj.pushKV("localservices", strprintf("%016x", services));
                 obj.pushKV("localservicesnames", GetServicesNames(services));
             }
-            obj.pushKV("localrelay", g_relay_txes);
+            if (node.peerman) {
+                obj.pushKV("localrelay", !node.peerman->IgnoresIncomingTxs());
+            }
             obj.pushKV("timeoffset", GetTimeOffset());
             if (node.connman) {
                 obj.pushKV("networkactive", node.connman->GetNetworkActive());

src/test/denialofservice_tests.cpp

@@ -79,7 +79,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) {
     auto connman = std::make_unique<CConnman>(config, 0x1337, 0x1337);
     auto peerLogic = std::make_unique<PeerManager>(
         config.GetChainParams(), *connman, nullptr, *m_node.scheduler,
-        *m_node.chainman, *m_node.mempool);
+        *m_node.chainman, *m_node.mempool, false);
 
     // Mock an outbound peer
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
@@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) {
     auto connman = std::make_unique<CConnmanTest>(config, 0x1337, 0x1337);
     auto peerLogic = std::make_unique<PeerManager>(
         config.GetChainParams(), *connman, nullptr, *m_node.scheduler,
-        *m_node.chainman, *m_node.mempool);
+        *m_node.chainman, *m_node.mempool, false);
 
     const Consensus::Params &consensusParams =
         config.GetChainParams().GetConsensus();
@@ -238,7 +238,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) {
     auto connman = std::make_unique<CConnman>(config, 0x1337, 0x1337);
     auto peerLogic = std::make_unique<PeerManager>(
         config.GetChainParams(), *connman, banman.get(), *m_node.scheduler,
-        *m_node.chainman, *m_node.mempool);
+        *m_node.chainman, *m_node.mempool, false);
 
     banman->ClearBanned();
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
@@ -297,7 +297,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) {
     auto connman = std::make_unique<CConnman>(config, 0x1337, 0x1337);
     auto peerLogic = std::make_unique<PeerManager>(
         config.GetChainParams(), *connman, banman.get(), *m_node.scheduler,
-        *m_node.chainman, *m_node.mempool);
+        *m_node.chainman, *m_node.mempool, false);
 
     banman->ClearBanned();
     int64_t nStartTime = GetTime();

src/test/util/setup_common.cpp

@@ -211,7 +211,7 @@ TestingSetup::TestingSetup(const std::string &chainName,
     m_node.connman = std::make_unique<CConnman>(config, 0x1337, 0x1337);
     m_node.peerman = std::make_unique<PeerManager>(
         chainparams, *m_node.connman, m_node.banman.get(), *m_node.scheduler,
-        *m_node.chainman, *m_node.mempool);
+        *m_node.chainman, *m_node.mempool, false);
     {
         CConnman::Options options;
         options.m_msgproc = m_node.peerman.get();