From f2e5f38f09ee40933f752680fe7d75ee8e529fae Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 28 Sep 2021 15:46:43 -0400 Subject: [PATCH] [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 --- src/addrman.cpp | 205 ++++++++++++++++++++++++------------------------ src/addrman.h | 40 +++++----- 2 files changed, 123 insertions(+), 122 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 0fa8edd316..99c4f00d6d 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -753,108 +753,6 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const } } -void CAddrMan::Check() const -{ - AssertLockHeld(cs); - - // Run consistency checks 1 in m_consistency_check_ratio times if enabled - if (m_consistency_check_ratio == 0) return; - if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; - - const int err{ForceCheckAddrman()}; - if (err) { - LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); - assert(false); - } -} - -int CAddrMan::ForceCheckAddrman() const -{ - AssertLockHeld(cs); - - LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); - - std::unordered_set setTried; - std::unordered_map mapNew; - - if (vRandom.size() != (size_t)(nTried + nNew)) - return -7; - - for (const auto& entry : mapInfo) { - int n = entry.first; - const CAddrInfo& info = entry.second; - if (info.fInTried) { - if (!info.nLastSuccess) - return -1; - if (info.nRefCount) - return -2; - setTried.insert(n); - } else { - if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS) - return -3; - if (!info.nRefCount) - return -4; - mapNew[n] = info.nRefCount; - } - const auto it{mapAddr.find(info)}; - if (it == mapAddr.end() || it->second != n) { - return -5; - } - if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) - return -14; - if (info.nLastTry < 0) - return -6; - if (info.nLastSuccess < 0) - return -8; - } - - if (setTried.size() != (size_t)nTried) - return -9; - if (mapNew.size() != (size_t)nNew) - return -10; - - for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) { - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvTried[n][i] != -1) { - if (!setTried.count(vvTried[n][i])) - return -11; - const auto it{mapInfo.find(vvTried[n][i])}; - if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { - return -17; - } - if (it->second.GetBucketPosition(nKey, false, n) != i) { - return -18; - } - setTried.erase(vvTried[n][i]); - } - } - } - - for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvNew[n][i] != -1) { - if (!mapNew.count(vvNew[n][i])) - return -12; - const auto it{mapInfo.find(vvNew[n][i])}; - if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) { - return -19; - } - if (--mapNew[vvNew[n][i]] == 0) - mapNew.erase(vvNew[n][i]); - } - } - } - - if (setTried.size()) - return -13; - if (mapNew.size()) - return -15; - if (nKey.IsNull()) - return -16; - - LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); - return 0; -} void CAddrMan::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const { @@ -1023,6 +921,109 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return mapInfo[id_old]; } +void CAddrMan::Check() const +{ + AssertLockHeld(cs); + + // Run consistency checks 1 in m_consistency_check_ratio times if enabled + if (m_consistency_check_ratio == 0) return; + if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; + + const int err{ForceCheckAddrman()}; + if (err) { + LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); + assert(false); + } +} + +int CAddrMan::ForceCheckAddrman() const +{ + AssertLockHeld(cs); + + LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); + + std::unordered_set setTried; + std::unordered_map mapNew; + + if (vRandom.size() != (size_t)(nTried + nNew)) + return -7; + + for (const auto& entry : mapInfo) { + int n = entry.first; + const CAddrInfo& info = entry.second; + if (info.fInTried) { + if (!info.nLastSuccess) + return -1; + if (info.nRefCount) + return -2; + setTried.insert(n); + } else { + if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS) + return -3; + if (!info.nRefCount) + return -4; + mapNew[n] = info.nRefCount; + } + const auto it{mapAddr.find(info)}; + if (it == mapAddr.end() || it->second != n) { + return -5; + } + if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) + return -14; + if (info.nLastTry < 0) + return -6; + if (info.nLastSuccess < 0) + return -8; + } + + if (setTried.size() != (size_t)nTried) + return -9; + if (mapNew.size() != (size_t)nNew) + return -10; + + for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) { + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvTried[n][i] != -1) { + if (!setTried.count(vvTried[n][i])) + return -11; + const auto it{mapInfo.find(vvTried[n][i])}; + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { + return -17; + } + if (it->second.GetBucketPosition(nKey, false, n) != i) { + return -18; + } + setTried.erase(vvTried[n][i]); + } + } + } + + for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[n][i] != -1) { + if (!mapNew.count(vvNew[n][i])) + return -12; + const auto it{mapInfo.find(vvNew[n][i])}; + if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) { + return -19; + } + if (--mapNew[vvNew[n][i]] == 0) + mapNew.erase(vvNew[n][i]); + } + } + } + + if (setTried.size()) + return -13; + if (mapNew.size()) + return -15; + if (nKey.IsNull()) + return -16; + + LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); + return 0; +} + size_t CAddrMan::size() const { LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead diff --git a/src/addrman.h b/src/addrman.h index bc2f934caa..d3d764c9c6 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -142,16 +142,16 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; class CAddrMan { public: + explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); + + ~CAddrMan(); + template void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); template void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs); - explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); - - ~CAddrMan(); - //! Return the number of (unique) addresses in all tables. size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -289,15 +289,15 @@ private: //! Swap two elements in vRandom. void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Move an entry from the "new" table(s) to the "tried" table - void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Delete an entry. It must not be in tried, and have refcount 0. void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Clear a position in a "new" table. This is the only place where entries are actually deleted. void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Move an entry from the "new" table(s) to the "tried" table + void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Mark an entry "good", possibly moving it from "new" to "tried". void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -310,19 +310,6 @@ private: //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); - //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Return a random to-be-evicted tried table address. - CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. - void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Perform consistency check, regardless of m_consistency_check_ratio. - //! @returns an error code or zero. - int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** * Return all or many randomly selected addresses, optionally by network. * @@ -349,6 +336,19 @@ private: //! Update an entry's service bits. void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. + void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Perform consistency check, regardless of m_consistency_check_ratio. + //! @returns an error code or zero. + int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); + friend class CAddrManTest; friend class CAddrManDeterministic; };