diff options
author | laanwj <126646+laanwj@users.noreply.github.com> | 2022-03-03 08:57:38 +0100 |
---|---|---|
committer | laanwj <126646+laanwj@users.noreply.github.com> | 2022-03-03 13:49:01 +0100 |
commit | 30308cc380a8176a5ec0e0bd2beed8b9c482ccf7 (patch) | |
tree | 75722477417ad2a8c3600dd8634d57befcdce58a /test | |
parent | 08bcfa27675da5c65e4c9eab7e7764eab0599298 (diff) | |
parent | 7d64ea4a01920bb55bc6de0de6766712ec792a11 (diff) | |
download | bitcoin-30308cc380a8176a5ec0e0bd2beed8b9c482ccf7.tar.xz |
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
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/feature_bind_port_discover.py | 78 | ||||
-rwxr-xr-x | test/functional/feature_bind_port_externalip.py | 75 | ||||
-rwxr-xr-x | test/functional/p2p_message_capture.py | 2 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 2 |
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', |