Merge bitcoin/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 #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

Author: fanquake

test/functional/p2p_getaddr_caching.py

@@ -6,7 +6,6 @@
 
 import time
 
-from test_framework.messages import msg_getaddr
 from test_framework.p2p import (
     P2PInterface,
     p2p_lock
@@ -21,6 +20,7 @@
 MAX_ADDR_TO_SEND = 1000
 MAX_PCT_ADDR_TO_SEND = 23
 
+
 class AddrReceiver(P2PInterface):
 
     def __init__(self):
@@ -70,11 +70,8 @@ def run_test(self):
         cur_mock_time = int(time.time())
         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
@@ -105,11 +102,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
@@ -123,5 +117,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

@@ -649,10 +649,11 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, **kwargs):
             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_SUBVERSION)