From 5faa7dd6d871eac1a0ec5c4a93f2ad7577781a56 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 24 Sep 2021 14:14:39 -0600 Subject: [PATCH 01/13] [move-only] Move CAddrMan function definitions to cpp In preparation for introducing the pimpl pattern to addrman, move all function bodies out of the header file. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/addrman.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++++ src/addrman.h | 99 ++++++------------------------------------------- 2 files changed, 111 insertions(+), 87 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 7c6b8fe64db..0fa8edd3164 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -119,6 +119,11 @@ CAddrMan::CAddrMan(std::vector asmap, bool deterministic, int32_t consiste } } +CAddrMan::~CAddrMan() +{ + nKey.SetNull(); +} + template void CAddrMan::Serialize(Stream& s_) const { @@ -1017,3 +1022,97 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return mapInfo[id_old]; } + +size_t CAddrMan::size() const +{ + LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead + return vRandom.size(); +} + +bool CAddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +{ + LOCK(cs); + int nAdd = 0; + Check(); + for (std::vector::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) + nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; + Check(); + if (nAdd) { + LogPrint(BCLog::ADDRMAN, "Added %i addresses from %s: %i tried, %i new\n", nAdd, source.ToString(), nTried, nNew); + } + return nAdd > 0; +} + +void CAddrMan::Good(const CService &addr, int64_t nTime) +{ + LOCK(cs); + Check(); + Good_(addr, /* test_before_evict */ true, nTime); + Check(); +} + +void CAddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +{ + LOCK(cs); + Check(); + Attempt_(addr, fCountFailure, nTime); + Check(); +} + + +void CAddrMan::ResolveCollisions() +{ + LOCK(cs); + Check(); + ResolveCollisions_(); + Check(); +} + +CAddrInfo CAddrMan::SelectTriedCollision() +{ + LOCK(cs); + Check(); + const CAddrInfo ret = SelectTriedCollision_(); + Check(); + return ret; +} + +CAddrInfo CAddrMan::Select(bool newOnly) const +{ + LOCK(cs); + Check(); + const CAddrInfo addrRet = Select_(newOnly); + Check(); + return addrRet; +} + +std::vector CAddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +{ + LOCK(cs); + Check(); + std::vector vAddr; + GetAddr_(vAddr, max_addresses, max_pct, network); + Check(); + return vAddr; +} + +void CAddrMan::Connected(const CService &addr, int64_t nTime) +{ + LOCK(cs); + Check(); + Connected_(addr, nTime); + Check(); +} + +void CAddrMan::SetServices(const CService &addr, ServiceFlags nServices) +{ + LOCK(cs); + Check(); + SetServices_(addr, nServices); + Check(); +} + +const std::vector& CAddrMan::GetAsmap() const +{ + return m_asmap; +} diff --git a/src/addrman.h b/src/addrman.h index 7dd8528bef1..bc2f934caa4 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -150,88 +150,33 @@ public: explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); - ~CAddrMan() - { - nKey.SetNull(); - } + ~CAddrMan(); //! Return the number of (unique) addresses in all tables. - size_t size() const - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead - return vRandom.size(); - } + size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Add addresses to addrman's new table. bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - int nAdd = 0; - Check(); - for (std::vector::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) - nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; - Check(); - if (nAdd) { - LogPrint(BCLog::ADDRMAN, "Added %i addresses from %s: %i tried, %i new\n", nAdd, source.ToString(), nTried, nNew); - } - return nAdd > 0; - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Mark an entry as accessible. void Good(const CService &addr, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - Good_(addr, /* test_before_evict */ true, nTime); - Check(); - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Mark an entry as connection attempted to. void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - Attempt_(addr, fCountFailure, nTime); - Check(); - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions() - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - ResolveCollisions_(); - Check(); - } + void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Randomly select an address in tried that another address is attempting to evict. - CAddrInfo SelectTriedCollision() - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - const CAddrInfo ret = SelectTriedCollision_(); - Check(); - return ret; - } + CAddrInfo SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); /** * Choose an address to connect to. */ - CAddrInfo Select(bool newOnly = false) const - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - const CAddrInfo addrRet = Select_(newOnly); - Check(); - return addrRet; - } + CAddrInfo Select(bool newOnly = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs); /** * Return all or many randomly selected addresses, optionally by network. @@ -241,36 +186,16 @@ public: * @param[in] network Select only addresses of this network (nullopt = all). */ std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - std::vector vAddr; - GetAddr_(vAddr, max_addresses, max_pct, network); - Check(); - return vAddr; - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Outer function for Connected_() void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - Connected_(addr, nTime); - Check(); - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); void SetServices(const CService &addr, ServiceFlags nServices) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - Check(); - SetServices_(addr, nServices); - Check(); - } + EXCLUSIVE_LOCKS_REQUIRED(!cs); - const std::vector& GetAsmap() const { return m_asmap; } + const std::vector& GetAsmap() const; private: //! A mutex to protect the inner data structures. From f2e5f38f09ee40933f752680fe7d75ee8e529fae Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 28 Sep 2021 15:46:43 -0400 Subject: [PATCH 02/13] [move-only] Match ordering of CAddrMan declarations and definitions Also move `Check` and `ForceCheckAddrman` to be after the `FunctionName_` functions. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/addrman.cpp | 205 ++++++++++++++++++++++++------------------------ src/addrman.h | 40 +++++----- 2 files changed, 123 insertions(+), 122 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 0fa8edd3164..99c4f00d6d2 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -753,108 +753,6 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const } } -void CAddrMan::Check() const -{ - AssertLockHeld(cs); - - // Run consistency checks 1 in m_consistency_check_ratio times if enabled - if (m_consistency_check_ratio == 0) return; - if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; - - const int err{ForceCheckAddrman()}; - if (err) { - LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); - assert(false); - } -} - -int CAddrMan::ForceCheckAddrman() const -{ - AssertLockHeld(cs); - - LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); - - std::unordered_set setTried; - std::unordered_map mapNew; - - if (vRandom.size() != (size_t)(nTried + nNew)) - return -7; - - for (const auto& entry : mapInfo) { - int n = entry.first; - const CAddrInfo& info = entry.second; - if (info.fInTried) { - if (!info.nLastSuccess) - return -1; - if (info.nRefCount) - return -2; - setTried.insert(n); - } else { - if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS) - return -3; - if (!info.nRefCount) - return -4; - mapNew[n] = info.nRefCount; - } - const auto it{mapAddr.find(info)}; - if (it == mapAddr.end() || it->second != n) { - return -5; - } - if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) - return -14; - if (info.nLastTry < 0) - return -6; - if (info.nLastSuccess < 0) - return -8; - } - - if (setTried.size() != (size_t)nTried) - return -9; - if (mapNew.size() != (size_t)nNew) - return -10; - - for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) { - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvTried[n][i] != -1) { - if (!setTried.count(vvTried[n][i])) - return -11; - const auto it{mapInfo.find(vvTried[n][i])}; - if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { - return -17; - } - if (it->second.GetBucketPosition(nKey, false, n) != i) { - return -18; - } - setTried.erase(vvTried[n][i]); - } - } - } - - for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvNew[n][i] != -1) { - if (!mapNew.count(vvNew[n][i])) - return -12; - const auto it{mapInfo.find(vvNew[n][i])}; - if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) { - return -19; - } - if (--mapNew[vvNew[n][i]] == 0) - mapNew.erase(vvNew[n][i]); - } - } - } - - if (setTried.size()) - return -13; - if (mapNew.size()) - return -15; - if (nKey.IsNull()) - return -16; - - LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); - return 0; -} void CAddrMan::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const { @@ -1023,6 +921,109 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return mapInfo[id_old]; } +void CAddrMan::Check() const +{ + AssertLockHeld(cs); + + // Run consistency checks 1 in m_consistency_check_ratio times if enabled + if (m_consistency_check_ratio == 0) return; + if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; + + const int err{ForceCheckAddrman()}; + if (err) { + LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); + assert(false); + } +} + +int CAddrMan::ForceCheckAddrman() const +{ + AssertLockHeld(cs); + + LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); + + std::unordered_set setTried; + std::unordered_map mapNew; + + if (vRandom.size() != (size_t)(nTried + nNew)) + return -7; + + for (const auto& entry : mapInfo) { + int n = entry.first; + const CAddrInfo& info = entry.second; + if (info.fInTried) { + if (!info.nLastSuccess) + return -1; + if (info.nRefCount) + return -2; + setTried.insert(n); + } else { + if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS) + return -3; + if (!info.nRefCount) + return -4; + mapNew[n] = info.nRefCount; + } + const auto it{mapAddr.find(info)}; + if (it == mapAddr.end() || it->second != n) { + return -5; + } + if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) + return -14; + if (info.nLastTry < 0) + return -6; + if (info.nLastSuccess < 0) + return -8; + } + + if (setTried.size() != (size_t)nTried) + return -9; + if (mapNew.size() != (size_t)nNew) + return -10; + + for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) { + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvTried[n][i] != -1) { + if (!setTried.count(vvTried[n][i])) + return -11; + const auto it{mapInfo.find(vvTried[n][i])}; + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { + return -17; + } + if (it->second.GetBucketPosition(nKey, false, n) != i) { + return -18; + } + setTried.erase(vvTried[n][i]); + } + } + } + + for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; n++) { + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[n][i] != -1) { + if (!mapNew.count(vvNew[n][i])) + return -12; + const auto it{mapInfo.find(vvNew[n][i])}; + if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) { + return -19; + } + if (--mapNew[vvNew[n][i]] == 0) + mapNew.erase(vvNew[n][i]); + } + } + } + + if (setTried.size()) + return -13; + if (mapNew.size()) + return -15; + if (nKey.IsNull()) + return -16; + + LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); + return 0; +} + size_t CAddrMan::size() const { LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead diff --git a/src/addrman.h b/src/addrman.h index bc2f934caa4..d3d764c9c64 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -142,16 +142,16 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; class CAddrMan { public: + explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); + + ~CAddrMan(); + template void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); template void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs); - explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); - - ~CAddrMan(); - //! Return the number of (unique) addresses in all tables. size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -289,15 +289,15 @@ private: //! Swap two elements in vRandom. void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Move an entry from the "new" table(s) to the "tried" table - void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Delete an entry. It must not be in tried, and have refcount 0. void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Clear a position in a "new" table. This is the only place where entries are actually deleted. void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Move an entry from the "new" table(s) to the "tried" table + void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Mark an entry "good", possibly moving it from "new" to "tried". void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -310,19 +310,6 @@ private: //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); - //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Return a random to-be-evicted tried table address. - CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. - void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Perform consistency check, regardless of m_consistency_check_ratio. - //! @returns an error code or zero. - int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** * Return all or many randomly selected addresses, optionally by network. * @@ -349,6 +336,19 @@ private: //! Update an entry's service bits. void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. + void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Perform consistency check, regardless of m_consistency_check_ratio. + //! @returns an error code or zero. + int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); + friend class CAddrManTest; friend class CAddrManDeterministic; }; From 8af5b54f973e11c847345418d8631bc301b96130 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 1 Sep 2021 11:21:29 -0700 Subject: [PATCH 03/13] [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. Introduce the pimpl pattern for CAddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics. Since the unit & fuzz tests currently rely on accessing CAddrMan internals, this commit introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files. Review hint: git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/Makefile.am | 1 + src/addrman.cpp | 157 ++++++++++++++++++++-------- src/addrman.h | 188 +++------------------------------ src/addrman_impl.h | 206 +++++++++++++++++++++++++++++++++++++ src/test/addrman_tests.cpp | 19 ++-- src/test/fuzz/addrman.cpp | 45 ++++---- 6 files changed, 371 insertions(+), 245 deletions(-) create mode 100644 src/addrman_impl.h diff --git a/src/Makefile.am b/src/Makefile.am index 52c8b85357b..72aa3829f4c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -117,6 +117,7 @@ endif BITCOIN_CORE_H = \ addrdb.h \ addrman.h \ + addrman_impl.h \ attributes.h \ banman.h \ base58.h \ diff --git a/src/addrman.cpp b/src/addrman.cpp index 99c4f00d6d2..2fc6ca29a12 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -101,7 +102,7 @@ double CAddrInfo::GetChance(int64_t nNow) const return fChance; } -CAddrMan::CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) +AddrManImpl::AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio) : insecure_rand{deterministic} , nKey{deterministic ? uint256{1} : insecure_rand.rand256()} , m_consistency_check_ratio{consistency_check_ratio} @@ -119,13 +120,13 @@ CAddrMan::CAddrMan(std::vector asmap, bool deterministic, int32_t consiste } } -CAddrMan::~CAddrMan() +AddrManImpl::~AddrManImpl() { nKey.SetNull(); } template -void CAddrMan::Serialize(Stream& s_) const +void AddrManImpl::Serialize(Stream& s_) const { LOCK(cs); @@ -228,7 +229,7 @@ void CAddrMan::Serialize(Stream& s_) const } template -void CAddrMan::Unserialize(Stream& s_) +void AddrManImpl::Unserialize(Stream& s_) { LOCK(cs); @@ -399,16 +400,7 @@ void CAddrMan::Unserialize(Stream& s_) } } -// explicit instantiation -template void CAddrMan::Serialize(CHashWriter& s) const; -template void CAddrMan::Serialize(CAutoFile& s) const; -template void CAddrMan::Serialize(CDataStream& s) const; -template void CAddrMan::Unserialize(CAutoFile& s); -template void CAddrMan::Unserialize(CHashVerifier& s); -template void CAddrMan::Unserialize(CDataStream& s); -template void CAddrMan::Unserialize(CHashVerifier& s); - -CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId) +CAddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId) { AssertLockHeld(cs); @@ -423,7 +415,7 @@ CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId) return nullptr; } -CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) +CAddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) { AssertLockHeld(cs); @@ -437,7 +429,7 @@ CAddrInfo* CAddrMan::Create(const CAddress& addr, const CNetAddr& addrSource, in return &mapInfo[nId]; } -void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) const +void AddrManImpl::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) const { AssertLockHeld(cs); @@ -461,7 +453,7 @@ void CAddrMan::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) const vRandom[nRndPos2] = nId1; } -void CAddrMan::Delete(int nId) +void AddrManImpl::Delete(int nId) { AssertLockHeld(cs); @@ -477,7 +469,7 @@ void CAddrMan::Delete(int nId) nNew--; } -void CAddrMan::ClearNew(int nUBucket, int nUBucketPos) +void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos) { AssertLockHeld(cs); @@ -494,7 +486,7 @@ void CAddrMan::ClearNew(int nUBucket, int nUBucketPos) } } -void CAddrMan::MakeTried(CAddrInfo& info, int nId) +void AddrManImpl::MakeTried(CAddrInfo& info, int nId) { AssertLockHeld(cs); @@ -547,7 +539,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) info.fInTried = true; } -void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime) +void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) { AssertLockHeld(cs); @@ -603,7 +595,7 @@ 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) +bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) { AssertLockHeld(cs); @@ -678,7 +670,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP return fNew; } -void CAddrMan::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) +void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) { AssertLockHeld(cs); @@ -702,7 +694,7 @@ void CAddrMan::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) } } -CAddrInfo CAddrMan::Select_(bool newOnly) const +CAddrInfo AddrManImpl::Select_(bool newOnly) const { AssertLockHeld(cs); @@ -753,8 +745,7 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const } } - -void CAddrMan::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const +void AddrManImpl::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const { AssertLockHeld(cs); @@ -789,7 +780,7 @@ void CAddrMan::GetAddr_(std::vector& vAddr, size_t max_addresses, size } } -void CAddrMan::Connected_(const CService& addr, int64_t nTime) +void AddrManImpl::Connected_(const CService& addr, int64_t nTime) { AssertLockHeld(cs); @@ -811,7 +802,7 @@ void CAddrMan::Connected_(const CService& addr, int64_t nTime) info.nTime = nTime; } -void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices) +void AddrManImpl::SetServices_(const CService& addr, ServiceFlags nServices) { AssertLockHeld(cs); @@ -831,7 +822,7 @@ void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices) info.nServices = nServices; } -void CAddrMan::ResolveCollisions_() +void AddrManImpl::ResolveCollisions_() { AssertLockHeld(cs); @@ -892,7 +883,7 @@ void CAddrMan::ResolveCollisions_() } } -CAddrInfo CAddrMan::SelectTriedCollision_() +CAddrInfo AddrManImpl::SelectTriedCollision_() { AssertLockHeld(cs); @@ -921,7 +912,7 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return mapInfo[id_old]; } -void CAddrMan::Check() const +void AddrManImpl::Check() const { AssertLockHeld(cs); @@ -936,7 +927,7 @@ void CAddrMan::Check() const } } -int CAddrMan::ForceCheckAddrman() const +int AddrManImpl::ForceCheckAddrman() const { AssertLockHeld(cs); @@ -1024,13 +1015,13 @@ int CAddrMan::ForceCheckAddrman() const return 0; } -size_t CAddrMan::size() const +size_t AddrManImpl::size() const { LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead return vRandom.size(); } -bool CAddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +bool AddrManImpl::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) { LOCK(cs); int nAdd = 0; @@ -1044,7 +1035,7 @@ bool CAddrMan::Add(const std::vector &vAddr, const CNetAddr& source, i return nAdd > 0; } -void CAddrMan::Good(const CService &addr, int64_t nTime) +void AddrManImpl::Good(const CService &addr, int64_t nTime) { LOCK(cs); Check(); @@ -1052,7 +1043,7 @@ void CAddrMan::Good(const CService &addr, int64_t nTime) Check(); } -void CAddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +void AddrManImpl::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) { LOCK(cs); Check(); @@ -1060,8 +1051,7 @@ void CAddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) Check(); } - -void CAddrMan::ResolveCollisions() +void AddrManImpl::ResolveCollisions() { LOCK(cs); Check(); @@ -1069,7 +1059,7 @@ void CAddrMan::ResolveCollisions() Check(); } -CAddrInfo CAddrMan::SelectTriedCollision() +CAddrInfo AddrManImpl::SelectTriedCollision() { LOCK(cs); Check(); @@ -1078,7 +1068,7 @@ CAddrInfo CAddrMan::SelectTriedCollision() return ret; } -CAddrInfo CAddrMan::Select(bool newOnly) const +CAddrInfo AddrManImpl::Select(bool newOnly) const { LOCK(cs); Check(); @@ -1087,7 +1077,7 @@ CAddrInfo CAddrMan::Select(bool newOnly) const return addrRet; } -std::vector CAddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const { LOCK(cs); Check(); @@ -1097,7 +1087,7 @@ std::vector CAddrMan::GetAddr(size_t max_addresses, size_t max_pct, st return vAddr; } -void CAddrMan::Connected(const CService &addr, int64_t nTime) +void AddrManImpl::Connected(const CService &addr, int64_t nTime) { LOCK(cs); Check(); @@ -1105,7 +1095,7 @@ void CAddrMan::Connected(const CService &addr, int64_t nTime) Check(); } -void CAddrMan::SetServices(const CService &addr, ServiceFlags nServices) +void AddrManImpl::SetServices(const CService &addr, ServiceFlags nServices) { LOCK(cs); Check(); @@ -1113,7 +1103,88 @@ void CAddrMan::SetServices(const CService &addr, ServiceFlags nServices) Check(); } -const std::vector& CAddrMan::GetAsmap() const +const std::vector& AddrManImpl::GetAsmap() const { return m_asmap; } + +CAddrMan::CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) + : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio)) {} + +CAddrMan::~CAddrMan() = default; + +template +void CAddrMan::Serialize(Stream& s_) const +{ + m_impl->Serialize(s_); +} + +template +void CAddrMan::Unserialize(Stream& s_) +{ + m_impl->Unserialize(s_); +} + +// explicit instantiation +template void CAddrMan::Serialize(CHashWriter& s) const; +template void CAddrMan::Serialize(CAutoFile& s) const; +template void CAddrMan::Serialize(CDataStream& s) const; +template void CAddrMan::Unserialize(CAutoFile& s); +template void CAddrMan::Unserialize(CHashVerifier& s); +template void CAddrMan::Unserialize(CDataStream& s); +template void CAddrMan::Unserialize(CHashVerifier& s); + +size_t CAddrMan::size() const +{ + return m_impl->size(); +} + +bool CAddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +{ + return m_impl->Add(vAddr, source, nTimePenalty); +} + +void CAddrMan::Good(const CService &addr, int64_t nTime) +{ + m_impl->Good(addr, nTime); +} + +void CAddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +{ + m_impl->Attempt(addr, fCountFailure, nTime); +} + +void CAddrMan::ResolveCollisions() +{ + m_impl->ResolveCollisions(); +} + +CAddrInfo CAddrMan::SelectTriedCollision() +{ + return m_impl->SelectTriedCollision(); +} + +CAddrInfo CAddrMan::Select(bool newOnly) const +{ + return m_impl->Select(newOnly); +} + +std::vector CAddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +{ + return m_impl->GetAddr(max_addresses, max_pct, network); +} + +void CAddrMan::Connected(const CService &addr, int64_t nTime) +{ + m_impl->Connected(addr, nTime); +} + +void CAddrMan::SetServices(const CService &addr, ServiceFlags nServices) +{ + m_impl->SetServices(addr, nServices); +} + +const std::vector& CAddrMan::GetAsmap() const +{ + return m_impl->GetAsmap(); +} diff --git a/src/addrman.h b/src/addrman.h index d3d764c9c64..2135295b892 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -19,6 +19,8 @@ #include #include +class AddrManImpl; + /** Default for -checkaddrman */ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; @@ -53,7 +55,7 @@ private: //! position in vRandom mutable int nRandomPos{-1}; - friend class CAddrMan; + friend class AddrManImpl; friend class CAddrManDeterministic; public: @@ -141,42 +143,41 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; */ class CAddrMan { + const std::unique_ptr m_impl; + public: explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); ~CAddrMan(); template - void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Serialize(Stream& s_) const; template - void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Unserialize(Stream& s_); //! Return the number of (unique) addresses in all tables. - size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + size_t size() const; //! Add addresses to addrman's new table. - bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); //! Mark an entry as accessible. - void Good(const CService &addr, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Good(const CService &addr, int64_t nTime = GetAdjustedTime()); //! Mark an entry as connection attempted to. - void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()); //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs); + void ResolveCollisions(); //! Randomly select an address in tried that another address is attempting to evict. - CAddrInfo SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); + CAddrInfo SelectTriedCollision(); /** * Choose an address to connect to. */ - CAddrInfo Select(bool newOnly = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + CAddrInfo Select(bool newOnly = false) const; /** * Return all or many randomly selected addresses, optionally by network. @@ -185,170 +186,15 @@ public: * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). */ - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const; //! Outer function for Connected_() - void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()); - void SetServices(const CService &addr, ServiceFlags nServices) - EXCLUSIVE_LOCKS_REQUIRED(!cs); + void SetServices(const CService &addr, ServiceFlags nServices); const std::vector& GetAsmap() const; -private: - //! A mutex to protect the inner data structures. - mutable Mutex cs; - - //! Source of random numbers for randomization in inner loops - mutable FastRandomContext insecure_rand GUARDED_BY(cs); - - //! secret key to randomize bucket select with - uint256 nKey; - - //! Serialization versions. - enum Format : uint8_t { - V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 - V1_DETERMINISTIC = 1, //!< for pre-asmap files - V2_ASMAP = 2, //!< for files including asmap version - V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format - }; - - //! The maximum format this software knows it can unserialize. Also, we always serialize - //! in this format. - //! The format (first byte in the serialized stream) can be higher than this and - //! still this software may be able to unserialize the file - if the second byte - //! (see `lowest_compatible` in `Unserialize()`) is less or equal to this. - static constexpr Format FILE_FORMAT = Format::V3_BIP155; - - //! The initial value of a field that is incremented every time an incompatible format - //! change is made (such that old software versions would not be able to parse and - //! understand the new file format). This is 32 because we overtook the "key size" - //! field which was 32 historically. - //! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead. - static constexpr uint8_t INCOMPATIBILITY_BASE = 32; - - //! last used nId - int nIdCount GUARDED_BY(cs){0}; - - //! table with information about all nIds - std::unordered_map mapInfo GUARDED_BY(cs); - - //! find an nId based on its network address - std::unordered_map mapAddr GUARDED_BY(cs); - - //! randomly-ordered vector of all nIds - //! This is mutable because it is unobservable outside the class, so any - //! changes to it (even in const methods) are also unobservable. - mutable std::vector vRandom GUARDED_BY(cs); - - // number of "tried" entries - int nTried GUARDED_BY(cs){0}; - - //! list of "tried" buckets - int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); - - //! number of (unique) "new" entries - int nNew GUARDED_BY(cs){0}; - - //! list of "new" buckets - int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); - - //! last time Good was called (memory only). Initially set to 1 so that "never" is strictly worse. - int64_t nLastGood GUARDED_BY(cs){1}; - - //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. - std::set m_tried_collisions; - - /** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */ - const int32_t m_consistency_check_ratio; - - // Compressed IP->ASN mapping, loaded from a file when a node starts. - // Should be always empty if no file was provided. - // This mapping is then used for bucketing nodes in Addrman. - // - // If asmap is provided, nodes will be bucketed by - // AS they belong to, in order to make impossible for a node - // to connect to several nodes hosted in a single AS. - // This is done in response to Erebus attack, but also to generally - // diversify the connections every node creates, - // especially useful when a large fraction of nodes - // operate under a couple of cloud providers. - // - // If a new asmap was provided, the existing records - // would be re-bucketed accordingly. - const std::vector m_asmap; - - //! Find an entry. - CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. - CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Swap two elements in vRandom. - void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Delete an entry. It must not be in tried, and have refcount 0. - void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Clear a position in a "new" table. This is the only place where entries are actually deleted. - void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Move an entry from the "new" table(s) to the "tried" table - void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Mark an entry "good", possibly moving it from "new" to "tried". - void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Add an entry to the "new" table. - bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Mark an entry as attempted to connect. - void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. - CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); - - /** - * Return all or many randomly selected addresses, optionally by network. - * - * @param[out] vAddr Vector of randomly selected addresses from vRandom. - * @param[in] max_addresses Maximum number of addresses to return (0 = all). - * @param[in] max_pct Maximum percentage of addresses to return (0 = all). - * @param[in] network Select only addresses of this network (nullopt = all). - */ - void GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); - - /** We have successfully connected to this peer. Calling this function - * updates the CAddress's nTime, which is used in our IsTerrible() - * decisions and gossiped to peers. Callers should be careful that updating - * this information doesn't leak topology information to network spies. - * - * net_processing calls this function when it *disconnects* from a peer to - * not leak information about currently connected peers. - * - * @param[in] addr The address of the peer we were connected to - * @param[in] nTime The time that we were last connected to this peer - */ - void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Update an entry's service bits. - void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. - void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Return a random to-be-evicted tried table address. - CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. - void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); - - //! Perform consistency check, regardless of m_consistency_check_ratio. - //! @returns an error code or zero. - int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); - friend class CAddrManTest; friend class CAddrManDeterministic; }; diff --git a/src/addrman_impl.h b/src/addrman_impl.h new file mode 100644 index 00000000000..ee4b55e5c4c --- /dev/null +++ b/src/addrman_impl.h @@ -0,0 +1,206 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_ADDRMAN_IMPL_H +#define BITCOIN_ADDRMAN_IMPL_H + +class AddrManImpl +{ +public: + AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio); + + ~AddrManImpl(); + + template + void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + + template + void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs); + + size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + + bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + void Good(const CService &addr, int64_t nTime) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + void Attempt(const CService &addr, bool fCountFailure, int64_t nTime) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs); + + CAddrInfo SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); + + CAddrInfo Select(bool newOnly) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + void Connected(const CService &addr, int64_t nTime) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + void SetServices(const CService &addr, ServiceFlags nServices) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + const std::vector& GetAsmap() const; + + friend class CAddrManTest; + friend class CAddrManDeterministic; + +private: + //! A mutex to protect the inner data structures. + mutable Mutex cs; + + //! Source of random numbers for randomization in inner loops + mutable FastRandomContext insecure_rand GUARDED_BY(cs); + + //! secret key to randomize bucket select with + uint256 nKey; + + //! Serialization versions. + enum Format : uint8_t { + V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 + V1_DETERMINISTIC = 1, //!< for pre-asmap files + V2_ASMAP = 2, //!< for files including asmap version + V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format + }; + + //! The maximum format this software knows it can unserialize. Also, we always serialize + //! in this format. + //! The format (first byte in the serialized stream) can be higher than this and + //! still this software may be able to unserialize the file - if the second byte + //! (see `lowest_compatible` in `Unserialize()`) is less or equal to this. + static constexpr Format FILE_FORMAT = Format::V3_BIP155; + + //! The initial value of a field that is incremented every time an incompatible format + //! change is made (such that old software versions would not be able to parse and + //! understand the new file format). This is 32 because we overtook the "key size" + //! field which was 32 historically. + //! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead. + static constexpr uint8_t INCOMPATIBILITY_BASE = 32; + + //! last used nId + int nIdCount GUARDED_BY(cs){0}; + + //! table with information about all nIds + std::unordered_map mapInfo GUARDED_BY(cs); + + //! find an nId based on its network address + std::unordered_map mapAddr GUARDED_BY(cs); + + //! randomly-ordered vector of all nIds + //! This is mutable because it is unobservable outside the class, so any + //! changes to it (even in const methods) are also unobservable. + mutable std::vector vRandom GUARDED_BY(cs); + + // number of "tried" entries + int nTried GUARDED_BY(cs){0}; + + //! list of "tried" buckets + int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); + + //! number of (unique) "new" entries + int nNew GUARDED_BY(cs){0}; + + //! list of "new" buckets + int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); + + //! last time Good was called (memory only). Initially set to 1 so that "never" is strictly worse. + int64_t nLastGood GUARDED_BY(cs){1}; + + //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. + std::set m_tried_collisions; + + /** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */ + const int32_t m_consistency_check_ratio; + + // Compressed IP->ASN mapping, loaded from a file when a node starts. + // Should be always empty if no file was provided. + // This mapping is then used for bucketing nodes in Addrman. + // + // If asmap is provided, nodes will be bucketed by + // AS they belong to, in order to make impossible for a node + // to connect to several nodes hosted in a single AS. + // This is done in response to Erebus attack, but also to generally + // diversify the connections every node creates, + // especially useful when a large fraction of nodes + // operate under a couple of cloud providers. + // + // If a new asmap was provided, the existing records + // would be re-bucketed accordingly. + const std::vector m_asmap; + + //! Find an entry. + CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. + CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Swap two elements in vRandom. + void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Delete an entry. It must not be in tried, and have refcount 0. + void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Clear a position in a "new" table. This is the only place where entries are actually deleted. + void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Move an entry from the "new" table(s) to the "tried" table + void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Mark an entry "good", possibly moving it from "new" to "tried". + void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Add an entry to the "new" table. + bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Mark an entry as attempted to connect. + void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. + CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); + + /** + * Return all or many randomly selected addresses, optionally by network. + * + * @param[out] vAddr Vector of randomly selected addresses from vRandom. + * @param[in] max_addresses Maximum number of addresses to return (0 = all). + * @param[in] max_pct Maximum percentage of addresses to return (0 = all). + * @param[in] network Select only addresses of this network (nullopt = all). + */ + void GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); + + /** We have successfully connected to this peer. Calling this function + * updates the CAddress's nTime, which is used in our IsTerrible() + * decisions and gossiped to peers. Callers should be careful that updating + * this information doesn't leak topology information to network spies. + * + * net_processing calls this function when it *disconnects* from a peer to + * not leak information about currently connected peers. + * + * @param[in] addr The address of the peer we were connected to + * @param[in] nTime The time that we were last connected to this peer + */ + void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Update an entry's service bits. + void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. + void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); + + //! Perform consistency check, regardless of m_consistency_check_ratio. + //! @returns an error code or zero. + int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); +}; + +#endif // BITCOIN_ADDRMAN_IMPL_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 01a492a20b8..41c298e0364 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -90,30 +91,30 @@ public: CAddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) { - LOCK(cs); - return CAddrMan::Find(addr, pnId); + LOCK(m_impl->cs); + return m_impl->Find(addr, pnId); } CAddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) { - LOCK(cs); - return CAddrMan::Create(addr, addrSource, pnId); + LOCK(m_impl->cs); + return m_impl->Create(addr, addrSource, pnId); } void Delete(int nId) { - LOCK(cs); - CAddrMan::Delete(nId); + LOCK(m_impl->cs); + m_impl->Delete(nId); } // Used to test deserialization std::pair GetBucketAndEntry(const CAddress& addr) { - LOCK(cs); - int nId = mapAddr[addr]; + LOCK(m_impl->cs); + int nId = m_impl->mapAddr[addr]; for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) { for (int entry = 0; entry < ADDRMAN_BUCKET_SIZE; ++entry) { - if (nId == vvNew[bucket][entry]) { + if (nId == m_impl->vvNew[bucket][entry]) { return std::pair(bucket, entry); } } diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 45ee778b871..6625b69b142 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -43,13 +44,13 @@ public: : CAddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0) , m_fuzzed_data_provider(fuzzed_data_provider) { - WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); + WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } /** * Generate a random address. Always returns a valid address. */ - CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(cs) + CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) { CNetAddr addr; if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { @@ -61,7 +62,7 @@ public: {4, ADDR_TORV3_SIZE}, {5, ADDR_I2P_SIZE}, {6, ADDR_CJDNS_SIZE}}; - uint8_t net = insecure_rand.randrange(5) + 1; // [1..5] + uint8_t net = m_impl->insecure_rand.randrange(5) + 1; // [1..5] if (net == 3) { net = 6; } @@ -69,7 +70,7 @@ public: CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); s << net; - s << insecure_rand.randbytes(net_len_map.at(net)); + s << m_impl->insecure_rand.randbytes(net_len_map.at(net)); s >> addr; } @@ -89,7 +90,7 @@ public: */ void Fill() { - LOCK(cs); + LOCK(m_impl->cs); // Add some of the addresses directly to the "tried" table. @@ -102,20 +103,20 @@ public: // the latter is exhausted it just returns 0. for (size_t i = 0; i < num_sources; ++i) { const auto source = RandAddr(); - const size_t num_addresses = insecure_rand.randrange(500) + 1; // [1..500] + const size_t num_addresses = m_impl->insecure_rand.randrange(500) + 1; // [1..500] for (size_t j = 0; j < num_addresses; ++j) { const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK}; - const auto time_penalty = insecure_rand.randrange(100000001); - Add_(addr, source, time_penalty); + const auto time_penalty = m_impl->insecure_rand.randrange(100000001); + m_impl->Add_(addr, source, time_penalty); - if (n > 0 && mapInfo.size() % n == 0) { - Good_(addr, false, GetTime()); + if (n > 0 && m_impl->mapInfo.size() % n == 0) { + m_impl->Good_(addr, false, GetTime()); } // Add 10% of the addresses from more than one source. - if (insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { - Add_(addr, prev_source, time_penalty); + if (m_impl->insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { + m_impl->Add_({addr}, prev_source, time_penalty); } } prev_source = source; @@ -131,10 +132,10 @@ public: */ bool operator==(const CAddrManDeterministic& other) { - LOCK2(cs, other.cs); + LOCK2(m_impl->cs, other.m_impl->cs); - if (mapInfo.size() != other.mapInfo.size() || nNew != other.nNew || - nTried != other.nTried) { + if (m_impl->mapInfo.size() != other.m_impl->mapInfo.size() || m_impl->nNew != other.m_impl->nNew || + m_impl->nTried != other.m_impl->nTried) { return false; } @@ -160,15 +161,15 @@ public: using Addresses = std::unordered_set; - const size_t num_addresses{mapInfo.size()}; + const size_t num_addresses{m_impl->mapInfo.size()}; Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; - for (const auto& [id, addr] : mapInfo) { + for (const auto& [id, addr] : m_impl->mapInfo) { addresses.insert(addr); } Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; - for (const auto& [id, addr] : other.mapInfo) { + for (const auto& [id, addr] : other.m_impl->mapInfo) { other_addresses.insert(addr); } @@ -176,14 +177,14 @@ public: return false; } - auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(cs, other.cs) { + auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) { if (id == -1 && other_id == -1) { return true; } if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { return false; } - return mapInfo.at(id) == other.mapInfo.at(other_id); + return m_impl->mapInfo.at(id) == other.m_impl->mapInfo.at(other_id); }; // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` @@ -191,7 +192,7 @@ public: // themselves may differ between `vvNew` and `other.vvNew`. for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { - if (!IdsReferToSameAddress(vvNew[i][j], other.vvNew[i][j])) { + if (!IdsReferToSameAddress(m_impl->vvNew[i][j], other.m_impl->vvNew[i][j])) { return false; } } @@ -200,7 +201,7 @@ public: // Same for `vvTried`. for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { - if (!IdsReferToSameAddress(vvTried[i][j], other.vvTried[i][j])) { + if (!IdsReferToSameAddress(m_impl->vvTried[i][j], other.m_impl->vvTried[i][j])) { return false; } } From 7cba9d56185b9325ce41d79364e448462fff0f6a Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 25 Aug 2021 15:40:59 -0700 Subject: [PATCH 04/13] [net, addrman] Remove external dependencies on CAddrInfo objects CAddrInfo objects are an implementation detail of how AddrMan manages and adds metadata to different records. Encapsulate this logic by updating Select & SelectTriedCollision to return the additional info that the callers need. --- src/addrman.cpp | 41 +++++++++++++++---------------- src/addrman.h | 16 +++++++++--- src/addrman_impl.h | 8 +++--- src/bench/addrman.cpp | 2 +- src/net.cpp | 13 +++++----- src/test/addrman_tests.cpp | 50 +++++++++++++++++++------------------- 6 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 2fc6ca29a12..324bab7292c 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -694,15 +694,13 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi } } -CAddrInfo AddrManImpl::Select_(bool newOnly) const +std::pair AddrManImpl::Select_(bool newOnly) const { AssertLockHeld(cs); - if (vRandom.empty()) - return CAddrInfo(); + if (vRandom.empty()) return {}; - if (newOnly && nNew == 0) - return CAddrInfo(); + if (newOnly && nNew == 0) return {}; // Use a 50% chance for choosing between tried and new table entries. if (!newOnly && @@ -720,8 +718,9 @@ CAddrInfo AddrManImpl::Select_(bool newOnly) const const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); const CAddrInfo& info{it_found->second}; - if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) - return info; + if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { + return {info, info.nLastTry}; + } fChanceFactor *= 1.2; } } else { @@ -738,8 +737,9 @@ CAddrInfo AddrManImpl::Select_(bool newOnly) const const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); const CAddrInfo& info{it_found->second}; - if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) - return info; + if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { + return {info, info.nLastTry}; + } fChanceFactor *= 1.2; } } @@ -883,11 +883,11 @@ void AddrManImpl::ResolveCollisions_() } } -CAddrInfo AddrManImpl::SelectTriedCollision_() +std::pair AddrManImpl::SelectTriedCollision_() { AssertLockHeld(cs); - if (m_tried_collisions.size() == 0) return CAddrInfo(); + if (m_tried_collisions.size() == 0) return {}; std::set::iterator it = m_tried_collisions.begin(); @@ -898,7 +898,7 @@ CAddrInfo AddrManImpl::SelectTriedCollision_() // If id_new not found in mapInfo remove it from m_tried_collisions if (mapInfo.count(id_new) != 1) { m_tried_collisions.erase(it); - return CAddrInfo(); + return {}; } const CAddrInfo& newInfo = mapInfo[id_new]; @@ -907,9 +907,8 @@ CAddrInfo AddrManImpl::SelectTriedCollision_() int tried_bucket = newInfo.GetTriedBucket(nKey, m_asmap); int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); - int id_old = vvTried[tried_bucket][tried_bucket_pos]; - - return mapInfo[id_old]; + const CAddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; + return {info_old, info_old.nLastTry}; } void AddrManImpl::Check() const @@ -1059,20 +1058,20 @@ void AddrManImpl::ResolveCollisions() Check(); } -CAddrInfo AddrManImpl::SelectTriedCollision() +std::pair AddrManImpl::SelectTriedCollision() { LOCK(cs); Check(); - const CAddrInfo ret = SelectTriedCollision_(); + const auto ret = SelectTriedCollision_(); Check(); return ret; } -CAddrInfo AddrManImpl::Select(bool newOnly) const +std::pair AddrManImpl::Select(bool newOnly) const { LOCK(cs); Check(); - const CAddrInfo addrRet = Select_(newOnly); + const auto addrRet = Select_(newOnly); Check(); return addrRet; } @@ -1159,12 +1158,12 @@ void CAddrMan::ResolveCollisions() m_impl->ResolveCollisions(); } -CAddrInfo CAddrMan::SelectTriedCollision() +std::pair CAddrMan::SelectTriedCollision() { return m_impl->SelectTriedCollision(); } -CAddrInfo CAddrMan::Select(bool newOnly) const +std::pair CAddrMan::Select(bool newOnly) const { return m_impl->Select(newOnly); } diff --git a/src/addrman.h b/src/addrman.h index 2135295b892..d176d0a42c1 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -171,13 +171,23 @@ public: //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. void ResolveCollisions(); - //! Randomly select an address in tried that another address is attempting to evict. - CAddrInfo SelectTriedCollision(); + /** + * Randomly select an address in the tried table that another address is + * attempting to evict. + * + * @return CAddress The record for the selected tried peer. + * int64_t The last time we attempted to connect to that peer. + */ + std::pair SelectTriedCollision(); /** * Choose an address to connect to. + * + * @param[in] newOnly Whether to only select addresses from the new table. + * @return CAddress The record for the selected peer. + * int64_t The last time we attempted to connect to that peer. */ - CAddrInfo Select(bool newOnly = false) const; + std::pair Select(bool newOnly = false) const; /** * Return all or many randomly selected addresses, optionally by network. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index ee4b55e5c4c..6752d5b81d1 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -31,9 +31,9 @@ public: void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs); - CAddrInfo SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::pair SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); - CAddrInfo Select(bool newOnly) const + std::pair Select(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const @@ -161,7 +161,7 @@ private: void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. - CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::pair Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Return all or many randomly selected addresses, optionally by network. @@ -193,7 +193,7 @@ private: void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); //! Return a random to-be-evicted tried table address. - CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); + std::pair SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index bebf86a09d9..b4c2e35f422 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -87,7 +87,7 @@ static void AddrManSelect(benchmark::Bench& bench) bench.run([&] { const auto& address = addrman.Select(); - assert(address.GetPort() > 0); + assert(address.first.GetPort() > 0); }); } diff --git a/src/net.cpp b/src/net.cpp index b8ff0b13ea8..df3b88725e4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2006,17 +2006,18 @@ void CConnman::ThreadOpenConnections(const std::vector connect) if (nTries > 100) break; - CAddrInfo addr; + CAddress addr; + int64_t addr_last_try{0}; if (fFeeler) { // First, try to get a tried table collision address. This returns // an empty (invalid) address if there are no collisions to try. - addr = addrman.SelectTriedCollision(); + std::tie(addr, addr_last_try) = addrman.SelectTriedCollision(); if (!addr.IsValid()) { // No tried table collisions. Select a new table address // for our feeler. - addr = addrman.Select(true); + std::tie(addr, addr_last_try) = addrman.Select(true); } else if (AlreadyConnectedToAddress(addr)) { // If test-before-evict logic would have us connect to a // peer that we're already connected to, just mark that @@ -2025,11 +2026,11 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // a currently-connected peer. addrman.Good(addr); // Select a new table address for our feeler instead. - addr = addrman.Select(true); + std::tie(addr, addr_last_try) = addrman.Select(true); } } else { // Not a feeler - addr = addrman.Select(); + std::tie(addr, addr_last_try) = addrman.Select(); } // Require outbound connections, other than feelers, to be to distinct network groups @@ -2046,7 +2047,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) continue; // only consider very recently tried nodes after 30 failed attempts - if (nANow - addr.nLastTry < 600 && nTries < 30) + if (nANow - addr_last_try < 600 && nTries < 30) continue; // for non-feelers, require all the services we'll want, diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 41c298e0364..022f60a8ede 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -172,14 +172,14 @@ BOOST_AUTO_TEST_CASE(addrman_simple) // Test: Does Addrman respond correctly when empty. BOOST_CHECK_EQUAL(addrman->size(), 0U); - CAddrInfo addr_null = addrman->Select(); + auto addr_null = addrman->Select().first; BOOST_CHECK_EQUAL(addr_null.ToString(), "[::]:0"); // Test: Does Addrman::Add work as expected. CService addr1 = ResolveService("250.1.1.1", 8333); BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman->size(), 1U); - CAddrInfo addr_ret1 = addrman->Select(); + auto addr_ret1 = addrman->Select().first; BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333"); // Test: Does IP address deduplication work correctly. @@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports) CService addr1_port = ResolveService("250.1.1.1", 8334); BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman.size(), 1U); - CAddrInfo addr_ret2 = addrman.Select(); + auto addr_ret2 = addrman.Select().first; BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333"); // Test: Add same IP but diff port to tried table, it doesn't get added. @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports) addrman.Good(CAddress(addr1_port, NODE_NONE)); BOOST_CHECK_EQUAL(addrman.size(), 1U); bool newOnly = true; - CAddrInfo addr_ret3 = addrman.Select(newOnly); + auto addr_ret3 = addrman.Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); } @@ -249,16 +249,16 @@ BOOST_AUTO_TEST_CASE(addrman_select) BOOST_CHECK_EQUAL(addrman.size(), 1U); bool newOnly = true; - CAddrInfo addr_ret1 = addrman.Select(newOnly); + auto addr_ret1 = addrman.Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333"); // Test: move addr to tried, select from new expected nothing returned. addrman.Good(CAddress(addr1, NODE_NONE)); BOOST_CHECK_EQUAL(addrman.size(), 1U); - CAddrInfo addr_ret2 = addrman.Select(newOnly); + auto addr_ret2 = addrman.Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret2.ToString(), "[::]:0"); - CAddrInfo addr_ret3 = addrman.Select(); + auto addr_ret3 = addrman.Select().first; BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); BOOST_CHECK_EQUAL(addrman.size(), 1U); @@ -291,7 +291,7 @@ BOOST_AUTO_TEST_CASE(addrman_select) // Test: Select pulls from new and tried regardless of port number. std::set ports; for (int i = 0; i < 20; ++i) { - ports.insert(addrman.Select().GetPort()); + ports.insert(addrman.Select().first.GetPort()); } BOOST_CHECK_EQUAL(ports.size(), 3U); } @@ -869,7 +869,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) BOOST_CHECK(addrman.size() == 0); // Empty addrman should return blank addrman info. - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); // Add twenty two addresses. CNetAddr source = ResolveIP("252.2.2.2"); @@ -880,7 +880,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) // No collisions yet. BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } // Ensure Good handles duplicates well. @@ -889,7 +889,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) addrman.Good(addr); BOOST_CHECK(addrman.size() == 22); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } } @@ -907,7 +907,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) // No collision yet. BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } // Collision between 36 and 19. @@ -916,11 +916,11 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) addrman.Good(addr36); BOOST_CHECK(addrman.size() == 36); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.19:0"); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.19:0"); // 36 should be discarded and 19 not evicted. addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); // Lets create two collisions. for (unsigned int i = 37; i < 59; i++) { @@ -929,7 +929,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) addrman.Good(addr); BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } // Cause a collision. @@ -938,16 +938,16 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) addrman.Good(addr59); BOOST_CHECK(addrman.size() == 59); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.10:0"); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.10:0"); // Cause a second collision. BOOST_CHECK(!addrman.Add({CAddress(addr36, NODE_NONE)}, source)); addrman.Good(addr36); BOOST_CHECK(addrman.size() == 59); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() != "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() != "[::]:0"); addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } BOOST_AUTO_TEST_CASE(addrman_evictionworks) @@ -957,7 +957,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) BOOST_CHECK(addrman.size() == 0); // Empty addrman should return blank addrman info. - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); // Add 35 addresses CNetAddr source = ResolveIP("252.2.2.2"); @@ -968,7 +968,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) // No collision yet. BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } // Collision between 36 and 19. @@ -977,7 +977,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) addrman.Good(addr); BOOST_CHECK_EQUAL(addrman.size(), 36); - CAddrInfo info = addrman.SelectTriedCollision(); + auto info = addrman.SelectTriedCollision().first; BOOST_CHECK_EQUAL(info.ToString(), "250.1.1.19:0"); // Ensure test of address fails, so that it is evicted. @@ -985,23 +985,23 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) // Should swap 36 for 19. addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); // If 36 was swapped for 19, then this should cause no collisions. BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source)); addrman.Good(addr); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); // If we insert 19 it should collide with 36 CService addr19 = ResolveService("250.1.1.19"); BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source)); addrman.Good(addr19); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.36:0"); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.36:0"); addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } BOOST_AUTO_TEST_CASE(load_addrman) From e3f1ea659c9eb1e8be4579923d6acaaab148c2ef Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 3 Sep 2021 17:26:56 -0700 Subject: [PATCH 05/13] [move-only] Move CAddrInfo to test-only header file Now that no bitcoind callers require knowledge of the CAddrInfo object, it can be moved into the test-only header file. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/addrman.h | 72 ----------------------------------- src/addrman_impl.h | 72 +++++++++++++++++++++++++++++++++++ src/test/fuzz/deserialize.cpp | 1 + 3 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index d176d0a42c1..33298df5cc1 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -24,78 +24,6 @@ class AddrManImpl; /** Default for -checkaddrman */ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; -/** - * Extended statistics about a CAddress - */ -class CAddrInfo : public CAddress -{ -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; - - //! last successful connection by us - int64_t nLastSuccess{0}; - - //! connection attempts since last successful attempt - int nAttempts{0}; - - //! reference count in new sets (memory only) - int nRefCount{0}; - - //! in tried set? (memory only) - bool fInTried{false}; - - //! position in vRandom - mutable int nRandomPos{-1}; - - friend class AddrManImpl; - friend class CAddrManDeterministic; - -public: - - SERIALIZE_METHODS(CAddrInfo, obj) - { - READWRITEAS(CAddress, obj); - READWRITE(obj.source, obj.nLastSuccess, obj.nAttempts); - } - - CAddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn), source(addrSource) - { - } - - CAddrInfo() : CAddress(), source() - { - } - - //! Calculate in which "tried" bucket this entry belongs - int GetTriedBucket(const uint256 &nKey, const std::vector &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 &asmap) const; - - //! Calculate in which "new" bucket this entry belongs, using its default source - int GetNewBucket(const uint256 &nKey, const std::vector &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; - - //! Calculate the relative chance this entry should be given when selecting nodes to connect to - double GetChance(int64_t nNow = GetAdjustedTime()) const; -}; - /** Stochastic address manager * * Design goals: diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 6752d5b81d1..fec98c416a1 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -5,6 +5,78 @@ #ifndef BITCOIN_ADDRMAN_IMPL_H #define BITCOIN_ADDRMAN_IMPL_H +/** + * Extended statistics about a CAddress + */ +class CAddrInfo : public CAddress +{ +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; + + //! last successful connection by us + int64_t nLastSuccess{0}; + + //! connection attempts since last successful attempt + int nAttempts{0}; + + //! reference count in new sets (memory only) + int nRefCount{0}; + + //! in tried set? (memory only) + bool fInTried{false}; + + //! position in vRandom + mutable int nRandomPos{-1}; + + friend class AddrManImpl; + friend class CAddrManDeterministic; + +public: + + SERIALIZE_METHODS(CAddrInfo, obj) + { + READWRITEAS(CAddress, obj); + READWRITE(obj.source, obj.nLastSuccess, obj.nAttempts); + } + + CAddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn), source(addrSource) + { + } + + CAddrInfo() : CAddress(), source() + { + } + + //! Calculate in which "tried" bucket this entry belongs + int GetTriedBucket(const uint256 &nKey, const std::vector &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 &asmap) const; + + //! Calculate in which "new" bucket this entry belongs, using its default source + int GetNewBucket(const uint256 &nKey, const std::vector &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; + + //! Calculate the relative chance this entry should be given when selecting nodes to connect to + double GetChance(int64_t nNow = GetAdjustedTime()) const; +}; + class AddrManImpl { public: diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 83ae1680e39..8297cfa481d 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include From 7cf41bbb38db5008f9b69037b88138076d6a6cc5 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 10 Sep 2021 14:32:42 -0600 Subject: [PATCH 06/13] [addrman] Change CAddrInfo access Since knowledge of CAddrInfo is limited to callsites that import addrman_impl.h, only objects in addrman.cpp or the tests have access. Thus we can remove calling them friends and make the members public. --- src/addrman_impl.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/addrman_impl.h b/src/addrman_impl.h index fec98c416a1..8da814b1472 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -17,7 +17,6 @@ public: //! last counted attempt (memory only) int64_t nLastCountAttempt{0}; -private: //! where knowledge about this address first came from CNetAddr source; @@ -36,11 +35,6 @@ private: //! position in vRandom mutable int nRandomPos{-1}; - friend class AddrManImpl; - friend class CAddrManDeterministic; - -public: - SERIALIZE_METHODS(CAddrInfo, obj) { READWRITEAS(CAddress, obj); From 40acd6fc9a8098fed85abf4fb727a5f0dff8a2ff Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 3 Sep 2021 17:26:10 -0700 Subject: [PATCH 07/13] [move-only] Move constants to test-only header Review hint: git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/addrman.h | 16 ---------------- src/addrman_impl.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index 33298df5cc1..688265d345f 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -53,22 +53,6 @@ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; * * Several indexes are kept for high performance. Setting m_consistency_check_ratio with the -checkaddrman * configuration option will introduce (expensive) consistency checks for the entire data structure. */ - -/** Total number of buckets for tried addresses */ -static constexpr int32_t ADDRMAN_TRIED_BUCKET_COUNT_LOG2{8}; -static constexpr int ADDRMAN_TRIED_BUCKET_COUNT{1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2}; - -/** Total number of buckets for new addresses */ -static constexpr int32_t ADDRMAN_NEW_BUCKET_COUNT_LOG2{10}; -static constexpr int ADDRMAN_NEW_BUCKET_COUNT{1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2}; - -/** Maximum allowed number of entries in buckets for new and tried addresses */ -static constexpr int32_t ADDRMAN_BUCKET_SIZE_LOG2{6}; -static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; - -/** - * Stochastical (IP) address manager - */ class CAddrMan { const std::unique_ptr m_impl; diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 8da814b1472..1d13df803d9 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -5,6 +5,16 @@ #ifndef BITCOIN_ADDRMAN_IMPL_H #define BITCOIN_ADDRMAN_IMPL_H +/** Total number of buckets for tried addresses */ +static constexpr int32_t ADDRMAN_TRIED_BUCKET_COUNT_LOG2{8}; +static constexpr int ADDRMAN_TRIED_BUCKET_COUNT{1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2}; +/** Total number of buckets for new addresses */ +static constexpr int32_t ADDRMAN_NEW_BUCKET_COUNT_LOG2{10}; +static constexpr int ADDRMAN_NEW_BUCKET_COUNT{1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2}; +/** Maximum allowed number of entries in buckets for new and tried addresses */ +static constexpr int32_t ADDRMAN_BUCKET_SIZE_LOG2{6}; +static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; + /** * Extended statistics about a CAddress */ From 14f9e000d05f82b364d5a142cafc70b10406b660 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 22 Sep 2021 15:47:49 -0600 Subject: [PATCH 08/13] [refactor] Update GetAddr_() function signature Update so the internal function signature matches the external one, as is the case for the other addrman functions. --- src/addrman.cpp | 14 ++++++++------ src/addrman_impl.h | 5 +++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 324bab7292c..40e087f5fb8 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -745,7 +745,7 @@ std::pair AddrManImpl::Select_(bool newOnly) const } } -void AddrManImpl::GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const { AssertLockHeld(cs); @@ -759,8 +759,9 @@ void AddrManImpl::GetAddr_(std::vector& vAddr, size_t max_addresses, s // gather a list of random nodes, skipping those of low quality const int64_t now{GetAdjustedTime()}; + std::vector addresses; for (unsigned int n = 0; n < vRandom.size(); n++) { - if (vAddr.size() >= nNodes) + if (addresses.size() >= nNodes) break; int nRndPos = insecure_rand.randrange(vRandom.size() - n) + n; @@ -776,8 +777,10 @@ void AddrManImpl::GetAddr_(std::vector& vAddr, size_t max_addresses, s // Filter for quality if (ai.IsTerrible(now)) continue; - vAddr.push_back(ai); + addresses.push_back(ai); } + + return addresses; } void AddrManImpl::Connected_(const CService& addr, int64_t nTime) @@ -1080,10 +1083,9 @@ std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, { LOCK(cs); Check(); - std::vector vAddr; - GetAddr_(vAddr, max_addresses, max_pct, network); + const auto addresses = GetAddr_(max_addresses, max_pct, network); Check(); - return vAddr; + return addresses; } void AddrManImpl::Connected(const CService &addr, int64_t nTime) diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 1d13df803d9..918034caf88 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -242,12 +242,13 @@ private: /** * Return all or many randomly selected addresses, optionally by network. * - * @param[out] vAddr Vector of randomly selected addresses from vRandom. * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * + * @returns A vector of randomly selected addresses from vRandom. */ - void GetAddr_(std::vector& vAddr, size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** We have successfully connected to this peer. Calling this function * updates the CAddress's nTime, which is used in our IsTerrible() From 29727c2aa1233f7c5b91a17884c405e0aef10c6e Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 3 Sep 2021 17:27:11 -0700 Subject: [PATCH 09/13] [doc] Update comments Maintain comments on the external interfaces rather than on the internal functions that implement them. --- src/addrman.cpp | 2 +- src/addrman.h | 17 +++++++++++++++-- src/addrman_impl.h | 27 --------------------------- 3 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 40e087f5fb8..3ccd3751bc6 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -23,7 +23,7 @@ static constexpr uint32_t ADDRMAN_TRIED_BUCKETS_PER_GROUP{8}; /** Over how many buckets entries with new addresses originating from a single group are spread */ static constexpr uint32_t ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP{64}; -/** Maximum number of times an address can be added to the new table */ +/** Maximum number of times an address can occur in the new table */ static constexpr int32_t ADDRMAN_NEW_BUCKETS_PER_ADDRESS{8}; /** How old addresses can maximally be */ static constexpr int64_t ADDRMAN_HORIZON_DAYS{30}; diff --git a/src/addrman.h b/src/addrman.h index 688265d345f..84c2bf2201b 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -74,7 +74,7 @@ public: //! Add addresses to addrman's new table. bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); - //! Mark an entry as accessible. + //! Mark an entry as accessible, possibly moving it from "new" to "tried". void Good(const CService &addr, int64_t nTime = GetAdjustedTime()); //! Mark an entry as connection attempted to. @@ -107,12 +107,25 @@ public: * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * + * @return A vector of randomly selected addresses from vRandom. */ std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const; - //! Outer function for Connected_() + /** We have successfully connected to this peer. Calling this function + * updates the CAddress's nTime, which is used in our IsTerrible() + * decisions and gossiped to peers. Callers should be careful that updating + * this information doesn't leak topology information to network spies. + * + * net_processing calls this function when it *disconnects* from a peer to + * not leak information about currently connected peers. + * + * @param[in] addr The address of the peer we were connected to + * @param[in] nTime The time that we were last connected to this peer + */ void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()); + //! Update an entry's service bits. void SetServices(const CService &addr, ServiceFlags nServices); const std::vector& GetAsmap() const; diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 918034caf88..160efb2c0ed 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -227,49 +227,22 @@ private: //! Move an entry from the "new" table(s) to the "tried" table void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Mark an entry "good", possibly moving it from "new" to "tried". void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Add an entry to the "new" table. bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Mark an entry as attempted to connect. void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. std::pair Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** - * Return all or many randomly selected addresses, optionally by network. - * - * @param[in] max_addresses Maximum number of addresses to return (0 = all). - * @param[in] max_pct Maximum percentage of addresses to return (0 = all). - * @param[in] network Select only addresses of this network (nullopt = all). - * - * @returns A vector of randomly selected addresses from vRandom. - */ std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** We have successfully connected to this peer. Calling this function - * updates the CAddress's nTime, which is used in our IsTerrible() - * decisions and gossiped to peers. Callers should be careful that updating - * this information doesn't leak topology information to network spies. - * - * net_processing calls this function when it *disconnects* from a peer to - * not leak information about currently connected peers. - * - * @param[in] addr The address of the peer we were connected to - * @param[in] nTime The time that we were last connected to this peer - */ void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Update an entry's service bits. void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); - //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Return a random to-be-evicted tried table address. std::pair SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. From 3c263d3f63c3598954ee2b65a0e721e3c22e52f8 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 10 Sep 2021 16:37:41 -0600 Subject: [PATCH 10/13] [includes] Fix up included files --- src/addrman.cpp | 9 +++++---- src/addrman.h | 8 +++----- src/addrman_impl.h | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 3ccd3751bc6..ef1538b6e9f 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -6,18 +6,19 @@ #include #include -#include #include -#include #include +#include +#include #include #include +#include +#include +#include #include #include #include -#include -#include /** Over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread */ static constexpr uint32_t ADDRMAN_TRIED_BUCKETS_PER_GROUP{8}; diff --git a/src/addrman.h b/src/addrman.h index 84c2bf2201b..b0944bcfd59 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -6,17 +6,15 @@ #ifndef BITCOIN_ADDRMAN_H #define BITCOIN_ADDRMAN_H -#include -#include #include #include -#include +#include #include #include +#include #include -#include -#include +#include #include class AddrManImpl; diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 160efb2c0ed..c37c7b47798 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -5,6 +5,21 @@ #ifndef BITCOIN_ADDRMAN_IMPL_H #define BITCOIN_ADDRMAN_IMPL_H +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + /** Total number of buckets for tried addresses */ static constexpr int32_t ADDRMAN_TRIED_BUCKET_COUNT_LOG2{8}; static constexpr int ADDRMAN_TRIED_BUCKET_COUNT{1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2}; From dd8f7f250095e58bbf4cd4effb481b52143bd1ed Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 10 Sep 2021 18:16:37 -0600 Subject: [PATCH 11/13] scripted-diff: Rename CAddrMan to AddrMan -BEGIN VERIFY SCRIPT- git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g' -END VERIFY SCRIPT- --- src/addrdb.cpp | 10 ++-- src/addrdb.h | 8 +-- src/addrman.cpp | 48 ++++++++--------- src/addrman.h | 10 ++-- src/addrman_impl.h | 4 +- src/bench/addrman.cpp | 18 +++---- src/net.cpp | 2 +- src/net.h | 4 +- src/net_processing.cpp | 10 ++-- src/net_processing.h | 4 +- src/netaddress.cpp | 2 +- src/netaddress.h | 2 +- src/node/context.h | 4 +- src/test/addrman_tests.cpp | 84 +++++++++++++++--------------- src/test/fuzz/addrman.cpp | 22 ++++---- src/test/fuzz/connman.cpp | 2 +- src/test/fuzz/deserialize.cpp | 2 +- src/test/util/setup_common.cpp | 2 +- test/functional/feature_addrman.py | 4 +- 19 files changed, 121 insertions(+), 121 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 94c77a6d896..50fd09101ee 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -170,21 +170,21 @@ bool CBanDB::Read(banmap_t& banSet) return true; } -bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr) +bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr) { const auto pathAddr = args.GetDataDirNet() / "peers.dat"; return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION); } -void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers) +void ReadFromStream(AddrMan& addr, CDataStream& ssPeers) { DeserializeDB(ssPeers, addr, false); } -std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman) +std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman) { auto check_addrman = std::clamp(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); int64_t nStart = GetTimeMillis(); const auto path_addr{args.GetDataDirNet() / "peers.dat"}; @@ -193,7 +193,7 @@ std::optional LoadAddrman(const std::vector& asmap, const A LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart); } catch (const DbNotFoundError&) { // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); LogPrintf("Creating peers.dat because the file was not found (%s)\n", path_addr); DumpPeerAddresses(args, *addrman); } catch (const std::exception& e) { diff --git a/src/addrdb.h b/src/addrdb.h index 33cc1f92040..19be4b5bb4e 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -14,14 +14,14 @@ #include class ArgsManager; -class CAddrMan; +class AddrMan; class CAddress; class CDataStream; struct bilingual_str; -bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr); +bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr); /** Only used by tests. */ -void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers); +void ReadFromStream(AddrMan& addr, CDataStream& ssPeers); /** Access to the banlist database (banlist.json) */ class CBanDB @@ -48,7 +48,7 @@ public: }; /** Returns an error string on failure */ -std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman); +std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman); /** * Dump the anchor IP address database (anchors.dat) diff --git a/src/addrman.cpp b/src/addrman.cpp index ef1538b6e9f..42d4b5a6e53 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -269,14 +269,14 @@ void AddrManImpl::Unserialize(Stream& s_) if (nNew > ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nNew < 0) { throw std::ios_base::failure( - strprintf("Corrupt CAddrMan serialization: nNew=%d, should be in [0, %d]", + strprintf("Corrupt AddrMan serialization: nNew=%d, should be in [0, %d]", nNew, ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); } if (nTried > ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nTried < 0) { throw std::ios_base::failure( - strprintf("Corrupt CAddrMan serialization: nTried=%d, should be in [0, %d]", + strprintf("Corrupt AddrMan serialization: nTried=%d, should be in [0, %d]", nTried, ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); } @@ -1110,83 +1110,83 @@ const std::vector& AddrManImpl::GetAsmap() const return m_asmap; } -CAddrMan::CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) +AddrMan::AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio)) {} -CAddrMan::~CAddrMan() = default; +AddrMan::~AddrMan() = default; template -void CAddrMan::Serialize(Stream& s_) const +void AddrMan::Serialize(Stream& s_) const { m_impl->Serialize(s_); } template -void CAddrMan::Unserialize(Stream& s_) +void AddrMan::Unserialize(Stream& s_) { m_impl->Unserialize(s_); } // explicit instantiation -template void CAddrMan::Serialize(CHashWriter& s) const; -template void CAddrMan::Serialize(CAutoFile& s) const; -template void CAddrMan::Serialize(CDataStream& s) const; -template void CAddrMan::Unserialize(CAutoFile& s); -template void CAddrMan::Unserialize(CHashVerifier& s); -template void CAddrMan::Unserialize(CDataStream& s); -template void CAddrMan::Unserialize(CHashVerifier& s); +template void AddrMan::Serialize(CHashWriter& s) const; +template void AddrMan::Serialize(CAutoFile& s) const; +template void AddrMan::Serialize(CDataStream& s) const; +template void AddrMan::Unserialize(CAutoFile& s); +template void AddrMan::Unserialize(CHashVerifier& s); +template void AddrMan::Unserialize(CDataStream& s); +template void AddrMan::Unserialize(CHashVerifier& s); -size_t CAddrMan::size() const +size_t AddrMan::size() const { return m_impl->size(); } -bool CAddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +bool AddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) { return m_impl->Add(vAddr, source, nTimePenalty); } -void CAddrMan::Good(const CService &addr, int64_t nTime) +void AddrMan::Good(const CService &addr, int64_t nTime) { m_impl->Good(addr, nTime); } -void CAddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +void AddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) { m_impl->Attempt(addr, fCountFailure, nTime); } -void CAddrMan::ResolveCollisions() +void AddrMan::ResolveCollisions() { m_impl->ResolveCollisions(); } -std::pair CAddrMan::SelectTriedCollision() +std::pair AddrMan::SelectTriedCollision() { return m_impl->SelectTriedCollision(); } -std::pair CAddrMan::Select(bool newOnly) const +std::pair AddrMan::Select(bool newOnly) const { return m_impl->Select(newOnly); } -std::vector CAddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const { return m_impl->GetAddr(max_addresses, max_pct, network); } -void CAddrMan::Connected(const CService &addr, int64_t nTime) +void AddrMan::Connected(const CService &addr, int64_t nTime) { m_impl->Connected(addr, nTime); } -void CAddrMan::SetServices(const CService &addr, ServiceFlags nServices) +void AddrMan::SetServices(const CService &addr, ServiceFlags nServices) { m_impl->SetServices(addr, nServices); } -const std::vector& CAddrMan::GetAsmap() const +const std::vector& AddrMan::GetAsmap() const { return m_impl->GetAsmap(); } diff --git a/src/addrman.h b/src/addrman.h index b0944bcfd59..fcb21478326 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -51,14 +51,14 @@ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; * * Several indexes are kept for high performance. Setting m_consistency_check_ratio with the -checkaddrman * configuration option will introduce (expensive) consistency checks for the entire data structure. */ -class CAddrMan +class AddrMan { const std::unique_ptr m_impl; public: - explicit CAddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); + explicit AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); - ~CAddrMan(); + ~AddrMan(); template void Serialize(Stream& s_) const; @@ -128,8 +128,8 @@ public: const std::vector& GetAsmap() const; - friend class CAddrManTest; - friend class CAddrManDeterministic; + friend class AddrManTest; + friend class AddrManDeterministic; }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/addrman_impl.h b/src/addrman_impl.h index c37c7b47798..f9deb171911 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -138,8 +138,8 @@ public: const std::vector& GetAsmap() const; - friend class CAddrManTest; - friend class CAddrManDeterministic; + friend class AddrManTest; + friend class AddrManDeterministic; private: //! A mutex to protect the inner data structures. diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index b4c2e35f422..d6834a239bd 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -53,14 +53,14 @@ static void CreateAddresses() } } -static void AddAddressesToAddrMan(CAddrMan& addrman) +static void AddAddressesToAddrMan(AddrMan& addrman) { for (size_t source_i = 0; source_i < NUM_SOURCES; ++source_i) { addrman.Add(g_addresses[source_i], g_sources[source_i]); } } -static void FillAddrMan(CAddrMan& addrman) +static void FillAddrMan(AddrMan& addrman) { CreateAddresses(); @@ -74,14 +74,14 @@ static void AddrManAdd(benchmark::Bench& bench) CreateAddresses(); bench.run([&] { - CAddrMan addrman{/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0}; + AddrMan addrman{/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0}; AddAddressesToAddrMan(addrman); }); } static void AddrManSelect(benchmark::Bench& bench) { - CAddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); FillAddrMan(addrman); @@ -93,7 +93,7 @@ static void AddrManSelect(benchmark::Bench& bench) static void AddrManGetAddr(benchmark::Bench& bench) { - CAddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); FillAddrMan(addrman); @@ -105,7 +105,7 @@ static void AddrManGetAddr(benchmark::Bench& bench) static void AddrManAddThenGood(benchmark::Bench& bench) { - auto markSomeAsGood = [](CAddrMan& addrman) { + auto markSomeAsGood = [](AddrMan& addrman) { for (size_t source_i = 0; source_i < NUM_SOURCES; ++source_i) { for (size_t addr_i = 0; addr_i < NUM_ADDRESSES_PER_SOURCE; ++addr_i) { addrman.Good(g_addresses[source_i][addr_i]); @@ -117,12 +117,12 @@ static void AddrManAddThenGood(benchmark::Bench& bench) bench.run([&] { // To make the benchmark independent of the number of evaluations, we always prepare a new addrman. - // This is necessary because CAddrMan::Good() method modifies the object, affecting the timing of subsequent calls + // This is necessary because AddrMan::Good() method modifies the object, affecting the timing of subsequent calls // to the same method and we want to do the same amount of work in every loop iteration. // // This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in - // CAddrMan::Good() will still be noticeable. - CAddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + // AddrMan::Good() will still be noticeable. + AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); AddAddressesToAddrMan(addrman); markSomeAsGood(addrman); diff --git a/src/net.cpp b/src/net.cpp index df3b88725e4..12a9dea98ee 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2438,7 +2438,7 @@ void CConnman::SetNetworkActive(bool active) } } -CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, CAddrMan& addrman_in, bool network_active) +CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, bool network_active) : addrman(addrman_in), nSeed0(nSeed0In), nSeed1(nSeed1In) { SetTryNewOutboundPeer(false); diff --git a/src/net.h b/src/net.h index 0a72ca888db..2742107a357 100644 --- a/src/net.h +++ b/src/net.h @@ -797,7 +797,7 @@ public: m_onion_binds = connOptions.onion_binds; } - CConnman(uint64_t seed0, uint64_t seed1, CAddrMan& addrman, bool network_active = true); + CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, bool network_active = true); ~CConnman(); bool Start(CScheduler& scheduler, const Options& options); @@ -1049,7 +1049,7 @@ private: std::vector vhListenSocket; std::atomic fNetworkActive{true}; bool fAddressesInitialized{false}; - CAddrMan& addrman; + AddrMan& addrman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); RecursiveMutex m_addr_fetches_mutex; std::vector vAddedNodes GUARDED_BY(cs_vAddedNodes); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e130272ff1e..c82c395da9d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -291,7 +291,7 @@ using PeerRef = std::shared_ptr; class PeerManagerImpl final : public PeerManager { public: - PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs); @@ -409,7 +409,7 @@ private: const CChainParams& m_chainparams; CConnman& m_connman; - CAddrMan& m_addrman; + AddrMan& m_addrman; /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */ BanMan* const m_banman; ChainstateManager& m_chainman; @@ -1419,14 +1419,14 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT); } -std::unique_ptr PeerManager::make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, +std::unique_ptr PeerManager::make(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs) { return std::make_unique(chainparams, connman, addrman, banman, chainman, pool, ignore_incoming_txs); } -PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, +PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs) : m_chainparams(chainparams), @@ -2653,7 +2653,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // table is also potentially detrimental because new-table entries // are subject to eviction in the event of addrman collisions. We // mitigate the information-leak by never calling - // CAddrMan::Connected() on block-relay-only peers; see + // AddrMan::Connected() on block-relay-only peers; see // FinalizeNode(). // // This moves an address from New to Tried table in Addrman, diff --git a/src/net_processing.h b/src/net_processing.h index 9d8d7885837..27bc40687a0 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -9,7 +9,7 @@ #include #include -class CAddrMan; +class AddrMan; class CChainParams; class CTxMemPool; class ChainstateManager; @@ -37,7 +37,7 @@ struct CNodeStateStats { class PeerManager : public CValidationInterface, public NetEventsInterface { public: - static std::unique_ptr make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, + static std::unique_ptr make(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs); virtual ~PeerManager() { } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index b2f4945e3bc..f9fff5a6d5e 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -165,7 +165,7 @@ void CNetAddr::SetLegacyIPv6(Span ipv6) } /** - * Create an "internal" address that represents a name or FQDN. CAddrMan uses + * Create an "internal" address that represents a name or FQDN. AddrMan uses * these fake addresses to keep track of which DNS seeds were used. * @returns Whether or not the operation was successful. * @see NET_INTERNAL, INTERNAL_IN_IPV6_PREFIX, CNetAddr::IsInternal(), CNetAddr::IsRFC4193() diff --git a/src/netaddress.h b/src/netaddress.h index cfb2edcd34b..66c8c48f08a 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -62,7 +62,7 @@ enum Network { NET_CJDNS, /// A set of addresses that represent the hash of a string or FQDN. We use - /// them in CAddrMan to keep track of which DNS seeds were used. + /// them in AddrMan to keep track of which DNS seeds were used. NET_INTERNAL, /// Dummy value to indicate the number of NET_* constants. diff --git a/src/node/context.h b/src/node/context.h index 135f9ea1c6a..26873345b46 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -12,7 +12,7 @@ class ArgsManager; class BanMan; -class CAddrMan; +class AddrMan; class CBlockPolicyEstimator; class CConnman; class CScheduler; @@ -39,7 +39,7 @@ class WalletClient; struct NodeContext { //! Init interface for initializing current process and connecting to other processes. interfaces::Init* init{nullptr}; - std::unique_ptr addrman; + std::unique_ptr addrman; std::unique_ptr connman; std::unique_ptr mempool; std::unique_ptr fee_estimator; diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 022f60a8ede..d532eed1da3 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -22,26 +22,26 @@ using namespace std::literals; -class CAddrManSerializationMock : public CAddrMan +class AddrManSerializationMock : public AddrMan { public: virtual void Serialize(CDataStream& s) const = 0; - CAddrManSerializationMock() - : CAddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100) + AddrManSerializationMock() + : AddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100) {} }; -class CAddrManUncorrupted : public CAddrManSerializationMock +class AddrManUncorrupted : public AddrManSerializationMock { public: void Serialize(CDataStream& s) const override { - CAddrMan::Serialize(s); + AddrMan::Serialize(s); } }; -class CAddrManCorrupted : public CAddrManSerializationMock +class AddrManCorrupted : public AddrManSerializationMock { public: void Serialize(CDataStream& s) const override @@ -67,7 +67,7 @@ public: } }; -static CDataStream AddrmanToStream(const CAddrManSerializationMock& _addrman) +static CDataStream AddrmanToStream(const AddrManSerializationMock& _addrman) { CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); ssPeersIn << Params().MessageStart(); @@ -77,14 +77,14 @@ static CDataStream AddrmanToStream(const CAddrManSerializationMock& _addrman) return CDataStream(vchData, SER_DISK, CLIENT_VERSION); } -class CAddrManTest : public CAddrMan +class AddrManTest : public AddrMan { private: bool deterministic; public: - explicit CAddrManTest(bool makeDeterministic = true, + explicit AddrManTest(bool makeDeterministic = true, std::vector asmap = std::vector()) - : CAddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) + : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) { deterministic = makeDeterministic; } @@ -166,7 +166,7 @@ BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(addrman_simple) { - auto addrman = std::make_unique(); + auto addrman = std::make_unique(); CNetAddr source = ResolveIP("252.2.2.2"); @@ -200,7 +200,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple) BOOST_CHECK(addrman->size() >= 1); // Test: reset addrman and test AddrMan::Add multiple addresses works as expected - addrman = std::make_unique(); + addrman = std::make_unique(); std::vector vAddr; vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE)); vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE)); @@ -210,7 +210,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple) BOOST_AUTO_TEST_CASE(addrman_ports) { - CAddrManTest addrman; + AddrManTest addrman; CNetAddr source = ResolveIP("252.2.2.2"); @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports) BOOST_AUTO_TEST_CASE(addrman_select) { - CAddrManTest addrman; + AddrManTest addrman; CNetAddr source = ResolveIP("252.2.2.2"); @@ -298,7 +298,7 @@ BOOST_AUTO_TEST_CASE(addrman_select) BOOST_AUTO_TEST_CASE(addrman_new_collisions) { - CAddrManTest addrman; + AddrManTest addrman; CNetAddr source = ResolveIP("252.2.2.2"); @@ -327,7 +327,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) BOOST_AUTO_TEST_CASE(addrman_tried_collisions) { - CAddrManTest addrman; + AddrManTest addrman; CNetAddr source = ResolveIP("252.2.2.2"); @@ -357,7 +357,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) BOOST_AUTO_TEST_CASE(addrman_find) { - CAddrManTest addrman; + AddrManTest addrman; BOOST_CHECK_EQUAL(addrman.size(), 0U); @@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(addrman_find) BOOST_AUTO_TEST_CASE(addrman_create) { - CAddrManTest addrman; + AddrManTest addrman; BOOST_CHECK_EQUAL(addrman.size(), 0U); @@ -410,7 +410,7 @@ BOOST_AUTO_TEST_CASE(addrman_create) BOOST_AUTO_TEST_CASE(addrman_delete) { - CAddrManTest addrman; + AddrManTest addrman; BOOST_CHECK_EQUAL(addrman.size(), 0U); @@ -430,7 +430,7 @@ BOOST_AUTO_TEST_CASE(addrman_delete) BOOST_AUTO_TEST_CASE(addrman_getaddr) { - CAddrManTest addrman; + AddrManTest addrman; // Test: Sanity check, GetAddr should never return anything if addrman // is empty. @@ -490,7 +490,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) { - CAddrManTest addrman; + AddrManTest addrman; CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -545,7 +545,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) { - CAddrManTest addrman; + AddrManTest addrman; CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); @@ -623,7 +623,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) // 101.8.0.0/16 AS8 BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) { - CAddrManTest addrman; + AddrManTest addrman; CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -678,7 +678,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) { - CAddrManTest addrman; + AddrManTest addrman; CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); @@ -760,9 +760,9 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) { std::vector asmap1 = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); - auto addrman_asmap1 = std::make_unique(true, asmap1); - auto addrman_asmap1_dup = std::make_unique(true, asmap1); - auto addrman_noasmap = std::make_unique(); + auto addrman_asmap1 = std::make_unique(true, asmap1); + auto addrman_asmap1_dup = std::make_unique(true, asmap1); + auto addrman_noasmap = std::make_unique(); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); CAddress addr = CAddress(ResolveService("250.1.1.1"), NODE_NONE); @@ -792,8 +792,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second); // deserializing non-asmaped peers.dat to asmaped addrman - addrman_asmap1 = std::make_unique(true, asmap1); - addrman_noasmap = std::make_unique(); + addrman_asmap1 = std::make_unique(true, asmap1); + addrman_noasmap = std::make_unique(); addrman_noasmap->Add({addr}, default_source); stream << *addrman_noasmap; stream >> *addrman_asmap1; @@ -804,8 +804,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second); // used to map to different buckets, now maps to the same bucket. - addrman_asmap1 = std::make_unique(true, asmap1); - addrman_noasmap = std::make_unique(); + addrman_asmap1 = std::make_unique(true, asmap1); + addrman_noasmap = std::make_unique(); CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE); addrman_noasmap->Add({addr, addr2}, default_source); @@ -825,7 +825,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid) { // Confirm that invalid addresses are ignored in unserialization. - auto addrman = std::make_unique(); + auto addrman = std::make_unique(); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; @@ -857,14 +857,14 @@ BOOST_AUTO_TEST_CASE(remove_invalid) BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size()); memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement)); - addrman = std::make_unique(); + addrman = std::make_unique(); stream >> *addrman; BOOST_CHECK_EQUAL(addrman->size(), 2); } BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) { - CAddrManTest addrman; + AddrManTest addrman; BOOST_CHECK(addrman.size() == 0); @@ -896,7 +896,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) BOOST_AUTO_TEST_CASE(addrman_noevict) { - CAddrManTest addrman; + AddrManTest addrman; // Add 35 addresses. CNetAddr source = ResolveIP("252.2.2.2"); @@ -952,7 +952,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) BOOST_AUTO_TEST_CASE(addrman_evictionworks) { - CAddrManTest addrman; + AddrManTest addrman; BOOST_CHECK(addrman.size() == 0); @@ -1006,7 +1006,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) BOOST_AUTO_TEST_CASE(load_addrman) { - CAddrManUncorrupted addrmanUncorrupted; + AddrManUncorrupted addrmanUncorrupted; CService addr1, addr2, addr3; BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); @@ -1025,7 +1025,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) // Test that the de-serialization does not throw an exception. CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); bool exceptionThrown = false; - CAddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman1.size() == 0); try { @@ -1042,7 +1042,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) // Test that ReadFromStream creates an addrman with the correct number of addrs. CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); - CAddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); ReadFromStream(addrman2, ssPeers2); BOOST_CHECK(addrman2.size() == 3); @@ -1051,12 +1051,12 @@ BOOST_AUTO_TEST_CASE(load_addrman) BOOST_AUTO_TEST_CASE(load_addrman_corrupted) { - CAddrManCorrupted addrmanCorrupted; + AddrManCorrupted addrmanCorrupted; // Test that the de-serialization of corrupted addrman throws an exception. CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); bool exceptionThrown = false; - CAddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; @@ -1072,7 +1072,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) // Test that ReadFromStream fails if peers.dat is corrupt CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); - CAddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure); } diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 6625b69b142..d68002667ac 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -28,20 +28,20 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); - CAddrMan addr_man(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addr_man(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); try { ReadFromStream(addr_man, data_stream); } catch (const std::exception&) { } } -class CAddrManDeterministic : public CAddrMan +class AddrManDeterministic : public AddrMan { public: FuzzedDataProvider& m_fuzzed_data_provider; - explicit CAddrManDeterministic(std::vector asmap, FuzzedDataProvider& fuzzed_data_provider) - : CAddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0) + explicit AddrManDeterministic(std::vector asmap, FuzzedDataProvider& fuzzed_data_provider) + : AddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0) , m_fuzzed_data_provider(fuzzed_data_provider) { WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); @@ -130,7 +130,7 @@ public: * - vvNew entries refer to the same addresses * - vvTried entries refer to the same addresses */ - bool operator==(const CAddrManDeterministic& other) + bool operator==(const AddrManDeterministic& other) { LOCK2(m_impl->cs, other.m_impl->cs); @@ -223,7 +223,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); std::vector asmap = ConsumeAsmap(fuzzed_data_provider); - auto addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); + auto addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); if (fuzzed_data_provider.ConsumeBool()) { const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION); @@ -232,10 +232,10 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) try { ds >> *addr_man_ptr; } catch (const std::ios_base::failure&) { - addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); + addr_man_ptr = std::make_unique(asmap, fuzzed_data_provider); } } - CAddrManDeterministic& addr_man = *addr_man_ptr; + AddrManDeterministic& addr_man = *addr_man_ptr; while (fuzzed_data_provider.ConsumeBool()) { CallOneOf( fuzzed_data_provider, @@ -284,7 +284,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) } }); } - const CAddrMan& const_addr_man{addr_man}; + const AddrMan& const_addr_man{addr_man}; (void)const_addr_man.GetAddr( /* max_addresses */ fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), /* max_pct */ fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), @@ -302,8 +302,8 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) SetMockTime(ConsumeTime(fuzzed_data_provider)); std::vector asmap = ConsumeAsmap(fuzzed_data_provider); - CAddrManDeterministic addr_man1{asmap, fuzzed_data_provider}; - CAddrManDeterministic addr_man2{asmap, fuzzed_data_provider}; + AddrManDeterministic addr_man1{asmap, fuzzed_data_provider}; + AddrManDeterministic addr_man2{asmap, fuzzed_data_provider}; CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 01741103e42..d381345a0d4 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -25,7 +25,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); CConnman connman{fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral(), addrman, fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 8297cfa481d..b1a07b482e9 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -189,7 +189,7 @@ FUZZ_TARGET_DESERIALIZE(blockmerkleroot, { BlockMerkleRoot(block, &mutated); }) FUZZ_TARGET_DESERIALIZE(addrman_deserialize, { - CAddrMan am(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan am(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); DeserializeFromFuzzingInput(buffer, am); }) FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, { diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 97e614379c9..ebefa9974e7 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -192,7 +192,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + m_node.addrman = std::make_unique(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); m_node.banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 5a8394db2e1..93d50c13692 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -109,7 +109,7 @@ class AddrmanTest(BitcoinTestFramework): self.stop_node(0) write_addrman(peers_dat, len_tried=-1) self.nodes[0].assert_start_raises_init_error( - expected_msg=init_error("Corrupt CAddrMan serialization: nTried=-1, should be in \\[0, 16384\\]:.*"), + expected_msg=init_error("Corrupt AddrMan serialization: nTried=-1, should be in \\[0, 16384\\]:.*"), match=ErrorMatch.FULL_REGEX, ) @@ -117,7 +117,7 @@ class AddrmanTest(BitcoinTestFramework): self.stop_node(0) write_addrman(peers_dat, len_new=-1) self.nodes[0].assert_start_raises_init_error( - expected_msg=init_error("Corrupt CAddrMan serialization: nNew=-1, should be in \\[0, 65536\\]:.*"), + expected_msg=init_error("Corrupt AddrMan serialization: nNew=-1, should be in \\[0, 65536\\]:.*"), match=ErrorMatch.FULL_REGEX, ) From 375750387e35ed751d1f5ab48860bdec93977f64 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 10 Sep 2021 18:33:25 -0600 Subject: [PATCH 12/13] scripted-diff: Rename CAddrInfo to AddrInfo -BEGIN VERIFY SCRIPT- git grep -l CAddrInfo src/ | xargs sed -i 's/CAddrInfo/AddrInfo/g' -END VERIFY SCRIPT- --- src/addrman.cpp | 70 +++++++++++++++++------------------ src/addrman_impl.h | 16 ++++---- src/test/addrman_tests.cpp | 56 ++++++++++++++-------------- src/test/fuzz/addrman.cpp | 10 ++--- src/test/fuzz/deserialize.cpp | 2 +- 5 files changed, 77 insertions(+), 77 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 42d4b5a6e53..c015026cbc2 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -41,7 +41,7 @@ static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10}; /** The maximum time we'll spend trying to resolve a tried table collision, in seconds */ static constexpr int64_t ADDRMAN_TEST_WINDOW{40*60}; // 40 minutes -int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector &asmap) const +int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector &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(); @@ -51,7 +51,7 @@ int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector &asma return tried_bucket; } -int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector &asmap) const +int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector &asmap) const { std::vector vchSourceGroupKey = src.GetGroup(asmap); uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash(); @@ -62,13 +62,13 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std: return new_bucket; } -int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const +int AddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const { uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? uint8_t{'N'} : uint8_t{'K'}) << nBucket << GetKey()).GetCheapHash(); return hash1 % ADDRMAN_BUCKET_SIZE; } -bool CAddrInfo::IsTerrible(int64_t nNow) const +bool AddrInfo::IsTerrible(int64_t nNow) const { if (nLastTry && nLastTry >= nNow - 60) // never remove things tried in the last minute return false; @@ -88,7 +88,7 @@ bool CAddrInfo::IsTerrible(int64_t nNow) const return false; } -double CAddrInfo::GetChance(int64_t nNow) const +double AddrInfo::GetChance(int64_t nNow) const { double fChance = 1.0; int64_t nSinceLastTry = std::max(nNow - nLastTry, 0); @@ -190,7 +190,7 @@ void AddrManImpl::Serialize(Stream& s_) const int nIds = 0; for (const auto& entry : mapInfo) { mapUnkIds[entry.first] = nIds; - const CAddrInfo &info = entry.second; + const AddrInfo &info = entry.second; if (info.nRefCount) { assert(nIds != nNew); // this means nNew was wrong, oh ow s << info; @@ -199,7 +199,7 @@ void AddrManImpl::Serialize(Stream& s_) const } nIds = 0; for (const auto& entry : mapInfo) { - const CAddrInfo &info = entry.second; + const AddrInfo &info = entry.second; if (info.fInTried) { assert(nIds != nTried); // this means nTried was wrong, oh ow s << info; @@ -283,7 +283,7 @@ void AddrManImpl::Unserialize(Stream& s_) // Deserialize entries from the new table. for (int n = 0; n < nNew; n++) { - CAddrInfo &info = mapInfo[n]; + AddrInfo &info = mapInfo[n]; s >> info; mapAddr[info] = n; info.nRandomPos = vRandom.size(); @@ -294,7 +294,7 @@ void AddrManImpl::Unserialize(Stream& s_) // Deserialize entries from the tried table. int nLost = 0; for (int n = 0; n < nTried; n++) { - CAddrInfo info; + AddrInfo info; s >> info; int nKBucket = info.GetTriedBucket(nKey, m_asmap); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); @@ -351,7 +351,7 @@ void AddrManImpl::Unserialize(Stream& s_) for (auto bucket_entry : bucket_entries) { int bucket{bucket_entry.first}; const int entry_index{bucket_entry.second}; - CAddrInfo& info = mapInfo[entry_index]; + AddrInfo& info = mapInfo[entry_index]; // Don't store the entry in the new bucket if it's not a valid address for our addrman if (!info.IsValid()) continue; @@ -401,7 +401,7 @@ void AddrManImpl::Unserialize(Stream& s_) } } -CAddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId) +AddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId) { AssertLockHeld(cs); @@ -416,12 +416,12 @@ CAddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId) return nullptr; } -CAddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) +AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) { AssertLockHeld(cs); int nId = nIdCount++; - mapInfo[nId] = CAddrInfo(addr, addrSource); + mapInfo[nId] = AddrInfo(addr, addrSource); mapAddr[addr] = nId; mapInfo[nId].nRandomPos = vRandom.size(); vRandom.push_back(nId); @@ -459,7 +459,7 @@ void AddrManImpl::Delete(int nId) AssertLockHeld(cs); assert(mapInfo.count(nId) != 0); - CAddrInfo& info = mapInfo[nId]; + AddrInfo& info = mapInfo[nId]; assert(!info.fInTried); assert(info.nRefCount == 0); @@ -477,7 +477,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos) // if there is an entry in the specified bucket, delete it. if (vvNew[nUBucket][nUBucketPos] != -1) { int nIdDelete = vvNew[nUBucket][nUBucketPos]; - CAddrInfo& infoDelete = mapInfo[nIdDelete]; + AddrInfo& infoDelete = mapInfo[nIdDelete]; assert(infoDelete.nRefCount > 0); infoDelete.nRefCount--; vvNew[nUBucket][nUBucketPos] = -1; @@ -487,7 +487,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos) } } -void AddrManImpl::MakeTried(CAddrInfo& info, int nId) +void AddrManImpl::MakeTried(AddrInfo& info, int nId) { AssertLockHeld(cs); @@ -515,7 +515,7 @@ void AddrManImpl::MakeTried(CAddrInfo& info, int nId) // find an item to evict int nIdEvict = vvTried[nKBucket][nKBucketPos]; assert(mapInfo.count(nIdEvict) == 1); - CAddrInfo& infoOld = mapInfo[nIdEvict]; + AddrInfo& infoOld = mapInfo[nIdEvict]; // Remove the to-be-evicted item from the tried set. infoOld.fInTried = false; @@ -548,13 +548,13 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT nLastGood = nTime; - CAddrInfo* pinfo = Find(addr, &nId); + AddrInfo* pinfo = Find(addr, &nId); // if not found, bail out if (!pinfo) return; - CAddrInfo& info = *pinfo; + AddrInfo& info = *pinfo; // check whether we are talking about the exact same CService (including same port) if (info != addr) @@ -605,7 +605,7 @@ bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTi bool fNew = false; int nId; - CAddrInfo* pinfo = Find(addr, &nId); + AddrInfo* pinfo = Find(addr, &nId); // Do not set a penalty for a source's self-announcement if (addr == source) { @@ -652,7 +652,7 @@ bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTi if (vvNew[nUBucket][nUBucketPos] != nId) { bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (!fInsert) { - CAddrInfo& infoExisting = mapInfo[vvNew[nUBucket][nUBucketPos]]; + AddrInfo& infoExisting = mapInfo[vvNew[nUBucket][nUBucketPos]]; if (infoExisting.IsTerrible() || (infoExisting.nRefCount > 1 && pinfo->nRefCount == 0)) { // Overwrite the existing new table entry. fInsert = true; @@ -675,13 +675,13 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi { AssertLockHeld(cs); - CAddrInfo* pinfo = Find(addr); + AddrInfo* pinfo = Find(addr); // if not found, bail out if (!pinfo) return; - CAddrInfo& info = *pinfo; + AddrInfo& info = *pinfo; // check whether we are talking about the exact same CService (including same port) if (info != addr) @@ -718,7 +718,7 @@ std::pair AddrManImpl::Select_(bool newOnly) const int nId = vvTried[nKBucket][nKBucketPos]; const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); - const CAddrInfo& info{it_found->second}; + const AddrInfo& info{it_found->second}; if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { return {info, info.nLastTry}; } @@ -737,7 +737,7 @@ std::pair AddrManImpl::Select_(bool newOnly) const int nId = vvNew[nUBucket][nUBucketPos]; const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); - const CAddrInfo& info{it_found->second}; + const AddrInfo& info{it_found->second}; if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { return {info, info.nLastTry}; } @@ -770,7 +770,7 @@ std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct const auto it{mapInfo.find(vRandom[n])}; assert(it != mapInfo.end()); - const CAddrInfo& ai{it->second}; + const AddrInfo& ai{it->second}; // Filter by network (optional) if (network != std::nullopt && ai.GetNetClass() != network) continue; @@ -788,13 +788,13 @@ void AddrManImpl::Connected_(const CService& addr, int64_t nTime) { AssertLockHeld(cs); - CAddrInfo* pinfo = Find(addr); + AddrInfo* pinfo = Find(addr); // if not found, bail out if (!pinfo) return; - CAddrInfo& info = *pinfo; + AddrInfo& info = *pinfo; // check whether we are talking about the exact same CService (including same port) if (info != addr) @@ -810,13 +810,13 @@ void AddrManImpl::SetServices_(const CService& addr, ServiceFlags nServices) { AssertLockHeld(cs); - CAddrInfo* pinfo = Find(addr); + AddrInfo* pinfo = Find(addr); // if not found, bail out if (!pinfo) return; - CAddrInfo& info = *pinfo; + AddrInfo& info = *pinfo; // check whether we are talking about the exact same CService (including same port) if (info != addr) @@ -839,7 +839,7 @@ void AddrManImpl::ResolveCollisions_() if (mapInfo.count(id_new) != 1) { erase_collision = true; } else { - CAddrInfo& info_new = mapInfo[id_new]; + AddrInfo& info_new = mapInfo[id_new]; // Which tried bucket to move the entry to. int tried_bucket = info_new.GetTriedBucket(nKey, m_asmap); @@ -850,7 +850,7 @@ void AddrManImpl::ResolveCollisions_() // Get the to-be-evicted address that is being tested int id_old = vvTried[tried_bucket][tried_bucket_pos]; - CAddrInfo& info_old = mapInfo[id_old]; + AddrInfo& info_old = mapInfo[id_old]; // Has successfully connected in last X hours if (GetAdjustedTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { @@ -905,13 +905,13 @@ std::pair AddrManImpl::SelectTriedCollision_() return {}; } - const CAddrInfo& newInfo = mapInfo[id_new]; + const AddrInfo& 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 CAddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; + const AddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; return {info_old, info_old.nLastTry}; } @@ -944,7 +944,7 @@ int AddrManImpl::ForceCheckAddrman() const for (const auto& entry : mapInfo) { int n = entry.first; - const CAddrInfo& info = entry.second; + const AddrInfo& info = entry.second; if (info.fInTried) { if (!info.nLastSuccess) return -1; diff --git a/src/addrman_impl.h b/src/addrman_impl.h index f9deb171911..157f7d5da65 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -33,7 +33,7 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; /** * Extended statistics about a CAddress */ -class CAddrInfo : public CAddress +class AddrInfo : public CAddress { public: //! last try whatsoever by us (memory only) @@ -60,17 +60,17 @@ public: //! position in vRandom mutable int nRandomPos{-1}; - SERIALIZE_METHODS(CAddrInfo, obj) + SERIALIZE_METHODS(AddrInfo, obj) { READWRITEAS(CAddress, obj); READWRITE(obj.source, obj.nLastSuccess, obj.nAttempts); } - CAddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn), source(addrSource) + AddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn), source(addrSource) { } - CAddrInfo() : CAddress(), source() + AddrInfo() : CAddress(), source() { } @@ -177,7 +177,7 @@ private: int nIdCount GUARDED_BY(cs){0}; //! table with information about all nIds - std::unordered_map mapInfo GUARDED_BY(cs); + std::unordered_map mapInfo GUARDED_BY(cs); //! find an nId based on its network address std::unordered_map mapAddr GUARDED_BY(cs); @@ -225,10 +225,10 @@ private: const std::vector m_asmap; //! Find an entry. - CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + AddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. - CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + AddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Swap two elements in vRandom. void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -240,7 +240,7 @@ private: void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Move an entry from the "new" table(s) to the "tried" table - void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + void MakeTried(AddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index d532eed1da3..69ad7c7e24f 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -62,7 +62,7 @@ public: CAddress addr = CAddress(serv, NODE_NONE); CNetAddr resolved; BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); - CAddrInfo info = CAddrInfo(addr, resolved); + AddrInfo info = AddrInfo(addr, resolved); s << info; } }; @@ -89,13 +89,13 @@ public: deterministic = makeDeterministic; } - CAddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) + AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) { LOCK(m_impl->cs); return m_impl->Find(addr, pnId); } - CAddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) + AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) { LOCK(m_impl->cs); return m_impl->Create(addr, addrSource, pnId); @@ -373,17 +373,17 @@ BOOST_AUTO_TEST_CASE(addrman_find) BOOST_CHECK(addrman.Add({addr3}, source1)); // Test: ensure Find returns an IP matching what we searched on. - CAddrInfo* info1 = addrman.Find(addr1); + AddrInfo* info1 = addrman.Find(addr1); BOOST_REQUIRE(info1); BOOST_CHECK_EQUAL(info1->ToString(), "250.1.2.1:8333"); // Test 18; Find does not discriminate by port number. - CAddrInfo* info2 = addrman.Find(addr2); + AddrInfo* info2 = addrman.Find(addr2); BOOST_REQUIRE(info2); BOOST_CHECK_EQUAL(info2->ToString(), info1->ToString()); // Test: Find returns another IP matching what we searched on. - CAddrInfo* info3 = addrman.Find(addr3); + AddrInfo* info3 = addrman.Find(addr3); BOOST_REQUIRE(info3); BOOST_CHECK_EQUAL(info3->ToString(), "251.255.2.1:8333"); } @@ -398,12 +398,12 @@ BOOST_AUTO_TEST_CASE(addrman_create) CNetAddr source1 = ResolveIP("250.1.2.1"); int nId; - CAddrInfo* pinfo = addrman.Create(addr1, source1, &nId); + AddrInfo* pinfo = addrman.Create(addr1, source1, &nId); // Test: The result should be the same as the input addr. BOOST_CHECK_EQUAL(pinfo->ToString(), "250.1.2.1:8333"); - CAddrInfo* info2 = addrman.Find(addr1); + AddrInfo* info2 = addrman.Find(addr1); BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:8333"); } @@ -424,7 +424,7 @@ BOOST_AUTO_TEST_CASE(addrman_delete) BOOST_CHECK_EQUAL(addrman.size(), 1U); addrman.Delete(nId); BOOST_CHECK_EQUAL(addrman.size(), 0U); - CAddrInfo* info2 = addrman.Find(addr1); + AddrInfo* info2 = addrman.Find(addr1); BOOST_CHECK(info2 == nullptr); } @@ -498,7 +498,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) CNetAddr source1 = ResolveIP("250.1.1.1"); - CAddrInfo info1 = CAddrInfo(addr1, source1); + AddrInfo info1 = AddrInfo(addr1, source1); uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); @@ -513,14 +513,14 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. - CAddrInfo info2 = CAddrInfo(addr2, source1); + AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); std::set buckets; for (int i = 0; i < 255; i++) { - CAddrInfo infoi = CAddrInfo( + AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetTriedBucket(nKey1, asmap); @@ -532,7 +532,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) buckets.clear(); for (int j = 0; j < 255; j++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); @@ -552,7 +552,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) CNetAddr source1 = ResolveIP("250.1.2.1"); - CAddrInfo info1 = CAddrInfo(addr1, source1); + AddrInfo info1 = AddrInfo(addr1, source1); uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); @@ -568,13 +568,13 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); // Test: Ports should not affect bucket placement in the addr - CAddrInfo info2 = CAddrInfo(addr2, source1); + AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); std::set buckets; for (int i = 0; i < 255; i++) { - CAddrInfo infoi = CAddrInfo( + AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetNewBucket(nKey1, asmap); @@ -586,7 +586,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) buckets.clear(); for (int j = 0; j < 4 * 255; j++) { - CAddrInfo infoj = CAddrInfo(CAddress( + AddrInfo infoj = AddrInfo(CAddress( ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); @@ -599,7 +599,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) buckets.clear(); for (int p = 0; p < 255; p++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); @@ -631,7 +631,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) CNetAddr source1 = ResolveIP("250.1.1.1"); - CAddrInfo info1 = CAddrInfo(addr1, source1); + AddrInfo info1 = AddrInfo(addr1, source1); uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); @@ -646,14 +646,14 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. - CAddrInfo info2 = CAddrInfo(addr2, source1); + AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); std::set buckets; for (int j = 0; j < 255; j++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("101." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("101." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); @@ -665,7 +665,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) buckets.clear(); for (int j = 0; j < 255; j++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1, asmap); @@ -685,7 +685,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) CNetAddr source1 = ResolveIP("250.1.2.1"); - CAddrInfo info1 = CAddrInfo(addr1, source1); + AddrInfo info1 = AddrInfo(addr1, source1); uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); @@ -701,13 +701,13 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); // Test: Ports should not affect bucket placement in the addr - CAddrInfo info2 = CAddrInfo(addr2, source1); + AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); std::set buckets; for (int i = 0; i < 255; i++) { - CAddrInfo infoi = CAddrInfo( + AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); int bucket = infoi.GetNewBucket(nKey1, asmap); @@ -719,7 +719,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) buckets.clear(); for (int j = 0; j < 4 * 255; j++) { - CAddrInfo infoj = CAddrInfo(CAddress( + AddrInfo infoj = AddrInfo(CAddress( ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); @@ -732,7 +732,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) buckets.clear(); for (int p = 0; p < 255; p++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("101." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); @@ -744,7 +744,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) buckets.clear(); for (int p = 0; p < 255; p++) { - CAddrInfo infoj = CAddrInfo( + AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1, asmap); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index d68002667ac..cfeab9dcdc2 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -142,24 +142,24 @@ public: // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. // Keys may be different. - using CAddrInfoHasher = std::function; - using CAddrInfoEq = std::function; + using AddrInfoHasher = std::function; + using AddrInfoEq = std::function; CNetAddrHash netaddr_hasher; - CAddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const CAddrInfo& a) { + AddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const AddrInfo& a) { return netaddr_hasher(static_cast(a)) ^ netaddr_hasher(a.source) ^ a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried; }; - CAddrInfoEq addrinfo_eq = [](const CAddrInfo& lhs, const CAddrInfo& rhs) { + AddrInfoEq addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) { return static_cast(lhs) == static_cast(rhs) && lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess && lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount && lhs.fInTried == rhs.fInTried; }; - using Addresses = std::unordered_set; + using Addresses = std::unordered_set; const size_t num_addresses{m_impl->mapInfo.size()}; diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index b1a07b482e9..a9325fa738b 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -105,7 +105,7 @@ FUZZ_TARGET_DESERIALIZE(block_filter_deserialize, { DeserializeFromFuzzingInput(buffer, block_filter); }) FUZZ_TARGET_DESERIALIZE(addr_info_deserialize, { - CAddrInfo addr_info; + AddrInfo addr_info; DeserializeFromFuzzingInput(buffer, addr_info); }) FUZZ_TARGET_DESERIALIZE(block_file_info_deserialize, { From 021f86953e8a1dff8ecc768186368d345c865cc2 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 10 Sep 2021 18:53:57 -0600 Subject: [PATCH 13/13] [style] Run changed files through clang formatter. --- src/addrman.cpp | 32 ++++++++++++++++---------------- src/addrman.h | 10 +++++----- src/addrman_impl.h | 24 ++++++++++++------------ src/test/addrman_tests.cpp | 2 +- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index c015026cbc2..c364a7710b4 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -41,7 +41,7 @@ static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10}; /** The maximum time we'll spend trying to resolve a tried table collision, in seconds */ static constexpr int64_t ADDRMAN_TEST_WINDOW{40*60}; // 40 minutes -int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector &asmap) const +int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector& 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(); @@ -51,7 +51,7 @@ int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector &asmap return tried_bucket; } -int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector &asmap) const +int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector& asmap) const { std::vector vchSourceGroupKey = src.GetGroup(asmap); uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash(); @@ -62,7 +62,7 @@ int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std:: return new_bucket; } -int AddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const +int AddrInfo::GetBucketPosition(const uint256& nKey, bool fNew, int nBucket) const { uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? uint8_t{'N'} : uint8_t{'K'}) << nBucket << GetKey()).GetCheapHash(); return hash1 % ADDRMAN_BUCKET_SIZE; @@ -190,7 +190,7 @@ void AddrManImpl::Serialize(Stream& s_) const int nIds = 0; for (const auto& entry : mapInfo) { mapUnkIds[entry.first] = nIds; - const AddrInfo &info = entry.second; + const AddrInfo& info = entry.second; if (info.nRefCount) { assert(nIds != nNew); // this means nNew was wrong, oh ow s << info; @@ -199,7 +199,7 @@ void AddrManImpl::Serialize(Stream& s_) const } nIds = 0; for (const auto& entry : mapInfo) { - const AddrInfo &info = entry.second; + const AddrInfo& info = entry.second; if (info.fInTried) { assert(nIds != nTried); // this means nTried was wrong, oh ow s << info; @@ -283,7 +283,7 @@ void AddrManImpl::Unserialize(Stream& s_) // Deserialize entries from the new table. for (int n = 0; n < nNew; n++) { - AddrInfo &info = mapInfo[n]; + AddrInfo& info = mapInfo[n]; s >> info; mapAddr[info] = n; info.nRandomPos = vRandom.size(); @@ -1024,7 +1024,7 @@ size_t AddrManImpl::size() const return vRandom.size(); } -bool AddrManImpl::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +bool AddrManImpl::Add(const std::vector& vAddr, const CNetAddr& source, int64_t nTimePenalty) { LOCK(cs); int nAdd = 0; @@ -1038,7 +1038,7 @@ bool AddrManImpl::Add(const std::vector &vAddr, const CNetAddr& source return nAdd > 0; } -void AddrManImpl::Good(const CService &addr, int64_t nTime) +void AddrManImpl::Good(const CService& addr, int64_t nTime) { LOCK(cs); Check(); @@ -1046,7 +1046,7 @@ void AddrManImpl::Good(const CService &addr, int64_t nTime) Check(); } -void AddrManImpl::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +void AddrManImpl::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) { LOCK(cs); Check(); @@ -1089,7 +1089,7 @@ std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, return addresses; } -void AddrManImpl::Connected(const CService &addr, int64_t nTime) +void AddrManImpl::Connected(const CService& addr, int64_t nTime) { LOCK(cs); Check(); @@ -1097,7 +1097,7 @@ void AddrManImpl::Connected(const CService &addr, int64_t nTime) Check(); } -void AddrManImpl::SetServices(const CService &addr, ServiceFlags nServices) +void AddrManImpl::SetServices(const CService& addr, ServiceFlags nServices) { LOCK(cs); Check(); @@ -1141,17 +1141,17 @@ size_t AddrMan::size() const return m_impl->size(); } -bool AddrMan::Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) +bool AddrMan::Add(const std::vector& vAddr, const CNetAddr& source, int64_t nTimePenalty) { return m_impl->Add(vAddr, source, nTimePenalty); } -void AddrMan::Good(const CService &addr, int64_t nTime) +void AddrMan::Good(const CService& addr, int64_t nTime) { m_impl->Good(addr, nTime); } -void AddrMan::Attempt(const CService &addr, bool fCountFailure, int64_t nTime) +void AddrMan::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) { m_impl->Attempt(addr, fCountFailure, nTime); } @@ -1176,12 +1176,12 @@ std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std return m_impl->GetAddr(max_addresses, max_pct, network); } -void AddrMan::Connected(const CService &addr, int64_t nTime) +void AddrMan::Connected(const CService& addr, int64_t nTime) { m_impl->Connected(addr, nTime); } -void AddrMan::SetServices(const CService &addr, ServiceFlags nServices) +void AddrMan::SetServices(const CService& addr, ServiceFlags nServices) { m_impl->SetServices(addr, nServices); } diff --git a/src/addrman.h b/src/addrman.h index fcb21478326..174ab4f811b 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -70,13 +70,13 @@ public: size_t size() const; //! Add addresses to addrman's new table. - bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); + bool Add(const std::vector& vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); //! Mark an entry as accessible, possibly moving it from "new" to "tried". - void Good(const CService &addr, int64_t nTime = GetAdjustedTime()); + void Good(const CService& addr, int64_t nTime = GetAdjustedTime()); //! Mark an entry as connection attempted to. - void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()); + void Attempt(const CService& addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()); //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. void ResolveCollisions(); @@ -121,10 +121,10 @@ public: * @param[in] addr The address of the peer we were connected to * @param[in] nTime The time that we were last connected to this peer */ - void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()); + void Connected(const CService& addr, int64_t nTime = GetAdjustedTime()); //! Update an entry's service bits. - void SetServices(const CService &addr, ServiceFlags nServices); + void SetServices(const CService& addr, ServiceFlags nServices); const std::vector& GetAsmap() const; diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 157f7d5da65..1dc7f25f9c4 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -111,29 +111,29 @@ public: size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) + bool Add(const std::vector& vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Good(const CService &addr, int64_t nTime) + void Good(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Attempt(const CService &addr, bool fCountFailure, int64_t nTime) + void Attempt(const CService& addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(!cs); void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs); std::pair SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::pair Select(bool newOnly) const + std::pair Select(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Connected(const CService &addr, int64_t nTime) + void Connected(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void SetServices(const CService &addr, ServiceFlags nServices) + void SetServices(const CService& addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(!cs); const std::vector& GetAsmap() const; @@ -225,10 +225,10 @@ private: const std::vector m_asmap; //! Find an entry. - AddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. - AddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Swap two elements in vRandom. void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -242,11 +242,11 @@ private: //! Move an entry from the "new" table(s) to the "tried" table void MakeTried(AddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); - void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); + void Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); + bool Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); + void Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); std::pair Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -254,7 +254,7 @@ private: void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); - void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + void SetServices_(const CService& addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 69ad7c7e24f..bd6f4702195 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -83,7 +83,7 @@ private: bool deterministic; public: explicit AddrManTest(bool makeDeterministic = true, - std::vector asmap = std::vector()) + std::vector asmap = std::vector()) : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) { deterministic = makeDeterministic;