From a725968143499b7982579fb2ae9ff086a10cff9e Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 13:57:00 +0100 Subject: [PATCH 1/8] Switch addrman key from vector to uint256 --- src/addrman.cpp | 13 +++++++------ src/addrman.h | 29 +++++++++++++++++++---------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 3fdd12cd2c..c42f64f8f6 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -8,30 +8,30 @@ using namespace std; -int CAddrInfo::GetTriedBucket(const std::vector &nKey) const +int CAddrInfo::GetTriedBucket(const uint256& nKey) const { CDataStream ss1(SER_GETHASH, 0); std::vector vchKey = GetKey(); - ss1 << nKey << vchKey; + ss1 << ((unsigned char)32) << nKey << vchKey; uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); CDataStream ss2(SER_GETHASH, 0); std::vector vchGroupKey = GetGroup(); - ss2 << nKey << vchGroupKey << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP); + ss2 << ((unsigned char)32) << nKey << vchGroupKey << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP); uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); return hash2 % ADDRMAN_TRIED_BUCKET_COUNT; } -int CAddrInfo::GetNewBucket(const std::vector &nKey, const CNetAddr& src) const +int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src) const { CDataStream ss1(SER_GETHASH, 0); std::vector vchGroupKey = GetGroup(); std::vector vchSourceGroupKey = src.GetGroup(); - ss1 << nKey << vchGroupKey << vchSourceGroupKey; + ss1 << ((unsigned char)32) << nKey << vchGroupKey << vchSourceGroupKey; uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); CDataStream ss2(SER_GETHASH, 0); - ss2 << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP); + ss2 << ((unsigned char)32) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP); uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); return hash2 % ADDRMAN_NEW_BUCKET_COUNT; } @@ -484,6 +484,7 @@ int CAddrMan::Check_() if (setTried.size()) return -13; if (mapNew.size()) return -15; + if (nKey.IsNull()) return -16; return 0; } diff --git a/src/addrman.h b/src/addrman.h index 340d409fcc..ed9250cda5 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -10,6 +10,7 @@ #include "random.h" #include "util.h" #include "sync.h" +#include "uint256.h" #include @@ -77,13 +78,13 @@ class CAddrInfo : public CAddress } // Calculate in which "tried" bucket this entry belongs - int GetTriedBucket(const std::vector &nKey) const; + int GetTriedBucket(const uint256 &nKey) const; // Calculate in which "new" bucket this entry belongs, given a certain source - int GetNewBucket(const std::vector &nKey, const CNetAddr& src) const; + int GetNewBucket(const uint256 &nKey, const CNetAddr& src) const; // Calculate in which "new" bucket this entry belongs, using its default source - int GetNewBucket(const std::vector &nKey) const + int GetNewBucket(const uint256 &nKey) const { return GetNewBucket(nKey, source); } @@ -171,10 +172,10 @@ class CAddrMan // critical section to protect the inner data structures mutable CCriticalSection cs; - // secret key to randomize bucket select with - std::vector nKey; + //! secret key to randomize bucket select with + uint256 nKey; - // last used nId + //! last used nId int nIdCount; // table with information about all nIds @@ -277,6 +278,7 @@ class CAddrMan unsigned char nVersion = 0; s << nVersion; + s << ((unsigned char)32); s << nKey; s << nNew; s << nTried; @@ -321,6 +323,9 @@ class CAddrMan unsigned char nVersion; s >> nVersion; + unsigned char nKeySize; + s >> nKeySize; + if (nKeySize != 32) throw std::ios_base::failure("Incorrect keysize in addrman"); s >> nKey; s >> nNew; s >> nTried; @@ -381,15 +386,19 @@ class CAddrMan CAddrMan() : vRandom(0), vvTried(ADDRMAN_TRIED_BUCKET_COUNT, std::vector(0)), vvNew(ADDRMAN_NEW_BUCKET_COUNT, std::set()) { - nKey.resize(32); - GetRandBytes(nKey.data(), 32); + nKey = GetRandHash(); nIdCount = 0; nTried = 0; nNew = 0; } - // Return the number of (unique) addresses in all tables. + ~CAddrMan() + { + nKey.SetNull(); + } + + //! Return the number of (unique) addresses in all tables. int size() { return vRandom.size(); @@ -503,7 +512,7 @@ class CAddrMan { LOCK(cs); std::vector().swap(vRandom); - GetRandBytes(nKey.data(), 32); + nKey = GetRandHash(); vvTried = std::vector>(ADDRMAN_TRIED_BUCKET_COUNT, std::vector(0)); vvNew = std::vector>(ADDRMAN_NEW_BUCKET_COUNT, std::set()); // Will need for Bitcoin rebase From 1aa38298169ef39a0060d23f74cbcc863b3a71dd Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 14:30:24 +0100 Subject: [PATCH 2/8] Make addrman's bucket placement deterministic. --- src/addrman.cpp | 382 ++++++++++++++++++++++-------------------------- src/addrman.h | 251 +++++++++++++++++-------------- 2 files changed, 319 insertions(+), 314 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index c42f64f8f6..e08deb1fa2 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -12,12 +12,12 @@ int CAddrInfo::GetTriedBucket(const uint256& nKey) const { CDataStream ss1(SER_GETHASH, 0); std::vector vchKey = GetKey(); - ss1 << ((unsigned char)32) << nKey << vchKey; + ss1 << nKey << vchKey; uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); CDataStream ss2(SER_GETHASH, 0); std::vector vchGroupKey = GetGroup(); - ss2 << ((unsigned char)32) << nKey << vchGroupKey << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP); + ss2 << nKey << vchGroupKey << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP); uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); return hash2 % ADDRMAN_TRIED_BUCKET_COUNT; } @@ -27,15 +27,24 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src) const CDataStream ss1(SER_GETHASH, 0); std::vector vchGroupKey = GetGroup(); std::vector vchSourceGroupKey = src.GetGroup(); - ss1 << ((unsigned char)32) << nKey << vchGroupKey << vchSourceGroupKey; + ss1 << nKey << vchGroupKey << vchSourceGroupKey; uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); CDataStream ss2(SER_GETHASH, 0); - ss2 << ((unsigned char)32) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP); + ss2 << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP); uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); return hash2 % ADDRMAN_NEW_BUCKET_COUNT; } +int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const +{ + CDataStream ss1(SER_GETHASH, 0); + std::vector vchKey = GetKey(); + ss1 << nKey << (fNew ? 'N' : 'K') << nBucket << vchKey; + uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); + return hash1 % ADDRMAN_BUCKET_SIZE; +} + bool CAddrInfo::IsTerrible(int64_t nNow) const { if (nLastTry && nLastTry >= nNow-60) // never remove things tried the last minute @@ -124,152 +133,93 @@ void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) vRandom[nRndPos2] = nId1; } -int CAddrMan::SelectTried(int nKBucket) +void CAddrMan::Delete(int nId) { - std::vector &vTried = vvTried[nKBucket]; - - // random shuffle the first few elements (using the entire list) - // find the least recently tried among them - int64_t nOldest = -1; - int nOldestPos = -1; - for (unsigned int i = 0; i < ADDRMAN_TRIED_ENTRIES_INSPECT_ON_EVICT && i < vTried.size(); i++) - { - int nPos = GetRandInt(vTried.size() - i) + i; - int nTemp = vTried[nPos]; - vTried[nPos] = vTried[i]; - vTried[i] = nTemp; - assert(nOldest == -1 || mapInfo.count(nTemp) == 1); - if (nOldest == -1 || mapInfo[nTemp].nLastSuccess < mapInfo[nOldest].nLastSuccess) { - nOldest = nTemp; - nOldestPos = nPos; - } - } + assert(mapInfo.count(nId) != 0); + CAddrInfo& info = mapInfo[nId]; + assert(!info.fInTried); + assert(info.nRefCount == 0); - return nOldestPos; + SwapRandom(info.nRandomPos, vRandom.size() - 1); + vRandom.pop_back(); + mapAddr.erase(info); + mapInfo.erase(nId); + nNew--; } -int CAddrMan::ShrinkNew(int nUBucket) +void CAddrMan::ClearNew(int nUBucket, int nUBucketPos) { - assert(nUBucket >= 0 && (unsigned int)nUBucket < vvNew.size()); - std::set &vNew = vvNew[nUBucket]; - - // first look for deletable items - for (std::set::iterator it = vNew.begin(); it != vNew.end(); it++) - { - assert(mapInfo.count(*it)); - CAddrInfo &info = mapInfo[*it]; - if (info.IsTerrible()) - { - if (--info.nRefCount == 0) - { - SwapRandom(info.nRandomPos, vRandom.size()-1); - vRandom.pop_back(); - mapAddr.erase(info); - mapInfo.erase(*it); - nNew--; - } - vNew.erase(it); - return 0; + // if there is an entry in the specified bucket, delete it. + if (vvNew[nUBucket][nUBucketPos] != -1) { + int nIdDelete = vvNew[nUBucket][nUBucketPos]; + CAddrInfo& infoDelete = mapInfo[nIdDelete]; + assert(infoDelete.nRefCount > 0); + infoDelete.nRefCount--; + vvNew[nUBucket][nUBucketPos] = -1; + if (infoDelete.nRefCount == 0) { + Delete(nIdDelete); } } - - // otherwise, select four randomly, and pick the oldest of those to replace - int n[4] = {GetRandInt(vNew.size()), GetRandInt(vNew.size()), GetRandInt(vNew.size()), GetRandInt(vNew.size())}; - int nI = 0; - int nOldest = -1; - for (std::set::iterator it = vNew.begin(); it != vNew.end(); it++) - { - if (nI == n[0] || nI == n[1] || nI == n[2] || nI == n[3]) - { - assert(nOldest == -1 || mapInfo.count(*it) == 1); - if (nOldest == -1 || mapInfo[*it].nTime < mapInfo[nOldest].nTime) - nOldest = *it; - } - nI++; - } - assert(mapInfo.count(nOldest) == 1); - CAddrInfo &info = mapInfo[nOldest]; - if (--info.nRefCount == 0) - { - SwapRandom(info.nRandomPos, vRandom.size()-1); - vRandom.pop_back(); - mapAddr.erase(info); - mapInfo.erase(nOldest); - nNew--; - } - vNew.erase(nOldest); - - return 1; } -void CAddrMan::MakeTried(CAddrInfo& info, int nId, int nOrigin) +void CAddrMan::MakeTried(CAddrInfo& info, int nId) { - assert(vvNew[nOrigin].count(nId) == 1); - // remove the entry from all new buckets - for (std::vector >::iterator it = vvNew.begin(); it != vvNew.end(); it++) - { - if (it->erase(nId)) + for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { + int pos = info.GetBucketPosition(nKey, true, bucket); + if (vvNew[bucket][pos] == nId) { + vvNew[bucket][pos] = -1; info.nRefCount--; + } } nNew--; assert(info.nRefCount == 0); - // what tried bucket to move the entry to + // which tried bucket to move the entry to int nKBucket = info.GetTriedBucket(nKey); - std::vector &vTried = vvTried[nKBucket]; - - // first check whether there is place to just add it - if (vTried.size() < ADDRMAN_TRIED_BUCKET_SIZE) - { - vTried.push_back(nId); - nTried++; - info.fInTried = true; - return; - } - - // otherwise, find an item to evict - int nPos = SelectTried(nKBucket); - - // find which new bucket it belongs to - assert(mapInfo.count(vTried[nPos]) == 1); - int nUBucket = mapInfo[vTried[nPos]].GetNewBucket(nKey); - std::set &vNew = vvNew[nUBucket]; - - // remove the to-be-replaced tried entry from the tried set - CAddrInfo& infoOld = mapInfo[vTried[nPos]]; - infoOld.fInTried = false; - infoOld.nRefCount = 1; - // do not update nTried, as we are going to move something else there immediately - - // check whether there is place in that one, - if (vNew.size() < ADDRMAN_NEW_BUCKET_SIZE) - { - // if so, move it back there - vNew.insert(vTried[nPos]); - } else { - // otherwise, move it to the new bucket nId came from (there is certainly place there) - vvNew[nOrigin].insert(vTried[nPos]); + int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); + + // first make space to add it (the existing tried entry there is moved to new, deleting whatever is there). + if (vvTried[nKBucket][nKBucketPos] != -1) { + // find an item to evict + int nIdEvict = vvTried[nKBucket][nKBucketPos]; + assert(mapInfo.count(nIdEvict) == 1); + CAddrInfo& infoOld = mapInfo[nIdEvict]; + + // Remove the to-be-evicted item from the tried set. + infoOld.fInTried = false; + vvTried[nKBucket][nKBucketPos] = -1; + nTried--; + + // find which new bucket it belongs to + int nUBucket = infoOld.GetNewBucket(nKey); + int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket); + ClearNew(nUBucket, nUBucketPos); + assert(vvNew[nUBucket][nUBucketPos] == -1); + + // Enter it into the new set again. + infoOld.nRefCount = 1; + vvNew[nUBucket][nUBucketPos] = nIdEvict; + nNew++; } - nNew++; + assert(vvTried[nKBucket][nKBucketPos] == -1); - vTried[nPos] = nId; - // we just overwrote an entry in vTried; no need to update nTried + vvTried[nKBucket][nKBucketPos] = nId; + nTried++; info.fInTried = true; - return; } -void CAddrMan::Good_(const CService &addr, int64_t nTime) +void CAddrMan::Good_(const CService& addr, int64_t nTime) { int nId; - CAddrInfo *pinfo = Find(addr, &nId); + CAddrInfo* pinfo = Find(addr, &nId); // if not found, bail out if (!pinfo) return; - CAddrInfo &info = *pinfo; + CAddrInfo& info = *pinfo; // check whether we are talking about the exact same CService (including same port) if (info != addr) @@ -278,22 +228,21 @@ void CAddrMan::Good_(const CService &addr, int64_t nTime) // update info info.nLastSuccess = nTime; info.nLastTry = nTime; - info.nTime = nTime; info.nAttempts = 0; + // nTime is not updated here, to avoid leaking information about + // currently-connected peers. // if it is already in the tried set, don't do anything else if (info.fInTried) return; // find a bucket it is in now - int nRnd = GetRandInt(vvNew.size()); + int nRnd = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT); int nUBucket = -1; - for (unsigned int n = 0; n < vvNew.size(); n++) - { - int nB = (n+nRnd) % vvNew.size(); - std::set &vNew = vvNew[nB]; - if (vNew.count(nId)) - { + for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { + int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT; + int nBpos = info.GetBucketPosition(nKey, true, nB); + if (vvNew[nB][nBpos] == nId) { nUBucket = nB; break; } @@ -301,25 +250,25 @@ void CAddrMan::Good_(const CService &addr, int64_t nTime) // if no bucket is found, something bad happened; // TODO: maybe re-add the node, but for now, just bail out - if (nUBucket == -1) return; + if (nUBucket == -1) + return; - LogPrint(BCLog::LogFlags::ADDRMAN, "Moving %s to tried", addr.ToString()); + LogPrint("addrman", "Moving %s to tried\n", addr.ToString()); // move nId to the tried tables - MakeTried(info, nId, nUBucket); + MakeTried(info, nId); } -bool CAddrMan::Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) +bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) { if (!addr.IsRoutable()) return false; bool fNew = false; int nId; - CAddrInfo *pinfo = Find(addr, &nId); + CAddrInfo* pinfo = Find(addr, &nId); - if (pinfo) - { + if (pinfo) { // periodically update nTime bool fCurrentlyOnline = (GetAdjustedTime() - addr.nTime < 24 * 60 * 60); int64_t nUpdateInterval = (fCurrentlyOnline ? 60 * 60 : 24 * 60 * 60); @@ -343,7 +292,7 @@ bool CAddrMan::Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimeP // stochastic test: previous nRefCount == N: 2^N times harder to increase it int nFactor = 1; - for (int n=0; nnRefCount; n++) + for (int n = 0; n < pinfo->nRefCount; n++) nFactor *= 2; if (nFactor > 1 && (GetRandInt(nFactor) != 0)) return false; @@ -355,13 +304,25 @@ bool CAddrMan::Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimeP } int nUBucket = pinfo->GetNewBucket(nKey, source); - std::set &vNew = vvNew[nUBucket]; - if (!vNew.count(nId)) - { - pinfo->nRefCount++; - if (vNew.size() == ADDRMAN_NEW_BUCKET_SIZE) - ShrinkNew(nUBucket); - vvNew[nUBucket].insert(nId); + int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket); + if (vvNew[nUBucket][nUBucketPos] != nId) { + bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; + if (!fInsert) { + CAddrInfo& infoExisting = mapInfo[vvNew[nUBucket][nUBucketPos]]; + if (infoExisting.IsTerrible() || (infoExisting.nRefCount > 1 && pinfo->nRefCount == 0)) { + // Overwrite the existing new table entry. + fInsert = true; + } + } + if (fInsert) { + ClearNew(nUBucket, nUBucketPos); + pinfo->nRefCount++; + vvNew[nUBucket][nUBucketPos] = nId; + } else { + if (pinfo->nRefCount == 0) { + Delete(nId); + } + } } return fNew; } @@ -392,37 +353,33 @@ CAddress CAddrMan::Select_(int nUnkBias) double nCorTried = sqrt(nTried) * (100.0 - nUnkBias); double nCorNew = sqrt(nNew) * nUnkBias; - if ((nCorTried + nCorNew)*GetRandInt(1<<30)/(1<<30) < nCorTried) - { + if ((nCorTried + nCorNew) * GetRandInt(1 << 30) / (1 << 30) < nCorTried) { // use a tried node double fChanceFactor = 1.0; - while(1) - { - int nKBucket = GetRandInt(vvTried.size()); - std::vector &vTried = vvTried[nKBucket]; - if (vTried.size() == 0) continue; - int nPos = GetRandInt(vTried.size()); - assert(mapInfo.count(vTried[nPos]) == 1); - CAddrInfo &info = mapInfo[vTried[nPos]]; - if (GetRandInt(1<<30) < fChanceFactor*info.GetChance()*(1<<30)) + while (1) { + int nKBucket = GetRandInt(ADDRMAN_TRIED_BUCKET_COUNT); + int nKBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE); + if (vvTried[nKBucket][nKBucketPos] == -1) + continue; + int nId = vvTried[nKBucket][nKBucketPos]; + assert(mapInfo.count(nId) == 1); + CAddrInfo& info = mapInfo[nId]; + if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30)) return info; fChanceFactor *= 1.2; } } else { // use a new node double fChanceFactor = 1.0; - while(1) - { - int nUBucket = GetRandInt(vvNew.size()); - std::set &vNew = vvNew[nUBucket]; - if (vNew.size() == 0) continue; - int nPos = GetRandInt(vNew.size()); - std::set::iterator it = vNew.begin(); - while (nPos--) - it++; - assert(mapInfo.count(*it) == 1); - CAddrInfo &info = mapInfo[*it]; - if (GetRandInt(1<<30) < fChanceFactor*info.GetChance()*(1<<30)) + while (1) { + int nUBucket = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT); + int nUBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE); + if (vvNew[nUBucket][nUBucketPos] == -1) + continue; + int nId = vvNew[nUBucket][nUBucketPos]; + assert(mapInfo.count(nId) == 1); + CAddrInfo& info = mapInfo[nId]; + if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30)) return info; fChanceFactor *= 1.2; } @@ -435,56 +392,73 @@ int CAddrMan::Check_() std::set setTried; std::map mapNew; - if (vRandom.size() != nTried + nNew) return -7; - - for (std::map::iterator it = mapInfo.begin(); it != mapInfo.end(); it++) - { - int n = it->first; - CAddrInfo& info = it->second; - if (info.fInTried) - { - - if (!info.nLastSuccess) return -1; - if (info.nRefCount) return -2; + if (vRandom.size() != nTried + nNew) + return -7; + + for (std::map::iterator it = mapInfo.begin(); it != mapInfo.end(); it++) { + int n = (*it).first; + CAddrInfo& info = (*it).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; + if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS) + return -3; + if (!info.nRefCount) + return -4; mapNew[n] = info.nRefCount; } - if (mapAddr[info] != n) return -5; - if (info.nRandomPos<0 || info.nRandomPos>=vRandom.size() || vRandom[info.nRandomPos] != n) return -14; - if (info.nLastTry < 0) return -6; - if (info.nLastSuccess < 0) return -8; + if (mapAddr[info] != n) + return -5; + if (info.nRandomPos < 0 || 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() != nTried) return -9; - if (mapNew.size() != nNew) return -10; - - for (int n=0; n &vTried = vvTried[n]; - for (std::vector::iterator it = vTried.begin(); it != vTried.end(); it++) - { - if (!setTried.count(*it)) return -11; - setTried.erase(*it); + if (setTried.size() != nTried) + return -9; + if (mapNew.size() != 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; + if (mapInfo[vvTried[n][i]].GetTriedBucket(nKey) != n) + return -17; + if (mapInfo[vvTried[n][i]].GetBucketPosition(nKey, false, n) != i) + return -18; + setTried.erase(vvTried[n][i]); + } } } - for (int n=0; n &vNew = vvNew[n]; - for (std::set::iterator it = vNew.begin(); it != vNew.end(); it++) - { - if (!mapNew.count(*it)) return -12; - if (--mapNew[*it] == 0) - mapNew.erase(*it); + 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; + if (mapInfo[vvNew[n][i]].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; + if (setTried.size()) + return -13; + if (mapNew.size()) + return -15; + if (nKey.IsNull()) + return -16; return 0; } diff --git a/src/addrman.h b/src/addrman.h index ed9250cda5..3ce9562508 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -10,7 +10,6 @@ #include "random.h" #include "util.h" #include "sync.h" -#include "uint256.h" #include @@ -77,18 +76,21 @@ class CAddrInfo : public CAddress Init(); } - // Calculate in which "tried" bucket this entry belongs + //! Calculate in which "tried" bucket this entry belongs int GetTriedBucket(const uint256 &nKey) const; - // Calculate in which "new" bucket this entry belongs, given a certain source + //! Calculate in which "new" bucket this entry belongs, given a certain source int GetNewBucket(const uint256 &nKey, const CNetAddr& src) const; - // Calculate in which "new" bucket this entry belongs, using its default source + //! Calculate in which "new" bucket this entry belongs, using its default source int GetNewBucket(const uint256 &nKey) const { return GetNewBucket(nKey, source); } + //! Calculate in which position of a bucket to store this entry. + int GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const; + // Determine whether the statistics about this entry are bad enough so that it can just be deleted bool IsTerrible(int64_t nNow = GetAdjustedTime()) const; @@ -132,8 +134,8 @@ class CAddrInfo : public CAddress // total number of buckets for new addresses #define ADDRMAN_NEW_BUCKET_COUNT 256 -// maximum allowed number of entries in buckets for new addresses -#define ADDRMAN_NEW_BUCKET_SIZE 64 +//! maximum allowed number of entries in buckets for new and tried addresses +#define ADDRMAN_BUCKET_SIZE 64 // over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread #define ADDRMAN_TRIED_BUCKETS_PER_GROUP 4 @@ -144,9 +146,6 @@ class CAddrInfo : public CAddress // in how many buckets for entries with new addresses a single address may occur #define ADDRMAN_NEW_BUCKETS_PER_ADDRESS 4 -// how many entries in a bucket with tried addresses are inspected, when selecting one to replace -#define ADDRMAN_TRIED_ENTRIES_INSPECT_ON_EVICT 4 - // how old addresses can maximally be #define ADDRMAN_HORIZON_DAYS 30 @@ -190,14 +189,14 @@ class CAddrMan // number of "tried" entries int nTried; - // list of "tried" buckets - std::vector > vvTried; + //! list of "tried" buckets + int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE]; // number of (unique) "new" entries int nNew; - // list of "new" buckets - std::vector > vvNew; + //! list of "new" buckets + int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE]; protected: @@ -211,17 +210,14 @@ class CAddrMan // Swap two elements in vRandom. void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2); - // Return position in given bucket to replace. - int SelectTried(int nKBucket); + //! Move an entry from the "new" table(s) to the "tried" table + void MakeTried(CAddrInfo& info, int nId); - // Remove an element from a "new" bucket. - // This is the only place where actual deletes occur. - // They are never deleted while in the "tried" table, only possibly evicted back to the "new" table. - int ShrinkNew(int nUBucket); + //! Delete an entry. It must not be in tried, and have refcount 0. + void Delete(int nId); - // Move an entry from the "new" table(s) to the "tried" table - // @pre vvUnkown[nOrigin].count(nId) != 0 - void MakeTried(CAddrInfo& info, int nId, int nOrigin); + //! Clear a position in a "new" table. This is the only place where entries are actually deleted. + void ClearNew(int nUBucket, int nUBucketPos); // Mark an entry "good", possibly moving it from "new" to "tried". void Good_(const CService &addr, int64_t nTime); @@ -248,70 +244,81 @@ class CAddrMan void Connected_(const CService &addr, int64_t nTime); public: - // serialized format: - // * version byte (currently 0) - // * nKey - // * nNew - // * nTried - // * number of "new" buckets - // * all nNew addrinfos in vvNew - // * all nTried addrinfos in vvTried - // * for each bucket: - // * number of elements - // * for each element: index - // - // Notice that vvTried, mapAddr and vVector are never encoded explicitly; - // they are instead reconstructed from the other information. - // - // vvNew is serialized, but only used if ADDRMAN_UNKOWN_BUCKET_COUNT didn't change, - // otherwise it is reconstructed as well. - // - // This format is more complex, but significantly smaller (at most 1.5 MiB), and supports - // changes to the ADDRMAN_ parameters without breaking the on-disk structure. - // - // We don't use ADD_SERIALIZE_METHODS since the serialization and deserialization code has - // very little in common. + /** + * serialized format: + * * version byte (currently 1) + * * 0x20 + nKey (serialized as if it were a vector, for backward compatibility) + * * nNew + * * nTried + * * number of "new" buckets XOR 2**30 + * * all nNew addrinfos in vvNew + * * all nTried addrinfos in vvTried + * * for each bucket: + * * number of elements + * * for each element: index + * + * 2**30 is xorred with the number of buckets to make addrman deserializer v0 detect it + * as incompatible. This is necessary because it did not check the version number on + * deserialization. + * + * Notice that vvTried, mapAddr and vVector are never encoded explicitly; + * they are instead reconstructed from the other information. + * + * vvNew is serialized, but only used if ADDRMAN_UNKOWN_BUCKET_COUNT didn't change, + * otherwise it is reconstructed as well. + * + * This format is more complex, but significantly smaller (at most 1.5 MiB), and supports + * changes to the ADDRMAN_ parameters without breaking the on-disk structure. + * + * We don't use ADD_SERIALIZE_METHODS since the serialization and deserialization code has + * very little in common. + */ template void Serialize(Stream &s) const { LOCK(cs); - unsigned char nVersion = 0; + unsigned char nVersion = 1; s << nVersion; s << ((unsigned char)32); s << nKey; s << nNew; s << nTried; - int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT; + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); s << nUBuckets; std::map mapUnkIds; int nIds = 0; for (std::map::const_iterator it = mapInfo.begin(); it != mapInfo.end(); it++) { - if (nIds == nNew) break; // this means nNew was wrong, oh ow mapUnkIds[it->first] = nIds; const CAddrInfo& info = it->second; if (info.nRefCount) { + assert(nIds != nNew); // this means nNew was wrong, oh ow s << info; nIds++; } } nIds = 0; for (std::map::const_iterator it = mapInfo.begin(); it != mapInfo.end(); it++) { - if (nIds == nTried) break; // this means nTried was wrong, oh ow const CAddrInfo& info = it->second; if (info.fInTried) { + assert(nIds != nTried); // this means nTried was wrong, oh ow s << info; nIds++; } } - for (std::vector >::const_iterator it = vvNew.begin(); it != vvNew.end(); it++) { - const std::set &vNew = (*it); - int nSize = vNew.size(); + for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { + int nSize = 0; + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[bucket][i] != -1) + nSize++; + } s << nSize; - for (std::set::const_iterator it2 = vNew.begin(); it2 != vNew.end(); it2++) { - int nIndex = mapUnkIds[*it2]; - s << nIndex; + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[bucket][i] != -1) { + int nIndex = mapUnkIds[vvNew[bucket][i]]; + s << nIndex; + } } } } @@ -321,81 +328,134 @@ class CAddrMan { LOCK(cs); + Clear(); + unsigned char nVersion; s >> nVersion; unsigned char nKeySize; s >> nKeySize; - if (nKeySize != 32) throw std::ios_base::failure("Incorrect keysize in addrman"); + if (nKeySize != 32) throw std::ios_base::failure("Incorrect keysize in addrman deserialization"); s >> nKey; s >> nNew; s >> nTried; int nUBuckets = 0; s >> nUBuckets; - nIdCount = 0; - mapInfo.clear(); - mapAddr.clear(); - vRandom.clear(); - vvTried = std::vector >(ADDRMAN_TRIED_BUCKET_COUNT, std::vector(0)); - vvNew = std::vector >(ADDRMAN_NEW_BUCKET_COUNT, std::set()); + if (nVersion != 0) { + nUBuckets ^= (1 << 30); + } + + // Deserialize entries from the new table. for (int n = 0; n < nNew; n++) { CAddrInfo &info = mapInfo[n]; s >> info; mapAddr[info] = n; info.nRandomPos = vRandom.size(); vRandom.push_back(n); - if (nUBuckets != ADDRMAN_NEW_BUCKET_COUNT) { - vvNew[info.GetNewBucket(nKey)].insert(n); - info.nRefCount++; + if (nVersion != 1 || nUBuckets != ADDRMAN_NEW_BUCKET_COUNT) { + // In case the new table data cannot be used (nVersion unknown, or bucket count wrong), + // immediately try to give them a reference based on their primary source address. + int nUBucket = info.GetNewBucket(nKey); + int nUBucketPos = info.GetBucketPosition(nKey, true, nUBucket); + if (vvNew[nUBucket][nUBucketPos] == -1) { + vvNew[nUBucket][nUBucketPos] = n; + info.nRefCount++; + } } } nIdCount = nNew; + + // Deserialize entries from the tried table. int nLost = 0; for (int n = 0; n < nTried; n++) { CAddrInfo info; s >> info; - std::vector &vTried = vvTried[info.GetTriedBucket(nKey)]; - if (vTried.size() < ADDRMAN_TRIED_BUCKET_SIZE) { + int nKBucket = info.GetTriedBucket(nKey); + int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); + if (vvTried[nKBucket][nKBucketPos] == -1) { info.nRandomPos = vRandom.size(); info.fInTried = true; vRandom.push_back(nIdCount); mapInfo[nIdCount] = info; mapAddr[info] = nIdCount; - vTried.push_back(nIdCount); + vvTried[nKBucket][nKBucketPos] = nIdCount; nIdCount++; } else { nLost++; } } nTried -= nLost; - for (int b = 0; b < nUBuckets; b++) { - std::set &vNew = vvNew[b]; + + // Deserialize positions in the new table (if possible). + for (int bucket = 0; bucket < nUBuckets; bucket++) { int nSize = 0; s >> nSize; for (int n = 0; n < nSize; n++) { int nIndex = 0; s >> nIndex; - CAddrInfo &info = mapInfo[nIndex]; - if (nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && info.nRefCount < ADDRMAN_NEW_BUCKETS_PER_ADDRESS) { - info.nRefCount++; - vNew.insert(nIndex); + if (nIndex >= 0 && nIndex < nNew) { + CAddrInfo &info = mapInfo[nIndex]; + int nUBucketPos = info.GetBucketPosition(nKey, true, bucket); + if (nVersion == 1 && nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && vvNew[bucket][nUBucketPos] == -1 && info.nRefCount < ADDRMAN_NEW_BUCKETS_PER_ADDRESS) { + info.nRefCount++; + vvNew[bucket][nUBucketPos] = nIndex; + } } } } + + // Prune new entries with refcount 0 (as a result of collisions). + int nLostUnk = 0; + for (std::map::const_iterator it = mapInfo.begin(); it != mapInfo.end(); ) { + if (it->second.fInTried == false && it->second.nRefCount == 0) { + std::map::const_iterator itCopy = it++; + Delete(itCopy->first); + nLostUnk++; + } else { + it++; + } + } + if (nLost + nLostUnk > 0) { + LogPrint("addrman", "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); + } + + Check(); + } + + unsigned int GetSerializeSize(int nType, int nVersion) const + { + return (CSizeComputer(nType, nVersion) << *this).size(); } - CAddrMan() : vRandom(0), vvTried(ADDRMAN_TRIED_BUCKET_COUNT, std::vector(0)), vvNew(ADDRMAN_NEW_BUCKET_COUNT, std::set()) + void Clear() { - nKey = GetRandHash(); + std::vector().swap(vRandom); + nKey = GetRandHash(); + for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { + for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { + vvNew[bucket][entry] = -1; + } + } + for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; bucket++) { + for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { + vvTried[bucket][entry] = -1; + } + } + + nIdCount = 0; + nTried = 0; + nNew = 0; + } + - nIdCount = 0; - nTried = 0; - nNew = 0; + CAddrMan() + { + Clear(); } ~CAddrMan() { - nKey.SetNull(); + nKey.SetNull(); } //! Return the number of (unique) addresses in all tables. @@ -404,7 +464,7 @@ class CAddrMan return vRandom.size(); } - // Consistency check + //! Consistency check void Check() { #ifdef DEBUG_ADDRMAN @@ -508,35 +568,6 @@ class CAddrMan } } - void Clear() - { - LOCK(cs); - std::vector().swap(vRandom); - nKey = GetRandHash(); - vvTried = std::vector>(ADDRMAN_TRIED_BUCKET_COUNT, std::vector(0)); - vvNew = std::vector>(ADDRMAN_NEW_BUCKET_COUNT, std::set()); - // Will need for Bitcoin rebase - // nKey = insecure_rand.rand256(); - //for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { - // for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { - // vvNew[bucket][entry] = -1; - // } - //} - //for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; bucket++) { - // for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { - // vvTried[bucket][entry] = -1; - // } - //} - - nIdCount = 0; - nTried = 0; - nNew = 0; - // Will need for Bitcoin rebase - // nLastGood = 1; //Initially at 1 so that "never" is strictly worse. - mapInfo.clear(); - mapAddr.clear(); - } - }; #endif // BITCOIN_ADDRMAN_H From a4c0bd6568a44c409f8ed15272035256d410d063 Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 14:56:58 +0100 Subject: [PATCH 3/8] Simplify hashing code --- src/addrman.cpp | 28 +++++++--------------------- src/uint256.h | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index e08deb1fa2..176bbadf65 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -3,6 +3,8 @@ // file COPYING or https://opensource.org/licenses/mit-license.php. #include "addrman.h" + +#include "hash.h" #include "random.h" #include "streams.h" @@ -10,38 +12,22 @@ using namespace std; int CAddrInfo::GetTriedBucket(const uint256& nKey) const { - CDataStream ss1(SER_GETHASH, 0); - std::vector vchKey = GetKey(); - ss1 << nKey << vchKey; - uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); - - CDataStream ss2(SER_GETHASH, 0); - std::vector vchGroupKey = GetGroup(); - ss2 << nKey << vchGroupKey << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP); - uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetHash().GetCheapHash(); + uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup() << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetHash().GetCheapHash(); return hash2 % ADDRMAN_TRIED_BUCKET_COUNT; } int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src) const { - CDataStream ss1(SER_GETHASH, 0); - std::vector vchGroupKey = GetGroup(); std::vector vchSourceGroupKey = src.GetGroup(); - ss1 << nKey << vchGroupKey << vchSourceGroupKey; - uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); - - CDataStream ss2(SER_GETHASH, 0); - ss2 << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP); - uint64_t hash2 = Hash(ss2.begin(), ss2.end()).GetUint64(); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup() << vchSourceGroupKey).GetHash().GetCheapHash(); + uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetHash().GetCheapHash(); return hash2 % ADDRMAN_NEW_BUCKET_COUNT; } int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const { - CDataStream ss1(SER_GETHASH, 0); - std::vector vchKey = GetKey(); - ss1 << nKey << (fNew ? 'N' : 'K') << nBucket << vchKey; - uint64_t hash1 = Hash(ss1.begin(), ss1.end()).GetUint64(); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << GetKey()).GetHash().GetCheapHash(); return hash1 % ADDRMAN_BUCKET_SIZE; } diff --git a/src/uint256.h b/src/uint256.h index 511eb447d0..47da60f74e 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -6,6 +6,8 @@ #ifndef BITCOIN_UINT256_H #define BITCOIN_UINT256_H +#include "crypto/common.h" + #include #include #include @@ -125,7 +127,23 @@ class uint160 : public base_blob<160> { class uint256 : public base_blob<256> { public: uint256() {} + uint256(const base_blob<256>& b) : base_blob<256>(b) {} explicit uint256(const std::vector& vch) : base_blob<256>(vch) {} + + /** A cheap hash function that just returns 64 bits from the result, it can be + * used when the contents are considered uniformly random. It is not appropriate + * when the value can easily be influenced from outside as e.g. a network adversary could + * provide values to trigger worst-case behavior. + */ + uint64_t GetCheapHash() const + { + return ReadLE64(data); + } + + /** A more secure, salted hash function. + * @note This hash is not stable between little and big endian. + */ + uint64_t GetHash(const uint256& salt) const; }; /* uint256 from const char *. From 4f2e4f9d5d6bd074fe6be4efd7c41adf2f3a20be Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 15:01:18 +0100 Subject: [PATCH 4/8] Do not bias outgoing connections towards fresh addresses --- src/addrman.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 176bbadf65..e0f37c7db7 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -61,8 +61,6 @@ double CAddrInfo::GetChance(int64_t nNow) const if (nSinceLastSeen < 0) nSinceLastSeen = 0; if (nSinceLastTry < 0) nSinceLastTry = 0; - fChance *= 600.0 / (600.0 + nSinceLastSeen); - // deprioritize very recent attempts away if (nSinceLastTry < 60*10) fChance *= 0.01; From f90e51613bf652ff4f40a36ce83800fbbe8c5793 Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 15:15:15 +0100 Subject: [PATCH 5/8] Always use a 50% chance to choose between tried and new entries --- src/addrman.cpp | 7 +++---- src/addrman.h | 12 +++++------- src/net.cpp | 3 +-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index e0f37c7db7..cf9d35c565 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -330,14 +330,13 @@ void CAddrMan::Attempt_(const CService &addr, int64_t nTime) info.nAttempts++; } -CAddress CAddrMan::Select_(int nUnkBias) +CAddress CAddrMan::Select_() { if (size() == 0) return CAddress(); - double nCorTried = sqrt(nTried) * (100.0 - nUnkBias); - double nCorNew = sqrt(nNew) * nUnkBias; - if ((nCorTried + nCorNew) * GetRandInt(1 << 30) / (1 << 30) < nCorTried) { + // Use a 50% chance for choosing between tried and new table entries. + if (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0)) { // use a tried node double fChanceFactor = 1.0; while (1) { diff --git a/src/addrman.h b/src/addrman.h index 3ce9562508..d6f11e162e 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -228,9 +228,8 @@ class CAddrMan // Mark an entry as attempted to connect. void Attempt_(const CService &addr, int64_t nTime); - // Select an address to connect to. - // nUnkBias determines how much to favor new addresses over tried ones (min=0, max=100) - CAddress Select_(int nUnkBias); + //! Select an address to connect to. + CAddress Select_(); #ifdef DEBUG_ADDRMAN // Perform consistency check. Returns an error code or zero. @@ -530,15 +529,14 @@ class CAddrMan } } - // Choose an address to connect to. - // nUnkBias determines how much "new" entries are favored over "tried" ones (0-100). - CAddress Select(int nUnkBias = 50) + //! Choose an address to connect to. + CAddress Select() { CAddress addrRet; { LOCK(cs); Check(); - addrRet = Select_(nUnkBias); + addrRet = Select_(); Check(); } return addrRet; diff --git a/src/net.cpp b/src/net.cpp index 179fc68695..1a9519a1c5 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1570,8 +1570,7 @@ void ThreadOpenConnections2(void* parg) int nTries = 0; while (true) { - // use an nUnkBias between 10 (no outgoing connections) and 90 (8 outgoing connections) - CAddress addr = addrman.Select(10 + min(nOutbound,8)*10); + CAddress addr = addrman.Select(); // if we selected an invalid address, restart if (!addr.IsValid() || setConnected.count(addr.GetGroup()) || IsLocal(addr)) From dc52ad851068149174cd822c686f15a77541d2e7 Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Fri, 4 Mar 2022 15:30:35 +0100 Subject: [PATCH 6/8] Scale up addrman --- src/addrman.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index d6f11e162e..e27e3d0c22 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -108,15 +108,15 @@ class CAddrInfo : public CAddress // // To that end: // * Addresses are organized into buckets. -// * Addresses that have not yet been tried go into 256 "new" buckets. -// * Based on the address range (/16 for IPv4) of source of the information, 32 buckets are selected at random +// * Addresses that have not yet been tried go into 1024 "new" buckets. +// * Based on the address range (/16 for IPv4) of source of the information, 64 buckets are selected at random // * The actual bucket is chosen from one of these, based on the range the address itself is located. -// * One single address can occur in up to 4 different buckets, to increase selection chances for addresses that +// * One single address can occur in up to 8 different buckets, to increase selection chances for addresses that // are seen frequently. The chance for increasing this multiplicity decreases exponentially. // * When adding a new address to a full bucket, a randomly chosen entry (with a bias favoring less recently seen // ones) is removed from it first. -// * Addresses of nodes that are known to be accessible go into 64 "tried" buckets. -// * Each address range selects at random 4 of these buckets. +// * Addresses of nodes that are known to be accessible go into 256 "tried" buckets. +// * Each address range selects at random 8 of these buckets. // * The actual bucket is chosen from one of these, based on the full address. // * When adding a new good address to a full bucket, a randomly chosen entry (with a bias favoring less recently // tried ones) is evicted from it, back to the "new" buckets. @@ -125,26 +125,26 @@ class CAddrInfo : public CAddress // * Several indexes are kept for high performance. Defining DEBUG_ADDRMAN will introduce frequent (and expensive) // consistency checks for the entire data structure. -// total number of buckets for tried addresses -#define ADDRMAN_TRIED_BUCKET_COUNT 64 +//! total number of buckets for tried addresses +#define ADDRMAN_TRIED_BUCKET_COUNT 256 // maximum allowed number of entries in buckets for tried addresses #define ADDRMAN_TRIED_BUCKET_SIZE 64 -// total number of buckets for new addresses -#define ADDRMAN_NEW_BUCKET_COUNT 256 +//! total number of buckets for new addresses +#define ADDRMAN_NEW_BUCKET_COUNT 1024 //! maximum allowed number of entries in buckets for new and tried addresses #define ADDRMAN_BUCKET_SIZE 64 -// over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread -#define ADDRMAN_TRIED_BUCKETS_PER_GROUP 4 +//! over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread +#define ADDRMAN_TRIED_BUCKETS_PER_GROUP 8 -// over how many buckets entries with new addresses originating from a single group are spread -#define ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP 32 +//! over how many buckets entries with new addresses originating from a single group are spread +#define ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP 64 -// in how many buckets for entries with new addresses a single address may occur -#define ADDRMAN_NEW_BUCKETS_PER_ADDRESS 4 +//! in how many buckets for entries with new addresses a single address may occur +#define ADDRMAN_NEW_BUCKETS_PER_ADDRESS 8 // how old addresses can maximally be #define ADDRMAN_HORIZON_DAYS 30 From 2648f058d117769fb5eb81fb829fe18671d8f8e2 Mon Sep 17 00:00:00 2001 From: pythonix <9782029+Pythonix@users.noreply.github.com> Date: Tue, 29 Mar 2022 20:45:20 +0200 Subject: [PATCH 7/8] Fix LogPrint --- src/addrman.cpp | 2 +- src/addrman.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index cf9d35c565..761c8a450c 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -237,7 +237,7 @@ void CAddrMan::Good_(const CService& addr, int64_t nTime) if (nUBucket == -1) return; - LogPrint("addrman", "Moving %s to tried\n", addr.ToString()); + LogPrint(BCLog::LogFlags::ADDRMAN, "Moving %s to tried", addr.ToString()); // move nId to the tried tables MakeTried(info, nId); diff --git a/src/addrman.h b/src/addrman.h index e27e3d0c22..f0d8cb3c9e 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -415,7 +415,7 @@ class CAddrMan } } if (nLost + nLostUnk > 0) { - LogPrint("addrman", "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); + LogPrint(BCLog::LogFlags::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); } Check(); From 35d0746bccc6e68f0b0408d00bbf22c58e736838 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 17 Apr 2022 12:12:11 -0400 Subject: [PATCH 8/8] Remove \n from LogPrint --- src/addrman.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/addrman.h b/src/addrman.h index f0d8cb3c9e..8f2606cb38 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -415,7 +415,7 @@ class CAddrMan } } if (nLost + nLostUnk > 0) { - LogPrint(BCLog::LogFlags::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); + LogPrint(BCLog::LogFlags::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions", nLostUnk, nLost); } Check();