test: Extend functional tests for addr relay

Summary:
```
This extends the functional test p2p_addr_relay.py.
It adds test coverage for address relay involving outbound peers, tests for both outgoing and incoming GETADDR requests and tests for -blocksonly mode.
```

Backport of [[bitcoin/bitcoin#21707 | core#21707]], [[bitcoin/bitcoin#21785 | core#21785]] (fix intermittent failure in the test) and [[bitcoin/bitcoin#22306 | core#22306]] (improvements to p2p_addr_relay.py, also fixes an intermittent issue).

Note to reviewers: you can look at the file after #22306 to get the final version of the test.

Depends on D10860.

Ref T1696.

Test Plan:
  ninja check-functional

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Subscribers: PiRK

Maniphest Tasks: T1696

Differential Revision: https://reviews.bitcoinabc.org/D10861

test/functional/p2p_addr_relay.py

@@ -8,70 +8,115 @@
 
 import time
 
-from test_framework.messages import NODE_NETWORK, CAddress, msg_addr
+from test_framework.messages import (
+    NODE_NETWORK,
+    CAddress,
+    msg_addr,
+    msg_getaddr,
+)
 from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
-# Keep this with length <= 10. Addresses from larger messages are not relayed.
-ADDRS = []
-num_ipv4_addrs = 10
-
-for i in range(num_ipv4_addrs):
-    addr = CAddress()
-    addr.time = int(time.time()) + i
-    addr.nServices = NODE_NETWORK
-    addr.ip = "123.123.123.{}".format(i % 256)
-    addr.port = 8333 + i
-    ADDRS.append(addr)
-
 
 class AddrReceiver(P2PInterface):
     num_ipv4_received = 0
+    test_addr_contents = False
+
+    def __init__(self, test_addr_contents=False):
+        super().__init__()
+        self.test_addr_contents = test_addr_contents
 
     def on_addr(self, message):
         for addr in message.addrs:
-            assert_equal(addr.nServices, NODE_NETWORK)
-            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.')
             self.num_ipv4_received += 1
+            if(self.test_addr_contents):
+                # relay_tests checks the content of the addr messages match
+                # expectations based on the message creation in setup_addr_msg
+                assert_equal(addr.nServices, NODE_NETWORK)
+                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.')
+
+    def addr_received(self):
+        return self.num_ipv4_received != 0
+
+    def getaddr_received(self):
+        return self.message_count['getaddr'] > 0
 
 
 class AddrTest(BitcoinTestFramework):
+    counter = 0
+    mocktime = int(time.time())
+
     def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
-        self.log.info('Create connection that sends addr messages')
-        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
+        self.oversized_addr_test()
+        self.relay_tests()
+        self.getaddr_tests()
+        self.blocksonly_mode_tests()
+
+    def setup_addr_msg(self, num):
+        addrs = []
+        for i in range(num):
+            addr = CAddress()
+            addr.time = self.mocktime + i
+            addr.nServices = NODE_NETWORK
+            addr.ip = f"123.123.123.{self.counter % 256}"
+            addr.port = 8333 + i
+            addrs.append(addr)
+            self.counter += 1
+
         msg = msg_addr()
+        msg.addrs = addrs
+        return msg
+
+    def send_addr_msg(self, source, msg, receivers):
+        source.send_and_ping(msg)
+        # pop m_next_addr_send timer
+        self.mocktime += 10 * 60
+        self.nodes[0].setmocktime(self.mocktime)
+        for peer in receivers:
+            peer.sync_send_with_ping()
+
+    def oversized_addr_test(self):
+        self.log.info('Send an addr message that is too large')
+        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
 
-        self.log.info('Send too-large addr message')
-        # more than 1000 addresses in one message
-        msg.addrs = ADDRS * 101
+        msg = self.setup_addr_msg(1010)
         with self.nodes[0].assert_debug_log(['addr message size = 1010']):
             addr_source.send_and_ping(msg)
 
+        self.nodes[0].disconnect_p2ps()
+
+    def relay_tests(self):
+        self.log.info('Test address relay')
         self.log.info(
             'Check that addr message content is relayed and added to addrman')
+        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
         num_receivers = 7
         receivers = []
         for _ in range(num_receivers):
-            receivers.append(self.nodes[0].add_p2p_connection(AddrReceiver()))
-        msg.addrs = ADDRS
+            receivers.append(
+                self.nodes[0].add_p2p_connection(
+                    AddrReceiver(
+                        test_addr_contents=True)))
+
+        # Keep this with length <= 10. Addresses from larger messages are not
+        # relayed.
+        num_ipv4_addrs = 10
+        msg = self.setup_addr_msg(num_ipv4_addrs)
         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',
+                'received: addr (301 bytes) peer=1',
             ]
         ):
-            addr_source.send_and_ping(msg)
-            self.nodes[0].setmocktime(int(time.time()) + 30 * 60)
-            for receiver in receivers:
-                receiver.sync_with_ping()
+            self.send_addr_msg(addr_source, msg, receivers)
 
         total_ipv4_received = sum(r.num_ipv4_received for r in receivers)
 
@@ -83,6 +128,108 @@ def run_test(self):
             num_ipv4_addrs *
             ipv4_branching_factor)
 
+        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))
+        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])
+        self.log.info(
+            'Check that the first addr message received from an outbound peer is not relayed')
+        # Currently, there is a flag that prevents the first addr message
+        # received from a new outbound peer to be relayed to others. Originally
+        # meant to prevent large GETADDR responses from being relayed, it now
+        # typically affects the self-announcement of the outbound peer which is
+        # often sent before the GETADDR response.
+        assert_equal(inbound_peer.num_ipv4_received, 0)
+
+        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])
+        assert_equal(inbound_peer.num_ipv4_received, 2)
+
+        self.log.info('Check address relay to outbound peers')
+        block_relay_peer = self.nodes[0].add_outbound_p2p_connection(
+            AddrReceiver(), p2p_idx=1, connection_type="block-relay-only")
+        msg3 = self.setup_addr_msg(2)
+        self.send_addr_msg(
+            inbound_peer, msg3,
+            [full_outbound_peer, block_relay_peer])
+
+        self.log.info(
+            'Check that addresses are relayed to full outbound peers')
+        assert_equal(full_outbound_peer.num_ipv4_received, 2)
+        self.log.info(
+            'Check that addresses are not relayed to block-relay-only outbound peers')
+        assert_equal(block_relay_peer.num_ipv4_received, 0)
+
+        self.nodes[0].disconnect_p2ps()
+
+    def getaddr_tests(self):
+        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")
+        full_outbound_peer.sync_with_ping()
+        assert full_outbound_peer.getaddr_received()
+
+        self.log.info(
+            'Check that we do not send a getaddr message upon connecting to a block-relay-only peer')
+        block_relay_peer = self.nodes[0].add_outbound_p2p_connection(
+            AddrReceiver(), p2p_idx=1, connection_type="block-relay-only")
+        block_relay_peer.sync_with_ping()
+        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.sync_with_ping()
+
+        # Add some addresses to addrman
+        for i in range(1000):
+            first_octet = i >> 8
+            second_octet = i % 256
+            a = f"{first_octet}.{second_octet}.1.1"
+            self.nodes[0].addpeeraddress(a, 8333)
+
+        full_outbound_peer.send_and_ping(msg_getaddr())
+        block_relay_peer.send_and_ping(msg_getaddr())
+        inbound_peer.send_and_ping(msg_getaddr())
+
+        self.mocktime += 5 * 60
+        self.nodes[0].setmocktime(self.mocktime)
+        inbound_peer.wait_until(lambda: inbound_peer.addr_received() is True)
+
+        assert_equal(full_outbound_peer.num_ipv4_received, 0)
+        assert_equal(block_relay_peer.num_ipv4_received, 0)
+        assert inbound_peer.num_ipv4_received > 100
+
+        self.nodes[0].disconnect_p2ps()
+
+    def blocksonly_mode_tests(self):
+        self.log.info('Test addr relay in -blocksonly mode')
+        self.restart_node(0, ["-blocksonly"])
+        self.mocktime = int(time.time())
+
+        self.log.info('Check that we send getaddr messages')
+        full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(
+            AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay")
+        full_outbound_peer.sync_with_ping()
+        assert full_outbound_peer.getaddr_received()
+
+        self.log.info('Check that we relay address messages')
+        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
+        msg = self.setup_addr_msg(2)
+        self.send_addr_msg(addr_source, msg, [full_outbound_peer])
+        assert_equal(full_outbound_peer.num_ipv4_received, 2)
+
+        self.nodes[0].disconnect_p2ps()
+
 
 if __name__ == '__main__':
     AddrTest().main()

test/functional/p2p_blocksonly.py

@@ -111,11 +111,7 @@ def blocks_relay_conn_tests(self):
         # Bump time forward to ensure nNextInvSend timer pops
         self.nodes[0].setmocktime(int(time.time()) + 60)
 
-        # Calling sync_with_ping twice requires that the node calls
-        # `ProcessMessage` twice, and thus ensures `SendMessages` must have
-        # been called at least once
-        conn.sync_with_ping()
-        conn.sync_with_ping()
+        conn.sync_send_with_ping()
         assert(int(txid, 16) not in conn.get_invs())
 
     def check_p2p_tx_violation(self, index=1):

test/functional/p2p_filter.py

@@ -198,8 +198,7 @@ def test_filter(self, filter_peer):
         filter_peer.merkleblock_received = False
         filter_peer.tx_received = False
         self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 90)
-        filter_peer.sync_with_ping()
-        filter_peer.sync_with_ping()
+        filter_peer.sync_send_with_ping()
         assert not filter_peer.merkleblock_received
         assert not filter_peer.tx_received
 

test/functional/test_framework/p2p.py

@@ -593,8 +593,16 @@ def send_and_ping(self, message, timeout=60):
         self.send_message(message)
         self.sync_with_ping(timeout=timeout)
 
-    # Sync up with the node
+    def sync_send_with_ping(self, timeout=60):
+        """Ensure SendMessages is called on this connection"""
+        # Calling sync_with_ping twice requires that the node calls
+        # `ProcessMessage` twice, and thus ensures `SendMessages` must have
+        # been called at least once
+        self.sync_with_ping()
+        self.sync_with_ping()
+
     def sync_with_ping(self, timeout=60):
+        """Ensure ProcessMessages is called on this connection"""
         self.send_message(msg_ping(nonce=self.ping_counter))
 
         def test_function():