Skip to content

Commit 632aad9

Browse files
committed
Make CAddrman::Select_ select buckets, not positions, first
The original CAddrMan behaviour (before commit e6b343d) was to pick a uniformly random non-empty bucket, and then pick a random element from that bucket. That commit, which introduced deterministic placement of entries in buckets, changed this to picking a uniformly random non-empty bucket position instead. This commit reverts the original high-level behavior, but in the deterministic placement model.
1 parent 113b863 commit 632aad9

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

src/addrman.cpp

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -709,38 +709,54 @@ std::pair<CAddress, int64_t> AddrManImpl::Select_(bool newOnly) const
709709
// use a tried node
710710
double fChanceFactor = 1.0;
711711
while (1) {
712+
// Pick a tried bucket, and an initial position in that bucket.
712713
int nKBucket = insecure_rand.randrange(ADDRMAN_TRIED_BUCKET_COUNT);
713714
int nKBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE);
714-
while (vvTried[nKBucket][nKBucketPos] == -1) {
715-
nKBucket = (nKBucket + insecure_rand.randbits(ADDRMAN_TRIED_BUCKET_COUNT_LOG2)) % ADDRMAN_TRIED_BUCKET_COUNT;
716-
nKBucketPos = (nKBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE;
715+
// Iterate over the positions of that bucket, starting at the initial one,
716+
// and looping around.
717+
int i;
718+
for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
719+
if (vvTried[nKBucket][(nKBucketPos + i) % ADDRMAN_BUCKET_SIZE] != -1) break;
717720
}
718-
int nId = vvTried[nKBucket][nKBucketPos];
721+
// If the bucket is entirely empty, start over with a (likely) different one.
722+
if (i == ADDRMAN_BUCKET_SIZE) continue;
723+
// Find the entry to return.
724+
int nId = vvTried[nKBucket][(nKBucketPos + i) % ADDRMAN_BUCKET_SIZE];
719725
const auto it_found{mapInfo.find(nId)};
720726
assert(it_found != mapInfo.end());
721727
const AddrInfo& info{it_found->second};
728+
// With probability GetChance() * fChanceFactor, return the entry.
722729
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
723730
return {info, info.nLastTry};
724731
}
732+
// Otherwise start over with a (likely) different bucket, and increased chance factor.
725733
fChanceFactor *= 1.2;
726734
}
727735
} else {
728736
// use a new node
729737
double fChanceFactor = 1.0;
730738
while (1) {
739+
// Pick a new bucket, and an initial position in that bucket.
731740
int nUBucket = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT);
732741
int nUBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE);
733-
while (vvNew[nUBucket][nUBucketPos] == -1) {
734-
nUBucket = (nUBucket + insecure_rand.randbits(ADDRMAN_NEW_BUCKET_COUNT_LOG2)) % ADDRMAN_NEW_BUCKET_COUNT;
735-
nUBucketPos = (nUBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE;
742+
// Iterate over the positions of that bucket, starting at the initial one,
743+
// and looping around.
744+
int i;
745+
for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
746+
if (vvNew[nUBucket][(nUBucketPos + i) % ADDRMAN_BUCKET_SIZE] != -1) break;
736747
}
737-
int nId = vvNew[nUBucket][nUBucketPos];
748+
// If the bucket is entirely empty, start over with a (likely) different one.
749+
if (i == ADDRMAN_BUCKET_SIZE) continue;
750+
// Find the entry to return.
751+
int nId = vvNew[nUBucket][(nUBucketPos + i) % ADDRMAN_BUCKET_SIZE];
738752
const auto it_found{mapInfo.find(nId)};
739753
assert(it_found != mapInfo.end());
740754
const AddrInfo& info{it_found->second};
755+
// With probability GetChance() * fChanceFactor, return the entry.
741756
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
742757
return {info, info.nLastTry};
743758
}
759+
// Otherwise start over with a (likely) different bucket, and increased chance factor.
744760
fChanceFactor *= 1.2;
745761
}
746762
}

0 commit comments

Comments
 (0)