[p2p] disallow sendrecon if not relaying transactions

glozow · Apr 18, 2022 · c0a6cce · c0a6cce
1 parent 0e55052
commit c0a6cce
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 20 deletions.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -2733,25 +2733,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         if (greatest_common_version >= WTXID_RELAY_VERSION) {
             m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY));
 
-            if (m_txreconciliation) {
-                // We announce reconciliation support only if this connection supports relaying
-                // transactions when they arrive (which is not the case for blocks-only nodes,
-                // block-relay-only connections, etc.).
-                // Also, we announce reconciliation support only for protocol versions above
-                // WTXID_RELAY per BIP-330.
-                if (peer->m_tx_relay && !m_ignore_incoming_txs) {
-                    const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId());
-                    // Per the protocol, only the inbound peer initiate reconciliations and the
-                    // outbound peer is supposed to only respond. Here we suggest our roles in
-                    // reconciliations (initiator/responder) based on the connection direction.
-                    // If the way we assign reconciliation roles is updated, reconciliation
-                    // protocol version should be bumped.
-                    m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDRECON,
-                                                                 !pfrom.IsInboundConn(),
-                                                                 pfrom.IsInboundConn(),
-                                                                 RECON_VERSION, recon_salt));
-                }
-            }
         }
 
         // Signal ADDRv2 support (BIP155).
@@ -2791,6 +2772,25 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message
             }
             if (fRelay) pfrom.m_relays_txs = true;
+            if (greatest_common_version >= WTXID_RELAY_VERSION && m_txreconciliation) {
+                // We announce reconciliation support only if this connection supports relaying
+                // transactions when they arrive (which is not the case for blocks-only nodes,
+                // block-relay-only connections, etc.).
+                // Also, we announce reconciliation support only for protocol versions above
+                // WTXID_RELAY per BIP-330.
+                if (pfrom.m_relays_txs && !m_ignore_incoming_txs) {
+                    const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId());
+                    // Per the protocol, only the inbound peer initiate reconciliations and the
+                    // outbound peer is supposed to only respond. Here we suggest our roles in
+                    // reconciliations (initiator/responder) based on the connection direction.
+                    // If the way we assign reconciliation roles is updated, reconciliation
+                    // protocol version should be bumped.
+                    m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDRECON,
+                                                                 !pfrom.IsInboundConn(),
+                                                                 pfrom.IsInboundConn(),
+                                                                 RECON_VERSION, recon_salt));
+                }
+            }
         }
 
         if((nServices & NODE_WITNESS))
@@ -3034,6 +3034,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             return;
         }
 
+        if (!peer->GetTxRelay()) {
+            // Disconnect peers that send a SENDRECON message if transaction relay is turned off.
+            LogPrint(BCLog::NET, "sendrecon received from non-transaction relaying peer=%d; disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+
         bool they_initiator;
         bool they_responder;
         uint32_t peer_recon_version;

diff --git a/test/functional/p2p_sendrecon.py b/test/functional/p2p_sendrecon.py
@@ -15,6 +15,7 @@
     P2PInterface,
     P2P_SERVICES,
     P2P_SUBVERSION,
+    P2P_VERSION,
 )
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
@@ -66,6 +67,17 @@ def run_test(self):
         peer.wait_for_verack()
         assert not peer.sendrecon_msg_received
 
+        self.log.info('SENDRECON for fRelay=false should not be sent')
+        peer = self.nodes[0].add_p2p_connection(SendReconSender(), 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 not peer.sendrecon_msg_received
+
         # Checks for the node *receiving* SENDRECON
         self.log.info('valid SENDRECON')
         peer = self.nodes[0].add_p2p_connection(SendReconSender(), send_version=True, wait_for_verack=False)
@@ -104,11 +116,17 @@ def run_test(self):
         peer.wait_for_disconnect()
 
         self.log.info('SENDRECON without WTXIDRELAY is ignored (recon state is erased after VERACK)')
-        with self.nodes[0].assert_debug_log(['Forget reconciliation state of peer=7']):
+        with self.nodes[0].assert_debug_log(['Forget reconciliation state of peer=8']):
             peer = self.nodes[0].add_p2p_connection(SendReconSender(wtxidrelay=False), send_version=True, wait_for_verack=False)
             peer.send_message(create_sendrecon_msg())
             peer.send_message(msg_verack())
 
+        self.log.info('SENDRECON from a block-relay-only peer triggers a disconnect')
+        peer = self.nodes[0].add_outbound_p2p_connection(
+            SendReconSender(), wait_for_verack=False, p2p_idx=0, connection_type="block-relay-only")
+        peer.send_message(create_sendrecon_msg())
+        peer.wait_for_disconnect()
+
         # Outbound
         self.log.info('SENDRECON sent to an outbound')
         peer = self.nodes[0].add_outbound_p2p_connection(