Merge bitcoin#28144: test: fix intermittent failure in p2p_getaddr_caching.py

8a20f76 test: drop duplicate getaddrs from p2p_getaddr_caching (Martin Zumsande)
feb0096 test: fix intermittent failure in p2p_getaddr_caching (Martin Zumsande)

Pull request description:

  Fixes bitcoin#28133

  In the consistency check, it's not enough to check that our address/port is unique, only the combination of source and target must be unique. Otherwise, the OS may reuse ports for connections to different `-addrbind`, which was happening in the failed runs.

  While at it, the second commit cleans up duplicate `getaddr` messages in `p2p_getaddr_caching.py` that do nothing but generate `Ignoring repeated "getaddr"` log messages (and cleans up some whitespace the python linter complains about).

ACKs for top commit:
  vasild:
    ACK 8a20f76

Tree-SHA512: eabe4727d7887f729074076f6333a918bba8cb34b8e3baaa83f167b441b0daa24f7c4824abcf03a9538a2ef14b2d826ff19aeffcb93a6c20735253a9678aac9c

Fixing Linting error - 'time' module imported but not used

Author: fanquake

test/functional/p2p_getaddr_caching.py

@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test addr response caching"""
 
-from test_framework.messages import msg_getaddr
 from test_framework.p2p import (
     P2PInterface,
     p2p_lock
@@ -19,6 +18,7 @@
 MAX_ADDR_TO_SEND = 1000
 MAX_PCT_ADDR_TO_SEND = 23
 
+
 class AddrReceiver(P2PInterface):
 
     def __init__(self):
@@ -68,11 +68,8 @@ def run_test(self):
         cur_mock_time = self.mocktime
         for i in range(N):
             addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver())
-            addr_receiver_local.send_and_ping(msg_getaddr())
             addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1)
-            addr_receiver_onion1.send_and_ping(msg_getaddr())
             addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2)
-            addr_receiver_onion2.send_and_ping(msg_getaddr())
 
             # Trigger response
             cur_mock_time += 5 * 60
@@ -103,11 +100,8 @@ def run_test(self):
 
         self.log.info('After time passed, see a new response to addr request')
         addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver())
-        addr_receiver_local.send_and_ping(msg_getaddr())
         addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1)
-        addr_receiver_onion1.send_and_ping(msg_getaddr())
         addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2)
-        addr_receiver_onion2.send_and_ping(msg_getaddr())
 
         # Trigger response
         cur_mock_time += 5 * 60
@@ -121,5 +115,6 @@ def run_test(self):
         assert(set(last_response_on_onion_bind1) != set(addr_receiver_onion1.get_received_addrs()))
         assert(set(last_response_on_onion_bind2) != set(addr_receiver_onion2.get_received_addrs()))
 
+
 if __name__ == '__main__':
     AddrTest().main()

test/functional/test_framework/test_node.py

@@ -713,10 +713,11 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru
             p2p_conn.sync_with_ping()
 
             # Consistency check that the node received our user agent string.
-            # Find our connection in getpeerinfo by our address:port, as it is unique.
+            # Find our connection in getpeerinfo by our address:port and theirs, as this combination is unique.
             sockname = p2p_conn._transport.get_extra_info("socket").getsockname()
             our_addr_and_port = f"{sockname[0]}:{sockname[1]}"
-            info = [peer for peer in self.getpeerinfo() if peer["addr"] == our_addr_and_port]
+            dst_addr_and_port = f"{p2p_conn.dstaddr}:{p2p_conn.dstport}"
+            info = [peer for peer in self.getpeerinfo() if peer["addr"] == our_addr_and_port and peer["addrbind"] == dst_addr_and_port]
             assert_equal(len(info), 1)
             assert_equal(info[0]["subver"], p2p_conn.strSubVer)