From 09dc073cff250afd47a3e219f35d1257add764e9 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 15 Jun 2021 12:21:06 -0700 Subject: [PATCH 1/5] [test] Allow AddrReceiver to be used more generally The `on_addr` functionality of `AddrReceiver` tests logic specific to how the addr messages are set up in the test bodies. To allow other callers to also use `AddrReceiver`, only apply the assertion logic if the caller indicates desirability by setting `test_addr_contents` to true when initializing the class. --- test/functional/p2p_addr_relay.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 87297989baf..a033027c71e 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -23,14 +23,22 @@ import time 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, 9) - 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, 9) + 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.') class GetAddrStore(P2PInterface): @@ -101,7 +109,7 @@ class AddrTest(BitcoinTestFramework): num_receivers = 7 receivers = [] for _ in range(num_receivers): - receivers.append(self.nodes[0].add_p2p_connection(AddrReceiver())) + 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. @@ -125,7 +133,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()) + inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(test_addr_contents=True)) full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(GetAddrStore(), p2p_idx=0, connection_type="outbound-full-relay") msg = self.setup_addr_msg(2) self.send_addr_msg(full_outbound_peer, msg, [inbound_peer]) From e8c67ea19ac4c0aec4a0b449ac3a4152f80dfff5 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 15 Jun 2021 12:29:46 -0700 Subject: [PATCH 2/5] [test] Add functionality to AddrReceiver Add two simple helper functions to `AddrReceiver` to support callers currently using `GetAddrStore` [used in next commit]. --- test/functional/p2p_addr_relay.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index a033027c71e..34c5d710538 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -40,6 +40,12 @@ class AddrReceiver(P2PInterface): 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 GetAddrStore(P2PInterface): getaddr_received = False From ef2f149bf2d12e2d14e441fdf701808f0f1dfb8e Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 15 Jun 2021 12:42:52 -0700 Subject: [PATCH 3/5] [test] Update GetAddrStore callers to use AddrReceiver --- test/functional/p2p_addr_relay.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 34c5d710538..1ffb2c774b6 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -140,7 +140,7 @@ class AddrTest(BitcoinTestFramework): 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(GetAddrStore(), p2p_idx=0, connection_type="outbound-full-relay") + 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') @@ -156,7 +156,7 @@ class AddrTest(BitcoinTestFramework): 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(GetAddrStore(), p2p_idx=1, connection_type="block-relay-only") + 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]) @@ -170,17 +170,17 @@ class AddrTest(BitcoinTestFramework): 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(GetAddrStore(), p2p_idx=0, connection_type="outbound-full-relay") + 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 + 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(GetAddrStore(), p2p_idx=1, connection_type="block-relay-only") + 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) + 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(GetAddrStore()) + inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver()) inbound_peer.sync_with_ping() # Add some addresses to addrman @@ -196,7 +196,7 @@ class AddrTest(BitcoinTestFramework): self.mocktime += 5 * 60 self.nodes[0].setmocktime(self.mocktime) - inbound_peer.wait_until(inbound_peer.addr_received) + 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) @@ -210,9 +210,9 @@ class AddrTest(BitcoinTestFramework): self.mocktime = int(time.time()) self.log.info('Check that we send getaddr messages') - full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(GetAddrStore(), p2p_idx=0, connection_type="outbound-full-relay") + 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 + assert full_outbound_peer.getaddr_received() self.log.info('Check that we relay address messages') addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) From 1d8193e2a2950fd957654b601e85ab888899c394 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 21 Jun 2021 10:05:05 -0700 Subject: [PATCH 4/5] [test] Remove GetAddrStore class --- test/functional/p2p_addr_relay.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 1ffb2c774b6..7c144af2e28 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -47,21 +47,6 @@ class AddrReceiver(P2PInterface): return self.message_count['getaddr'] > 0 -class GetAddrStore(P2PInterface): - getaddr_received = False - num_ipv4_received = 0 - - def on_getaddr(self, message): - self.getaddr_received = True - - def on_addr(self, message): - for addr in message.addrs: - self.num_ipv4_received += 1 - - def addr_received(self): - return self.num_ipv4_received != 0 - - class AddrTest(BitcoinTestFramework): counter = 0 mocktime = int(time.time()) From 6168eb06b2044f00f18727b955b672fc91c60bd7 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 21 Jun 2021 10:13:37 -0700 Subject: [PATCH 5/5] [test] Prevent intermittent issue Since m_next_addr_send is on a Poisson distribution, increase the mocktime bump to ensure we don't experience flakiness in the tests. Closes #22243. --- test/functional/p2p_addr_relay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 7c144af2e28..1a414959b90 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -78,7 +78,7 @@ class AddrTest(BitcoinTestFramework): def send_addr_msg(self, source, msg, receivers): source.send_and_ping(msg) # pop m_next_addr_send timer - self.mocktime += 5 * 60 + self.mocktime += 10 * 60 self.nodes[0].setmocktime(self.mocktime) for peer in receivers: peer.sync_send_with_ping()