merge bitcoin#19763: don't relay to the address' originator

src/net_processing.cpp

@@ -2006,7 +2006,23 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid)
     });
 }
 
-static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& connman)
+/**
+ * Relay (gossip) an address to a few randomly chosen nodes.
+ * We choose the same nodes within a given 24h window (if the list of connected
+ * nodes does not change) and we don't relay to nodes that already know an
+ * address. So within 24h we will likely relay a given address once. This is to
+ * prevent a peer from unjustly giving their address better propagation by sending
+ * it to us repeatedly.
+ * @param[in] originator The peer that sent us the address. We don't want to relay it back.
+ * @param[in] addr Address to relay.
+ * @param[in] fReachable Whether the address' network is reachable. We relay unreachable
+ * addresses less.
+ * @param[in] connman Connection manager to choose nodes to relay to.
+ */
+static void RelayAddress(const CNode& originator,
+                         const CAddress& addr,
+                         bool fReachable,
+                         const CConnman& connman)
 {
     if (!fReachable && !addr.IsRelayable()) return;
 
@@ -2023,8 +2039,8 @@ static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman&
     std::array<std::pair<uint64_t, CNode*>,2> best{{{0, nullptr}, {0, nullptr}}};
     assert(nRelayNodes <= best.size());
 
-    auto sortfunc = [&best, &hasher, nRelayNodes, addr](CNode* pnode) {
-        if (pnode->RelayAddrsWithConn() && pnode->IsAddrCompatible(addr)) {
+    auto sortfunc = [&best, &hasher, nRelayNodes, &originator, &addr](CNode* pnode) {
+        if (pnode->RelayAddrsWithConn() && pnode != &originator && pnode->IsAddrCompatible(addr)) {
             uint64_t hashKey = CSipHasher(hasher).Write(pnode->GetId()).Finalize();
             for (unsigned int i = 0; i < nRelayNodes; i++) {
                 if (hashKey > best[i].first) {
@@ -3329,7 +3345,8 @@ void PeerManagerImpl::ProcessMessage(
             bool fReachable = IsReachable(addr);
             if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable())
             {
-                RelayAddress(addr, fReachable, m_connman);
+                // Relay to a limited number of other nodes
+                RelayAddress(pfrom, addr, fReachable, m_connman);
             }
             // Do not store addresses outside our network
             if (fReachable)

test/functional/p2p_addr_relay.py

@@ -17,22 +17,28 @@
     assert_equal,
 )
 
+# Keep this with length <= 10. Addresses from larger messages are not relayed.
 ADDRS = []
+num_ipv4_addrs = 10
 
 class AddrReceiver(P2PInterface):
+    num_ipv4_received = 0
+
     def on_addr(self, message):
         for addr in message.addrs:
             assert_equal(addr.nServices, 1)
+            if not 8333 <= addr.port < 8343:
+                raise AssertionError("Invalid addr.port of {} (8333-8342 expected)".format(addr.port))
             assert addr.ip.startswith('123.123.123.')
-            assert (8333 <= addr.port < 8343)
+            self.num_ipv4_received += 1
 
 
 class AddrTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
-        for i in range(10):
+        for i in range(num_ipv4_addrs):
             addr = CAddress()
             addr.time = int(self.mocktime) + i
             addr.nServices = NODE_NETWORK
@@ -45,21 +51,33 @@ def run_test(self):
         msg = msg_addr()
 
         self.log.info('Send too-large addr message')
-        msg.addrs = ADDRS * 101
+        msg.addrs = ADDRS * 101  # more than 1000 addresses in one message
         with self.nodes[0].assert_debug_log(['addr message size = 1010']):
             addr_source.send_and_ping(msg)
 
         self.log.info('Check that addr message content is relayed and added to addrman')
-        addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver())
+        num_receivers = 7
+        receivers = []
+        for _ in range(num_receivers):
+            receivers.append(self.nodes[0].add_p2p_connection(AddrReceiver()))
         msg.addrs = ADDRS
-        with self.nodes[0].assert_debug_log([
-                'Added 10 addresses from 127.0.0.1: 0 tried',
+        with self.nodes[0].assert_debug_log(
+            [
+                'Added {} addresses from 127.0.0.1: 0 tried'.format(num_ipv4_addrs),
                 'received: addr (301 bytes) peer=0',
-                'sending addr (301 bytes) peer=1',
-        ]):
+            ]
+        ):
             addr_source.send_and_ping(msg)
             self.bump_mocktime(30 * 60)
-            addr_receiver.sync_with_ping()
+            for receiver in receivers:
+                receiver.sync_with_ping()
+
+        total_ipv4_received = sum(r.num_ipv4_received for r in receivers)
+
+        # Every IPv4 address must be relayed to two peers, other than the
+        # originating node (addr_source).
+        ipv4_branching_factor = 2
+        assert_equal(total_ipv4_received, num_ipv4_addrs * ipv4_branching_factor)
 
 
 if __name__ == '__main__':