Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

addrman fixes #59

Closed
wants to merge 10 commits into from
Closed

addrman fixes #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9e2e43d
update addrman test suite to match bitcoin core
slowriot Sep 1, 2021
d1899e4
restore CHashWriterSHA256's GetCheapHash function to maintain consist…
slowriot Sep 1, 2021
f310525
switch addrman's cheaphash algorithms back to SHA256, so bucket selec…
slowriot Sep 1, 2021
38ef337
allow addrman to operate in deterministic mode, with fixed known nKey
slowriot Sep 1, 2021
154555a
adjust addrman new collision test methodology to be consistent with l…
slowriot Sep 1, 2021
0811c94
addrman - assert exclusive locks are held in all necessary functions,…
slowriot Sep 2, 2021
3342fab
addrman consistency check, no longer gated behind an unnecessary define
slowriot Sep 2, 2021
a4f8253
addrman - clean up SelectTriedCollision and Select logic, remove unne…
slowriot Sep 2, 2021
b173d7d
adjust the addrman Good function to always test before evict; the sec…
slowriot Sep 2, 2021
bdadc30
add two more addrman testcases for CAddrDB - caddrdb_read and caddrdb…
slowriot Sep 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ AC_ARG_ENABLE([natpmp-default],
AC_ARG_ENABLE(tests,
AS_HELP_STRING([--disable-tests],[do not compile tests (default is to compile)]),
[use_tests=$enableval],
[use_tests=no])
[use_tests=yes])

AC_ARG_ENABLE(gui-tests,
AS_HELP_STRING([--disable-gui-tests],[do not compile GUI tests (default is to compile if GUI and tests enabled)]),
Expand Down Expand Up @@ -343,7 +343,7 @@ AC_ARG_ENABLE([werror],
AC_ARG_ENABLE([external-signer],
[AS_HELP_STRING([--enable-external-signer],[compile external signer support (default is no, requires Boost::Process)])],
[use_external_signer=$enableval],
[use_external_signer=no])
[use_external_signer=yes])

AC_LANG_PUSH([C++])

Expand Down Expand Up @@ -387,7 +387,7 @@ if test "x$enable_debug" = xyes; then
[[$CXXFLAG_WERROR]])

AX_CHECK_PREPROC_FLAG([-DDEBUG],[[DEBUG_CPPFLAGS="$DEBUG_CPPFLAGS -DDEBUG"]],,[[$CXXFLAG_WERROR]])
AX_CHECK_PREPROC_FLAG([-DDEBUG_LOCKORDER],[[DEBUG_CPPFLAGS="$DEBUG_CPPFLAGS -DDEBUG_LOCKORDER"]],,[[$CXXFLAG_WERROR]])
dnl AX_CHECK_PREPROC_FLAG([-DDEBUG_LOCKORDER],[[DEBUG_CPPFLAGS="$DEBUG_CPPFLAGS -DDEBUG_LOCKORDER"]],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-ftrapv],[DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -ftrapv"],,[[$CXXFLAG_WERROR]])
fi

Expand Down Expand Up @@ -453,22 +453,31 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
dnl some compilers will ignore -Wformat-security without -Wformat, so just combine the two here.
AX_CHECK_COMPILE_FLAG([-Wformat -Wformat-security],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat -Wformat-security"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wshadow-field],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wshadow-all],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-all"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wswitch],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wswitch"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wthread-safety],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wredundant-decls],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wunused-variable],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-variable"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wdate-time],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdate-time"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wconditional-uninitialized],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wconditional-uninitialized"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wsign-compare],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wsign-compare"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wduplicated-branches],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-branches"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wduplicated-cond],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-cond"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wlogical-op],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wlogical-op"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Woverloaded-virtual],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Woverloaded-virtual"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wsuggest-override],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wsuggest-override"],,[[$CXXFLAG_WERROR]],
[AC_LANG_SOURCE([[struct A { virtual void f(); }; struct B : A { void f() final; };]])])
AX_CHECK_COMPILE_FLAG([-Wunreachable-code-loop-increment],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code-loop-increment"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Warray-bounds],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Warray-bounds"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wdisabled-optimization],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdisabled-optimization"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wuninitialized],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wuninitialized"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Winit-self],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Winit-self"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wmissing-include-dirs],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wmissing-include-dirs"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wunsafe-loop-optimizations],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunsafe-loop-optimizations"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wvector-operation-performance],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvector-operation-performance"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Winvalid-pch],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Winvalid-pch"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wcast-align],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wcast-align"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wnormalized],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wnormalized"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wpacked],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wpacked"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wpointer-arith],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wpointer-arith"],,[[$CXXFLAG_WERROR]])

if test x$suppress_external_warnings != xno ; then
AX_CHECK_COMPILE_FLAG([-Wdocumentation],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"],,[[$CXXFLAG_WERROR]])
Expand All @@ -477,13 +486,13 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all
dnl unknown options if any other warning is produced. Test the -Wfoo case, and
dnl set the -Wno-foo case if it works.
AX_CHECK_COMPILE_FLAG([-Wunused-parameter],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wself-assign],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-self-assign"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wunused-parameter],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wunused-local-typedef],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-local-typedef"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-implicit-fallthrough"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wdeprecated-copy],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-copy"],,[[$CXXFLAG_WERROR]])
fi


dnl Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
AX_CHECK_COMPILE_FLAG([-fno-extended-identifiers],[[CXXFLAGS="$CXXFLAGS -fno-extended-identifiers"]],,[[$CXXFLAG_WERROR]])

Expand Down
43 changes: 36 additions & 7 deletions src/addrman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
{
uint64_t hash1 = (CHashWriterKeccak(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
uint64_t hash2 = (CHashWriterKeccak(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash();
uint64_t hash1 = (CHashWriterSHA256(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
uint64_t hash2 = (CHashWriterSHA256(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);
Expand All @@ -24,8 +24,8 @@ int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asma
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 = (CHashWriterKeccak(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
uint64_t hash2 = (CHashWriterKeccak(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
uint64_t hash1 = (CHashWriterSHA256(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
uint64_t hash2 = (CHashWriterSHA256(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);
Expand All @@ -34,7 +34,7 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std:

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

Expand Down Expand Up @@ -75,6 +75,8 @@ double CAddrInfo::GetChance(int64_t nNow) const

CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
{
AssertLockHeld(cs);

std::map<CNetAddr, int>::iterator it = mapAddr.find(addr);
if (it == mapAddr.end())
return nullptr;
Expand All @@ -88,6 +90,8 @@ CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)

CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId)
{
AssertLockHeld(cs);

int nId = nIdCount++;
mapInfo[nId] = CAddrInfo(addr, addrSource);
mapAddr[addr] = nId;
Expand All @@ -100,6 +104,8 @@ CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, in

void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2)
{
AssertLockHeld(cs);

if (nRndPos1 == nRndPos2)
return;

Expand All @@ -120,6 +126,8 @@ void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2)

void CAddrMan::Delete(int nId)
{
AssertLockHeld(cs);

assert(mapInfo.count(nId) != 0);
CAddrInfo& info = mapInfo[nId];
assert(!info.fInTried);
Expand All @@ -134,6 +142,8 @@ void CAddrMan::Delete(int nId)

void CAddrMan::ClearNew(int nUBucket, int nUBucketPos)
{
AssertLockHeld(cs);

// if there is an entry in the specified bucket, delete it.
if (vvNew[nUBucket][nUBucketPos] != -1) {
int nIdDelete = vvNew[nUBucket][nUBucketPos];
Expand All @@ -149,6 +159,8 @@ void CAddrMan::ClearNew(int nUBucket, int nUBucketPos)

void CAddrMan::MakeTried(CAddrInfo& info, int nId)
{
AssertLockHeld(cs);

// remove the entry from all new buckets
for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) {
int pos = info.GetBucketPosition(nKey, true, bucket);
Expand Down Expand Up @@ -197,6 +209,8 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId)

void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime)
{
AssertLockHeld(cs);

int nId;

nLastGood = nTime;
Expand Down Expand Up @@ -263,6 +277,8 @@ void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime

bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty)
{
AssertLockHeld(cs);

if (!addr.IsRoutable())
return false;

Expand Down Expand Up @@ -336,6 +352,8 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP

void CAddrMan::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime)
{
AssertLockHeld(cs);

CAddrInfo* pinfo = Find(addr);

// if not found, bail out
Expand All @@ -358,6 +376,8 @@ void CAddrMan::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime)

CAddrInfo CAddrMan::Select_(bool newOnly)
{
AssertLockHeld(cs);

if (size() == 0)
return CAddrInfo();

Expand Down Expand Up @@ -403,9 +423,10 @@ CAddrInfo CAddrMan::Select_(bool newOnly)
}
}

#ifdef DEBUG_ADDRMAN
int CAddrMan::Check_()
{
AssertLockHeld(cs);

std::set<int> setTried;
std::map<int, int> mapNew;

Expand Down Expand Up @@ -479,10 +500,11 @@ int CAddrMan::Check_()

return 0;
}
#endif

void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct)
{
AssertLockHeld(cs);

size_t nNodes = vRandom.size();
if (max_pct != 0) {
nNodes = max_pct * nNodes / 100;
Expand All @@ -508,6 +530,8 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size

void CAddrMan::Connected_(const CService& addr, int64_t nTime)
{
AssertLockHeld(cs);

CAddrInfo* pinfo = Find(addr);

// if not found, bail out
Expand All @@ -528,6 +552,8 @@ void CAddrMan::Connected_(const CService& addr, int64_t nTime)

void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices)
{
AssertLockHeld(cs);

CAddrInfo* pinfo = Find(addr);

// if not found, bail out
Expand All @@ -546,6 +572,7 @@ void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices)

void CAddrMan::ResolveCollisions_()
{
AssertLockHeld(cs);
for (std::set<int>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) {
int id_new = *it;

Expand Down Expand Up @@ -605,6 +632,8 @@ void CAddrMan::ResolveCollisions_()

CAddrInfo CAddrMan::SelectTriedCollision_()
{
AssertLockHeld(cs);

if (m_tried_collisions.size() == 0) return CAddrInfo();

std::set<int>::iterator it = m_tried_collisions.begin();
Expand Down
Loading