From bb97b1ffa9f02bf9c05f653602cfb1cf48efb7fa Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 23 Oct 2024 14:20:12 -0400 Subject: [PATCH 1/2] test: fix intermittent timeout in p2p_seednodes.py On some CI runs, the timer in ThreadOpenConnection was only started *after* the mocktime was set. Fix this by waiting for the first connection attempt, which happens after the timer was started. Also convert some comments into log messages/add a log, so that the test isn't completely silent. --- test/functional/p2p_seednode.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_seednode.py b/test/functional/p2p_seednode.py index 6c510a6a0b0..0881a8ca4e7 100755 --- a/test/functional/p2p_seednode.py +++ b/test/functional/p2p_seednode.py @@ -20,17 +20,18 @@ class P2PSeedNodes(BitcoinTestFramework): self.disable_autoconnect = False def test_no_seednode(self): - # Check that if no seednode is provided, the node proceeds as usual (without waiting) + self.log.info("Check that if no seednode is provided, the node proceeds as usual (without waiting)") with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["Empty addrman, adding seednode", f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode"], timeout=ADD_NEXT_SEEDNODE): self.restart_node(0) def test_seednode_empty_addrman(self): seed_node = "0.0.0.1" - # Check that the seednode is added to m_addr_fetches on bootstrap on an empty addrman + self.log.info("Check that the seednode is immediately added on bootstrap on an empty addrman") with self.nodes[0].assert_debug_log(expected_msgs=[f"Empty addrman, adding seednode ({seed_node}) to addrfetch"], timeout=ADD_NEXT_SEEDNODE): self.restart_node(0, extra_args=[f'-seednode={seed_node}']) def test_seednode_addrman_unreachable_peers(self): + self.log.info("Check that if addrman is non-empty, seednodes are queried with a delay") seed_node = "0.0.0.2" node = self.nodes[0] # Fill the addrman with unreachable nodes @@ -40,8 +41,10 @@ class P2PSeedNodes(BitcoinTestFramework): node.addpeeraddress(ip, port) # Restart the node so seednode is processed again - with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5): + with node.assert_debug_log(expected_msgs=["trying v1 connection"], timeout=ADD_NEXT_SEEDNODE): self.restart_node(0, extra_args=[f'-seednode={seed_node}']) + + with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5): node.setmocktime(int(time.time()) + ADD_NEXT_SEEDNODE + 1) def run_test(self): @@ -52,4 +55,3 @@ class P2PSeedNodes(BitcoinTestFramework): if __name__ == '__main__': P2PSeedNodes(__file__).main() - From 6c9fe7b73ea1572b8b56c716ab13d9866f91c6e9 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 23 Oct 2024 14:38:06 -0400 Subject: [PATCH 2/2] test: Prevent connection attempts to random IPs in p2p_seednodes.py These addrs aren't unreachable as the test claims. Specify a (non-working) proxy to make sure the connections fails even if the addr was reachable. Co-authored-by: Vasil Dimov --- test/functional/p2p_seednode.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_seednode.py b/test/functional/p2p_seednode.py index 0881a8ca4e7..5a9fca3278a 100755 --- a/test/functional/p2p_seednode.py +++ b/test/functional/p2p_seednode.py @@ -30,7 +30,7 @@ class P2PSeedNodes(BitcoinTestFramework): with self.nodes[0].assert_debug_log(expected_msgs=[f"Empty addrman, adding seednode ({seed_node}) to addrfetch"], timeout=ADD_NEXT_SEEDNODE): self.restart_node(0, extra_args=[f'-seednode={seed_node}']) - def test_seednode_addrman_unreachable_peers(self): + def test_seednode_non_empty_addrman(self): self.log.info("Check that if addrman is non-empty, seednodes are queried with a delay") seed_node = "0.0.0.2" node = self.nodes[0] @@ -40,9 +40,9 @@ class P2PSeedNodes(BitcoinTestFramework): port = 8333 + i node.addpeeraddress(ip, port) - # Restart the node so seednode is processed again + # Restart the node so seednode is processed again. Specify a non-working proxy to make sure no actual connections to random IPs are attempted. with node.assert_debug_log(expected_msgs=["trying v1 connection"], timeout=ADD_NEXT_SEEDNODE): - self.restart_node(0, extra_args=[f'-seednode={seed_node}']) + self.restart_node(0, extra_args=[f'-seednode={seed_node}', '-proxy=127.0.0.1:1']) with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5): node.setmocktime(int(time.time()) + ADD_NEXT_SEEDNODE + 1) @@ -50,7 +50,7 @@ class P2PSeedNodes(BitcoinTestFramework): def run_test(self): self.test_no_seednode() self.test_seednode_empty_addrman() - self.test_seednode_addrman_unreachable_peers() + self.test_seednode_non_empty_addrman() if __name__ == '__main__':