p2p: Clarify sendtxrcncl policies

Author: naumenkogs

src/net_processing.cpp

@@ -3273,11 +3273,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 
         if (greatest_common_version >= WTXID_RELAY_VERSION && m_txreconciliation) {
             // Per BIP-330, we announce txreconciliation support if:
-            // - protocol version per the VERSION message supports WTXID_RELAY;
-            // - we intended to exchange transactions over this connection while establishing it
-            //   and the peer indicated support for transaction relay in the VERSION message;
+            // - protocol version per the peer's VERSION message supports WTXID_RELAY;
+            // - transaction relay is supported per the peer's VERSION message (see m_relays_txs);
+            // - this is not a block-relay-only connection and not a feeler (see m_relays_txs);
+            // - this is not an addr fetch connection;
             // - we are not in -blocksonly mode.
-            if (pfrom.m_relays_txs && !m_ignore_incoming_txs) {
+            if (pfrom.m_relays_txs && !pfrom.IsAddrFetchConn() && !m_ignore_incoming_txs) {
                 const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId());
                 m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL,
                                                              TXRECONCILIATION_VERSION, recon_salt));
@@ -3496,20 +3497,27 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         }
 
         if (pfrom.fSuccessfullyConnected) {
-            // Disconnect peers that send a SENDTXRCNCL message after VERACK.
             LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received after verack from peer=%d; disconnecting\n", pfrom.GetId());
             pfrom.fDisconnect = true;
             return;
         }
 
-        if (!peer->GetTxRelay()) {
-            // Disconnect peers that send a SENDTXRCNCL message even though we indicated we don't
-            // support transaction relay.
+        // Peer must not offer us reconciliations if we specified no tx relay support in VERSION.
+        if (RejectIncomingTxs(pfrom)) {
             LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d to which we indicated no tx relay; disconnecting\n", pfrom.GetId());
             pfrom.fDisconnect = true;
             return;
         }
 
+        // Peer must not offer us reconciliations if they specified no tx relay support in VERSION.
+        // This flag might also be false in other cases, but the RejectIncomingTxs check above
+        // eliminates them, so that this flag fully represents what we are looking for.
+        if (!pfrom.m_relays_txs) {
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d which indicated no tx relay to us; disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+
         uint32_t peer_txreconcl_version;
         uint64_t remote_salt;
         vRecv >> peer_txreconcl_version >> remote_salt;

test/functional/p2p_sendtxrcncl.py

@@ -10,6 +10,7 @@
     msg_verack,
     msg_version,
     msg_wtxidrelay,
+    NODE_BLOOM,
 )
 from test_framework.p2p import (
     P2PInterface,
@@ -104,6 +105,22 @@ def run_test(self):
         assert not peer.sendtxrcncl_msg_received
         self.nodes[0].disconnect_p2ps()
 
+        self.log.info('SENDTXRCNCL for fRelay=false should not be sent (with NODE_BLOOM offered)')
+        self.restart_node(0, ["-peerbloomfilters", "-txreconciliation"])
+        peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False)
+        no_txrelay_version_msg = msg_version()
+        no_txrelay_version_msg.nVersion = P2P_VERSION
+        no_txrelay_version_msg.strSubVer = P2P_SUBVERSION
+        no_txrelay_version_msg.nServices = P2P_SERVICES
+        no_txrelay_version_msg.relay = 0
+        peer.send_message(no_txrelay_version_msg)
+        peer.wait_for_verack()
+        assert peer.nServices & NODE_BLOOM != 0
+        assert not peer.sendtxrcncl_msg_received
+        self.nodes[0].disconnect_p2ps()
+
+        self.restart_node(0, ["-txreconciliation"])
+
         self.log.info('valid SENDTXRCNCL received')
         peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
         with self.nodes[0].assert_debug_log(["received: sendtxrcncl"]):
@@ -113,6 +130,15 @@ def run_test(self):
             peer.send_message(create_sendtxrcncl_msg())
             peer.wait_for_disconnect()
 
+        self.restart_node(0, [])
+        self.log.info('SENDTXRCNCL if no txreconciliation supported is ignored')
+        peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
+        with self.nodes[0].assert_debug_log(['ignored, as our node does not have txreconciliation enabled']):
+            peer.send_message(create_sendtxrcncl_msg())
+        self.nodes[0].disconnect_p2ps()
+
+        self.restart_node(0, ["-txreconciliation"])
+
         self.log.info('SENDTXRCNCL with version=0 triggers a disconnect')
         sendtxrcncl_low_version = create_sendtxrcncl_msg()
         sendtxrcncl_low_version.version = 0
@@ -125,7 +151,7 @@ def run_test(self):
         sendtxrcncl_higher_version = create_sendtxrcncl_msg()
         sendtxrcncl_higher_version.version = 2
         peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
-        with self.nodes[0].assert_debug_log(['Register peer=6']):
+        with self.nodes[0].assert_debug_log(['Register peer=1']):
             peer.send_message(sendtxrcncl_higher_version)
         self.nodes[0].disconnect_p2ps()
 
@@ -160,6 +186,12 @@ def run_test(self):
         assert not peer.sendtxrcncl_msg_received
         self.nodes[0].disconnect_p2ps()
 
+        self.log.info("SENDTXRCNCL should not be sent if addrfetch")
+        peer = self.nodes[0].add_outbound_p2p_connection(
+            SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="addr-fetch")
+        assert not peer.sendtxrcncl_msg_received
+        self.nodes[0].disconnect_p2ps()
+
         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=0, connection_type="block-relay-only")