Merge bitcoin/bitcoin#22627: [addrman] De-duplicate Add() function

60e0cbdd574bb9109bcad1e0c27c7936a534a0e7 [addrman] Merge the two Add() functions (Amiti Uttarwar)

Pull request description:

  This PR merges the two definitions of this overloaded function to reduce code duplication.

  When these functions were introduced in https://github.com/bitcoin/bitcoin/commit/5fee401fe14aa6459428a26a82f764db70a6a0b9, there were multiple places that invoked `Add()` with a single addr and a vector of addrs each, so it made sense to overload the function. I could see how the small difference in log statement was more meaningful when a peer was added via IRC :)

  Now, the definition of `Add()` that takes in a single address is only invoked from the hidden/test-only RPC `addpeeraddress`. These changes should not cause any observable difference, and are covered by the existing tests that use this RPC endpoint.

ACKs for top commit:
  jnewbery:
    Code review ACK 60e0cbdd574bb9109bcad1e0c27c7936a534a0e7
  Zero-1729:
    crACK 60e0cbd
  fanquake:
    ACK 60e0cbdd574bb9109bcad1e0c27c7936a534a0e7

Tree-SHA512: 782fb2ac6d2d403ba7d7ff543197ca42b610b9a8806952d271e57e2ee3527ad1a94af4ebbad5371b5e95d77df07c56ccc8c1d5a2c82cdecb0d2b5085b3bdd5ee

3 files changed, 40 insertions, 51 deletions

diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 3c64461605..cead0489b0 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
 
     // Test: Does Addrman::Add work as expected.
     CService addr1 = ResolveService("250.1.1.1", 8333);
-    BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), 1U);
     CAddrInfo addr_ret1 = addrman.Select();
     BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
@@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
     // Test: Does IP address deduplication work correctly.
     //  Expected dup IP should not be added.
     CService addr1_dup = ResolveService("250.1.1.1", 8333);
-    BOOST_CHECK(!addrman.Add(CAddress(addr1_dup, NODE_NONE), source));
+    BOOST_CHECK(!addrman.Add({CAddress(addr1_dup, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), 1U);
 
 
@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
     // success.
 
     CService addr2 = ResolveService("250.1.1.2", 8333);
-    BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
     BOOST_CHECK(addrman.size() >= 1);
 
     // Test: AddrMan::Clear() should empty the new table.
@@ -172,11 +172,11 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
 
     // Test 7; Addr with same IP but diff port does not replace existing addr.
     CService addr1 = ResolveService("250.1.1.1", 8333);
-    BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), 1U);
 
     CService addr1_port = ResolveService("250.1.1.1", 8334);
-    BOOST_CHECK(!addrman.Add(CAddress(addr1_port, NODE_NONE), source));
+    BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), 1U);
     CAddrInfo addr_ret2 = addrman.Select();
     BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333");
@@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
 
     // Test: Select from new with 1 addr in new.
     CService addr1 = ResolveService("250.1.1.1", 8333);
-    BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), 1U);
 
     bool newOnly = true;
@@ -223,20 +223,20 @@ BOOST_AUTO_TEST_CASE(addrman_select)
     CService addr3 = ResolveService("250.3.2.2", 9999);
     CService addr4 = ResolveService("250.3.3.3", 9999);
 
-    BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), ResolveService("250.3.1.1", 8333)));
-    BOOST_CHECK(addrman.Add(CAddress(addr3, NODE_NONE), ResolveService("250.3.1.1", 8333)));
-    BOOST_CHECK(addrman.Add(CAddress(addr4, NODE_NONE), ResolveService("250.4.1.1", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr3, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr4, NODE_NONE)}, ResolveService("250.4.1.1", 8333)));
 
     // Add three addresses to tried table.
     CService addr5 = ResolveService("250.4.4.4", 8333);
     CService addr6 = ResolveService("250.4.5.5", 7777);
     CService addr7 = ResolveService("250.4.6.6", 8333);
 
-    BOOST_CHECK(addrman.Add(CAddress(addr5, NODE_NONE), ResolveService("250.3.1.1", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
     addrman.Good(CAddress(addr5, NODE_NONE));
-    BOOST_CHECK(addrman.Add(CAddress(addr6, NODE_NONE), ResolveService("250.3.1.1", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
     addrman.Good(CAddress(addr6, NODE_NONE));
-    BOOST_CHECK(addrman.Add(CAddress(addr7, NODE_NONE), ResolveService("250.1.1.3", 8333)));
+    BOOST_CHECK(addrman.Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333)));
     addrman.Good(CAddress(addr7, NODE_NONE));
 
     // Test: 6 addrs + 1 addr from last test = 7.
@@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
 
     while (num_addrs < 22) {  // Magic number! 250.1.1.1 - 250.1.1.22 do not collide with deterministic key = 1
         CService addr = ResolveService("250.1.1." + ToString(++num_addrs));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
 
         //Test: No collision in new table yet.
         BOOST_CHECK_EQUAL(addrman.size(), num_addrs);
@@ -271,11 +271,11 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
     //Test: new table collision!
     CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs));
     uint32_t collisions{1};
-    BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
 
     CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs));
-    BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
 }
 
@@ -291,7 +291,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
 
     while (num_addrs < 64) {  // Magic number! 250.1.1.1 - 250.1.1.64 do not collide with deterministic key = 1
         CService addr = ResolveService("250.1.1." + ToString(++num_addrs));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
         addrman.Good(CAddress(addr, NODE_NONE));
 
         //Test: No collision in tried table yet.
@@ -301,11 +301,11 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
     //Test: tried table collision!
     CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs));
     uint32_t collisions{1};
-    BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
 
     CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs));
-    BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
     BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
 }
 
@@ -322,9 +322,9 @@ BOOST_AUTO_TEST_CASE(addrman_find)
     CNetAddr source1 = ResolveIP("250.1.2.1");
     CNetAddr source2 = ResolveIP("250.1.2.2");
 
-    BOOST_CHECK(addrman.Add(addr1, source1));
-    BOOST_CHECK(!addrman.Add(addr2, source2));
-    BOOST_CHECK(addrman.Add(addr3, source1));
+    BOOST_CHECK(addrman.Add({addr1}, source1));
+    BOOST_CHECK(!addrman.Add({addr2}, source2));
+    BOOST_CHECK(addrman.Add({addr3}, source1));
 
     // Test: ensure Find returns an IP matching what we searched on.
     CAddrInfo* info1 = addrman.Find(addr1);
@@ -406,11 +406,8 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
     CNetAddr source2 = ResolveIP("250.2.3.3");
 
     // Test: Ensure GetAddr works with new addresses.
-    BOOST_CHECK(addrman.Add(addr1, source1));
-    BOOST_CHECK(addrman.Add(addr2, source2));
-    BOOST_CHECK(addrman.Add(addr3, source1));
-    BOOST_CHECK(addrman.Add(addr4, source2));
-    BOOST_CHECK(addrman.Add(addr5, source1));
+    BOOST_CHECK(addrman.Add({addr1, addr3, addr5}, source1));
+    BOOST_CHECK(addrman.Add({addr2, addr4}, source2));
 
     BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U);
     // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down.
@@ -431,7 +428,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
 
         // Ensure that for all addrs in addrman, isTerrible == false.
         addr.nTime = GetAdjustedTime();
-        addrman.Add(addr, ResolveIP(strAddr));
+        addrman.Add({addr}, ResolveIP(strAddr));
         if (i % 8 == 0)
             addrman.Good(addr);
     }
@@ -726,7 +723,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     CNetAddr default_source;
 
 
-    addrman_asmap1.Add(addr, default_source);
+    addrman_asmap1.Add({addr}, default_source);
 
     stream << addrman_asmap1;
     // serizalizing/deserializing addrman with the same asmap
@@ -751,7 +748,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     // deserializing non-asmaped peers.dat to asmaped addrman
     addrman_asmap1.Clear();
     addrman_noasmap.Clear();
-    addrman_noasmap.Add(addr, default_source);
+    addrman_noasmap.Add({addr}, default_source);
     stream << addrman_noasmap;
     stream >> addrman_asmap1;
     std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1.GetBucketAndEntry(addr);
@@ -765,8 +762,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     addrman_noasmap.Clear();
     CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
     CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE);
-    addrman_noasmap.Add(addr, default_source);
-    addrman_noasmap.Add(addr2, default_source);
+    addrman_noasmap.Add({addr, addr2}, default_source);
     std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap.GetBucketAndEntry(addr1);
     std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap.GetBucketAndEntry(addr2);
     BOOST_CHECK(bucketAndEntry_noasmap_addr1.first != bucketAndEntry_noasmap_addr2.first);
@@ -833,7 +829,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
     CNetAddr source = ResolveIP("252.2.2.2");
     for (unsigned int i = 1; i < 23; i++) {
         CService addr = ResolveService("250.1.1."+ToString(i));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
         addrman.Good(addr);
 
         // No collisions yet.
@@ -860,7 +856,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
     CNetAddr source = ResolveIP("252.2.2.2");
     for (unsigned int i = 1; i < 36; i++) {
         CService addr = ResolveService("250.1.1."+ToString(i));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
         addrman.Good(addr);
 
         // No collision yet.
@@ -870,7 +866,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
 
     // Collision between 36 and 19.
     CService addr36 = ResolveService("250.1.1.36");
-    BOOST_CHECK(addrman.Add(CAddress(addr36, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr36, NODE_NONE)}, source));
     addrman.Good(addr36);
 
     BOOST_CHECK(addrman.size() == 36);
@@ -883,7 +879,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
     // Lets create two collisions.
     for (unsigned int i = 37; i < 59; i++) {
         CService addr = ResolveService("250.1.1."+ToString(i));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
         addrman.Good(addr);
 
         BOOST_CHECK(addrman.size() == i);
@@ -892,14 +888,14 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
 
     // Cause a collision.
     CService addr59 = ResolveService("250.1.1.59");
-    BOOST_CHECK(addrman.Add(CAddress(addr59, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr59, NODE_NONE)}, source));
     addrman.Good(addr59);
     BOOST_CHECK(addrman.size() == 59);
 
     BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.10:0");
 
     // Cause a second collision.
-    BOOST_CHECK(!addrman.Add(CAddress(addr36, NODE_NONE), source));
+    BOOST_CHECK(!addrman.Add({CAddress(addr36, NODE_NONE)}, source));
     addrman.Good(addr36);
     BOOST_CHECK(addrman.size() == 59);
 
@@ -921,7 +917,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     CNetAddr source = ResolveIP("252.2.2.2");
     for (unsigned int i = 1; i < 36; i++) {
         CService addr = ResolveService("250.1.1."+ToString(i));
-        BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+        BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
         addrman.Good(addr);
 
         // No collision yet.
@@ -931,7 +927,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
 
     // Collision between 36 and 19.
     CService addr = ResolveService("250.1.1.36");
-    BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
+    BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
     addrman.Good(addr);
 
     BOOST_CHECK_EQUAL(addrman.size(), 36);
@@ -946,14 +942,14 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
 
     // If 36 was swapped for 19, then this should cause no collisions.
-    BOOST_CHECK(!addrman.Add(CAddress(addr, NODE_NONE), source));
+    BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source));
     addrman.Good(addr);
 
     BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
 
     // If we insert 19 it should collide with 36
     CService addr19 = ResolveService("250.1.1.19");
-    BOOST_CHECK(!addrman.Add(CAddress(addr19, NODE_NONE), source));
+    BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source));
     addrman.Good(addr19);
 
     BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.36:0");
diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp
index 60fba5730a..bc41180a8f 100644
--- a/src/test/fuzz/addrman.cpp
+++ b/src/test/fuzz/addrman.cpp
@@ -253,13 +253,6 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
                 (void)addr_man.SelectTriedCollision();
             },
             [&] {
-                const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
-                const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider);
-                if (opt_address && opt_net_addr) {
-                    addr_man.Add(*opt_address, *opt_net_addr, fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, 100000000));
-                }
-            },
-            [&] {
                 std::vector<CAddress> addresses;
                 while (fuzzed_data_provider.ConsumeBool()) {
                     const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 1915f9c7d5..9ac200cbc7 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -112,9 +112,9 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
     // Add three addresses to new table.
     CService source;
     BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false));
-    BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr1, NODE_NONE), source));
-    BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr2, NODE_NONE), source));
-    BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr3, NODE_NONE), source));
+    std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)};
+    BOOST_CHECK(addrmanUncorrupted.Add(addresses, source));
+    BOOST_CHECK(addrmanUncorrupted.size() == 3);
 
     // Test that the de-serialization does not throw an exception.
     CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);