Skip to content

Commit

Permalink
Increase test coverage for addrman and addrinfo
Browse files Browse the repository at this point in the history
Adds several unittests for CAddrMan and CAddrInfo.
Increases the accuracy of addrman tests.
Removes non-determinism in tests by overriding the random number generator.
Extracts testing code from addrman class to test class.
  • Loading branch information
EthanHeilman authored and Warrows committed May 27, 2019
1 parent d4da015 commit 3355c86
Show file tree
Hide file tree
Showing 3 changed files with 391 additions and 48 deletions.
24 changes: 14 additions & 10 deletions src/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ void CAddrMan::Good_(const CService& addr, int64_t nTime)
return;

// find a bucket it is in now
int nRnd = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT);
int nRnd = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
int nUBucket = -1;
for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT;
Expand Down Expand Up @@ -281,7 +281,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP
int nFactor = 1;
for (int n = 0; n < pinfo->nRefCount; n++)
nFactor *= 2;
if (nFactor > 1 && (GetRandInt(nFactor) != 0))
if (nFactor > 1 && (RandomInt(nFactor) != 0))
return false;
} else {
pinfo = Create(addr, source, &nId);
Expand Down Expand Up @@ -342,37 +342,37 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
return CAddrInfo();

// Use a 50% chance for choosing between tried and new table entries.
if (!newOnly && (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0))) {
if (!newOnly && (nTried > 0 && (nNew == 0 || RandomInt(2) == 0))) {
// use a tried node
double fChanceFactor = 1.0;
while (1) {
int nKBucket = GetRandInt(ADDRMAN_TRIED_BUCKET_COUNT);
int nKBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT);
int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
while (vvTried[nKBucket][nKBucketPos] == -1) {
nKBucket = (nKBucket + insecure_rand()) % ADDRMAN_TRIED_BUCKET_COUNT;
nKBucketPos = (nKBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
}
int nId = vvTried[nKBucket][nKBucketPos];
assert(mapInfo.count(nId) == 1);
CAddrInfo& info = mapInfo[nId];
if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
if (RandomInt(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(ADDRMAN_NEW_BUCKET_COUNT);
int nUBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
while (vvNew[nUBucket][nUBucketPos] == -1) {
nUBucket = (nUBucket + insecure_rand()) % ADDRMAN_NEW_BUCKET_COUNT;
nUBucketPos = (nUBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
}
int nId = vvNew[nUBucket][nUBucketPos];
assert(mapInfo.count(nId) == 1);
CAddrInfo& info = mapInfo[nId];
if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
return info;
fChanceFactor *= 1.2;
}
Expand Down Expand Up @@ -468,7 +468,7 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr)
if (vAddr.size() >= nNodes)
break;

int nRndPos = GetRandInt(vRandom.size() - n) + n;
int nRndPos = RandomInt(vRandom.size() - n) + n;
SwapRandom(n, nRndPos);
assert(mapInfo.count(vRandom[n]) == 1);

Expand Down Expand Up @@ -497,3 +497,7 @@ void CAddrMan::Connected_(const CService& addr, int64_t nTime)
if (nTime - info.nTime > nUpdateInterval)
info.nTime = nTime;
}

int CAddrMan::RandomInt(int nMax){
return GetRandInt(nMax);
}
15 changes: 6 additions & 9 deletions src/addrman.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,6 @@ class CAddrMan
//! critical section to protect the inner data structures
mutable CCriticalSection cs;

//! secret key to randomize bucket select with
uint256 nKey;

//! last used nId
int nIdCount;

Expand All @@ -204,6 +201,9 @@ class CAddrMan
int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE];

protected:
//! secret key to randomize bucket select with
uint256 nKey;

//! Find an entry.
CAddrInfo* Find(const CNetAddr& addr, int* pnId = NULL);

Expand Down Expand Up @@ -235,6 +235,9 @@ 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);

//! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic.
virtual int RandomInt(int nMax);

#ifdef DEBUG_ADDRMAN
//! Perform consistency check. Returns an error code or zero.
int Check_();
Expand Down Expand Up @@ -570,12 +573,6 @@ class CAddrMan
Check();
}
}

//! Ensure that bucket placement is always the same for testing purposes.
void MakeDeterministic(){
nKey.SetNull(); //Do not use outside of tests.
}

};

#endif // BITCOIN_ADDRMAN_H
Loading

0 comments on commit 3355c86

Please sign in to comment.