Skip to content

Commit

Permalink
[addrman] Make GetBucket functions members of CAddrMan
Browse files Browse the repository at this point in the history 
GetTriedBucket(), GetNewBucket() and GetBucketPosition() all require data
from addrman (nKey and asmap), so make them members of that class rather
than passing those things through to CAddrInfo on each call.
  • Loading branch information
@jnewbery
jnewbery committed Nov 13, 2020
1 parent 4963175 commit 7d02625
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 132 deletions.
88 changes: 44 additions & 44 deletions src/addrman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,6 @@
#include <logging.h>
#include <serialize.h>

int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool>& asmap) const
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash();
int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n", ToStringIP(), mapped_as, tried_bucket);
return tried_bucket;
}

int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector<bool>& asmap) const
{
std::vector<unsigned char> vchSourceGroupKey = src.GetGroup(asmap);
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n", ToStringIP(), mapped_as, new_bucket);
return new_bucket;
}

int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << GetKey()).GetCheapHash();
return hash1 % ADDRMAN_BUCKET_SIZE;
}

bool CAddrInfo::IsTerrible(int64_t nNow) const
{
if (nLastTry && nLastTry >= nNow - 60) // never remove things tried in the last minute
Expand Down Expand Up @@ -149,7 +122,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId)
{
// remove the entry from all new buckets
for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) {
int pos = info.GetBucketPosition(nKey, true, bucket);
const int pos{GetBucketPosition(info, true, bucket)};
if (vvNew[bucket][pos] == nId) {
vvNew[bucket][pos] = -1;
info.nRefCount--;
Expand All @@ -160,8 +133,8 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId)
assert(info.nRefCount == 0);

// which tried bucket to move the entry to
int nKBucket = info.GetTriedBucket(nKey, m_asmap);
int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket);
const int nKBucket{GetTriedBucket(info)};
const int nKBucketPos{GetBucketPosition(info, 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) {
Expand All @@ -176,8 +149,8 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId)
nTried--;

// find which new bucket it belongs to
int nUBucket = infoOld.GetNewBucket(nKey, m_asmap);
int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket);
const int nUBucket{GetNewBucket(infoOld)};
const int nUBucketPos{GetBucketPosition(infoOld, true, nUBucket)};
ClearNew(nUBucket, nUBucketPos);
assert(vvNew[nUBucket][nUBucketPos] == -1);

Expand Down Expand Up @@ -227,7 +200,7 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime
int nUBucket = -1;
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);
const int nBpos{GetBucketPosition(info, true, nB)};
if (vvNew[nB][nBpos] == nId) {
nUBucket = nB;
break;
Expand All @@ -240,8 +213,8 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime
return;

// which tried bucket to move the entry to
int tried_bucket = info.GetTriedBucket(nKey, m_asmap);
int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket);
const int tried_bucket{GetTriedBucket(info)};
const int tried_bucket_pos{GetBucketPosition(info, false, tried_bucket)};

// Will moving this address into tried evict another entry?
if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) {
Expand Down Expand Up @@ -308,8 +281,8 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP
fNew = true;
}

int nUBucket = pinfo->GetNewBucket(nKey, source, m_asmap);
int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket);
const int nUBucket{GetNewBucket(*pinfo)};
const int nUBucketPos{GetBucketPosition(*pinfo, true, nUBucket)};
if (vvNew[nUBucket][nUBucketPos] != nId) {
bool fInsert = vvNew[nUBucket][nUBucketPos] == -1;
if (!fInsert) {
Expand Down Expand Up @@ -446,9 +419,9 @@ int CAddrMan::Check_()
if (vvTried[n][i] != -1) {
if (!setTried.count(vvTried[n][i]))
return -11;
if (mapInfo[vvTried[n][i]].GetTriedBucket(nKey, m_asmap) != n)
if (GetTriedBucket(mapInfo[vvTried[n][i]]) != n)
return -17;
if (mapInfo[vvTried[n][i]].GetBucketPosition(nKey, false, n) != i)
if (GetBucketPosition(mapInfo[vvTried[n][i]], false, n) != i)
return -18;
setTried.erase(vvTried[n][i]);
}
Expand All @@ -460,7 +433,7 @@ int CAddrMan::Check_()
if (vvNew[n][i] != -1) {
if (!mapNew.count(vvNew[n][i]))
return -12;
if (mapInfo[vvNew[n][i]].GetBucketPosition(nKey, true, n) != i)
if (GetBucketPosition(mapInfo[vvNew[n][i]], true, n) != i)
return -19;
if (--mapNew[vvNew[n][i]] == 0)
mapNew.erase(vvNew[n][i]);
Expand Down Expand Up @@ -556,8 +529,8 @@ void CAddrMan::ResolveCollisions_()
CAddrInfo& info_new = mapInfo[id_new];

// Which tried bucket to move the entry to.
int tried_bucket = info_new.GetTriedBucket(nKey, m_asmap);
int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket);
const int tried_bucket{GetTriedBucket(info_new)};
const int tried_bucket_pos{GetBucketPosition(info_new, false, tried_bucket)};
if (!info_new.IsValid()) { // id_new may no longer map to a valid address
erase_collision = true;
} else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty
Expand Down Expand Up @@ -620,14 +593,41 @@ CAddrInfo CAddrMan::SelectTriedCollision_()
CAddrInfo& newInfo = mapInfo[id_new];

// which tried bucket to move the entry to
int tried_bucket = newInfo.GetTriedBucket(nKey, m_asmap);
int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket);
const int tried_bucket{GetTriedBucket(newInfo)};
const int tried_bucket_pos{GetBucketPosition(newInfo, false, tried_bucket)};

int id_old = vvTried[tried_bucket][tried_bucket_pos];

return mapInfo[id_old];
}

int CAddrMan::GetTriedBucket(const CAddrInfo& addr) const
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << addr.GetKey()).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << addr.GetGroup(m_asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash();
int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
uint32_t mapped_as = addr.GetMappedAS(m_asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n", addr.ToStringIP(), mapped_as, tried_bucket);
return tried_bucket;
}

int CAddrMan::GetNewBucket(const CAddrInfo& addr, const CNetAddr& src) const
{
std::vector<unsigned char> vchSourceGroupKey = src.GetGroup(m_asmap);
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << addr.GetGroup(m_asmap) << vchSourceGroupKey).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
uint32_t mapped_as = addr.GetMappedAS(m_asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n", addr.ToStringIP(), mapped_as, new_bucket);
return new_bucket;
}

int CAddrMan::GetBucketPosition(const CAddrInfo& addr, bool fNew, int nBucket) const
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << addr.GetKey()).GetCheapHash();
return hash1 % ADDRMAN_BUCKET_SIZE;
}

std::vector<bool> DecodeAsmap(fs::path path)
{
std::vector<bool> bits;
Expand Down
47 changes: 22 additions & 25 deletions src/addrman.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@
*/
class CAddrInfo : public CAddress
{
friend class CAddrMan;

public:
//! last try whatsoever by us (memory only)
int64_t nLastTry{0};

//! last counted attempt (memory only)
int64_t nLastCountAttempt{0};

private:
//! where knowledge about this address first came from
CNetAddr source;

Expand All @@ -55,10 +56,6 @@ class CAddrInfo : public CAddress
//! position in vRandom
int nRandomPos{-1};

friend class CAddrMan;

public:

SERIALIZE_METHODS(CAddrInfo, obj)
{
READWRITEAS(CAddress, obj);
Expand All @@ -73,21 +70,6 @@ class CAddrInfo : public CAddress
{
}

//! Calculate in which "tried" bucket this entry belongs
int GetTriedBucket(const uint256& nKey, const std::vector<bool>& asmap) const;

//! Calculate in which "new" bucket this entry belongs, given a certain source
int GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector<bool>& asmap) const;

//! Calculate in which "new" bucket this entry belongs, using its default source
int GetNewBucket(const uint256& nKey, const std::vector<bool>& asmap) const
{
return GetNewBucket(nKey, source, asmap);
}

//! 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;

Expand Down Expand Up @@ -264,6 +246,21 @@ friend class CAddrManTest;
//! Update an entry's service bits.
void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);

//! Calculate in which "tried" bucket an entry belongs
int GetTriedBucket(const CAddrInfo& addr) const;

//! Calculate in which "new" bucket an entry belongs, given a certain source
int GetNewBucket(const CAddrInfo& addr, const CNetAddr& src) const;

//! Calculate in which "new" bucket an entry belongs, using its default source
int GetNewBucket(const CAddrInfo& addr) const
{
return GetNewBucket(addr, addr.source);
}

//! Calculate in which position of a bucket to store an entry.
int GetBucketPosition(const CAddrInfo& addr, bool fNew, int nBucket) const;

public:
//! Serialization versions.
enum class Format : uint8_t {
Expand Down Expand Up @@ -441,8 +438,8 @@ friend class CAddrManTest;
for (int n = 0; n < nTried; n++) {
CAddrInfo info;
s >> info;
int nKBucket = info.GetTriedBucket(nKey, m_asmap);
int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket);
const int nKBucket{GetTriedBucket(info)};
const int nKBucketPos{GetBucketPosition(info, false, nKBucket)};
if (vvTried[nKBucket][nKBucketPos] == -1) {
info.nRandomPos = vRandom.size();
info.fInTried = true;
Expand Down Expand Up @@ -484,7 +481,7 @@ friend class CAddrManTest;
for (int n = 0; n < nNew; n++) {
CAddrInfo &info = mapInfo[n];
int bucket = entryToBucket[n];
int nUBucketPos = info.GetBucketPosition(nKey, true, bucket);
int nUBucketPos{GetBucketPosition(info, true, bucket)};
if (format >= Format::V2_ASMAP && nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && vvNew[bucket][nUBucketPos] == -1 &&
info.nRefCount < ADDRMAN_NEW_BUCKETS_PER_ADDRESS && serialized_asmap_version == supplied_asmap_version) {
// Bucketing has not changed, using existing bucket positions for the new table
Expand All @@ -494,8 +491,8 @@ friend class CAddrManTest;
// In case the new table data cannot be used (format unknown, bucket count wrong or new asmap),
// try to give them a reference based on their primary source address.
LogPrint(BCLog::ADDRMAN, "Bucketing method was updated, re-bucketing addrman entries from disk\n");
bucket = info.GetNewBucket(nKey, m_asmap);
nUBucketPos = info.GetBucketPosition(nKey, true, bucket);
bucket = GetNewBucket(info);
nUBucketPos = GetBucketPosition(info, true, bucket);
if (vvNew[bucket][nUBucketPos] == -1) {
vvNew[bucket][nUBucketPos] = n;
info.nRefCount++;
Expand Down
Loading

0 comments on commit 7d02625

Please sign in to comment.