Merge bitcoin/bitcoin#20196: net: fix GetListenPort() to derive the proper port

7d64ea4a01920bb55bc6de0de6766712ec792a11 net: only assume all local addresses if listening on any (Vasil Dimov)
0cfc0cd32239d3c08d2121e028b297022450b320 net: fix GetListenPort() to derive the proper port (Vasil Dimov)
f98cdcb3574ee661223e1a09e1762b2cc85fab2f net: pass Span by value to CaptureMessage() (Vasil Dimov)
3cb9d9c861710c0cff018bee90b1969193d3ed68 net: make CaptureMessage() mockable (Vasil Dimov)
43868ba416727effd515a471e2a337c3b8cacc37 timedata: rename variables to match the coding style (Vasil Dimov)
60da1eaa1113e7318e273144e7ef9c8895d7ed54 timedata: make it possible to reset the state (Vasil Dimov)

Pull request description:

  `GetListenPort()` uses a simple logic: "if `-port=P` is given, then we
  must be listening on `P`, otherwise we must be listening on `8333`".
  This is however not true if `-bind=` has been provided with `:port` part
  or if `-whitebind=` has been provided. Thus, extend `GetListenPort()` to
  return the port from `-bind=` or `-whitebind=`, if any.

  Also, if `-bind=` is provided then we would bind only to a particular address
  and should not add all the other addresses of the machine to the list of
  local addresses.

  Fixes https://github.com/bitcoin/bitcoin/issues/20184

ACKs for top commit:
  sipa:
    utACK 7d64ea4a01920bb55bc6de0de6766712ec792a11. I didn't review the tests in detail.
  jonatack:
    re-ACK 7d64ea4a01920bb55bc6de0de6766712ec792a11 per `git range-diff 08bcfa27 35ec977 7d64ea4`, changes are rebase-only, light re-review, re-ran the new tests locally

Tree-SHA512: 45135ab9c0ec3cc8c83e3b3e58a1c1f77eaeaba00618d54f1010db1d23d6db7d9c0dc7807e72ebc34e8b2d0e91f1e0d0e9239d13b90f1bdce8be84459e7837f0

4 files changed, 156 insertions, 1 deletions

diff --git a/test/functional/feature_bind_port_discover.py b/test/functional/feature_bind_port_discover.py
new file mode 100755
index 0000000000..6e07f2f16c
--- /dev/null
+++ b/test/functional/feature_bind_port_discover.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test that -discover does not add all interfaces' addresses if we listen on only some of them
+"""
+
+from test_framework.test_framework import BitcoinTestFramework, SkipTest
+from test_framework.util import assert_equal
+
+# We need to bind to a routable address for this test to exercise the relevant code
+# and also must have another routable address on another interface which must not
+# be named "lo" or "lo0".
+# To set these routable addresses on the machine, use:
+# Linux:
+# ifconfig lo:0 1.1.1.1/32 up && ifconfig lo:1 2.2.2.2/32 up  # to set up
+# ifconfig lo:0 down && ifconfig lo:1 down  # to remove it, after the test
+# FreeBSD:
+# ifconfig em0 1.1.1.1/32 alias && ifconfig wlan0 2.2.2.2/32 alias  # to set up
+# ifconfig em0 1.1.1.1 -alias && ifconfig wlan0 2.2.2.2 -alias  # to remove it, after the test
+ADDR1 = '1.1.1.1'
+ADDR2 = '2.2.2.2'
+
+BIND_PORT = 31001
+
+class BindPortDiscoverTest(BitcoinTestFramework):
+    def set_test_params(self):
+        # Avoid any -bind= on the command line. Force the framework to avoid adding -bind=127.0.0.1.
+        self.setup_clean_chain = True
+        self.bind_to_localhost_only = False
+        self.extra_args = [
+            ['-discover', f'-port={BIND_PORT}'], # bind on any
+            ['-discover', f'-bind={ADDR1}:{BIND_PORT}'],
+        ]
+        self.num_nodes = len(self.extra_args)
+
+    def add_options(self, parser):
+        parser.add_argument(
+            "--ihave1111and2222", action='store_true', dest="ihave1111and2222",
+            help=f"Run the test, assuming {ADDR1} and {ADDR2} are configured on the machine",
+            default=False)
+
+    def skip_test_if_missing_module(self):
+        if not self.options.ihave1111and2222:
+            raise SkipTest(
+                f"To run this test make sure that {ADDR1} and {ADDR2} (routable addresses) are "
+                "assigned to the interfaces on this machine and rerun with --ihave1111and2222")
+
+    def run_test(self):
+        self.log.info(
+                "Test that if -bind= is not passed then all addresses are "
+                "added to localaddresses")
+        found_addr1 = False
+        found_addr2 = False
+        for local in self.nodes[0].getnetworkinfo()['localaddresses']:
+            if local['address'] == ADDR1:
+                found_addr1 = True
+                assert_equal(local['port'], BIND_PORT)
+            if local['address'] == ADDR2:
+                found_addr2 = True
+                assert_equal(local['port'], BIND_PORT)
+        assert found_addr1
+        assert found_addr2
+
+        self.log.info(
+                "Test that if -bind= is passed then only that address is "
+                "added to localaddresses")
+        found_addr1 = False
+        for local in self.nodes[1].getnetworkinfo()['localaddresses']:
+            if local['address'] == ADDR1:
+                found_addr1 = True
+                assert_equal(local['port'], BIND_PORT)
+            assert local['address'] != ADDR2
+        assert found_addr1
+
+if __name__ == '__main__':
+    BindPortDiscoverTest().main()
diff --git a/test/functional/feature_bind_port_externalip.py b/test/functional/feature_bind_port_externalip.py
new file mode 100755
index 0000000000..6a74ce5738
--- /dev/null
+++ b/test/functional/feature_bind_port_externalip.py
@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test that the proper port is used for -externalip=
+"""
+
+from test_framework.test_framework import BitcoinTestFramework, SkipTest
+from test_framework.util import assert_equal, p2p_port
+
+# We need to bind to a routable address for this test to exercise the relevant code.
+# To set a routable address on the machine use:
+# Linux:
+# ifconfig lo:0 1.1.1.1/32 up  # to set up
+# ifconfig lo:0 down  # to remove it, after the test
+# FreeBSD:
+# ifconfig lo0 1.1.1.1/32 alias  # to set up
+# ifconfig lo0 1.1.1.1 -alias  # to remove it, after the test
+ADDR = '1.1.1.1'
+
+# array of tuples [arguments, expected port in localaddresses]
+EXPECTED = [
+    [['-externalip=2.2.2.2',       '-port=30001'],                        30001],
+    [['-externalip=2.2.2.2',       '-port=30002', f'-bind={ADDR}'],       30002],
+    [['-externalip=2.2.2.2',                      f'-bind={ADDR}'],       'default_p2p_port'],
+    [['-externalip=2.2.2.2',       '-port=30003', f'-bind={ADDR}:30004'], 30004],
+    [['-externalip=2.2.2.2',                      f'-bind={ADDR}:30005'], 30005],
+    [['-externalip=2.2.2.2:30006', '-port=30007'],                        30006],
+    [['-externalip=2.2.2.2:30008', '-port=30009', f'-bind={ADDR}'],       30008],
+    [['-externalip=2.2.2.2:30010',                f'-bind={ADDR}'],       30010],
+    [['-externalip=2.2.2.2:30011', '-port=30012', f'-bind={ADDR}:30013'], 30011],
+    [['-externalip=2.2.2.2:30014',                f'-bind={ADDR}:30015'], 30014],
+    [['-externalip=2.2.2.2',       '-port=30016', f'-bind={ADDR}:30017',
+                                             f'-whitebind={ADDR}:30018'], 30017],
+    [['-externalip=2.2.2.2',       '-port=30019',
+                                             f'-whitebind={ADDR}:30020'], 30020],
+]
+
+class BindPortExternalIPTest(BitcoinTestFramework):
+    def set_test_params(self):
+        # Avoid any -bind= on the command line. Force the framework to avoid adding -bind=127.0.0.1.
+        self.setup_clean_chain = True
+        self.bind_to_localhost_only = False
+        self.num_nodes = len(EXPECTED)
+        self.extra_args = list(map(lambda e: e[0], EXPECTED))
+
+    def add_options(self, parser):
+        parser.add_argument(
+            "--ihave1111", action='store_true', dest="ihave1111",
+            help=f"Run the test, assuming {ADDR} is configured on the machine",
+            default=False)
+
+    def skip_test_if_missing_module(self):
+        if not self.options.ihave1111:
+            raise SkipTest(
+                f"To run this test make sure that {ADDR} (a routable address) is assigned "
+                "to one of the interfaces on this machine and rerun with --ihave1111")
+
+    def run_test(self):
+        self.log.info("Test the proper port is used for -externalip=")
+        for i in range(len(EXPECTED)):
+            expected_port = EXPECTED[i][1]
+            if expected_port == 'default_p2p_port':
+                expected_port = p2p_port(i)
+            found = False
+            for local in self.nodes[i].getnetworkinfo()['localaddresses']:
+                if local['address'] == '2.2.2.2':
+                    assert_equal(local['port'], expected_port)
+                    found = True
+                    break
+            assert found
+
+if __name__ == '__main__':
+    BindPortExternalIPTest().main()
diff --git a/test/functional/p2p_message_capture.py b/test/functional/p2p_message_capture.py
index edde9a6ecf..0a7ae44de4 100755
--- a/test/functional/p2p_message_capture.py
+++ b/test/functional/p2p_message_capture.py
@@ -20,7 +20,7 @@ LENGTH_SIZE = 4
 MSGTYPE_SIZE = 12
 
 def mini_parser(dat_file):
-    """Parse a data file created by CaptureMessage.
+    """Parse a data file created by CaptureMessageToFile.
 
     From the data file we'll only check the structure.
 
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index d15edcedd2..b0f24e3b97 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -276,6 +276,7 @@ BASE_SCRIPTS = [
     'feature_minchainwork.py',
     'rpc_estimatefee.py',
     'rpc_getblockstats.py',
+    'feature_bind_port_externalip.py',
     'wallet_create_tx.py --legacy-wallet',
     'wallet_send.py --legacy-wallet',
     'wallet_send.py --descriptors',
@@ -291,6 +292,7 @@ BASE_SCRIPTS = [
     'feature_loadblock.py',
     'p2p_dos_header_tree.py',
     'p2p_add_connections.py',
+    'feature_bind_port_discover.py',
     'p2p_unrequested_blocks.py',
     'p2p_blockfilters.py',
     'p2p_message_capture.py',