From 04c28e3eef00427751d3cd5aecd05858910ba1c8 Mon Sep 17 00:00:00 2001 From: Duke Date: Sat, 20 May 2023 06:12:40 -0700 Subject: [PATCH] Disable run-time asserts in addrman Run-time asserts are a horrible anti-pattern littered across code inherited from BTC. One could maybe argue they are the right thing to do in some situations but not when managing the peer database. Crashing our full node and potentially corrupting our wallet or block index is INSANE in the case of some inconsistencies in peers.dat . --- src/addrman.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 15ced2f61..a832ffc4c 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -143,15 +143,17 @@ void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) if (nRndPos1 == nRndPos2) return; - assert(nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()); + // assert(nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()); int nId1 = vRandom[nRndPos1]; int nId2 = vRandom[nRndPos2]; const auto it_1{mapInfo.find(nId1)}; const auto it_2{mapInfo.find(nId2)}; - assert(it_1 != mapInfo.end()); - assert(it_2 != mapInfo.end()); + + if( (it_1 == mapInfo.end()) || (it_2 == mapInfo.end())) { + return; + } it_1->second.nRandomPos = nRndPos2; it_2->second.nRandomPos = nRndPos1; @@ -167,8 +169,8 @@ void CAddrMan::Delete(int nId) const auto it{mapInfo.find(nId)}; if (it != mapInfo.end()) { CAddrInfo& info = (*it).second; - assert(!info.fInTried); - assert(info.nRefCount == 0); + // assert(!info.fInTried); + // assert(info.nRefCount == 0); SwapRandom(info.nRandomPos, vRandom.size() - 1); vRandom.pop_back(); @@ -189,7 +191,10 @@ void CAddrMan::ClearNew(int nUBucket, int nUBucketPos) const auto it{mapInfo.find(nIdDelete)}; if (it != mapInfo.end()) { CAddrInfo& infoDelete = (*it).second; - assert(infoDelete.nRefCount > 0); + // assert(infoDelete.nRefCount > 0); + if (infoDelete.nRefCount == 0) { + return; + } infoDelete.nRefCount--; vvNew[nUBucket][nUBucketPos] = -1; if (infoDelete.nRefCount == 0) { @@ -217,7 +222,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) } nNew--; - assert(info.nRefCount == 0); + //assert(info.nRefCount == 0); // which tried bucket to move the entry to int nKBucket = info.GetTriedBucket(nKey, m_asmap); @@ -227,7 +232,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) if (vvTried[nKBucket][nKBucketPos] != -1) { // find an item to evict int nIdEvict = vvTried[nKBucket][nKBucketPos]; - assert(mapInfo.count(nIdEvict) == 1); + //assert(mapInfo.count(nIdEvict) == 1); CAddrInfo& infoOld = mapInfo[nIdEvict]; // Remove the to-be-evicted item from the tried set. @@ -239,14 +244,14 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) int nUBucket = infoOld.GetNewBucket(nKey, m_asmap); int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket); ClearNew(nUBucket, nUBucketPos); - assert(vvNew[nUBucket][nUBucketPos] == -1); + //assert(vvNew[nUBucket][nUBucketPos] == -1); // Enter it into the new set again. infoOld.nRefCount = 1; vvNew[nUBucket][nUBucketPos] = nIdEvict; nNew++; } - assert(vvTried[nKBucket][nKBucketPos] == -1); + //assert(vvTried[nKBucket][nKBucketPos] == -1); vvTried[nKBucket][nKBucketPos] = nId; nTried++; @@ -665,7 +670,7 @@ void CAddrMan::GetAddr_(std::vector& vAddr, bool wants_addrv2) if (vAddr.size() >= nNodes) break; - assert(mapInfo.count(vRandom[n]) == 1); + // assert(mapInfo.count(vRandom[n]) == 1); const CAddrInfo& ai = mapInfo[vRandom[n]]; if (!ai.IsTerrible()) {