Merge bitcoin/bitcoin#21528: [p2p] Reduce addr blackholes

3f7250b328b8b2f5d63f323702445ac5c989b73d [test] Use the new endpoint to improve tests (Amiti Uttarwar)
3893da06db1eb622f540605700f8663f8d87b2df [RPC] Add field to getpeerinfo to indicate if addr relay is enabled (Amiti Uttarwar)
0980ca78cd930a00c9985d7f00083a3b8e8be89e [test] Test that we intentionally select addr relay peers. (Amiti Uttarwar)
c061599e40dc3d379c10b914765061a7a8449dd7 [net_processing] Remove RelayAddrsWithPeer function (Amiti Uttarwar)
201e4964816f8896cfe7b4f6d8ddbfffe7102f87 [net_processing] Introduce new field to indicate if addr relay is enabled (Amiti Uttarwar)
1d1ef2db7ea0d93c7dab4a9800ec74afa7a019eb [net_processing] Defer initializing m_addr_known (Amiti Uttarwar)
6653fa3328b5608fcceda1c6ea8e68c5d58739ec [test] Update p2p_addr_relay test to prepare (Amiti Uttarwar)
2fcaec7bbb96d6fe72a7e3a5744b0c35c79733e8 [net_processing] Introduce SetupAddressRelay (Amiti Uttarwar)

Pull request description:

  This PR builds on the test refactors extracted into #22306 (first 5 commits).

  This PR aims to reduce addr blackholes. When we receive an `addr` message that contains 10 or less addresses, we forward them to 1-2 peers. This is the main technique we use for self advertisements, so sending to peers that wouldn't relay would effectively "blackhole" the trickle. Although we cannot prevent this in a malicious case, we can improve it for the normal, honest cases, and reduce the overall likelihood of occurrence. Two known cases where peers would not participate in addr relay are if they have connected to you as a block-relay-only connection, or if they are a light client.

  This implementation defers initialization of `m_addr_known` until it is needed, then uses its presence to decide if the peer is participating in addr relay. For outbound (not block-relay-only) peers, we initialize the filter before sending the initial self announcement when processing their `version` message. For inbound peers, we initialize the filter if/when we get an addr related message (`ADDR`, `ADDRV2`, `GETADDR`). We do NOT initialize the filter based on a `SENDADDRV2` message.

  To communicate about these changes beyond bitcoin core & to (try to) ensure that no other software would be disrupted, I have:
  - Posted to the [mailing list](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018784.html)
  - Researched other open source clients to confirm compatibility, opened issues in all the projects & documented in https://github.com/bitcoin/bitcoin/pull/21528#issuecomment-809906430. Many have confirmed that this change would not be problematic.
  - Raised as topic during [bitcoin-core-dev meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-03-25.html#l-954)
  - Raised as topic during [bitcoin p2p meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-04-20.html#l-439)

ACKs for top commit:
  jnewbery:
    reACK 3f7250b328b8b2f5d63f323702445ac5c989b73d
  glozow:
    ACK 3f7250b328b8b2f5d63f323702445ac5c989b73d
  ajtowns:
    utACK 3f7250b328b8b2f5d63f323702445ac5c989b73d

Tree-SHA512: 29069282af684c1cd37d107c395fdd432dcccb11626f3c2dabfe92fdc4c85e74c7c4056fbdfa88017fec240506639b72ac6c311f8ce7c583112eb15f47e421af

2 files changed, 78 insertions, 5 deletions

diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py
index ff1d85a9be..95743a1bf5 100755
--- a/test/functional/p2p_addr_relay.py
+++ b/test/functional/p2p_addr_relay.py
@@ -11,14 +11,15 @@ from test_framework.messages import (
     NODE_NETWORK,
     NODE_WITNESS,
     msg_addr,
-    msg_getaddr
+    msg_getaddr,
+    msg_verack
 )
 from test_framework.p2p import (
     P2PInterface,
     p2p_lock,
 )
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import assert_equal, assert_greater_than
 import random
 import time
 
@@ -27,10 +28,12 @@ class AddrReceiver(P2PInterface):
     num_ipv4_received = 0
     test_addr_contents = False
     _tokens = 1
+    send_getaddr = True
 
-    def __init__(self, test_addr_contents=False):
+    def __init__(self, test_addr_contents=False, send_getaddr=True):
         super().__init__()
         self.test_addr_contents = test_addr_contents
+        self.send_getaddr = send_getaddr
 
     def on_addr(self, message):
         for addr in message.addrs:
@@ -60,6 +63,11 @@ class AddrReceiver(P2PInterface):
     def addr_received(self):
         return self.num_ipv4_received != 0
 
+    def on_version(self, message):
+        self.send_message(msg_verack())
+        if (self.send_getaddr):
+            self.send_message(msg_getaddr())
+
     def getaddr_received(self):
         return self.message_count['getaddr'] > 0
 
@@ -75,6 +83,10 @@ class AddrTest(BitcoinTestFramework):
     def run_test(self):
         self.oversized_addr_test()
         self.relay_tests()
+        self.inbound_blackhole_tests()
+
+        # This test populates the addrman, which can impact the node's behavior
+        # in subsequent tests
         self.getaddr_tests()
         self.blocksonly_mode_tests()
         self.rate_limit_tests()
@@ -156,7 +168,7 @@ class AddrTest(BitcoinTestFramework):
         self.nodes[0].disconnect_p2ps()
 
         self.log.info('Check relay of addresses received from outbound peers')
-        inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True))
+        inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True, send_getaddr=False))
         full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay")
         msg = self.setup_addr_msg(2)
         self.send_addr_msg(full_outbound_peer, msg, [inbound_peer])
@@ -167,6 +179,9 @@ class AddrTest(BitcoinTestFramework):
         # of the outbound peer which is often sent before the GETADDR response.
         assert_equal(inbound_peer.num_ipv4_received, 0)
 
+        # Send an empty ADDR message to intialize address relay on this connection.
+        inbound_peer.send_and_ping(msg_addr())
+
         self.log.info('Check that subsequent addr messages sent from an outbound peer are relayed')
         msg2 = self.setup_addr_msg(2)
         self.send_addr_msg(full_outbound_peer, msg2, [inbound_peer])
@@ -184,7 +199,64 @@ class AddrTest(BitcoinTestFramework):
 
         self.nodes[0].disconnect_p2ps()
 
+    def sum_addr_messages(self, msgs_dict):
+        return sum(bytes_received for (msg, bytes_received) in msgs_dict.items() if msg in ['addr', 'addrv2', 'getaddr'])
+
+    def inbound_blackhole_tests(self):
+        self.log.info('Check that we only relay addresses to inbound peers who have previously sent us addr related messages')
+
+        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
+        receiver_peer = self.nodes[0].add_p2p_connection(AddrReceiver())
+        blackhole_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False))
+        initial_addrs_received = receiver_peer.num_ipv4_received
+
+        peerinfo = self.nodes[0].getpeerinfo()
+        assert_equal(peerinfo[0]['addr_relay_enabled'], True)  # addr_source
+        assert_equal(peerinfo[1]['addr_relay_enabled'], True)  # receiver_peer
+        assert_equal(peerinfo[2]['addr_relay_enabled'], False)  # blackhole_peer
+
+        # addr_source sends 2 addresses to node0
+        msg = self.setup_addr_msg(2)
+        addr_source.send_and_ping(msg)
+        self.mocktime += 30 * 60
+        self.nodes[0].setmocktime(self.mocktime)
+        receiver_peer.sync_with_ping()
+        blackhole_peer.sync_with_ping()
+
+        peerinfo = self.nodes[0].getpeerinfo()
+
+        # Confirm node received addr-related messages from receiver peer
+        assert_greater_than(self.sum_addr_messages(peerinfo[1]['bytesrecv_per_msg']), 0)
+        # And that peer received addresses
+        assert_equal(receiver_peer.num_ipv4_received - initial_addrs_received, 2)
+
+        # Confirm node has not received addr-related messages from blackhole peer
+        assert_equal(self.sum_addr_messages(peerinfo[2]['bytesrecv_per_msg']), 0)
+        # And that peer did not receive addresses
+        assert_equal(blackhole_peer.num_ipv4_received, 0)
+
+        self.log.info("After blackhole peer sends addr message, it becomes eligible for addr gossip")
+        blackhole_peer.send_and_ping(msg_addr())
+
+        # Confirm node has now received addr-related messages from blackhole peer
+        assert_greater_than(self.sum_addr_messages(peerinfo[1]['bytesrecv_per_msg']), 0)
+        assert_equal(self.nodes[0].getpeerinfo()[2]['addr_relay_enabled'], True)
+
+        msg = self.setup_addr_msg(2)
+        self.send_addr_msg(addr_source, msg, [receiver_peer, blackhole_peer])
+
+        # And that peer received addresses
+        assert_equal(blackhole_peer.num_ipv4_received, 2)
+
+        self.nodes[0].disconnect_p2ps()
+
     def getaddr_tests(self):
+        # In the previous tests, the node answered GETADDR requests with an
+        # empty addrman. Due to GETADDR response caching (see
+        # CConnman::GetAddresses), the node would continue to provide 0 addrs
+        # in response until enough time has passed or the node is restarted.
+        self.restart_node(0)
+
         self.log.info('Test getaddr behavior')
         self.log.info('Check that we send a getaddr message upon connecting to an outbound-full-relay peer')
         full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay")
@@ -197,7 +269,7 @@ class AddrTest(BitcoinTestFramework):
         assert_equal(block_relay_peer.getaddr_received(), False)
 
         self.log.info('Check that we answer getaddr messages only from inbound peers')
-        inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver())
+        inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False))
         inbound_peer.sync_with_ping()
 
         # Add some addresses to addrman
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index cc80b543cd..b7d5bd8fab 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -438,6 +438,7 @@ class P2PInterface(P2PConnection):
             self.send_message(msg_sendaddrv2())
         self.send_message(msg_verack())
         self.nServices = message.nServices
+        self.send_message(msg_getaddr())
 
     # Connection helper methods