Merge bitcoin/bitcoin#26396: net: Avoid SetTxRelay for feeler connections

fa24239 net: Avoid SetTxRelay for feeler connections (MacroFake)

Pull request description:

  Seems odd to reserve memory for the struct (the heaviest member being `m_tx_inventory_known_filter`) when it is never used.

  This also avoids sending out `msg_sendtxrcncl` before disconnecting. This shouldn't matter, as other messages, such as `msg_wtxidrelay`, `msg_sendaddrv2`, `msg_verack` or `msg_getaddr` are still sent. Though, it allows to test the changes here as a side-effect.

ACKs for top commit:
  naumenkogs:
    ACK fa24239
  vasild:
    ACK fa24239
  jonatack:
    ACK fa24239
  mzumsande:
    ACK fa24239

Tree-SHA512: d7604c7eb4df8f2de811e600bdd312440ee03e508d3a0f09ae79f7f2d3eeec663bfd47a2d079fa50b756d61e35dfa998de068a7b9afaf35378fa0e62a538263d

Author: MacroFake

src/net.h

@@ -489,10 +489,8 @@ class CNode
     /** Whether this peer provides all services that we want. Used for eviction decisions */
     std::atomic_bool m_has_all_wanted_services{false};
 
-    /** Whether we should relay transactions to this peer (their version
-     *  message did not include fRelay=false and this is not a block-relay-only
-     *  connection). This only changes from false to true. It will never change
-     *  back to false. Used only in inbound eviction logic. */
+    /** Whether we should relay transactions to this peer. This only changes
+     * from false to true. It will never change back to false. */
     std::atomic_bool m_relays_txs{false};
 
     /** Whether this peer has loaded a bloom filter. Used only in inbound

src/net_processing.cpp

@@ -395,9 +395,7 @@ struct Peer {
 private:
     Mutex m_tx_relay_mutex;
 
-    /** Transaction relay data. Will be a nullptr if we're not relaying
-     *  transactions with this peer (e.g. if it's a block-relay-only peer or
-     *  the peer has sent us fRelay=false with bloom filters disabled). */
+    /** Transaction relay data. May be a nullptr. */
     std::unique_ptr<TxRelay> m_tx_relay GUARDED_BY(m_tx_relay_mutex);
 };
 
@@ -3256,12 +3254,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         }
         peer->m_starting_height = starting_height;
 
-        // We only initialize the Peer::TxRelay m_relay_txs data structure if:
+        // Only initialize the Peer::TxRelay m_relay_txs data structure if:
         // - this isn't an outbound block-relay-only connection, and
+        // - this isn't an outbound feeler connection, and
         // - fRelay=true (the peer wishes to receive transaction announcements)
         //   or we're offering NODE_BLOOM to this peer. NODE_BLOOM means that
         //   the peer may turn on transaction relay later.
         if (!pfrom.IsBlockOnlyConn() &&
+            !pfrom.IsFeelerConn() &&
             (fRelay || (peer->m_our_services & NODE_BLOOM))) {
             auto* const tx_relay = peer->SetTxRelay();
             {

test/functional/p2p_sendtxrcncl.py

@@ -39,6 +39,12 @@ def __init__(self):
     def on_sendtxrcncl(self, message):
         self.sendtxrcncl_msg_received = message
 
+
+class P2PFeelerReceiver(SendTxrcnclReceiver):
+    def on_version(self, message):
+        pass  # feeler connections can not send any message other than their own version
+
+
 class PeerTrackMsgOrder(P2PInterface):
     def __init__(self):
         super().__init__()
@@ -163,6 +169,11 @@ def run_test(self):
         assert not peer.sendtxrcncl_msg_received
         peer.peer_disconnect()
 
+        self.log.info("SENDTXRCNCL should not be sent if feeler")
+        peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=2, connection_type="feeler")
+        assert not peer.sendtxrcncl_msg_received
+        peer.peer_disconnect()
+
         self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect')
         peer = self.nodes[0].add_outbound_p2p_connection(
             PeerNoVerack(), wait_for_verack=False, p2p_idx=3, connection_type="block-relay-only")