Merge bitcoin/bitcoin#26261: p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`

5c832c3820253affc65c0ed490e26e5b0a4d5c9b p2p, refactor: return `std::optional<CNetAddr>` in `LookupHost` (brunoerg)
34bcdfc6a65de906c65edccdd96fe15219081cd2 p2p, refactor: return vector/optional<CService> in `Lookup` (brunoerg)
7799eb125b7a1146f8251be5d26df574236212a9 p2p, refactor: return `std::vector<CNetAddr>` in `LookupHost` (brunoerg)
5c1774a563dcc237a88df69569cd94fe81e908f7 p2p, refactor: return `std::vector<CNetAddr>` in `LookupIntern` (brunoerg)

Pull request description:

  Continuation of #26078.

  To improve readability instead of returning a bool and passing stuff by reference, this PR changes:

  - `LookupHost` to return `std::vector<CNetAddr>`
  - `LookupHost` to return `std::optional<CNetAddr>`
  - `Lookup` to return `std::vector<CService>`
  - `Lookup` to return `std::optional<CService>`.
  - `LookupIntern` to return `std::vector<CNetAddr>`

  As discussed in #26078, it would be better to avoid using `optional` in some cases, but for specific `Lookup` and `LookupHost` functions it's necessary to use `optional` to verify if they were able to catch some data from their overloaded function.

ACKs for top commit:
  achow101:
    ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b
  stickies-v:
    re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b - just addressing two nits, no other changes
  theStack:
    re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b

Tree-SHA512: ea346fdc54463999646269bd600cd4a1590ef958001d2f0fc2be608ca51e1b4365efccca76dd4972b023e12fcc6e67d226608b0df7beb901bdeadd19948df840

1 files changed, 27 insertions, 22 deletions

diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 54d923e4a4..328e7f81a0 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -33,16 +33,16 @@ 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)
 {
-    CService serv;
-    BOOST_CHECK_MESSAGE(Lookup(ip, serv, port, false), strprintf("failed to resolve: %s:%i", ip, port));
-    return serv;
+    const std::optional<CService> serv{Lookup(ip, port, false)};
+    BOOST_CHECK_MESSAGE(serv.has_value(), strprintf("failed to resolve: %s:%i", ip, port));
+    return serv.value_or(CService{});
 }
 
 
@@ -948,18 +948,23 @@ BOOST_AUTO_TEST_CASE(load_addrman)
 {
     AddrMan addrman{EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)};
 
-    CService addr1, addr2, addr3;
-    BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
-    BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false));
-    BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false));
-    BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false));
-    BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false));
+    std::optional<CService> addr1, addr2, addr3, addr4;
+    addr1 = Lookup("250.7.1.1", 8333, false);
+    BOOST_CHECK(addr1.has_value());
+    addr2 = Lookup("250.7.2.2", 9999, false);
+    BOOST_CHECK(addr2.has_value());
+    addr3 = Lookup("250.7.3.3", 9999, false);
+    BOOST_CHECK(addr3.has_value());
+    addr3 = Lookup("250.7.3.3"s, 9999, false);
+    BOOST_CHECK(addr3.has_value());
+    addr4 = Lookup("250.7.3.3\0example.com"s, 9999, false);
+    BOOST_CHECK(!addr4.has_value());
 
     // Add three addresses to new table.
-    CService source;
-    BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false));
-    std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)};
-    BOOST_CHECK(addrman.Add(addresses, source));
+    const std::optional<CService> source{Lookup("252.5.1.1", 8333, false)};
+    BOOST_CHECK(source.has_value());
+    std::vector<CAddress> addresses{CAddress(addr1.value(), NODE_NONE), CAddress(addr2.value(), NODE_NONE), CAddress(addr3.value(), NODE_NONE)};
+    BOOST_CHECK(addrman.Add(addresses, source.value()));
     BOOST_CHECK(addrman.Size() == 3);
 
     // Test that the de-serialization does not throw an exception.
@@ -1004,12 +1009,12 @@ static CDataStream MakeCorruptPeersDat()
     int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
     s << nUBuckets;
 
-    CService serv;
-    BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false));
-    CAddress addr = CAddress(serv, NODE_NONE);
-    CNetAddr resolved;
-    BOOST_CHECK(LookupHost("252.2.2.2", resolved, false));
-    AddrInfo info = AddrInfo(addr, resolved);
+    const std::optional<CService> serv{Lookup("252.1.1.1", 7777, false)};
+    BOOST_REQUIRE(serv.has_value());
+    CAddress addr = CAddress(serv.value(), NODE_NONE);
+    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;