Merge #21328: net, refactor: pass uint16 CService::port as uint16

52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack)
6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack)
2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack)
6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack)

Pull request description:

  As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch.

ACKs for top commit:
  practicalswift:
    cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct
  MarcoFalke:
    cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61
  vasild:
    ACK 52dd40a9febec1f4e70d777821b6764830bdec61

Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432

7 files changed, 20 insertions, 15 deletions

diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 37ff8a9afe..d438537606 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -100,7 +100,7 @@ static CNetAddr ResolveIP(const std::string& ip)
     return addr;
 }
 
-static CService ResolveService(const std::string& ip, const int port = 0)
+static CService ResolveService(const std::string& ip, uint16_t port = 0)
 {
     CService serv;
     BOOST_CHECK_MESSAGE(Lookup(ip, serv, port, false), strprintf("failed to resolve: %s:%i", ip, port));
diff --git a/src/test/fuzz/asmap_direct.cpp b/src/test/fuzz/asmap_direct.cpp
index 8b7822dc16..8ca4de3919 100644
--- a/src/test/fuzz/asmap_direct.cpp
+++ b/src/test/fuzz/asmap_direct.cpp
@@ -2,8 +2,9 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <test/fuzz/fuzz.h>
+#include <netaddress.h>
 #include <util/asmap.h>
+#include <test/fuzz/fuzz.h>
 
 #include <cstdint>
 #include <optional>
diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp
index 7be8b13743..cf2fa33744 100644
--- a/src/test/fuzz/netbase_dns_lookup.cpp
+++ b/src/test/fuzz/netbase_dns_lookup.cpp
@@ -18,7 +18,7 @@ FUZZ_TARGET(netbase_dns_lookup)
     const std::string name = fuzzed_data_provider.ConsumeRandomLengthString(512);
     const unsigned int max_results = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
     const bool allow_lookup = fuzzed_data_provider.ConsumeBool();
-    const int default_port = fuzzed_data_provider.ConsumeIntegral<int>();
+    const uint16_t default_port = fuzzed_data_provider.ConsumeIntegral<uint16_t>();
 
     auto fuzzed_dns_lookup_function = [&](const std::string&, bool) {
         std::vector<CNetAddr> resolved_addresses;
diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp
index e5cc4cabe5..c3a6eed089 100644
--- a/src/test/fuzz/socks5.cpp
+++ b/src/test/fuzz/socks5.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <netaddress.h>
 #include <netbase.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
@@ -38,7 +39,7 @@ FUZZ_TARGET_INIT(socks5, initialize_socks5)
     // This Socks5(...) fuzzing harness would have caught CVE-2017-18350 within
     // a few seconds of fuzzing.
     (void)Socks5(fuzzed_data_provider.ConsumeRandomLengthString(512),
-                 fuzzed_data_provider.ConsumeIntegral<int>(),
+                 fuzzed_data_provider.ConsumeIntegral<uint16_t>(),
                  fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr,
                  fuzzed_sock);
 }
diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp
index 93b4948a2f..286375f7ae 100644
--- a/src/test/fuzz/string.cpp
+++ b/src/test/fuzz/string.cpp
@@ -5,6 +5,7 @@
 #include <blockfilter.h>
 #include <clientversion.h>
 #include <logging.h>
+#include <netaddress.h>
 #include <netbase.h>
 #include <outputtype.h>
 #include <rpc/client.h>
@@ -82,7 +83,7 @@ FUZZ_TARGET(string)
 #ifndef WIN32
     (void)ShellEscape(random_string_1);
 #endif // WIN32
-    int port_out;
+    uint16_t port_out;
     std::string host_out;
     SplitHostPort(random_string_1, port_out, host_out);
     (void)TimingResistantEqual(random_string_1, random_string_2);
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 3172f119bd..c336705d50 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -8,6 +8,7 @@
 #include <clientversion.h>
 #include <cstdint>
 #include <net.h>
+#include <netaddress.h>
 #include <netbase.h>
 #include <serialize.h>
 #include <span.h>
@@ -91,7 +92,7 @@ BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup)
 BOOST_AUTO_TEST_CASE(cnode_listen_port)
 {
     // test default
-    uint16_t port = GetListenPort();
+    uint16_t port{GetListenPort()};
     BOOST_CHECK(port == Params().GetDefaultPort());
     // test set port
     uint16_t altPort = 12345;
diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index 66ad7bb5ea..33b56624a8 100644
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <net_permissions.h>
+#include <netaddress.h>
 #include <netbase.h>
 #include <protocol.h>
 #include <serialize.h>
@@ -83,31 +84,31 @@ BOOST_AUTO_TEST_CASE(netbase_properties)
 
 }
 
-bool static TestSplitHost(std::string test, std::string host, int port)
+bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port)
 {
     std::string hostOut;
-    int portOut = -1;
+    uint16_t portOut{0};
     SplitHostPort(test, portOut, hostOut);
     return hostOut == host && port == portOut;
 }
 
 BOOST_AUTO_TEST_CASE(netbase_splithost)
 {
-    BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", -1));
-    BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", -1));
+    BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", 0));
+    BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", 0));
     BOOST_CHECK(TestSplitHost("www.bitcoincore.org:80", "www.bitcoincore.org", 80));
     BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]:80", "www.bitcoincore.org", 80));
-    BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", -1));
+    BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", 0));
     BOOST_CHECK(TestSplitHost("127.0.0.1:8333", "127.0.0.1", 8333));
-    BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", -1));
+    BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", 0));
     BOOST_CHECK(TestSplitHost("[127.0.0.1]:8333", "127.0.0.1", 8333));
-    BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", -1));
+    BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", 0));
     BOOST_CHECK(TestSplitHost("[::ffff:127.0.0.1]:8333", "::ffff:127.0.0.1", 8333));
     BOOST_CHECK(TestSplitHost("[::]:8333", "::", 8333));
-    BOOST_CHECK(TestSplitHost("::8333", "::8333", -1));
+    BOOST_CHECK(TestSplitHost("::8333", "::8333", 0));
     BOOST_CHECK(TestSplitHost(":8333", "", 8333));
     BOOST_CHECK(TestSplitHost("[]:8333", "", 8333));
-    BOOST_CHECK(TestSplitHost("", "", -1));
+    BOOST_CHECK(TestSplitHost("", "", 0));
 }
 
 bool static TestParse(std::string src, std::string canon)