Skip to content

Commit 3355c86

Browse files
EthanHeilmanWarrows
authored andcommitted
Increase test coverage for addrman and addrinfo
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.
1 parent d4da015 commit 3355c86

File tree

3 files changed

+391
-48
lines changed

3 files changed

+391
-48
lines changed

src/addrman.cpp

+14-10
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ void CAddrMan::Good_(const CService& addr, int64_t nTime)
224224
return;
225225

226226
// find a bucket it is in now
227-
int nRnd = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT);
227+
int nRnd = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
228228
int nUBucket = -1;
229229
for (unsigned int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) {
230230
int nB = (n + nRnd) % ADDRMAN_NEW_BUCKET_COUNT;
@@ -281,7 +281,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP
281281
int nFactor = 1;
282282
for (int n = 0; n < pinfo->nRefCount; n++)
283283
nFactor *= 2;
284-
if (nFactor > 1 && (GetRandInt(nFactor) != 0))
284+
if (nFactor > 1 && (RandomInt(nFactor) != 0))
285285
return false;
286286
} else {
287287
pinfo = Create(addr, source, &nId);
@@ -342,37 +342,37 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
342342
return CAddrInfo();
343343

344344
// Use a 50% chance for choosing between tried and new table entries.
345-
if (!newOnly && (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0))) {
345+
if (!newOnly && (nTried > 0 && (nNew == 0 || RandomInt(2) == 0))) {
346346
// use a tried node
347347
double fChanceFactor = 1.0;
348348
while (1) {
349-
int nKBucket = GetRandInt(ADDRMAN_TRIED_BUCKET_COUNT);
350-
int nKBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
349+
int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT);
350+
int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
351351
while (vvTried[nKBucket][nKBucketPos] == -1) {
352352
nKBucket = (nKBucket + insecure_rand()) % ADDRMAN_TRIED_BUCKET_COUNT;
353353
nKBucketPos = (nKBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
354354
}
355355
int nId = vvTried[nKBucket][nKBucketPos];
356356
assert(mapInfo.count(nId) == 1);
357357
CAddrInfo& info = mapInfo[nId];
358-
if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
358+
if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
359359
return info;
360360
fChanceFactor *= 1.2;
361361
}
362362
} else {
363363
// use a new node
364364
double fChanceFactor = 1.0;
365365
while (1) {
366-
int nUBucket = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT);
367-
int nUBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
366+
int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
367+
int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
368368
while (vvNew[nUBucket][nUBucketPos] == -1) {
369369
nUBucket = (nUBucket + insecure_rand()) % ADDRMAN_NEW_BUCKET_COUNT;
370370
nUBucketPos = (nUBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
371371
}
372372
int nId = vvNew[nUBucket][nUBucketPos];
373373
assert(mapInfo.count(nId) == 1);
374374
CAddrInfo& info = mapInfo[nId];
375-
if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
375+
if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
376376
return info;
377377
fChanceFactor *= 1.2;
378378
}
@@ -468,7 +468,7 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr)
468468
if (vAddr.size() >= nNodes)
469469
break;
470470

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

@@ -497,3 +497,7 @@ void CAddrMan::Connected_(const CService& addr, int64_t nTime)
497497
if (nTime - info.nTime > nUpdateInterval)
498498
info.nTime = nTime;
499499
}
500+
501+
int CAddrMan::RandomInt(int nMax){
502+
return GetRandInt(nMax);
503+
}

src/addrman.h

+6-9
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,6 @@ class CAddrMan
176176
//! critical section to protect the inner data structures
177177
mutable CCriticalSection cs;
178178

179-
//! secret key to randomize bucket select with
180-
uint256 nKey;
181-
182179
//! last used nId
183180
int nIdCount;
184181

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

206203
protected:
204+
//! secret key to randomize bucket select with
205+
uint256 nKey;
206+
207207
//! Find an entry.
208208
CAddrInfo* Find(const CNetAddr& addr, int* pnId = NULL);
209209

@@ -235,6 +235,9 @@ class CAddrMan
235235
//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
236236
CAddrInfo Select_(bool newOnly);
237237

238+
//! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic.
239+
virtual int RandomInt(int nMax);
240+
238241
#ifdef DEBUG_ADDRMAN
239242
//! Perform consistency check. Returns an error code or zero.
240243
int Check_();
@@ -570,12 +573,6 @@ class CAddrMan
570573
Check();
571574
}
572575
}
573-
574-
//! Ensure that bucket placement is always the same for testing purposes.
575-
void MakeDeterministic(){
576-
nKey.SetNull(); //Do not use outside of tests.
577-
}
578-
579576
};
580577

581578
#endif // BITCOIN_ADDRMAN_H

0 commit comments

Comments
 (0)