diff options
author | Amiti Uttarwar <amiti@uttarwar.org> | 2021-07-22 17:00:36 -0700 |
---|---|---|
committer | Amiti Uttarwar <amiti@uttarwar.org> | 2021-08-15 22:35:13 -0700 |
commit | 60e0cbdd574bb9109bcad1e0c27c7936a534a0e7 (patch) | |
tree | b3c49dced04e42234b507682e033706b99d12232 | |
parent | 3facf0a8ae99a03f5e2fb709b8d86c47328d363c (diff) | |
download | bitcoin-60e0cbdd574bb9109bcad1e0c27c7936a534a0e7.tar.xz |
[addrman] Merge the two Add() functions
Merge the two definitions of this overloaded function to reduce code
duplication.
-rw-r--r-- | src/addrman.h | 17 | ||||
-rw-r--r-- | src/rpc/net.cpp | 2 | ||||
-rw-r--r-- | src/test/addrman_tests.cpp | 78 | ||||
-rw-r--r-- | src/test/fuzz/addrman.cpp | 7 | ||||
-rw-r--r-- | src/test/net_tests.cpp | 6 |
5 files changed, 42 insertions, 68 deletions
diff --git a/src/addrman.h b/src/addrman.h index 3ee8c3ee09..fa155fb00a 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -517,22 +517,7 @@ public: return vRandom.size(); } - //! Add a single address. - bool Add(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty = 0) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - bool fRet = false; - Check(); - fRet |= Add_(addr, source, nTimePenalty); - Check(); - if (fRet) { - LogPrint(BCLog::ADDRMAN, "Added %s from %s: %i tried, %i new\n", addr.ToStringIPPort(), source.ToString(), nTried, nNew); - } - return fRet; - } - - //! Add multiple addresses. + //! Add addresses to addrman's new table. bool Add(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0) EXCLUSIVE_LOCKS_REQUIRED(!cs) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 4d066b8957..861b889118 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -951,7 +951,7 @@ static RPCHelpMan addpeeraddress() address.nTime = GetAdjustedTime(); // The source address is set equal to the address. This is equivalent to the peer // announcing itself. - if (node.addrman->Add(address, address)) success = true; + if (node.addrman->Add({address}, address)) success = true; } obj.pushKV("success", success); 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); |