Merge bitcoin#29393: i2p: log connection was refused due to arbitrary port

achow101 · PastaPastaPasta · commit bd607f049dd5 · 2024-10-24T11:16:21.000-05:00
5b358cd i2p: log connection was refused due to arbitrary port (brunoerg) Pull request description: For I2P, we do not try to connect if port is != 0. However, we do not have anything that indicates it or any error when trying to connect with port != 0. This PR adds a log for it. Also, it improves the functional test. With this log we can ensure the reason we won't connect is the port, in the current test, we cannot ensure it. ACKs for top commit: jonatack: ACK 5b358cd epiccurious: re-ACK 5b358cd. achow101: ACK 5b358cd kristapsk: re-ACK 5b358cd vasild: ACK 5b358cd Tree-SHA512: 027245afa771c9295fff0bfd17c251dca4a9f4c739e5773922de3c030a65ef05d96291edcbdeeaa50ba3add61f75f28d8c00be503e03fc33d3491d1956fc549f
diff --git a/src/i2p.cpp b/src/i2p.cpp
@@ -186,6 +186,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
     // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy
     // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.
     if (to.GetPort() != I2P_SAM31_PORT) {
+        Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort());
         proxy_error = false;
         return false;
     }
diff --git a/test/functional/p2p_i2p_ports.py b/test/functional/p2p_i2p_ports.py
@@ -6,36 +6,28 @@
 Test ports handling for I2P hosts
 """
 
-import re
 
 from test_framework.test_framework import BitcoinTestFramework
 
+PROXY = "127.0.0.1:60000"
 
 class I2PPorts(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         # The test assumes that an I2P SAM proxy is not listening here.
-        self.extra_args = [["-i2psam=127.0.0.1:60000"]]
+        self.extra_args = [[f"-i2psam={PROXY}"]]
 
     def run_test(self):
         node = self.nodes[0]
 
         self.log.info("Ensure we don't try to connect if port!=0")
         addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p:8333"
-        raised = False
-        try:
-            with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
-                node.addnode(node=addr, command="onetry")
-        except AssertionError as e:
-            raised = True
-            if not re.search(r"Expected messages .* does not partially match log", str(e)):
-                raise AssertionError(f"Assertion raised as expected, but with an unexpected message: {str(e)}")
-        if not raised:
-            raise AssertionError("Assertion should have been raised")
+        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}, connection refused due to arbitrary port 8333"]):
+            node.addnode(node=addr, command="onetry")
 
         self.log.info("Ensure we try to connect if port=0 and get an error due to missing I2P proxy")
         addr = "h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0"
-        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
+        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}: Cannot connect to {PROXY}"]):
             node.addnode(node=addr, command="onetry")
 
 

Original file line number	Diff line number	Diff line change
`@@ -186,6 +186,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)`
`186`	`186`	`// Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy`
`187`	`187`	`// when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.`
`188`	`188`	`if (to.GetPort() != I2P_SAM31_PORT) {`
	`189`	`+ Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort());`
`189`	`190`	`proxy_error = false;`
`190`	`191`	`return false;`
`191`	`192`	`}`