Skip to content

Commit f2e5f38

Browse files
amitiuttarwaramitiuttarwar
committed
[move-only] Match ordering of CAddrMan declarations and definitions
Also move `Check` and `ForceCheckAddrman` to be after the `FunctionName_` functions. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
1 parent 5faa7dd commit f2e5f38

File tree

2 files changed

+123
-122
lines changed

2 files changed

+123
-122
lines changed

‎src/addrman.cpp‎

Lines changed: 103 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -753,108 +753,6 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const
753753
}
754754
}
755755

756-
void CAddrMan::Check() const
757-
{
758-
AssertLockHeld(cs);
759-
760-
// Run consistency checks 1 in m_consistency_check_ratio times if enabled
761-
if (m_consistency_check_ratio == 0) return;
762-
if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return;
763-
764-
const int err{ForceCheckAddrman()};
765-
if (err) {
766-
LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err);
767-
assert(false);
768-
}
769-
}
770-
771-
int CAddrMan::ForceCheckAddrman() const
772-
{
773-
AssertLockHeld(cs);
774-
775-
LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size());
776-
777-
std::unordered_set<int> setTried;
778-
std::unordered_map<int, int> mapNew;
779-
780-
if (vRandom.size() != (size_t)(nTried + nNew))
781-
return -7;
782-
783-
for (const auto& entry : mapInfo) {
784-
int n = entry.first;
785-
const CAddrInfo& info = entry.second;
786-
if (info.fInTried) {
787-
if (!info.nLastSuccess)
788-
return -1;
789-
if (info.nRefCount)
790-
return -2;
791-
setTried.insert(n);
792-
} else {
793-
if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS)
794-
return -3;
795-
if (!info.nRefCount)
796-
return -4;
797-
mapNew[n] = info.nRefCount;
798-
}
799-
const auto it{mapAddr.find(info)};
800-
if (it == mapAddr.end() || it->second != n) {
801-
return -5;
802-
}
803-
if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n)
804-
return -14;
805-
if (info.nLastTry < 0)
806-
return -6;
807-
if (info.nLastSuccess < 0)
808-
return -8;
809-
}
810-
811-
if (setTried.size() != (size_t)nTried)
812-
return -9;
813-
if (mapNew.size() != (size_t)nNew)
814-
return -10;
815-
816-
for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) {
817-
for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) {
818-
if (vvTried[n][i] != -1) {
819-
if (!setTried.count(vvTried[n][i]))
820-
return -11;
821-
const auto it{mapInfo.find(vvTried[n][i])};
822-
if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) {
823-
return -17;
824-
}
825-
if (it->second.GetBucketPosition(nKey, false, n) != i) {
826-
return -18;
827-
}
828-
setTried.erase(vvTried[n][i]);
829-
}
830-
}
831-
}
832-
833-
for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
834-
for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) {
835-
if (vvNew[n][i] != -1) {
836-
if (!mapNew.count(vvNew[n][i]))
837-
return -12;
838-
const auto it{mapInfo.find(vvNew[n][i])};
839-
if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) {
840-
return -19;
841-
}
842-
if (--mapNew[vvNew[n][i]] == 0)
843-
mapNew.erase(vvNew[n][i]);
844-
}
845-
}
846-
}
847-
848-
if (setTried.size())
849-
return -13;
850-
if (mapNew.size())
851-
return -15;
852-
if (nKey.IsNull())
853-
return -16;
854-
855-
LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n");
856-
return 0;
857-
}
858756

859757
void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) const
860758
{
@@ -1023,6 +921,109 @@ CAddrInfo CAddrMan::SelectTriedCollision_()
1023921
return mapInfo[id_old];
1024922
}
1025923

924+
void CAddrMan::Check() const
925+
{
926+
AssertLockHeld(cs);
927+
928+
// Run consistency checks 1 in m_consistency_check_ratio times if enabled
929+
if (m_consistency_check_ratio == 0) return;
930+
if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return;
931+
932+
const int err{ForceCheckAddrman()};
933+
if (err) {
934+
LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err);
935+
assert(false);
936+
}
937+
}
938+
939+
int CAddrMan::ForceCheckAddrman() const
940+
{
941+
AssertLockHeld(cs);
942+
943+
LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size());
944+
945+
std::unordered_set<int> setTried;
946+
std::unordered_map<int, int> mapNew;
947+
948+
if (vRandom.size() != (size_t)(nTried + nNew))
949+
return -7;
950+
951+
for (const auto& entry : mapInfo) {
952+
int n = entry.first;
953+
const CAddrInfo& info = entry.second;
954+
if (info.fInTried) {
955+
if (!info.nLastSuccess)
956+
return -1;
957+
if (info.nRefCount)
958+
return -2;
959+
setTried.insert(n);
960+
} else {
961+
if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS)
962+
return -3;
963+
if (!info.nRefCount)
964+
return -4;
965+
mapNew[n] = info.nRefCount;
966+
}
967+
const auto it{mapAddr.find(info)};
968+
if (it == mapAddr.end() || it->second != n) {
969+
return -5;
970+
}
971+
if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n)
972+
return -14;
973+
if (info.nLastTry < 0)
974+
return -6;
975+
if (info.nLastSuccess < 0)
976+
return -8;
977+
}
978+
979+
if (setTried.size() != (size_t)nTried)
980+
return -9;
981+
if (mapNew.size() != (size_t)nNew)
982+
return -10;
983+
984+
for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) {
985+
for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) {
986+
if (vvTried[n][i] != -1) {
987+
if (!setTried.count(vvTried[n][i]))
988+
return -11;
989+
const auto it{mapInfo.find(vvTried[n][i])};
990+
if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) {
991+
return -17;
992+
}
993+
if (it->second.GetBucketPosition(nKey, false, n) != i) {
994+
return -18;
995+
}
996+
setTried.erase(vvTried[n][i]);
997+
}
998+
}
999+
}
1000+
1001+
for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
1002+
for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) {
1003+
if (vvNew[n][i] != -1) {
1004+
if (!mapNew.count(vvNew[n][i]))
1005+
return -12;
1006+
const auto it{mapInfo.find(vvNew[n][i])};
1007+
if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) {
1008+
return -19;
1009+
}
1010+
if (--mapNew[vvNew[n][i]] == 0)
1011+
mapNew.erase(vvNew[n][i]);
1012+
}
1013+
}
1014+
}
1015+
1016+
if (setTried.size())
1017+
return -13;
1018+
if (mapNew.size())
1019+
return -15;
1020+
if (nKey.IsNull())
1021+
return -16;
1022+
1023+
LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n");
1024+
return 0;
1025+
}
1026+
10261027
size_t CAddrMan::size() const
10271028
{
10281029
LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead

‎src/addrman.h‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,16 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2};
142142
class CAddrMan
143143
{
144144
public:
145+
explicit CAddrMan(std::vector<bool> asmap, bool deterministic, int32_t consistency_check_ratio);
146+
147+
~CAddrMan();
148+
145149
template <typename Stream>
146150
void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
147151

148152
template <typename Stream>
149153
void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
150154

151-
explicit CAddrMan(std::vector<bool> asmap, bool deterministic, int32_t consistency_check_ratio);
152-
153-
~CAddrMan();
154-
155155
//! Return the number of (unique) addresses in all tables.
156156
size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
157157

@@ -289,15 +289,15 @@ class CAddrMan
289289
//! Swap two elements in vRandom.
290290
void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs);
291291

292-
//! Move an entry from the "new" table(s) to the "tried" table
293-
void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
294-
295292
//! Delete an entry. It must not be in tried, and have refcount 0.
296293
void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
297294

298295
//! Clear a position in a "new" table. This is the only place where entries are actually deleted.
299296
void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs);
300297

298+
//! Move an entry from the "new" table(s) to the "tried" table
299+
void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
300+
301301
//! Mark an entry "good", possibly moving it from "new" to "tried".
302302
void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs);
303303

@@ -310,19 +310,6 @@ class CAddrMan
310310
//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
311311
CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs);
312312

313-
//! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
314-
void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs);
315-
316-
//! Return a random to-be-evicted tried table address.
317-
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
318-
319-
//! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected.
320-
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);
321-
322-
//! Perform consistency check, regardless of m_consistency_check_ratio.
323-
//! @returns an error code or zero.
324-
int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs);
325-
326313
/**
327314
* Return all or many randomly selected addresses, optionally by network.
328315
*
@@ -349,6 +336,19 @@ class CAddrMan
349336
//! Update an entry's service bits.
350337
void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);
351338

339+
//! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
340+
void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs);
341+
342+
//! Return a random to-be-evicted tried table address.
343+
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
344+
345+
//! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected.
346+
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);
347+
348+
//! Perform consistency check, regardless of m_consistency_check_ratio.
349+
//! @returns an error code or zero.
350+
int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs);
351+
352352
friend class CAddrManTest;
353353
friend class CAddrManDeterministic;
354354
};

0 commit comments

Comments
 (0)