This small PR adds test coverage for the scenario of connecting to ourself, leading to an immediate disconnect: https://github.com/bitcoin/bitcoin/blob/2f6dca4d1c01ee47275a4292f128d714736837a1/src/net_processing.cpp#L3729-L3735
This logic has been first introduced by Satoshi in October 2009, together with a couple of other changes and a version bump to “v0.1.6 BETA” (see commit cc0b4c3b62367a2aebe5fc1f4d0ed4b97e9c2ac9).
The "connect to ourself" detection logic has been first introduced
by Satoshi in October 2009, together with a couple of other changes
and a version bump to "v0.1.6 BETA" (see commit
cc0b4c3b62367a2aebe5fc1f4d0ed4b97e9c2ac9).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | fjahr, kevkevinpal, tdb3, maflcko |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
88@@ -88,6 +89,12 @@ def run_test(self):
89 with node.assert_debug_log([f"feeler connection completed"]):
90 self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True)
91
92+ self.log.info("Check that connecting to ourself leads to immediate disconnect")
93+ with node.assert_debug_log(["connected to self", "disconnecting"]):
94+ node_listen_addr = f"127.0.0.1:{p2p_port(0)}"
tACK 5d2fb14bafe4e80c0a482d99e5ebde07c477f000
Nice find!
tACK 5d2fb14
looks good to me and I ran it locally on my machine and looks to work well
88@@ -88,6 +89,12 @@ def run_test(self):
89 with node.assert_debug_log([f"feeler connection completed"]):
90 self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True)
91
92+ self.log.info("Check that connecting to ourself leads to immediate disconnect")
93+ with node.assert_debug_log(["connected to self", "disconnecting"]):
94+ node_listen_addr = f"127.0.0.1:{p2p_port(0)}"
95+ node.addconnection(node_listen_addr, "outbound-full-relay", self.options.v2transport)
self.options.v2transport as the argument, enabling the script list in test_runner to specify whether or not v2transport should be used for the test.
ACK 5d2fb14bafe4e80c0a482d99e5ebde07c477f000 Good addition. Tested locally with the following:
test_runner (passed)src/bitcoind -regtest -bind=127.0.0.1:1234), and modifying p2p_handshake to run the test against it (node_listen_addr = f"127.0.0.1:{1234}"). Test failed as expected (expected messages weren’t found).
