Merge bitcoin/bitcoin#20233: addrman: Make consistency checks a runtime option

a4d78546b0858602c60c03fdf8b35ca666ab2e56 [addrman] Make addrman consistency checks a runtime option (John Newbery)
10aac241455a3270462d49b53732477ed97623e7 [tests] Make deterministic addrman use nKey = 1 (John Newbery)
fa9710f62c29c7f8d71c9f281001c9b5e70946bf [addrman] Add deterministic argument to CAddrMan ctor (John Newbery)
ee458d84fc187d69f002ebead6fccc4f4f9c0744 Add missing const to CAddrMan::Check_() (MarcoFalke)

Pull request description:

  CAddrMan has internal consistency checks. Currently, these are only run when the program is compiled with the  `DEBUG_ADDRMAN` option. This option is not enabled on any of our CI builds, and it's likely that no-one is running them at all.

  This PR makes consistency checks a (hidden) runtime option that can be enabled with `-checkaddrman`, where `-checkaddrman=n` will result in the consistency checks running every n operations (similar to `-checkmempool=n`). We set the ratio to 1/100 for our unit tests, and leave it disabled by default for all networks. Additionally, a consistency check failure now asserts, rather than logging and continuing. This matches the behavior of CTxMemPool and TxRequestTracker, where a failed consistency check asserts.

ACKs for top commit:
  jonatack:
    ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56 per `git diff 00fd089 a4d7854`, tested by adding logging similar to #22479 and running with `-checkaddrman=<n>` for various values 0/1/10/100 etc, tested the updated docs with `bitcoind -help-debug | grep -A2 "checkaddrman\|checkmempool"` and verified rebased on master that compiling with `CPPFLAGS="-DDEBUG_ADDRMAN"` no longer causes the build to error.
  mzumsande:
    Code-review ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56
  theStack:
    Code-review ACK a4d78546b0858602c60c03fdf8b35ca666ab2e56

Tree-SHA512: eaee003f7a99154822c5b5efbc62008d32c1efbecc6fec6e183427f6b2ae5d30b3be7924e3a7271b1a1de91517f5bd2a70011d45358c3105c6a0702f12b70f7c

1 files changed, 16 insertions, 7 deletions

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 96139182d3..8e2fc67569 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -431,11 +431,14 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const
     }
 }
 
-#ifdef DEBUG_ADDRMAN
-int CAddrMan::Check_()
+int CAddrMan::Check_() const
 {
     AssertLockHeld(cs);
 
+    // Run consistency checks 1 in m_consistency_check_ratio times if enabled
+    if (m_consistency_check_ratio == 0) return 0;
+    if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return 0;
+
     std::unordered_set<int> setTried;
     std::unordered_map<int, int> mapNew;
 
@@ -458,8 +461,10 @@ int CAddrMan::Check_()
                 return -4;
             mapNew[n] = info.nRefCount;
         }
-        if (mapAddr[info] != n)
+        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)
@@ -478,10 +483,13 @@ int CAddrMan::Check_()
             if (vvTried[n][i] != -1) {
                 if (!setTried.count(vvTried[n][i]))
                     return -11;
-                if (mapInfo[vvTried[n][i]].GetTriedBucket(nKey, m_asmap) != n)
+                const auto it{mapInfo.find(vvTried[n][i])};
+                if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) {
                     return -17;
-                if (mapInfo[vvTried[n][i]].GetBucketPosition(nKey, false, n) != i)
+                }
+                if (it->second.GetBucketPosition(nKey, false, n) != i) {
                     return -18;
+                }
                 setTried.erase(vvTried[n][i]);
             }
         }
@@ -492,8 +500,10 @@ int CAddrMan::Check_()
             if (vvNew[n][i] != -1) {
                 if (!mapNew.count(vvNew[n][i]))
                     return -12;
-                if (mapInfo[vvNew[n][i]].GetBucketPosition(nKey, true, n) != i)
+                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]);
             }
@@ -509,7 +519,6 @@ int CAddrMan::Check_()
 
     return 0;
 }
-#endif
 
 void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) const
 {