Skip to content

Commit 60e0cbd

Browse files
amitiuttarwaramitiuttarwar
committed
[addrman] Merge the two Add() functions
Merge the two definitions of this overloaded function to reduce code duplication.
1 parent 3facf0a commit 60e0cbd

File tree

5 files changed

+42
-68
lines changed

5 files changed

+42
-68
lines changed

‎src/addrman.h‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -517,22 +517,7 @@ class CAddrMan
517517
return vRandom.size();
518518
}
519519

520-
//! Add a single address.
521-
bool Add(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty = 0)
522-
EXCLUSIVE_LOCKS_REQUIRED(!cs)
523-
{
524-
LOCK(cs);
525-
bool fRet = false;
526-
Check();
527-
fRet |= Add_(addr, source, nTimePenalty);
528-
Check();
529-
if (fRet) {
530-
LogPrint(BCLog::ADDRMAN, "Added %s from %s: %i tried, %i new\n", addr.ToStringIPPort(), source.ToString(), nTried, nNew);
531-
}
532-
return fRet;
533-
}
534-
535-
//! Add multiple addresses.
520+
//! Add addresses to addrman's new table.
536521
bool Add(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0)
537522
EXCLUSIVE_LOCKS_REQUIRED(!cs)
538523
{

‎src/rpc/net.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,7 @@ static RPCHelpMan addpeeraddress()
951951
address.nTime = GetAdjustedTime();
952952
// The source address is set equal to the address. This is equivalent to the peer
953953
// announcing itself.
954-
if (node.addrman->Add(address, address)) success = true;
954+
if (node.addrman->Add({address}, address)) success = true;
955955
}
956956

957957
obj.pushKV("success", success);

‎src/test/addrman_tests.cpp‎

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,15 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
126126

127127
// Test: Does Addrman::Add work as expected.
128128
CService addr1 = ResolveService("250.1.1.1", 8333);
129-
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
129+
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
130130
BOOST_CHECK_EQUAL(addrman.size(), 1U);
131131
CAddrInfo addr_ret1 = addrman.Select();
132132
BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
133133

134134
// Test: Does IP address deduplication work correctly.
135135
// Expected dup IP should not be added.
136136
CService addr1_dup = ResolveService("250.1.1.1", 8333);
137-
BOOST_CHECK(!addrman.Add(CAddress(addr1_dup, NODE_NONE), source));
137+
BOOST_CHECK(!addrman.Add({CAddress(addr1_dup, NODE_NONE)}, source));
138138
BOOST_CHECK_EQUAL(addrman.size(), 1U);
139139

140140

@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
145145
// success.
146146

147147
CService addr2 = ResolveService("250.1.1.2", 8333);
148-
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
148+
BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
149149
BOOST_CHECK(addrman.size() >= 1);
150150

151151
// Test: AddrMan::Clear() should empty the new table.
@@ -172,11 +172,11 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
172172

173173
// Test 7; Addr with same IP but diff port does not replace existing addr.
174174
CService addr1 = ResolveService("250.1.1.1", 8333);
175-
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
175+
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
176176
BOOST_CHECK_EQUAL(addrman.size(), 1U);
177177

178178
CService addr1_port = ResolveService("250.1.1.1", 8334);
179-
BOOST_CHECK(!addrman.Add(CAddress(addr1_port, NODE_NONE), source));
179+
BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
180180
BOOST_CHECK_EQUAL(addrman.size(), 1U);
181181
CAddrInfo addr_ret2 = addrman.Select();
182182
BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333");
@@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
199199

200200
// Test: Select from new with 1 addr in new.
201201
CService addr1 = ResolveService("250.1.1.1", 8333);
202-
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
202+
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
203203
BOOST_CHECK_EQUAL(addrman.size(), 1U);
204204

205205
bool newOnly = true;
@@ -223,20 +223,20 @@ BOOST_AUTO_TEST_CASE(addrman_select)
223223
CService addr3 = ResolveService("250.3.2.2", 9999);
224224
CService addr4 = ResolveService("250.3.3.3", 9999);
225225

226-
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), ResolveService("250.3.1.1", 8333)));
227-
BOOST_CHECK(addrman.Add(CAddress(addr3, NODE_NONE), ResolveService("250.3.1.1", 8333)));
228-
BOOST_CHECK(addrman.Add(CAddress(addr4, NODE_NONE), ResolveService("250.4.1.1", 8333)));
226+
BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
227+
BOOST_CHECK(addrman.Add({CAddress(addr3, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
228+
BOOST_CHECK(addrman.Add({CAddress(addr4, NODE_NONE)}, ResolveService("250.4.1.1", 8333)));
229229

230230
// Add three addresses to tried table.
231231
CService addr5 = ResolveService("250.4.4.4", 8333);
232232
CService addr6 = ResolveService("250.4.5.5", 7777);
233233
CService addr7 = ResolveService("250.4.6.6", 8333);
234234

235-
BOOST_CHECK(addrman.Add(CAddress(addr5, NODE_NONE), ResolveService("250.3.1.1", 8333)));
235+
BOOST_CHECK(addrman.Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
236236
addrman.Good(CAddress(addr5, NODE_NONE));
237-
BOOST_CHECK(addrman.Add(CAddress(addr6, NODE_NONE), ResolveService("250.3.1.1", 8333)));
237+
BOOST_CHECK(addrman.Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
238238
addrman.Good(CAddress(addr6, NODE_NONE));
239-
BOOST_CHECK(addrman.Add(CAddress(addr7, NODE_NONE), ResolveService("250.1.1.3", 8333)));
239+
BOOST_CHECK(addrman.Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333)));
240240
addrman.Good(CAddress(addr7, NODE_NONE));
241241

242242
// Test: 6 addrs + 1 addr from last test = 7.
@@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
262262

263263
while (num_addrs < 22) { // Magic number! 250.1.1.1 - 250.1.1.22 do not collide with deterministic key = 1
264264
CService addr = ResolveService("250.1.1." + ToString(++num_addrs));
265-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
265+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
266266

267267
//Test: No collision in new table yet.
268268
BOOST_CHECK_EQUAL(addrman.size(), num_addrs);
@@ -271,11 +271,11 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
271271
//Test: new table collision!
272272
CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs));
273273
uint32_t collisions{1};
274-
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
274+
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
275275
BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
276276

277277
CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs));
278-
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
278+
BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
279279
BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
280280
}
281281

@@ -291,7 +291,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
291291

292292
while (num_addrs < 64) { // Magic number! 250.1.1.1 - 250.1.1.64 do not collide with deterministic key = 1
293293
CService addr = ResolveService("250.1.1." + ToString(++num_addrs));
294-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
294+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
295295
addrman.Good(CAddress(addr, NODE_NONE));
296296

297297
//Test: No collision in tried table yet.
@@ -301,11 +301,11 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
301301
//Test: tried table collision!
302302
CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs));
303303
uint32_t collisions{1};
304-
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
304+
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
305305
BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
306306

307307
CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs));
308-
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
308+
BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
309309
BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions);
310310
}
311311

@@ -322,9 +322,9 @@ BOOST_AUTO_TEST_CASE(addrman_find)
322322
CNetAddr source1 = ResolveIP("250.1.2.1");
323323
CNetAddr source2 = ResolveIP("250.1.2.2");
324324

325-
BOOST_CHECK(addrman.Add(addr1, source1));
326-
BOOST_CHECK(!addrman.Add(addr2, source2));
327-
BOOST_CHECK(addrman.Add(addr3, source1));
325+
BOOST_CHECK(addrman.Add({addr1}, source1));
326+
BOOST_CHECK(!addrman.Add({addr2}, source2));
327+
BOOST_CHECK(addrman.Add({addr3}, source1));
328328

329329
// Test: ensure Find returns an IP matching what we searched on.
330330
CAddrInfo* info1 = addrman.Find(addr1);
@@ -406,11 +406,8 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
406406
CNetAddr source2 = ResolveIP("250.2.3.3");
407407

408408
// Test: Ensure GetAddr works with new addresses.
409-
BOOST_CHECK(addrman.Add(addr1, source1));
410-
BOOST_CHECK(addrman.Add(addr2, source2));
411-
BOOST_CHECK(addrman.Add(addr3, source1));
412-
BOOST_CHECK(addrman.Add(addr4, source2));
413-
BOOST_CHECK(addrman.Add(addr5, source1));
409+
BOOST_CHECK(addrman.Add({addr1, addr3, addr5}, source1));
410+
BOOST_CHECK(addrman.Add({addr2, addr4}, source2));
414411

415412
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U);
416413
// Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down.
@@ -431,7 +428,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
431428

432429
// Ensure that for all addrs in addrman, isTerrible == false.
433430
addr.nTime = GetAdjustedTime();
434-
addrman.Add(addr, ResolveIP(strAddr));
431+
addrman.Add({addr}, ResolveIP(strAddr));
435432
if (i % 8 == 0)
436433
addrman.Good(addr);
437434
}
@@ -726,7 +723,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
726723
CNetAddr default_source;
727724

728725

729-
addrman_asmap1.Add(addr, default_source);
726+
addrman_asmap1.Add({addr}, default_source);
730727

731728
stream << addrman_asmap1;
732729
// serizalizing/deserializing addrman with the same asmap
@@ -751,7 +748,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
751748
// deserializing non-asmaped peers.dat to asmaped addrman
752749
addrman_asmap1.Clear();
753750
addrman_noasmap.Clear();
754-
addrman_noasmap.Add(addr, default_source);
751+
addrman_noasmap.Add({addr}, default_source);
755752
stream << addrman_noasmap;
756753
stream >> addrman_asmap1;
757754
std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1.GetBucketAndEntry(addr);
@@ -765,8 +762,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
765762
addrman_noasmap.Clear();
766763
CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
767764
CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE);
768-
addrman_noasmap.Add(addr, default_source);
769-
addrman_noasmap.Add(addr2, default_source);
765+
addrman_noasmap.Add({addr, addr2}, default_source);
770766
std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap.GetBucketAndEntry(addr1);
771767
std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap.GetBucketAndEntry(addr2);
772768
BOOST_CHECK(bucketAndEntry_noasmap_addr1.first != bucketAndEntry_noasmap_addr2.first);
@@ -833,7 +829,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
833829
CNetAddr source = ResolveIP("252.2.2.2");
834830
for (unsigned int i = 1; i < 23; i++) {
835831
CService addr = ResolveService("250.1.1."+ToString(i));
836-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
832+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
837833
addrman.Good(addr);
838834

839835
// No collisions yet.
@@ -860,7 +856,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
860856
CNetAddr source = ResolveIP("252.2.2.2");
861857
for (unsigned int i = 1; i < 36; i++) {
862858
CService addr = ResolveService("250.1.1."+ToString(i));
863-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
859+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
864860
addrman.Good(addr);
865861

866862
// No collision yet.
@@ -870,7 +866,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
870866

871867
// Collision between 36 and 19.
872868
CService addr36 = ResolveService("250.1.1.36");
873-
BOOST_CHECK(addrman.Add(CAddress(addr36, NODE_NONE), source));
869+
BOOST_CHECK(addrman.Add({CAddress(addr36, NODE_NONE)}, source));
874870
addrman.Good(addr36);
875871

876872
BOOST_CHECK(addrman.size() == 36);
@@ -883,7 +879,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
883879
// Lets create two collisions.
884880
for (unsigned int i = 37; i < 59; i++) {
885881
CService addr = ResolveService("250.1.1."+ToString(i));
886-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
882+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
887883
addrman.Good(addr);
888884

889885
BOOST_CHECK(addrman.size() == i);
@@ -892,14 +888,14 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
892888

893889
// Cause a collision.
894890
CService addr59 = ResolveService("250.1.1.59");
895-
BOOST_CHECK(addrman.Add(CAddress(addr59, NODE_NONE), source));
891+
BOOST_CHECK(addrman.Add({CAddress(addr59, NODE_NONE)}, source));
896892
addrman.Good(addr59);
897893
BOOST_CHECK(addrman.size() == 59);
898894

899895
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.10:0");
900896

901897
// Cause a second collision.
902-
BOOST_CHECK(!addrman.Add(CAddress(addr36, NODE_NONE), source));
898+
BOOST_CHECK(!addrman.Add({CAddress(addr36, NODE_NONE)}, source));
903899
addrman.Good(addr36);
904900
BOOST_CHECK(addrman.size() == 59);
905901

@@ -921,7 +917,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
921917
CNetAddr source = ResolveIP("252.2.2.2");
922918
for (unsigned int i = 1; i < 36; i++) {
923919
CService addr = ResolveService("250.1.1."+ToString(i));
924-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
920+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
925921
addrman.Good(addr);
926922

927923
// No collision yet.
@@ -931,7 +927,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
931927

932928
// Collision between 36 and 19.
933929
CService addr = ResolveService("250.1.1.36");
934-
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
930+
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
935931
addrman.Good(addr);
936932

937933
BOOST_CHECK_EQUAL(addrman.size(), 36);
@@ -946,14 +942,14 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
946942
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
947943

948944
// If 36 was swapped for 19, then this should cause no collisions.
949-
BOOST_CHECK(!addrman.Add(CAddress(addr, NODE_NONE), source));
945+
BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source));
950946
addrman.Good(addr);
951947

952948
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
953949

954950
// If we insert 19 it should collide with 36
955951
CService addr19 = ResolveService("250.1.1.19");
956-
BOOST_CHECK(!addrman.Add(CAddress(addr19, NODE_NONE), source));
952+
BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source));
957953
addrman.Good(addr19);
958954

959955
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.36:0");

‎src/test/fuzz/addrman.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,6 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
252252
[&] {
253253
(void)addr_man.SelectTriedCollision();
254254
},
255-
[&] {
256-
const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
257-
const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider);
258-
if (opt_address && opt_net_addr) {
259-
addr_man.Add(*opt_address, *opt_net_addr, fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, 100000000));
260-
}
261-
},
262255
[&] {
263256
std::vector<CAddress> addresses;
264257
while (fuzzed_data_provider.ConsumeBool()) {

‎src/test/net_tests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
112112
// Add three addresses to new table.
113113
CService source;
114114
BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false));
115-
BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr1, NODE_NONE), source));
116-
BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr2, NODE_NONE), source));
117-
BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr3, NODE_NONE), source));
115+
std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)};
116+
BOOST_CHECK(addrmanUncorrupted.Add(addresses, source));
117+
BOOST_CHECK(addrmanUncorrupted.size() == 3);
118118

119119
// Test that the de-serialization does not throw an exception.
120120
CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);

0 commit comments

Comments
 (0)