[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

Author: amitiuttarwar

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<int> setTried;
-    std::unordered_map<int, int> 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<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> 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<int> setTried;
+    std::unordered_map<int, int> 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

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<bool> asmap, bool deterministic, int32_t consistency_check_ratio);
+
+    ~CAddrMan();
+
     template <typename Stream>
     void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     template <typename Stream>
     void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
-    explicit CAddrMan(std::vector<bool> 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 @@ class CAddrMan
     //! 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 @@ class CAddrMan
     //! 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 @@ class CAddrMan
     //! 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;
 };