Skip to content

Commit d1dc6b8

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#23780: refactor, test: update addrman_tests.cpp to use output from AddrMan::Good()
bf4f817 refactor: addrman_select test (josibake) 5a64dc0 refactor: addrman_evictionworks test (josibake) e281fcc refactor: addrman_noevict test (josibake) 8bdd924 refactor: addrman_selecttriedcollisions test (josibake) Pull request description: As a follow-up to #23713 , this PR refactors the remaining tests in `src/tests/addrman_tests.cpp` to use the output from `AddrMan::Good()` where appropriate. ACKs for top commit: naumenkogs: ACK bf4f817 mzumsande: Code Review ACK bf4f817 Tree-SHA512: 93cc127aecff42c1c174daa04911af7e3460a5c40ddf96952fe4a6ab86fa1ff22d66724326abb709008d7f9f79c26c55c6d62753c40059c9ac60f869507ec913
2 parents 1c41fb9 + bf4f817 commit d1dc6b8

File tree

1 file changed

+34
-39
lines changed

1 file changed

+34
-39
lines changed

‎src/test/addrman_tests.cpp

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
194194
BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
195195

196196
// Test: move addr to tried, select from new expected nothing returned.
197-
addrman.Good(CAddress(addr1, NODE_NONE));
197+
BOOST_CHECK(addrman.Good(CAddress(addr1, NODE_NONE)));
198198
BOOST_CHECK_EQUAL(addrman.size(), 1U);
199199
auto addr_ret2 = addrman.Select(newOnly).first;
200200
BOOST_CHECK_EQUAL(addr_ret2.ToString(), "[::]:0");
@@ -220,11 +220,11 @@ BOOST_AUTO_TEST_CASE(addrman_select)
220220
CService addr7 = ResolveService("250.4.6.6", 8333);
221221

222222
BOOST_CHECK(addrman.Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
223-
addrman.Good(CAddress(addr5, NODE_NONE));
223+
BOOST_CHECK(addrman.Good(CAddress(addr5, NODE_NONE)));
224224
BOOST_CHECK(addrman.Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333)));
225-
addrman.Good(CAddress(addr6, NODE_NONE));
225+
BOOST_CHECK(addrman.Good(CAddress(addr6, NODE_NONE)));
226226
BOOST_CHECK(addrman.Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333)));
227-
addrman.Good(CAddress(addr7, NODE_NONE));
227+
BOOST_CHECK(addrman.Good(CAddress(addr7, NODE_NONE)));
228228

229229
// Test: 6 addrs + 1 addr from last test = 7.
230230
BOOST_CHECK_EQUAL(addrman.size(), 7U);
@@ -816,19 +816,21 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
816816
for (unsigned int i = 1; i < 23; i++) {
817817
CService addr = ResolveService("250.1.1." + ToString(i));
818818
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
819-
addrman.Good(addr);
820819

821-
// No collisions yet.
822-
BOOST_CHECK(addrman.size() == i);
820+
// No collisions in tried.
821+
BOOST_CHECK(addrman.Good(addr));
823822
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
824823
}
825824

826825
// Ensure Good handles duplicates well.
826+
// If an address is a duplicate, Good will return false but will not count it as a collision.
827827
for (unsigned int i = 1; i < 23; i++) {
828828
CService addr = ResolveService("250.1.1." + ToString(i));
829-
addrman.Good(addr);
830829

831-
BOOST_CHECK(addrman.size() == 22);
830+
// Unable to add duplicate address to tried table.
831+
BOOST_CHECK(!addrman.Good(addr));
832+
833+
// Verify duplicate address not marked as a collision.
832834
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
833835
}
834836
}
@@ -842,49 +844,45 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
842844
for (unsigned int i = 1; i < 36; i++) {
843845
CService addr = ResolveService("250.1.1." + ToString(i));
844846
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
845-
addrman.Good(addr);
846847

847848
// No collision yet.
848-
BOOST_CHECK(addrman.size() == i);
849-
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
849+
BOOST_CHECK(addrman.Good(addr));
850850
}
851851

852-
// Collision between 36 and 19.
852+
// Collision in tried table between 36 and 19.
853853
CService addr36 = ResolveService("250.1.1.36");
854854
BOOST_CHECK(addrman.Add({CAddress(addr36, NODE_NONE)}, source));
855-
addrman.Good(addr36);
856-
857-
BOOST_CHECK(addrman.size() == 36);
855+
BOOST_CHECK(!addrman.Good(addr36));
858856
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.19:0");
859857

860858
// 36 should be discarded and 19 not evicted.
859+
// This means we keep 19 in the tried table and
860+
// 36 stays in the new table.
861861
addrman.ResolveCollisions();
862862
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
863863

864864
// Lets create two collisions.
865865
for (unsigned int i = 37; i < 59; i++) {
866866
CService addr = ResolveService("250.1.1." + ToString(i));
867867
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
868-
addrman.Good(addr);
869-
870-
BOOST_CHECK(addrman.size() == i);
871-
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
868+
BOOST_CHECK(addrman.Good(addr));
872869
}
873870

874-
// Cause a collision.
871+
// Cause a collision in the tried table.
875872
CService addr59 = ResolveService("250.1.1.59");
876873
BOOST_CHECK(addrman.Add({CAddress(addr59, NODE_NONE)}, source));
877-
addrman.Good(addr59);
878-
BOOST_CHECK(addrman.size() == 59);
874+
BOOST_CHECK(!addrman.Good(addr59));
879875

880876
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.10:0");
881877

882-
// Cause a second collision.
878+
// Cause a second collision in the new table.
883879
BOOST_CHECK(!addrman.Add({CAddress(addr36, NODE_NONE)}, source));
884-
addrman.Good(addr36);
885-
BOOST_CHECK(addrman.size() == 59);
886880

881+
// 36 still cannot be moved from new to tried due to colliding with 19
882+
BOOST_CHECK(!addrman.Good(addr36));
887883
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() != "[::]:0");
884+
885+
// Resolve all collisions.
888886
addrman.ResolveCollisions();
889887
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
890888
}
@@ -903,19 +901,16 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
903901
for (unsigned int i = 1; i < 36; i++) {
904902
CService addr = ResolveService("250.1.1." + ToString(i));
905903
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
906-
addrman.Good(addr);
907904

908905
// No collision yet.
909-
BOOST_CHECK(addrman.size() == i);
910-
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
906+
BOOST_CHECK(addrman.Good(addr));
911907
}
912908

913909
// Collision between 36 and 19.
914910
CService addr = ResolveService("250.1.1.36");
915911
BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source));
916-
addrman.Good(addr);
912+
BOOST_CHECK(!addrman.Good(addr));
917913

918-
BOOST_CHECK_EQUAL(addrman.size(), 36);
919914
auto info = addrman.SelectTriedCollision().first;
920915
BOOST_CHECK_EQUAL(info.ToString(), "250.1.1.19:0");
921916

@@ -926,17 +921,17 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
926921
addrman.ResolveCollisions();
927922
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
928923

929-
// If 36 was swapped for 19, then this should cause no collisions.
930-
BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source));
931-
addrman.Good(addr);
932-
924+
// If 36 was swapped for 19, then adding 36 to tried should fail because we
925+
// are attempting to add a duplicate.
926+
// We check this by verifying Good() returns false and also verifying that
927+
// we have no collisions.
928+
BOOST_CHECK(!addrman.Good(addr));
933929
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
934930

935-
// If we insert 19 it should collide with 36
931+
// 19 should fail as a collision (not a duplicate) if we now attempt to move
932+
// it to the tried table.
936933
CService addr19 = ResolveService("250.1.1.19");
937-
BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source));
938-
addrman.Good(addr19);
939-
934+
BOOST_CHECK(!addrman.Good(addr19));
940935
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.36:0");
941936

942937
addrman.ResolveCollisions();

0 commit comments

Comments
 (0)