p2p, refactor: return std::optional<CNetAddr> in LookupHost

Author: brunoerg

src/bench/addrman.cpp

@@ -72,13 +72,6 @@ static void FillAddrMan(AddrMan& addrman)
     AddAddressesToAddrMan(addrman);
 }
 
-static CNetAddr ResolveIP(const std::string& ip)
-{
-    CNetAddr addr;
-    LookupHost(ip, addr, false);
-    return addr;
-}
-
 /* Benchmarks */
 
 static void AddrManAdd(benchmark::Bench& bench)
@@ -128,7 +121,7 @@ static void AddrManSelectByNetwork(benchmark::Bench& bench)
     i2p_service.SetSpecial("udhdrtrcetjm5sxzskjyr5ztpeszydbh4dpl3pl4utgqqw2v4jna.b32.i2p");
     CAddress i2p_address(i2p_service, NODE_NONE);
     i2p_address.nTime = Now<NodeSeconds>();
-    CNetAddr source = ResolveIP("252.2.2.2");
+    const CNetAddr source{LookupHost("252.2.2.2", false).value()};
     addrman.Add({i2p_address}, source);
 
     FillAddrMan(addrman);

src/httpserver.cpp

@@ -170,12 +170,8 @@ static bool ClientAllowed(const CNetAddr& netaddr)
 static bool InitHTTPAllowList()
 {
     rpc_allow_subnets.clear();
-    CNetAddr localv4;
-    CNetAddr localv6;
-    LookupHost("127.0.0.1", localv4, false);
-    LookupHost("::1", localv6, false);
-    rpc_allow_subnets.push_back(CSubNet(localv4, 8));      // always allow IPv4 local subnet
-    rpc_allow_subnets.push_back(CSubNet(localv6));         // always allow IPv6 localhost
+    rpc_allow_subnets.push_back(CSubNet{LookupHost("127.0.0.1", false).value(), 8});  // always allow IPv4 local subnet
+    rpc_allow_subnets.push_back(CSubNet{LookupHost("::1", false).value()});  // always allow IPv6 localhost
     for (const std::string& strAllow : gArgs.GetArgs("-rpcallowip")) {
         CSubNet subnet;
         LookupSubNet(strAllow, subnet);
@@ -338,8 +334,8 @@ static bool HTTPBindAddresses(struct evhttp* http)
         LogPrintf("Binding RPC on address %s port %i\n", i->first, i->second);
         evhttp_bound_socket *bind_handle = evhttp_bind_socket_with_handle(http, i->first.empty() ? nullptr : i->first.c_str(), i->second);
         if (bind_handle) {
-            CNetAddr addr;
-            if (i->first.empty() || (LookupHost(i->first, addr, false) && addr.IsBindAny())) {
+            const std::optional<CNetAddr> addr{LookupHost(i->first, false)};
+            if (i->first.empty() || (addr.has_value() && addr->IsBindAny())) {
                 LogPrintf("WARNING: the RPC server is not safe to expose to untrusted networks such as the public internet\n");
             }
             boundSockets.push_back(bind_handle);

src/mapport.cpp

@@ -175,10 +175,10 @@ static bool ProcessUpnp()
                 LogPrintf("UPnP: GetExternalIPAddress() returned %d\n", r);
             } else {
                 if (externalIPAddress[0]) {
-                    CNetAddr resolved;
-                    if (LookupHost(externalIPAddress, resolved, false)) {
-                        LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToStringAddr());
-                        AddLocal(resolved, LOCAL_MAPPED);
+                    std::optional<CNetAddr> resolved{LookupHost(externalIPAddress, false)};
+                    if (resolved.has_value()) {
+                        LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved->ToStringAddr());
+                        AddLocal(resolved.value(), LOCAL_MAPPED);
                     }
                 } else {
                     LogPrintf("UPnP: GetExternalIPAddress failed.\n");

src/netbase.cpp

@@ -173,16 +173,10 @@ std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolut
     return LookupIntern(strHost, nMaxSolutions, fAllowLookup, dns_lookup_function);
 }
 
-bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function)
+std::optional<CNetAddr> LookupHost(const std::string& name, bool fAllowLookup, DNSLookupFn dns_lookup_function)
 {
-    if (!ContainsNoNUL(name)) {
-        return false;
-    }
     const std::vector<CNetAddr> addresses{LookupHost(name, 1, fAllowLookup, dns_lookup_function)};
-    if(addresses.empty())
-        return false;
-    addr = addresses.front();
-    return true;
+    return addresses.empty() ? std::nullopt : std::make_optional(addresses.front());
 }
 
 std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
@@ -669,27 +663,27 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out)
 
     const size_t slash_pos{subnet_str.find_last_of('/')};
     const std::string str_addr{subnet_str.substr(0, slash_pos)};
-    CNetAddr addr;
+    const std::optional<CNetAddr> addr{LookupHost(str_addr, /*fAllowLookup=*/false)};
 
-    if (LookupHost(str_addr, addr, /*fAllowLookup=*/false)) {
+    if (addr.has_value()) {
         if (slash_pos != subnet_str.npos) {
             const std::string netmask_str{subnet_str.substr(slash_pos + 1)};
             uint8_t netmask;
             if (ParseUInt8(netmask_str, &netmask)) {
                 // Valid number; assume CIDR variable-length subnet masking.
-                subnet_out = CSubNet{addr, netmask};
+                subnet_out = CSubNet{addr.value(), netmask};
                 return subnet_out.IsValid();
             } else {
                 // Invalid number; try full netmask syntax. Never allow lookup for netmask.
-                CNetAddr full_netmask;
-                if (LookupHost(netmask_str, full_netmask, /*fAllowLookup=*/false)) {
-                    subnet_out = CSubNet{addr, full_netmask};
+                const std::optional<CNetAddr> full_netmask{LookupHost(netmask_str, /*fAllowLookup=*/false)};
+                if (full_netmask.has_value()) {
+                    subnet_out = CSubNet{addr.value(), full_netmask.value()};
                     return subnet_out.IsValid();
                 }
             }
         } else {
             // Single IP subnet (<ipv4>/32 or <ipv6>/128).
-            subnet_out = CSubNet{addr};
+            subnet_out = CSubNet{addr.value()};
             return subnet_out.IsValid();
         }
     }

src/netbase.h

@@ -117,13 +117,13 @@ std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolut
 /**
  * Resolve a host string to its first corresponding network address.
  *
- * @returns The resulting network addresses to which the specified host
- *          string resolved.
+ * @returns The resulting network address to which the specified host
+ *          string resolved or std::nullopt if host does not resolve to an address.
  *
- * @see LookupHost(const std::string&, uint16_t, bool, DNSLookupFn)
+ * @see LookupHost(const std::string&, unsigned int, bool, DNSLookupFn)
  *      for additional parameter descriptions.
  */
-bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
+std::optional<CNetAddr> LookupHost(const std::string& name, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
 
 /**
  * Resolve a service string to its corresponding service.

src/qt/optionsdialog.cpp

@@ -406,9 +406,8 @@ void OptionsDialog::updateProxyValidationState()
 
 void OptionsDialog::updateDefaultProxyNets()
 {
-    CNetAddr ui_proxy_netaddr;
-    LookupHost(ui->proxyIp->text().toStdString(), ui_proxy_netaddr, /*fAllowLookup=*/false);
-    const CService ui_proxy{ui_proxy_netaddr, ui->proxyPort->text().toUShort()};
+    const std::optional<CNetAddr> ui_proxy_netaddr{LookupHost(ui->proxyIp->text().toStdString(), /*fAllowLookup=*/false)};
+    const CService ui_proxy{ui_proxy_netaddr.value_or(CNetAddr{}), ui->proxyPort->text().toUShort()};
 
     Proxy proxy;
     bool has_proxy;

src/rpc/net.cpp

@@ -713,9 +713,10 @@ static RPCHelpMan setban()
         isSubnet = true;
 
     if (!isSubnet) {
-        CNetAddr resolved;
-        LookupHost(request.params[0].get_str(), resolved, false);
-        netAddr = resolved;
+        const std::optional<CNetAddr> addr{LookupHost(request.params[0].get_str(), false)};
+        if (addr.has_value()) {
+            netAddr = addr.value();
+        }
     }
     else
         LookupSubNet(request.params[0].get_str(), subNet);
@@ -943,11 +944,11 @@ static RPCHelpMan addpeeraddress()
     const bool tried{request.params[2].isNull() ? false : request.params[2].get_bool()};
 
     UniValue obj(UniValue::VOBJ);
-    CNetAddr net_addr;
+    std::optional<CNetAddr> net_addr{LookupHost(addr_string, false)};
     bool success{false};
 
-    if (LookupHost(addr_string, net_addr, false)) {
-        CService service{net_addr, port};
+    if (net_addr.has_value()) {
+        CService service{net_addr.value(), port};
         CAddress address{MaybeFlipIPv6toCJDNS(service), ServiceFlags{NODE_NETWORK | NODE_WITNESS}};
         address.nTime = Now<NodeSeconds>();
         // The source address is set equal to the address. This is equivalent to the peer

src/test/addrman_tests.cpp

@@ -33,9 +33,9 @@ static int32_t GetCheckRatio(const NodeContext& node_ctx)
 
 static CNetAddr ResolveIP(const std::string& ip)
 {
-    CNetAddr addr;
-    BOOST_CHECK_MESSAGE(LookupHost(ip, addr, false), strprintf("failed to resolve: %s", ip));
-    return addr;
+    const std::optional<CNetAddr> addr{LookupHost(ip, false)};
+    BOOST_CHECK_MESSAGE(addr.has_value(), strprintf("failed to resolve: %s", ip));
+    return addr.value_or(CNetAddr{});
 }
 
 static CService ResolveService(const std::string& ip, uint16_t port = 0)
@@ -1012,9 +1012,9 @@ static CDataStream MakeCorruptPeersDat()
     const std::optional<CService> serv{Lookup("252.1.1.1", 7777, false)};
     BOOST_REQUIRE(serv.has_value());
     CAddress addr = CAddress(serv.value(), NODE_NONE);
-    CNetAddr resolved;
-    BOOST_REQUIRE(LookupHost("252.2.2.2", resolved, false));
-    AddrInfo info = AddrInfo(addr, resolved);
+    std::optional<CNetAddr> resolved{LookupHost("252.2.2.2", false)};
+    BOOST_REQUIRE(resolved.has_value());
+    AddrInfo info = AddrInfo(addr, resolved.value());
     s << info;
 
     return s;

src/test/fuzz/netbase_dns_lookup.cpp

@@ -36,9 +36,9 @@ FUZZ_TARGET(netbase_dns_lookup)
         assert(resolved_addresses.size() <= max_results || max_results == 0);
     }
     {
-        CNetAddr resolved_address;
-        if (LookupHost(name, resolved_address, allow_lookup, fuzzed_dns_lookup_function)) {
-            assert(!resolved_address.IsInternal());
+        const std::optional<CNetAddr> resolved_address{LookupHost(name, allow_lookup, fuzzed_dns_lookup_function)};
+        if (resolved_address.has_value()) {
+            assert(!resolved_address.value().IsInternal());
         }
     }
     {

src/test/net_tests.cpp

@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     CNetAddr addr;
 
     // IPv4, INADDR_ANY
-    BOOST_REQUIRE(LookupHost("0.0.0.0", addr, false));
+    addr = LookupHost("0.0.0.0", false).value();
     BOOST_REQUIRE(!addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv4());
 
@@ -144,7 +144,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     BOOST_CHECK_EQUAL(addr.ToStringAddr(), "0.0.0.0");
 
     // IPv4, INADDR_NONE
-    BOOST_REQUIRE(LookupHost("255.255.255.255", addr, false));
+    addr = LookupHost("255.255.255.255", false).value();
     BOOST_REQUIRE(!addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv4());
 
@@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     BOOST_CHECK_EQUAL(addr.ToStringAddr(), "255.255.255.255");
 
     // IPv4, casual
-    BOOST_REQUIRE(LookupHost("12.34.56.78", addr, false));
+    addr = LookupHost("12.34.56.78", false).value();
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv4());
 
@@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     BOOST_CHECK_EQUAL(addr.ToStringAddr(), "12.34.56.78");
 
     // IPv6, in6addr_any
-    BOOST_REQUIRE(LookupHost("::", addr, false));
+    addr = LookupHost("::", false).value();
     BOOST_REQUIRE(!addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
 
@@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     BOOST_CHECK_EQUAL(addr.ToStringAddr(), "::");
 
     // IPv6, casual
-    BOOST_REQUIRE(LookupHost("1122:3344:5566:7788:9900:aabb:ccdd:eeff", addr, false));
+    addr = LookupHost("1122:3344:5566:7788:9900:aabb:ccdd:eeff", false).value();
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
 
@@ -186,14 +186,14 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     // id of "32", return the address as "fe80::1%32".
     const std::string link_local{"fe80::1"};
     const std::string scoped_addr{link_local + "%32"};
-    BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
+    addr = LookupHost(scoped_addr, false).value();
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
     BOOST_CHECK(!addr.IsBindAny());
     BOOST_CHECK_EQUAL(addr.ToStringAddr(), scoped_addr);
 
     // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
-    BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
+    addr = LookupHost(link_local + "%0", false).value();
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
     BOOST_CHECK(!addr.IsBindAny());
@@ -318,10 +318,9 @@ BOOST_AUTO_TEST_CASE(cnetaddr_tostring_canonical_ipv6)
         {"2001:db8:aaaa:bbbb:cccc:dddd:eeee:AaAa", "2001:db8:aaaa:bbbb:cccc:dddd:eeee:aaaa"},
     };
     for (const auto& [input_address, expected_canonical_representation_output] : canonical_representations_ipv6) {
-        CNetAddr net_addr;
-        BOOST_REQUIRE(LookupHost(input_address, net_addr, false));
-        BOOST_REQUIRE(net_addr.IsIPv6());
-        BOOST_CHECK_EQUAL(net_addr.ToStringAddr(), expected_canonical_representation_output);
+        const std::optional<CNetAddr> net_addr{LookupHost(input_address, false)};
+        BOOST_REQUIRE(net_addr.value().IsIPv6());
+        BOOST_CHECK_EQUAL(net_addr.value().ToStringAddr(), expected_canonical_representation_output);
     }
 }
 
@@ -334,12 +333,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v1)
     BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000000000000000");
     s.clear();
 
-    BOOST_REQUIRE(LookupHost("1.2.3.4", addr, false));
+    addr = LookupHost("1.2.3.4", false).value();
     s << addr;
     BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000ffff01020304");
     s.clear();
 
-    BOOST_REQUIRE(LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", addr, false));
+    addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value();
     s << addr;
     BOOST_CHECK_EQUAL(HexStr(s), "1a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b");
     s.clear();
@@ -370,12 +369,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v2)
     BOOST_CHECK_EQUAL(HexStr(s), "021000000000000000000000000000000000");
     s.clear();
 
-    BOOST_REQUIRE(LookupHost("1.2.3.4", addr, false));
+    addr = LookupHost("1.2.3.4", false).value();
     s << addr;
     BOOST_CHECK_EQUAL(HexStr(s), "010401020304");
     s.clear();
 
-    BOOST_REQUIRE(LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", addr, false));
+    addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value();
     s << addr;
     BOOST_CHECK_EQUAL(HexStr(s), "02101a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b");
     s.clear();