diff --git a/doc/developer-notes.md b/doc/developer-notes.md index ce8652f529..ab0783840e 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -160,11 +160,57 @@ public: } // namespace foo ``` +Coding Style (C++ named arguments) +------------------------------ + +When passing named arguments, use a format that clang-tidy understands. The +argument names can otherwise not be verified by clang-tidy. + +For example: + +```c++ +void function(Addrman& addrman, bool clear); + +int main() +{ + function(g_addrman, /*clear=*/false); +} +``` + +### Running clang-tidy + +To run clang-tidy on Ubuntu/Debian, install the dependencies: + +```sh +apt install clang-tidy bear clang +``` + +Then, pass clang as compiler to configure, and use bear to produce the `compile_commands.json`: + +```sh +./autogen.sh && ./configure CC=clang CXX=clang++ +make clean && bear make -j $(nproc) # For bear 2.x +make clean && bear -- make -j $(nproc) # For bear 3.x +``` + +To run clang-tidy on all source files: + +```sh +( cd ./src/ && run-clang-tidy -j $(nproc) ) +``` + +To run clang-tidy on the changed source lines: + +```sh +git diff | ( cd ./src/ && clang-tidy-diff -p2 -j $(nproc) ) +``` + Coding Style (Python) --------------------- Refer to [/test/functional/README.md#style-guidelines](/test/functional/README.md#style-guidelines). + Coding Style (Doxygen-compatible comments) ------------------------------------------ diff --git a/doc/fuzzing.md b/doc/fuzzing.md index f7fac188ce..31d5de130c 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -64,6 +64,15 @@ block^@M-^?M-^?M-^?M-^?M-^?nM-^?M-^? In this case the fuzzer managed to create a `block` message which when passed to `ProcessMessage(...)` increased coverage. +It is possible to specify `dashd` arguments to the `fuzz` executable. +Depending on the test, they may be ignored or consumed and alter the behavior +of the test. Just make sure to use double-dash to distinguish them from the +fuzzer's own arguments: + +```sh +$ FUZZ=address_deserialize_v2 src/test/fuzz/fuzz -runs=1 fuzz_seed_corpus/address_deserialize_v2 --checkaddrman=5 --printtoconsole=1 +``` + ## Fuzzing corpora The project's collection of seed corpora is found in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo. diff --git a/src/Makefile.am b/src/Makefile.am index 0c2e16a6db..ae6b3863e2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -266,6 +266,7 @@ BITCOIN_CORE_H = \ netaddress.h \ netbase.h \ netfulfilledman.h \ + netgroup.h \ netmessagemaker.h \ node/blockstorage.h \ node/coin.h \ @@ -492,6 +493,7 @@ libbitcoin_server_a_SOURCES = \ miner.cpp \ net.cpp \ netfulfilledman.cpp \ + netgroup.cpp \ net_processing.cpp \ node/blockstorage.cpp \ node/coin.cpp \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 23dd88893c..68bab60903 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -182,10 +183,10 @@ 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 NetGroupManager& netgroupman, const ArgsManager& args, std::unique_ptr& addrman) { auto check_addrman = std::clamp(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); int64_t nStart = GetTimeMillis(); const auto path_addr{gArgs.GetDataDirNet() / "peers.dat"}; @@ -194,7 +195,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(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const DbInconsistentError& e) { @@ -203,9 +204,18 @@ std::optional LoadAddrman(const std::vector& asmap, const A // with frequent corruption, we are restoring old behaviour that does the same, silently. // // TODO: Evaluate cause and fix, revert this change at some point. - addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating peers.dat because of invalid or corrupt file (%s)\n", e.what()); DumpPeerAddresses(args, *addrman); + } catch (const InvalidAddrManVersionError&) { + if (!RenameOver(path_addr, (fs::path)path_addr + ".bak")) { + addrman = nullptr; + return strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again.")); + } + // Addrman can be in an inconsistent state after failure, reset it + addrman = std::make_unique(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr))); + DumpPeerAddresses(args, *addrman); } catch (const std::exception& e) { addrman = nullptr; return strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."), diff --git a/src/addrdb.h b/src/addrdb.h index 4996f577b5..b0f4c4170d 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -18,6 +18,7 @@ class ArgsManager; class AddrMan; class CAddress; class CDataStream; +class NetGroupManager; struct bilingual_str; bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr); @@ -49,7 +50,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 NetGroupManager& netgroupman, 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 aa05b7a098..cd18b7228a 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -7,6 +7,8 @@ #include #include +#include +#include #include #include #include @@ -41,17 +43,17 @@ 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 NetGroupManager& netgroupman) 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(); + uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << netgroupman.GetGroup(*this) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash(); return hash2 % ADDRMAN_TRIED_BUCKET_COUNT; } -int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector& asmap) const +int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const NetGroupManager& netgroupman) const { - std::vector vchSourceGroupKey = src.GetGroup(asmap); - uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash(); + std::vector vchSourceGroupKey = netgroupman.GetGroup(src); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << netgroupman.GetGroup(*this) << vchSourceGroupKey).GetCheapHash(); uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash(); return hash2 % ADDRMAN_NEW_BUCKET_COUNT; } @@ -99,11 +101,11 @@ double AddrInfo::GetChance(int64_t nNow) const return fChance; } -AddrManImpl::AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio) +AddrManImpl::AddrManImpl(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio) : insecure_rand{deterministic} , nKey{deterministic ? uint256{1} : insecure_rand.rand256()} , m_consistency_check_ratio{consistency_check_ratio} - , m_asmap{std::move(asmap)} + , m_netgroupman{netgroupman} { for (auto& bucket : vvNew) { for (auto& entry : bucket) { @@ -218,11 +220,7 @@ void AddrManImpl::Serialize(Stream& s_) const } // Store asmap checksum after bucket entries so that it // can be ignored by older clients for backward compatibility. - uint256 asmap_checksum; - if (m_asmap.size() != 0) { - asmap_checksum = SerializeHash(m_asmap); - } - s << asmap_checksum; + s << m_netgroupman.GetAsmapChecksum(); } template @@ -248,7 +246,7 @@ void AddrManImpl::Unserialize(Stream& s_) s >> compat; const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE; if (lowest_compatible > FILE_FORMAT) { - throw std::ios_base::failure(strprintf( + throw InvalidAddrManVersionError(strprintf( "Unsupported format of addrman database: %u. It is compatible with formats >=%u, " "but the maximum supported by this version of %s is %u.", uint8_t{format}, uint8_t{lowest_compatible}, PACKAGE_NAME, uint8_t{FILE_FORMAT})); @@ -292,7 +290,7 @@ void AddrManImpl::Unserialize(Stream& s_) for (int n = 0; n < nTried; n++) { AddrInfo info; s >> info; - int nKBucket = info.GetTriedBucket(nKey, m_asmap); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); if (info.IsValid() && vvTried[nKBucket][nKBucketPos] == -1) { @@ -329,10 +327,7 @@ void AddrManImpl::Unserialize(Stream& s_) // If the bucket count and asmap checksum haven't changed, then attempt // to restore the entries to the buckets/positions they were in before // serialization. - uint256 supplied_asmap_checksum; - if (m_asmap.size() != 0) { - supplied_asmap_checksum = SerializeHash(m_asmap); - } + uint256 supplied_asmap_checksum{m_netgroupman.GetAsmapChecksum()}; uint256 serialized_asmap_checksum; if (format >= Format::V2_ASMAP) { s >> serialized_asmap_checksum; @@ -365,7 +360,7 @@ void AddrManImpl::Unserialize(Stream& s_) } else { // In case the new table data cannot be used (bucket count wrong or new asmap), // try to give them a reference based on their primary source address. - bucket = info.GetNewBucket(nKey, m_asmap); + bucket = info.GetNewBucket(nKey, m_netgroupman); bucket_position = info.GetBucketPosition(nKey, true, bucket); if (vvNew[bucket][bucket_position] == -1) { vvNew[bucket][bucket_position] = entry_index; @@ -389,7 +384,7 @@ void AddrManImpl::Unserialize(Stream& s_) LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost); } - const int check_code{ForceCheckAddrman()}; + const int check_code{CheckAddrman()}; if (check_code != 0) { throw DbInconsistentError(strprintf( "Corrupt data. Consistency check failed with code %s", @@ -487,7 +482,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) AssertLockHeld(cs); // remove the entry from all new buckets - const int start_bucket{info.GetNewBucket(nKey, m_asmap)}; + const int start_bucket{info.GetNewBucket(nKey, m_netgroupman)}; for (int n = 0; n < ADDRMAN_NEW_BUCKET_COUNT; ++n) { const int bucket{(start_bucket + n) % ADDRMAN_NEW_BUCKET_COUNT}; const int pos{info.GetBucketPosition(nKey, true, bucket)}; @@ -502,7 +497,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) assert(info.nRefCount == 0); // which tried bucket to move the entry to - int nKBucket = info.GetTriedBucket(nKey, m_asmap); + int nKBucket = info.GetTriedBucket(nKey, m_netgroupman); int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); // first make space to add it (the existing tried entry there is moved to new, deleting whatever is there). @@ -518,7 +513,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) nTried--; // find which new bucket it belongs to - int nUBucket = infoOld.GetNewBucket(nKey, m_asmap); + int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman); int nUBucketPos = infoOld.GetBucketPosition(nKey, true, nUBucket); ClearNew(nUBucket, nUBucketPos); assert(vvNew[nUBucket][nUBucketPos] == -1); @@ -588,7 +583,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ nNew++; } - int nUBucket = pinfo->GetNewBucket(nKey, source, m_asmap); + int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman); int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket); bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (vvNew[nUBucket][nUBucketPos] != nId) { @@ -604,7 +599,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ pinfo->nRefCount++; vvNew[nUBucket][nUBucketPos] = nId; LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_asmap), nUBucket, nUBucketPos); + addr.ToString(), m_netgroupman.GetMappedAS(addr), nUBucket, nUBucketPos); } else { if (pinfo->nRefCount == 0) { Delete(nId); @@ -614,7 +609,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ return fInsert; } -void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) +bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) { AssertLockHeld(cs); @@ -625,8 +620,7 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT AddrInfo* pinfo = Find(addr, &nId); // if not found, bail out - if (!pinfo) - return; + if (!pinfo) return false; AddrInfo& info = *pinfo; @@ -638,16 +632,14 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT // currently-connected peers. // if it is already in the tried set, don't do anything else - if (info.fInTried) - return; + if (info.fInTried) return false; // if it is not in new, something bad happened - if (!Assume(info.nRefCount > 0)) { - return; - } + if (!Assume(info.nRefCount > 0)) return false; + // which tried bucket to move the entry to - int tried_bucket = info.GetTriedBucket(nKey, m_asmap); + int tried_bucket = info.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); // Will moving this address into tried evict another entry? @@ -663,13 +655,15 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT addr.ToString(), m_tried_collisions.size()); } + return false; } else { // move nId to the tried tables MakeTried(info, nId); if (fLogIPs) { LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos); + addr.ToString(), m_netgroupman.GetMappedAS(addr), tried_bucket, tried_bucket_pos); } + return true; } } @@ -873,7 +867,7 @@ void AddrManImpl::ResolveCollisions_() AddrInfo& info_new = mapInfo[id_new]; // Which tried bucket to move the entry to. - int tried_bucket = info_new.GetTriedBucket(nKey, m_asmap); + int tried_bucket = info_new.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket); if (!info_new.IsValid()) { // id_new may no longer map to a valid address erase_collision = true; @@ -941,13 +935,36 @@ std::pair AddrManImpl::SelectTriedCollision_() 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 = newInfo.GetTriedBucket(nKey, m_netgroupman); int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); const AddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]]; return {info_old, info_old.nLastTry}; } +std::optional AddrManImpl::FindAddressEntry_(const CAddress& addr) +{ + AssertLockHeld(cs); + + AddrInfo* addr_info = Find(addr); + + if (!addr_info) return std::nullopt; + + if(addr_info->fInTried) { + int bucket{addr_info->GetTriedBucket(nKey, m_netgroupman)}; + return AddressPosition(/*tried_in=*/true, + /*multiplicity_in=*/1, + /*bucket_in=*/bucket, + /*position_in=*/addr_info->GetBucketPosition(nKey, false, bucket)); + } else { + int bucket{addr_info->GetNewBucket(nKey, m_netgroupman)}; + return AddressPosition(/*tried_in=*/false, + /*multiplicity_in=*/addr_info->nRefCount, + /*bucket_in=*/bucket, + /*position_in=*/addr_info->GetBucketPosition(nKey, true, bucket)); + } +} + void AddrManImpl::Check() const { AssertLockHeld(cs); @@ -956,18 +973,19 @@ void AddrManImpl::Check() const if (m_consistency_check_ratio == 0) return; if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; - const int err{ForceCheckAddrman()}; + const int err{CheckAddrman()}; if (err) { LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); assert(false); } } -int AddrManImpl::ForceCheckAddrman() const +int AddrManImpl::CheckAddrman() const { AssertLockHeld(cs); - LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); + LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE( + strprintf("new %i, tried %i, total %u", nNew, nTried, vRandom.size()), BCLog::ADDRMAN); std::unordered_set setTried; std::unordered_map mapNew; @@ -1014,7 +1032,7 @@ int AddrManImpl::ForceCheckAddrman() const 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) { + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_netgroupman) != n) { return -17; } if (it->second.GetBucketPosition(nKey, false, n) != i) { @@ -1047,7 +1065,6 @@ int AddrManImpl::ForceCheckAddrman() const if (nKey.IsNull()) return -16; - LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); return 0; } @@ -1066,12 +1083,13 @@ bool AddrManImpl::Add(const std::vector& vAddr, const CNetAddr& source return ret; } -void AddrManImpl::Good(const CService& addr, int64_t nTime) +bool AddrManImpl::Good(const CService& addr, int64_t nTime) { LOCK(cs); Check(); - Good_(addr, /* test_before_evict */ true, nTime); + auto ret = Good_(addr, /*test_before_evict=*/true, nTime); Check(); + return ret; } void AddrManImpl::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) @@ -1133,6 +1151,15 @@ void AddrManImpl::SetServices(const CService& addr, ServiceFlags nServices) Check(); } +std::optional AddrManImpl::FindAddressEntry(const CAddress& addr) +{ + LOCK(cs); + Check(); + auto entry = FindAddressEntry_(addr); + Check(); + return entry; +} + AddrInfo AddrManImpl::GetAddressInfo(const CService& addr) { AddrInfo addrRet; @@ -1145,13 +1172,8 @@ AddrInfo AddrManImpl::GetAddressInfo(const CService& addr) return addrRet; } -const std::vector& AddrManImpl::GetAsmap() const -{ - return m_asmap; -} - -AddrMan::AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) - : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio)) {} +AddrMan::AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio) + : m_impl(std::make_unique(netgroupman, deterministic, consistency_check_ratio)) {} AddrMan::~AddrMan() = default; @@ -1185,9 +1207,9 @@ bool AddrMan::Add(const std::vector& vAddr, const CNetAddr& source, in return m_impl->Add(vAddr, source, nTimePenalty); } -void AddrMan::Good(const CService& addr, int64_t nTime) +bool AddrMan::Good(const CService& addr, int64_t nTime) { - m_impl->Good(addr, nTime); + return m_impl->Good(addr, nTime); } void AddrMan::Attempt(const CService& addr, bool fCountFailure, int64_t nTime) @@ -1230,7 +1252,7 @@ AddrInfo AddrMan::GetAddressInfo(const CService& addr) return m_impl->GetAddressInfo(addr); } -const std::vector& AddrMan::GetAsmap() const +std::optional AddrMan::FindAddressEntry(const CAddress& addr) { - return m_impl->GetAsmap(); + return m_impl->FindAddressEntry(addr); } diff --git a/src/addrman.h b/src/addrman.h index 2d5579eee8..a951825faa 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -7,6 +7,7 @@ #define BITCOIN_ADDRMAN_H #include +#include #include #include #include @@ -17,12 +18,6 @@ #include #include -class AddrInfo; -class AddrManImpl; - -/** Default for -checkaddrman */ -static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; - class DbInconsistentError : public std::exception { using std::exception::exception; @@ -33,6 +28,43 @@ public: const char* what() const noexcept override { return error.c_str(); } }; +class InvalidAddrManVersionError : public std::ios_base::failure +{ +public: + InvalidAddrManVersionError(std::string msg) : std::ios_base::failure(msg) { } +}; + +class AddrInfo; +class AddrManImpl; + +/** Default for -checkaddrman */ +static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; + +/** Test-only struct, capturing info about an address in AddrMan */ +struct AddressPosition { + // Whether the address is in the new or tried table + const bool tried; + + // Addresses in the tried table should always have a multiplicity of 1. + // Addresses in the new table can have multiplicity between 1 and + // ADDRMAN_NEW_BUCKETS_PER_ADDRESS + const int multiplicity; + + // If the address is in the new table, the bucket and position are + // populated based on the first source who sent the address. + // In certain edge cases, this may not be where the address is currently + // located. + const int bucket; + const int position; + + bool operator==(AddressPosition other) { + return std::tie(tried, multiplicity, bucket, position) == + std::tie(other.tried, other.multiplicity, other.bucket, other.position); + } + explicit AddressPosition(bool tried_in, int multiplicity_in, int bucket_in, int position_in) + : tried{tried_in}, multiplicity{multiplicity_in}, bucket{bucket_in}, position{position_in} {} +}; + /** Stochastic address manager * * Design goals: @@ -64,10 +96,11 @@ public: */ class AddrMan { +protected: const std::unique_ptr m_impl; public: - explicit AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); + explicit AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio); ~AddrMan(); @@ -91,8 +124,14 @@ public: * @return true if at least one address is successfully added. */ 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()); + /** + * Mark an address record as accessible and attempt to move it to addrman's tried table. + * + * @param[in] addr Address record to attempt to move to tried table. + * @param[in] nTime The time that we were last connected to this peer. + * @return true if the address is successfully moved from the new table to the tried table. + */ + bool 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()); @@ -148,10 +187,14 @@ public: //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. AddrInfo GetAddressInfo(const CService& addr); - const std::vector& GetAsmap() const; - - friend class AddrManTest; - friend class AddrManDeterministic; + /** Test-only function + * Find the address record in AddrMan and return information about its + * position. + * @param[in] addr The address record to look up. + * @return Information about the address record in AddrMan + * or nullopt if address is not found. + */ + std::optional FindAddressEntry(const CAddress& addr); }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 1e29b69da6..77333bb9b8 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -6,6 +6,7 @@ #define BITCOIN_ADDRMAN_IMPL_H #include +#include #include #include #include @@ -75,15 +76,15 @@ public: } //! Calculate in which "tried" bucket this entry belongs - int GetTriedBucket(const uint256 &nKey, const std::vector &asmap) const; + int GetTriedBucket(const uint256& nKey, const NetGroupManager& netgroupman) 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; + int GetNewBucket(const uint256& nKey, const CNetAddr& src, const NetGroupManager& netgroupman) const; //! Calculate in which "new" bucket this entry belongs, using its default source - int GetNewBucket(const uint256 &nKey, const std::vector &asmap) const + int GetNewBucket(const uint256& nKey, const NetGroupManager& netgroupman) const { - return GetNewBucket(nKey, source, asmap); + return GetNewBucket(nKey, source, netgroupman); } //! Calculate in which position of a bucket to store this entry. @@ -99,7 +100,7 @@ public: class AddrManImpl { public: - AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio); + AddrManImpl(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio); ~AddrManImpl(); @@ -114,7 +115,7 @@ public: bool Add(const std::vector& vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void Good(const CService& addr, int64_t nTime) + bool Good(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(!cs); void Attempt(const CService& addr, bool fCountFailure, int64_t nTime) @@ -136,11 +137,12 @@ public: void SetServices(const CService& addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(!cs); - AddrInfo GetAddressInfo(const CService& addr); + std::optional FindAddressEntry(const CAddress& addr) + EXCLUSIVE_LOCKS_REQUIRED(!cs); - const std::vector& GetAsmap() const; + AddrInfo GetAddressInfo(const CService& addr) + EXCLUSIVE_LOCKS_REQUIRED(!cs); - friend class AddrManTest; friend class AddrManDeterministic; private: @@ -211,21 +213,8 @@ private: /** 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; + /** Reference to the netgroup manager. netgroupman must be constructed before addrman and destructed after. */ + const NetGroupManager& m_netgroupman; //! Find an entry. AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -249,7 +238,7 @@ private: * @see AddrMan::Add() for parameters. */ bool AddSingle(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); - void Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); + bool Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); bool Add_(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -269,12 +258,15 @@ private: std::pair SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); - //! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. + std::optional FindAddressEntry_(const CAddress& addr) 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); + int CheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); }; #endif // BITCOIN_ADDRMAN_IMPL_H diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 535dad15b3..9eee2f7d3e 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -14,6 +15,9 @@ static constexpr size_t NUM_SOURCES = 64; static constexpr size_t NUM_ADDRESSES_PER_SOURCE = 256; +static NetGroupManager EMPTY_NETGROUPMAN{std::vector()}; +static constexpr uint32_t ADDRMAN_CONSISTENCY_CHECK_RATIO{0}; + static std::vector g_sources; static std::vector> g_addresses; @@ -72,14 +76,14 @@ static void AddrManAdd(benchmark::Bench& bench) CreateAddresses(); bench.run([&] { - AddrMan addrman{/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0}; + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; AddAddressesToAddrMan(addrman); }); } static void AddrManSelect(benchmark::Bench& bench) { - AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; FillAddrMan(addrman); @@ -91,7 +95,7 @@ static void AddrManSelect(benchmark::Bench& bench) static void AddrManGetAddr(benchmark::Bench& bench) { - AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; FillAddrMan(addrman); @@ -101,40 +105,33 @@ static void AddrManGetAddr(benchmark::Bench& bench) }); } -static void AddrManGood(benchmark::Bench& bench) +static void AddrManAddThenGood(benchmark::Bench& bench) { - /* Create many AddrMan objects - one to be modified at each loop iteration. - * This is necessary because the 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. */ - - bench.epochs(5).epochIterations(1); - const size_t addrman_count{bench.epochs() * bench.epochIterations()}; - - std::vector> addrmans(addrman_count); - for (size_t i{0}; i < addrman_count; ++i) { - addrmans[i] = std::make_unique(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); - FillAddrMan(*addrmans[i]); - } - 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) { - if (addr_i % 32 == 0) { - addrman.Good(g_addresses[source_i][addr_i]); - } + addrman.Good(g_addresses[source_i][addr_i]); } } }; - uint64_t i = 0; + CreateAddresses(); + bench.run([&] { - markSomeAsGood(*addrmans.at(i)); - ++i; + // To make the benchmark independent of the number of evaluations, we always prepare a new addrman. + // 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 + // AddrMan::Good() will still be noticeable. + AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; + AddAddressesToAddrMan(addrman); + + markSomeAsGood(addrman); }); } BENCHMARK(AddrManAdd); BENCHMARK(AddrManSelect); BENCHMARK(AddrManGetAddr); -BENCHMARK(AddrManGood); +BENCHMARK(AddrManAddThenGood); diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 16d622ebc0..02ae5de7fd 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -16,6 +16,8 @@ const std::function G_TEST_LOG_FUN{}; +const std::function()> G_TEST_COMMAND_LINE_ARGUMENTS{}; + namespace { void GenerateTemplateResults(const std::vector& benchmarkResults, const fs::path& file, const char* tpl) diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h index 030d6ebf6a..27df08fb69 100644 --- a/src/bench/nanobench.h +++ b/src/bench/nanobench.h @@ -33,7 +33,7 @@ // see https://semver.org/ #define ANKERL_NANOBENCH_VERSION_MAJOR 4 // incompatible API changes #define ANKERL_NANOBENCH_VERSION_MINOR 3 // backwards-compatible changes -#define ANKERL_NANOBENCH_VERSION_PATCH 4 // backwards-compatible bug fixes +#define ANKERL_NANOBENCH_VERSION_PATCH 6 // backwards-compatible bug fixes /////////////////////////////////////////////////////////////////////////////////////////////////// // public facing api - as minimal as possible @@ -88,13 +88,15 @@ } while (0) #endif -#if defined(__linux__) && defined(PERF_EVENT_IOC_ID) && defined(PERF_COUNT_HW_REF_CPU_CYCLES) && defined(PERF_FLAG_FD_CLOEXEC) && \ - !defined(ANKERL_NANOBENCH_DISABLE_PERF_COUNTERS) -// only enable perf counters on kernel 3.14 which seems to have all the necessary defines. The three PERF_... defines are not in -// kernel 2.6.32 (all others are). -# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 1 -#else -# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 0 +#define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 0 +#if defined(__linux__) && !defined(ANKERL_NANOBENCH_DISABLE_PERF_COUNTERS) +# include +# if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 14, 0) +// PERF_COUNT_HW_REF_CPU_CYCLES only available since kernel 3.3 +// PERF_FLAG_FD_CLOEXEC since kernel 3.14 +# undef ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS +# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 1 +# endif #endif #if defined(__clang__) @@ -2210,20 +2212,20 @@ struct IterationLogic::Impl { columns.emplace_back(10, 1, "err%", "%", rErrorMedian * 100.0); double rInsMedian = -1.0; - if (mResult.has(Result::Measure::instructions)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::instructions)) { rInsMedian = mResult.median(Result::Measure::instructions); columns.emplace_back(18, 2, "ins/" + mBench.unit(), "", rInsMedian / mBench.batch()); } double rCycMedian = -1.0; - if (mResult.has(Result::Measure::cpucycles)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::cpucycles)) { rCycMedian = mResult.median(Result::Measure::cpucycles); columns.emplace_back(18, 2, "cyc/" + mBench.unit(), "", rCycMedian / mBench.batch()); } if (rInsMedian > 0.0 && rCycMedian > 0.0) { columns.emplace_back(9, 3, "IPC", "", rCycMedian <= 0.0 ? 0.0 : rInsMedian / rCycMedian); } - if (mResult.has(Result::Measure::branchinstructions)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::branchinstructions)) { double rBraMedian = mResult.median(Result::Measure::branchinstructions); columns.emplace_back(17, 2, "bra/" + mBench.unit(), "", rBraMedian / mBench.batch()); if (mResult.has(Result::Measure::branchmisses)) { @@ -2402,6 +2404,14 @@ public: return (a + divisor / 2) / divisor; } + ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") + static inline uint32_t mix(uint32_t x) noexcept { + x ^= x << 13; + x ^= x >> 17; + x ^= x << 5; + return x; + } + template ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") void calibrate(Op&& op) { @@ -2441,15 +2451,10 @@ public: uint64_t const numIters = 100000U + (std::random_device{}() & 3); uint64_t n = numIters; uint32_t x = 1234567; - auto fn = [&]() { - x ^= x << 13; - x ^= x >> 17; - x ^= x << 5; - }; beginMeasure(); while (n-- > 0) { - fn(); + x = mix(x); } endMeasure(); detail::doNotOptimizeAway(x); @@ -2459,8 +2464,8 @@ public: beginMeasure(); while (n-- > 0) { // we now run *twice* so we can easily calculate the overhead - fn(); - fn(); + x = mix(x); + x = mix(x); } endMeasure(); detail::doNotOptimizeAway(x); diff --git a/src/init.cpp b/src/init.cpp index 4e1383a683..1bd565b760 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -287,6 +288,7 @@ void PrepareShutdown(NodeContext& node) node.connman.reset(); node.banman.reset(); node.addrman.reset(); + node.netgroupman.reset(); if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(*node.mempool); @@ -1523,8 +1525,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)}; { - // Initialize addrman - assert(!node.addrman); // Read asmap file if configured std::vector asmap; @@ -1551,8 +1551,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("Using /16 prefix for IP bucketing\n"); } + // Initialize netgroup manager + assert(!node.netgroupman); + node.netgroupman = std::make_unique(std::move(asmap)); + + // Initialize addrman + assert(!node.addrman); uiInterface.InitMessage(_("Loading P2P addresses…").translated); - if (const auto error{LoadAddrman(asmap, args, node.addrman)}) { + if (const auto error{LoadAddrman(*node.netgroupman, args, node.addrman)}) { return InitError(*error); } } @@ -1560,7 +1566,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.banman); node.banman = std::make_unique(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); - node.connman = std::make_unique(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()), *node.addrman, args.GetBoolArg("-networkactive", true)); + node.connman = std::make_unique(GetRand(std::numeric_limits::max()), + GetRand(std::numeric_limits::max()), + *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, diff --git a/src/logging/timer.h b/src/logging/timer.h index 80835af5e2..168c292435 100644 --- a/src/logging/timer.h +++ b/src/logging/timer.h @@ -27,10 +27,12 @@ public: Timer( std::string prefix, std::string end_msg, - BCLog::LogFlags log_category = BCLog::LogFlags::ALL) : + BCLog::LogFlags log_category = BCLog::LogFlags::ALL, + bool msg_on_completion = true) : m_prefix(std::move(prefix)), m_title(std::move(end_msg)), - m_log_category(log_category) + m_log_category(log_category), + m_message_on_completion(msg_on_completion) { this->Log(strprintf("%s started", m_title)); m_start_t = GetTime(); @@ -38,7 +40,11 @@ public: ~Timer() { - this->Log(strprintf("%s completed", m_title)); + if (m_message_on_completion) { + this->Log(strprintf("%s completed", m_title)); + } else { + this->Log("completed"); + } } void Log(const std::string& msg) @@ -74,14 +80,17 @@ private: std::chrono::microseconds m_start_t{}; //! Log prefix; usually the name of the function this was created in. - const std::string m_prefix{}; + const std::string m_prefix; //! A descriptive message of what is being timed. - const std::string m_title{}; + const std::string m_title; //! Forwarded on to LogPrint if specified - has the effect of only //! outputting the timing log when a particular debug= category is specified. - const BCLog::LogFlags m_log_category{}; + const BCLog::LogFlags m_log_category; + + //! Whether to output the message again on completion. + const bool m_message_on_completion; }; } // namespace BCLog @@ -91,6 +100,8 @@ private: BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) #define LOG_TIME_MILLIS_WITH_CATEGORY(end_msg, log_category) \ BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) +#define LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(end_msg, log_category) \ + BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category, /* msg_on_completion=*/false) #define LOG_TIME_SECONDS(end_msg) \ BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg) diff --git a/src/net.cpp b/src/net.cpp index 0da4e01dd9..f94011c899 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2688,7 +2688,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: - setConnected.insert(pnode->addr.GetGroup(addrman.GetAsmap())); + setConnected.insert(m_netgroupman.GetGroup(pnode->addr)); } // no default case, so the compiler can warn about missing cases } } @@ -2778,7 +2778,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) || !HasAllDesirableServiceFlags(addr.nServices) || - setConnected.count(addr.GetGroup(addrman.GetAsmap()))) continue; + setConnected.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToString()); break; @@ -2822,12 +2822,12 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe bool isMasternode = dmn != nullptr; // Require outbound connections, other than feelers, to be to distinct network groups - if (!fFeeler && setConnected.count(addr.GetGroup(addrman.GetAsmap()))) { + if (!fFeeler && setConnected.count(m_netgroupman.GetGroup(addr))) { break; } // if we selected an invalid address, restart - if (!addr.IsValid() || setConnected.count(addr.GetGroup(addrman.GetAsmap()))) + if (!addr.IsValid() || setConnected.count(m_netgroupman.GetGroup(addr))) break; // don't try to connect to masternodes that we already have a connection to (most likely inbound) @@ -3488,8 +3488,12 @@ void CConnman::SetNetworkActive(bool active, CMasternodeSync* const mn_sync) uiInterface.NotifyNetworkActiveChanged(fNetworkActive); } -CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, bool network_active) : - addrman(addrman_in), nSeed0(nSeed0In), nSeed1(nSeed1In) +CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, + const NetGroupManager& netgroupman, bool network_active) + : addrman(addrman_in) + , m_netgroupman{netgroupman} + , nSeed0(nSeed0In) + , nSeed1(nSeed1In) { SetTryNewOutboundPeer(false); @@ -4069,7 +4073,7 @@ void CConnman::GetNodeStats(std::vector& vstats) const } vstats.emplace_back(); pnode->CopyStats(vstats.back()); - vstats.back().m_mapped_as = pnode->addr.GetMappedAS(addrman.GetAsmap()); + vstats.back().m_mapped_as = m_netgroupman.GetMappedAS(pnode->addr); } } @@ -4374,9 +4378,9 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const return CSipHasher(nSeed0, nSeed1).Write(id); } -uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const +uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const { - std::vector vchNetGroup(ad.GetGroup(addrman.GetAsmap())); + std::vector vchNetGroup(m_netgroupman.GetGroup(address)); return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize(); } diff --git a/src/net.h b/src/net.h index a4d6a7dfcd..b29ef25e79 100644 --- a/src/net.h +++ b/src/net.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -991,7 +992,9 @@ public: m_onion_binds = connOptions.onion_binds; } - CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, bool network_active = true); + CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, + bool network_active = true); + ~CConnman(); bool Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync, CScheduler& scheduler, const Options& options) @@ -1505,6 +1508,7 @@ private: std::atomic fNetworkActive{true}; bool fAddressesInitialized{false}; AddrMan& addrman; + const NetGroupManager& m_netgroupman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); Mutex m_addr_fetches_mutex; std::vector m_added_nodes GUARDED_BY(m_added_nodes_mutex); diff --git a/src/netaddress.cpp b/src/netaddress.cpp index c4b639df3e..cc443713b6 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -728,107 +727,6 @@ Network CNetAddr::GetNetClass() const return m_net; } -uint32_t CNetAddr::GetMappedAS(const std::vector &asmap) const { - uint32_t net_class = GetNetClass(); - if (asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { - return 0; // Indicates not found, safe because AS0 is reserved per RFC7607. - } - std::vector ip_bits(128); - if (HasLinkedIPv4()) { - // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits) - for (int8_t byte_i = 0; byte_i < 12; ++byte_i) { - for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { - ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1; - } - } - uint32_t ipv4 = GetLinkedIPv4(); - for (int i = 0; i < 32; ++i) { - ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1; - } - } else { - // Use all 128 bits of the IPv6 address otherwise - assert(IsIPv6()); - for (int8_t byte_i = 0; byte_i < 16; ++byte_i) { - uint8_t cur_byte = m_addr[byte_i]; - for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { - ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1; - } - } - } - uint32_t mapped_as = Interpret(asmap, ip_bits); - return mapped_as; -} - -/** - * Get the canonical identifier of our network group - * - * The groups are assigned in a way where it should be costly for an attacker to - * obtain addresses with many different group identifiers, even if it is cheap - * to obtain addresses with the same identifier. - * - * @note No two connections will be attempted to addresses with the same network - * group. - */ -std::vector CNetAddr::GetGroup(const std::vector &asmap) const -{ - std::vector vchRet; - uint32_t net_class = GetNetClass(); - // If non-empty asmap is supplied and the address is IPv4/IPv6, - // return ASN to be used for bucketing. - uint32_t asn = GetMappedAS(asmap); - if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR). - vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket - for (int i = 0; i < 4; i++) { - vchRet.push_back((asn >> (8 * i)) & 0xFF); - } - return vchRet; - } - - vchRet.push_back(net_class); - int nBits{0}; - - if (IsLocal()) { - // all local addresses belong to the same group - } else if (IsInternal()) { - // all internal-usage addresses get their own group - nBits = ADDR_INTERNAL_SIZE * 8; - } else if (!IsRoutable()) { - // all other unroutable addresses belong to the same group - } else if (HasLinkedIPv4()) { - // IPv4 addresses (and mapped IPv4 addresses) use /16 groups - uint32_t ipv4 = GetLinkedIPv4(); - vchRet.push_back((ipv4 >> 24) & 0xFF); - vchRet.push_back((ipv4 >> 16) & 0xFF); - return vchRet; - } else if (IsTor() || IsI2P()) { - nBits = 4; - } else if (IsCJDNS()) { - // Treat in the same way as Tor and I2P because the address in all of - // them is "random" bytes (derived from a public key). However in CJDNS - // the first byte is a constant 0xfc, so the random bytes come after it. - // Thus skip the constant 8 bits at the start. - nBits = 12; - } else if (IsHeNet()) { - // for he.net, use /36 groups - nBits = 36; - } else { - // for the rest of the IPv6 network, use /32 groups - nBits = 32; - } - - // Push our address onto vchRet. - const size_t num_bytes = nBits / 8; - vchRet.insert(vchRet.end(), m_addr.begin(), m_addr.begin() + num_bytes); - nBits %= 8; - // ...for the last byte, push nBits and for the rest of the byte push 1's - if (nBits > 0) { - assert(num_bytes < m_addr.size()); - vchRet.push_back(m_addr[num_bytes] | ((1 << (8 - nBits)) - 1)); - } - - return vchRet; -} - std::vector CNetAddr::GetAddrBytes() const { if (IsAddrV1Compatible()) { diff --git a/src/netaddress.h b/src/netaddress.h index 6525a0cf4a..29736f8057 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -211,12 +211,6 @@ public: //! Whether this address has a linked IPv4 address (see GetLinkedIPv4()). bool HasLinkedIPv4() const; - // The AS on the BGP path to the node we use to diversify - // peers in AddrMan bucketing based on the AS infrastructure. - // The ip->AS mapping depends on how asmap is constructed. - uint32_t GetMappedAS(const std::vector &asmap) const; - - std::vector GetGroup(const std::vector &asmap) const; std::vector GetAddrBytes() const; int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const; diff --git a/src/netgroup.cpp b/src/netgroup.cpp new file mode 100644 index 0000000000..5f42d6c719 --- /dev/null +++ b/src/netgroup.cpp @@ -0,0 +1,111 @@ +// 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. + +#include + +#include +#include + +uint256 NetGroupManager::GetAsmapChecksum() const +{ + if (!m_asmap.size()) return {}; + + return SerializeHash(m_asmap); +} + +std::vector NetGroupManager::GetGroup(const CNetAddr& address) const +{ + std::vector vchRet; + // If non-empty asmap is supplied and the address is IPv4/IPv6, + // return ASN to be used for bucketing. + uint32_t asn = GetMappedAS(address); + if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR). + vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket + for (int i = 0; i < 4; i++) { + vchRet.push_back((asn >> (8 * i)) & 0xFF); + } + return vchRet; + } + + vchRet.push_back(address.GetNetClass()); + int nStartByte{0}; + int nBits{0}; + + if (address.IsLocal()) { + // all local addresses belong to the same group + } else if (address.IsInternal()) { + // All internal-usage addresses get their own group. + // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes(). + nStartByte = INTERNAL_IN_IPV6_PREFIX.size(); + nBits = ADDR_INTERNAL_SIZE * 8; + } else if (!address.IsRoutable()) { + // all other unroutable addresses belong to the same group + } else if (address.HasLinkedIPv4()) { + // IPv4 addresses (and mapped IPv4 addresses) use /16 groups + uint32_t ipv4 = address.GetLinkedIPv4(); + vchRet.push_back((ipv4 >> 24) & 0xFF); + vchRet.push_back((ipv4 >> 16) & 0xFF); + return vchRet; + } else if (address.IsTor() || address.IsI2P()) { + nBits = 4; + } else if (address.IsCJDNS()) { + // Treat in the same way as Tor and I2P because the address in all of + // them is "random" bytes (derived from a public key). However in CJDNS + // the first byte is a constant 0xfc, so the random bytes come after it. + // Thus skip the constant 8 bits at the start. + nBits = 12; + } else if (address.IsHeNet()) { + // for he.net, use /36 groups + nBits = 36; + } else { + // for the rest of the IPv6 network, use /32 groups + nBits = 32; + } + + // Push our address onto vchRet. + auto addr_bytes = address.GetAddrBytes(); + const size_t num_bytes = nBits / 8; + vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes); + nBits %= 8; + // ...for the last byte, push nBits and for the rest of the byte push 1's + if (nBits > 0) { + assert(num_bytes < addr_bytes.size()); + vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1)); + } + + return vchRet; +} + +uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const +{ + uint32_t net_class = address.GetNetClass(); + if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) { + return 0; // Indicates not found, safe because AS0 is reserved per RFC7607. + } + std::vector ip_bits(128); + if (address.HasLinkedIPv4()) { + // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits) + for (int8_t byte_i = 0; byte_i < 12; ++byte_i) { + for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { + ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1; + } + } + uint32_t ipv4 = address.GetLinkedIPv4(); + for (int i = 0; i < 32; ++i) { + ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1; + } + } else { + // Use all 128 bits of the IPv6 address otherwise + assert(address.IsIPv6()); + auto addr_bytes = address.GetAddrBytes(); + for (int8_t byte_i = 0; byte_i < 16; ++byte_i) { + uint8_t cur_byte = addr_bytes[byte_i]; + for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) { + ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1; + } + } + } + uint32_t mapped_as = Interpret(m_asmap, ip_bits); + return mapped_as; +} diff --git a/src/netgroup.h b/src/netgroup.h new file mode 100644 index 0000000000..2dd63ec66b --- /dev/null +++ b/src/netgroup.h @@ -0,0 +1,66 @@ +// 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_NETGROUP_H +#define BITCOIN_NETGROUP_H + +#include +#include + +#include + +/** + * Netgroup manager + */ +class NetGroupManager { +public: + explicit NetGroupManager(std::vector asmap) + : m_asmap{std::move(asmap)} + {} + + /** Get a checksum identifying the asmap being used. */ + uint256 GetAsmapChecksum() const; + + /** + * Get the canonical identifier of the network group for address. + * + * The groups are assigned in a way where it should be costly for an attacker to + * obtain addresses with many different group identifiers, even if it is cheap + * to obtain addresses with the same identifier. + * + * @note No two connections will be attempted to addresses with the same network + * group. + */ + std::vector GetGroup(const CNetAddr& address) const; + + /** + * Get the autonomous system on the BGP path to address. + * + * The ip->AS mapping depends on how asmap is constructed. + */ + uint32_t GetMappedAS(const CNetAddr& address) const; + +private: + /** Compressed IP->ASN mapping, loaded from a file when a node starts. + * + * This mapping is then used for bucketing nodes in Addrman and for + * ensuring we connect to a diverse set of peers in Connman. The map is + * empty if no file was provided. + * + * 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 is provided, the existing addrman records are + * re-bucketed. + * + * This is initialized in the constructor, const, and therefore is + * thread-safe. */ + const std::vector m_asmap; +}; + +#endif // BITCOIN_NETGROUP_H diff --git a/src/node/context.cpp b/src/node/context.cpp index 075ad5eb06..aa63c1301d 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index 0c1b60f192..10351b2f2c 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -29,6 +29,7 @@ class CScheduler; class CSporkManager; class CTxMemPool; class CMNHFManager; +class NetGroupManager; class PeerManager; struct CJContext; struct LLMQContext; @@ -59,6 +60,7 @@ struct NodeContext { std::unique_ptr addrman; std::unique_ptr connman; std::unique_ptr mempool; + std::unique_ptr netgroupman; std::unique_ptr fee_estimator; std::unique_ptr peerman; std::unique_ptr chainman; diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index b9a91f0fb6..78d8331eb3 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #if defined(QT_STATICPLUGIN) #include @@ -40,6 +41,8 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); const std::function G_TEST_LOG_FUN{}; +const std::function()> G_TEST_COMMAND_LINE_ARGUMENTS{}; + // This is all you need to run all the tests int main(int argc, char* argv[]) { diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index be89fea835..8af1bcf441 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -225,6 +225,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "upgradetohd", 3, "rescan"}, { "getnodeaddresses", 0, "count"}, { "addpeeraddress", 1, "port"}, + { "addpeeraddress", 2, "tried"}, { "stop", 0, "wait" }, { "verifychainlock", 2, "blockHeight" }, { "verifyislock", 3, "maxHeight" }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 29b99c9e51..b8bdd38939 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -970,6 +970,7 @@ static RPCHelpMan addpeeraddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address of the peer"}, {"port", RPCArg::Type::NUM, RPCArg::Optional::NO, "The port of the peer"}, + {"tried", RPCArg::Type::BOOL, /* default */ "false", "If true, attempt to add the peer to the tried addresses table"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -978,8 +979,8 @@ static RPCHelpMan addpeeraddress() }, }, RPCExamples{ - HelpExampleCli("addpeeraddress", "\"1.2.3.4\" 9999") - + HelpExampleRpc("addpeeraddress", "\"1.2.3.4\", 9999") + HelpExampleCli("addpeeraddress", "\"1.2.3.4\" 9999 true") + + HelpExampleRpc("addpeeraddress", "\"1.2.3.4\", 9999, true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -991,6 +992,7 @@ static RPCHelpMan addpeeraddress() const std::string& addr_string{request.params[0].get_str()}; const uint16_t port = request.params[1].get_int(); + const bool tried{request.params[2].isTrue()}; UniValue obj(UniValue::VOBJ); CNetAddr net_addr; @@ -1001,7 +1003,13 @@ static RPCHelpMan addpeeraddress() address.nTime = GetAdjustedTime(); // The source address is set equal to the address. This is equivalent to the peer // announcing itself. - if (node.addrman->Add({address}, address)) success = true; + if (node.addrman->Add({address}, address)) { + success = true; + if (tried) { + // Attempt to move the address to the tried addresses table. + node.addrman->Good(address); + } + } } obj.pushKV("success", success); diff --git a/src/test/README.md b/src/test/README.md index e46152b7e8..cf76f12007 100644 --- a/src/test/README.md +++ b/src/test/README.md @@ -33,19 +33,31 @@ the `src/qt/test/test_main.cpp` file. ### Running individual tests -`test_dash` has some built-in command-line arguments; for -example, to run just the `getarg_tests` verbosely: +`test_dash` accepts the command line arguments from the boost framework. +For example, to run just the `getarg_tests` suite of tests: - test_dash --log_level=all --run_test=getarg_tests -- DEBUG_LOG_OUT +```bash +test_dash --log_level=all --run_test=getarg_tests +``` `log_level` controls the verbosity of the test framework, which logs when a -test case is entered, for example. The `DEBUG_LOG_OUT` after the two dashes -redirects the debug log, which would normally go to a file in the test datadir +test case is entered, for example. `test_dash` also accepts the command +line arguments accepted by `dashd`. Use `--` to separate both types of +arguments: + +```bash +test_dash --log_level=all --run_test=getarg_tests -- -printtoconsole=1 +``` + +The `-printtoconsole=1` after the two dashes redirects the debug log, which +would normally go to a file in the test datadir (`BasicTestingSetup::m_path_root`), to the standard terminal output. ... or to run just the doubledash test: - test_dash --run_test=getarg_tests/doubledash +```bash +test_dash --run_test=getarg_tests/doubledash +``` Run `test_dash --help` for the full list. @@ -68,9 +80,35 @@ on failure. For running individual tests verbosely, refer to the section To write to logs from unit tests you need to use specific message methods provided by Boost. The simplest is `BOOST_TEST_MESSAGE`. -For debugging you can launch the `test_dash` executable with `gdb`or `lldb` and +For debugging you can launch the `test_dash` executable with `gdb` or `lldb` and start debugging, just like you would with any other program: ```bash gdb src/test/test_dash ``` + +#### Segmentation faults + +If you hit a segmentation fault during a test run, you can diagnose where the fault +is happening by running `gdb ./src/test/test_dash` and then using the `bt` command +within gdb. + +Another tool that can be used to resolve segmentation faults is +[valgrind](https://valgrind.org/). + +If for whatever reason you want to produce a core dump file for this fault, you can do +that as well. By default, the boost test runner will intercept system errors and not +produce a core file. To bypass this, add `--catch_system_errors=no` to the +`test_dash` arguments and ensure that your ulimits are set properly (e.g. `ulimit -c +unlimited`). + +Running the tests and hitting a segmentation fault should now produce a file called `core` +(on Linux platforms, the file name will likely depend on the contents of +`/proc/sys/kernel/core_pattern`). + +You can then explore the core dump using +```bash +gdb src/test/test_dash core + +(gbd) bt # produce a backtrace for where a segfault occurred +``` diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 05fb06ec0a..f75bdf4384 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -20,120 +21,14 @@ using namespace std::literals; -class AddrManSerializationMock : public AddrMan +static NetGroupManager EMPTY_NETGROUPMAN{std::vector()}; +static const bool DETERMINISTIC{true}; + +static int32_t GetCheckRatio(const NodeContext& node_ctx) { -public: - virtual void Serialize(CDataStream& s) const = 0; - - AddrManSerializationMock() - : AddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100) - {} -}; - -class AddrManUncorrupted : public AddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - AddrMan::Serialize(s); - } -}; - -class AddrManCorrupted : public AddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. - unsigned char nVersion = 1; - s << nVersion; - s << ((unsigned char)32); - s << uint256::ONE; - s << 10; // nNew - s << 10; // nTried - - int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); - s << nUBuckets; - - CService serv; - BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); - CAddress addr = CAddress(serv, NODE_NONE); - CNetAddr resolved; - BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); - AddrInfo info = AddrInfo(addr, resolved); - s << info; - } -}; - -static CDataStream AddrmanToStream(const AddrManSerializationMock& _addrman) -{ - CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); - ssPeersIn << Params().MessageStart(); - ssPeersIn << _addrman; - std::string str = ssPeersIn.str(); - std::vector vchData(str.begin(), str.end()); - return CDataStream(vchData, SER_DISK, CLIENT_VERSION); + return std::clamp(node_ctx.args->GetArg("-checkaddrman", 100), 0, 1000000); } -class AddrManTest : public AddrMan -{ -private: - bool deterministic; - -public: - explicit AddrManTest(bool makeDeterministic = true, - std::vector asmap = std::vector()) - : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) - { - deterministic = makeDeterministic; - } - - AddrInfo* Find(const CService& addr, int* pnId = nullptr) - { - LOCK(m_impl->cs); - return m_impl->Find(addr, pnId); - } - - AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) - { - LOCK(m_impl->cs); - return m_impl->Create(addr, addrSource, pnId); - } - - void Delete(int nId) - { - LOCK(m_impl->cs); - m_impl->Delete(nId); - } - - // Used to test deserialization - std::pair GetBucketAndEntry(const CAddress& 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 == m_impl->vvNew[bucket][entry]) { - return std::pair(bucket, entry); - } - } - } - return std::pair(-1, -1); - } - - // Simulates connection failure so that we can test eviction of offline nodes - void SimConnFail(const CService& addr) - { - int64_t nLastSuccess = 1; - // Set last good connection in the deep past. - Good(addr, nLastSuccess); - - bool count_failure = false; - int64_t nLastTry = GetAdjustedTime()-61; - Attempt(addr, count_failure, nLastTry); - } -}; - static CNetAddr ResolveIP(const std::string& ip) { CNetAddr addr; @@ -149,7 +44,8 @@ static CService ResolveService(const std::string& ip, uint16_t port = 0) } -static std::vector FromBytes(const unsigned char* source, int vector_size) { +static std::vector FromBytes(const unsigned char* source, int vector_size) +{ std::vector result(vector_size); for (int byte_i = 0; byte_i < vector_size / 8; ++byte_i) { unsigned char cur_byte = source[byte_i]; @@ -160,12 +56,11 @@ static std::vector FromBytes(const unsigned char* source, int vector_size) return result; } - BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(addrman_simple) { - auto addrman = std::make_unique(); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); @@ -199,7 +94,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(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); 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)); @@ -209,57 +104,57 @@ BOOST_AUTO_TEST_CASE(addrman_simple) BOOST_AUTO_TEST_CASE(addrman_ports) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); - BOOST_CHECK_EQUAL(addrman.size(), 0U); + BOOST_CHECK_EQUAL(addrman->size(), 0U); // Test 7; Addr with same IP but diff port does not replace existing addr. CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 1U); CService addr1_port = ResolveService("250.1.1.1", 8334); - BOOST_CHECK(addrman.Add({CAddress(addr1_port, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), 2U); - auto addr_ret2 = addrman.Select().first; + BOOST_CHECK(addrman->Add({CAddress(addr1_port, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 2U); + auto addr_ret2 = addrman->Select().first; BOOST_CHECK(addr_ret2.ToString() == "250.1.1.1:8333" || addr_ret2.ToString() == "250.1.1.1:8334"); // Test: Add same IP but diff port to tried table; this converts the entry with // the specified port to tried, but not the other. - addrman.Good(CAddress(addr1_port, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.size(), 2U); + addrman->Good(CAddress(addr1_port, NODE_NONE)); + BOOST_CHECK_EQUAL(addrman->size(), 2U); bool newOnly = true; - auto addr_ret3 = addrman.Select(newOnly).first; + auto addr_ret3 = addrman->Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); } BOOST_AUTO_TEST_CASE(addrman_select) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); // Test: Select from new with 1 addr in new. CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 1U); bool newOnly = true; - auto addr_ret1 = addrman.Select(newOnly).first; + 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); - auto addr_ret2 = addrman.Select(newOnly).first; + BOOST_CHECK(addrman->Good(CAddress(addr1, NODE_NONE))); + BOOST_CHECK_EQUAL(addrman->size(), 1U); + auto addr_ret2 = addrman->Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret2.ToString(), "[::]:0"); - auto addr_ret3 = addrman.Select().first; + auto addr_ret3 = addrman->Select().first; BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); - BOOST_CHECK_EQUAL(addrman.size(), 1U); + BOOST_CHECK_EQUAL(addrman->size(), 1U); // Add three addresses to new table. @@ -267,173 +162,133 @@ BOOST_AUTO_TEST_CASE(addrman_select) CService addr3 = ResolveService("250.3.2.2", 9999); CService addr4 = ResolveService("250.3.3.3", 9999); - BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); - BOOST_CHECK(addrman.Add({CAddress(addr3, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); - BOOST_CHECK(addrman.Add({CAddress(addr4, NODE_NONE)}, ResolveService("250.4.1.1", 8333))); + BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman->Add({CAddress(addr3, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman->Add({CAddress(addr4, NODE_NONE)}, ResolveService("250.4.1.1", 8333))); // Add three addresses to tried table. CService addr5 = ResolveService("250.4.4.4", 8333); CService addr6 = ResolveService("250.4.5.5", 7777); CService addr7 = ResolveService("250.4.6.6", 8333); - BOOST_CHECK(addrman.Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); - addrman.Good(CAddress(addr5, NODE_NONE)); - BOOST_CHECK(addrman.Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); - addrman.Good(CAddress(addr6, NODE_NONE)); - BOOST_CHECK(addrman.Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333))); - addrman.Good(CAddress(addr7, NODE_NONE)); + BOOST_CHECK(addrman->Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman->Good(CAddress(addr5, NODE_NONE))); + BOOST_CHECK(addrman->Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman->Good(CAddress(addr6, NODE_NONE))); + BOOST_CHECK(addrman->Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333))); + BOOST_CHECK(addrman->Good(CAddress(addr7, NODE_NONE))); // Test: 6 addrs + 1 addr from last test = 7. - BOOST_CHECK_EQUAL(addrman.size(), 7U); + BOOST_CHECK_EQUAL(addrman->size(), 7U); // 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().first.GetPort()); + ports.insert(addrman->Select().first.GetPort()); } BOOST_CHECK_EQUAL(ports.size(), 3U); } BOOST_AUTO_TEST_CASE(addrman_new_collisions) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); uint32_t num_addrs{0}; - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); + BOOST_CHECK_EQUAL(addrman->size(), num_addrs); - while (num_addrs < 22) { // Magic number! 250.1.1.1 - 250.1.1.22 do not collide with deterministic key = 1 + while (num_addrs < 22) { // Magic number! 250.1.1.1 - 250.1.1.22 do not collide with deterministic key = 1 CService addr = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); - //Test: No collision in new table yet. - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); + // Test: No collision in new table yet. + BOOST_CHECK_EQUAL(addrman->size(), num_addrs); } - //Test: new table collision! + // Test: new table collision! CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); uint32_t collisions{1}; - BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), num_addrs - collisions); CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), num_addrs - collisions); +} + +BOOST_AUTO_TEST_CASE(addrman_new_multiplicity) +{ + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); + CAddress addr{CAddress(ResolveService("253.3.3.3", 8333), NODE_NONE)}; + int64_t start_time{GetAdjustedTime()}; + addr.nTime = start_time; + + // test that multiplicity stays at 1 if nTime doesn't increase + for (unsigned int i = 1; i < 20; ++i) { + std::string addr_ip{ToString(i % 256) + "." + ToString(i >> 8 % 256) + ".1.1"}; + CNetAddr source{ResolveIP(addr_ip)}; + addrman->Add({addr}, source); + } + AddressPosition addr_pos = addrman->FindAddressEntry(addr).value(); + BOOST_CHECK_EQUAL(addr_pos.multiplicity, 1U); + BOOST_CHECK_EQUAL(addrman->size(), 1U); + + // if nTime increases, an addr can occur in up to 8 buckets + // The acceptance probability decreases exponentially with existing multiplicity - + // choose number of iterations such that it gets to 8 with deterministic addrman. + for (unsigned int i = 1; i < 400; ++i) { + std::string addr_ip{ToString(i % 256) + "." + ToString(i >> 8 % 256) + ".1.1"}; + CNetAddr source{ResolveIP(addr_ip)}; + addr.nTime = start_time + i; + addrman->Add({addr}, source); + } + AddressPosition addr_pos_multi = addrman->FindAddressEntry(addr).value(); + BOOST_CHECK_EQUAL(addr_pos_multi.multiplicity, 8U); + // multiplicity doesn't affect size + BOOST_CHECK_EQUAL(addrman->size(), 1U); } BOOST_AUTO_TEST_CASE(addrman_tried_collisions) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source = ResolveIP("252.2.2.2"); uint32_t num_addrs{0}; - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); + BOOST_CHECK_EQUAL(addrman->size(), num_addrs); - while (num_addrs < 64) { // Magic number! 250.1.1.1 - 250.1.1.64 do not collide with deterministic key = 1 + while (num_addrs < 35) { // Magic number! 250.1.1.1 - 250.1.1.35 do not collide in tried with deterministic key = 1 CService addr = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(CAddress(addr, NODE_NONE)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); + + // Test: Add to tried without collision + BOOST_CHECK(addrman->Good(CAddress(addr, NODE_NONE))); - //Test: No collision in tried table yet. - BOOST_CHECK_EQUAL(addrman.size(), num_addrs); } - //Test: tried table collision! + // Test: Unable to add to tried table due to collision! CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); - uint32_t collisions{1}; - BOOST_CHECK(!addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK(!addrman->Good(CAddress(addr1, NODE_NONE))); + // Test: Add the next address to tried without collision CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); - BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source)); - BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); + BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK(addrman->Good(CAddress(addr2, NODE_NONE))); } -BOOST_AUTO_TEST_CASE(addrman_find) -{ - AddrManTest addrman; - - BOOST_CHECK_EQUAL(addrman.size(), 0U); - - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); - CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); - CAddress addr3 = CAddress(ResolveService("251.255.2.1", 8333), NODE_NONE); - - CNetAddr source1 = ResolveIP("250.1.2.1"); - CNetAddr source2 = ResolveIP("250.1.2.2"); - - BOOST_CHECK(addrman.Add({addr1}, source1)); - BOOST_CHECK(addrman.Add({addr2}, source2)); - BOOST_CHECK(addrman.Add({addr3}, source1)); - - // Test: ensure Find returns an IP/port matching what we searched on. - AddrInfo* info1 = addrman.Find(addr1); - BOOST_REQUIRE(info1); - BOOST_CHECK_EQUAL(info1->ToString(), "250.1.2.1:8333"); - - // Test; Find discriminates by port number. - AddrInfo* info2 = addrman.Find(addr2); - BOOST_REQUIRE(info2); - BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:9999"); - - // Test: Find returns another IP matching what we searched on. - AddrInfo* info3 = addrman.Find(addr3); - BOOST_REQUIRE(info3); - BOOST_CHECK_EQUAL(info3->ToString(), "251.255.2.1:8333"); -} - -BOOST_AUTO_TEST_CASE(addrman_create) -{ - AddrManTest addrman; - - BOOST_CHECK_EQUAL(addrman.size(), 0U); - - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); - CNetAddr source1 = ResolveIP("250.1.2.1"); - - int 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"); - - AddrInfo* info2 = addrman.Find(addr1); - BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:8333"); -} - - -BOOST_AUTO_TEST_CASE(addrman_delete) -{ - AddrManTest addrman; - - BOOST_CHECK_EQUAL(addrman.size(), 0U); - - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); - CNetAddr source1 = ResolveIP("250.1.2.1"); - - int nId; - addrman.Create(addr1, source1, &nId); - - // Test: Delete should actually delete the addr. - BOOST_CHECK_EQUAL(addrman.size(), 1U); - addrman.Delete(nId); - BOOST_CHECK_EQUAL(addrman.size(), 0U); - AddrInfo* info2 = addrman.Find(addr1); - BOOST_CHECK(info2 == nullptr); -} BOOST_AUTO_TEST_CASE(addrman_getaddr) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); // Test: Sanity check, GetAddr should never return anything if addrman // is empty. - BOOST_CHECK_EQUAL(addrman.size(), 0U); - std::vector vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt); + BOOST_CHECK_EQUAL(addrman->size(), 0U); + std::vector vAddr1 = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt); BOOST_CHECK_EQUAL(vAddr1.size(), 0U); CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); @@ -450,18 +305,18 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) CNetAddr source2 = ResolveIP("250.2.3.3"); // Test: Ensure GetAddr works with new addresses. - BOOST_CHECK(addrman.Add({addr1, addr3, addr5}, source1)); - BOOST_CHECK(addrman.Add({addr2, addr4}, source2)); + BOOST_CHECK(addrman->Add({addr1, addr3, addr5}, source1)); + BOOST_CHECK(addrman->Add({addr2, addr4}, source2)); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U); + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt).size(), 5U); // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).size(), 1U); + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/2500, /*max_pct=*/23, /*network=*/std::nullopt).size(), 1U); // Test: Ensure GetAddr works with new and tried addresses. - addrman.Good(CAddress(addr1, NODE_NONE)); - addrman.Good(CAddress(addr2, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).size(), 1U); + BOOST_CHECK(addrman->Good(CAddress(addr1, NODE_NONE))); + BOOST_CHECK(addrman->Good(CAddress(addr2, NODE_NONE))); + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt).size(), 5U); + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/2500, /*max_pct=*/23, /*network=*/std::nullopt).size(), 1U); // Test: Ensure GetAddr still returns 23% when addrman has many addrs. for (unsigned int i = 1; i < (8 * 256); i++) { @@ -472,24 +327,22 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) // Ensure that for all addrs in addrman, isTerrible == false. addr.nTime = GetAdjustedTime(); - addrman.Add({addr}, ResolveIP(strAddr)); + addrman->Add({addr}, ResolveIP(strAddr)); if (i % 8 == 0) - addrman.Good(addr); + addrman->Good(addr); } - std::vector vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt); + std::vector vAddr = addrman->GetAddr(/*max_addresses=*/2500, /*max_pct=*/23, /*network=*/std::nullopt); - size_t percent23 = (addrman.size() * 23) / 100; + size_t percent23 = (addrman->size() * 23) / 100; BOOST_CHECK_EQUAL(vAddr.size(), percent23); BOOST_CHECK_EQUAL(vAddr.size(), 461U); // (Addrman.size() < number of addresses added) due to address collisions. - BOOST_CHECK_EQUAL(addrman.size(), 2006U); + BOOST_CHECK_EQUAL(addrman->size(), 2006U); } BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) { - AddrManTest addrman; - CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -501,27 +354,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap; // use /16 - - BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, asmap), 40); + BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN), 40); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info1.GetTriedBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN) != info1.GetTriedBucket(nKey2, EMPTY_NETGROUPMAN)); // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN) != info2.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetTriedBucket(nKey1, asmap); + int bucket = infoi.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same /16 prefix should @@ -533,7 +384,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix should map to more than @@ -543,8 +394,6 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) { - AddrManTest addrman; - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); @@ -555,27 +404,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap; // use /16 - // Test: Make sure the buckets are what we expect - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), 786); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, asmap), 786); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN), 786); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, EMPTY_NETGROUPMAN), 786); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN) != info1.GetNewBucket(nKey2, EMPTY_NETGROUPMAN)); // Test: Ports should not affect bucket placement in the addr AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, EMPTY_NETGROUPMAN), info2.GetNewBucket(nKey1, EMPTY_NETGROUPMAN)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetNewBucket(nKey1, asmap); + int bucket = infoi.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same group (\16 prefix for IPv4) should @@ -588,7 +435,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the same source groups should map to NO MORE @@ -600,7 +447,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, EMPTY_NETGROUPMAN); buckets.insert(bucket); } // Test: IP addresses in the different source groups should map to MORE @@ -621,7 +468,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket_legacy) // 101.8.0.0/16 AS8 BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) { - AddrManTest addrman; + std::vector asmap = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8); + NetGroupManager ngm_asmap{asmap}; CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -634,27 +482,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8); - - BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, asmap), 236); + BOOST_CHECK_EQUAL(info1.GetTriedBucket(nKey1, ngm_asmap), 236); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info1.GetTriedBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, ngm_asmap) != info1.GetTriedBucket(nKey2, ngm_asmap)); // Test: Two addresses with same IP but different ports can map to // different buckets because they have different keys. AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK(info1.GetTriedBucket(nKey1, asmap) != info2.GetTriedBucket(nKey1, asmap)); + BOOST_CHECK(info1.GetTriedBucket(nKey1, ngm_asmap) != info2.GetTriedBucket(nKey1, ngm_asmap)); std::set buckets; for (int j = 0; j < 255; j++) { AddrInfo infoj = AddrInfo( CAddress(ResolveService("101." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("101." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix MAY map to more than @@ -666,7 +512,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250." + ToString(j) + ".1.1"), NODE_NONE), ResolveIP("250." + ToString(j) + ".1.1")); - int bucket = infoj.GetTriedBucket(nKey1, asmap); + int bucket = infoj.GetTriedBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different /16 prefix MAY NOT map to more than @@ -676,7 +522,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) { - AddrManTest addrman; + std::vector asmap = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8); + NetGroupManager ngm_asmap{asmap}; CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); @@ -688,27 +535,25 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) uint256 nKey1 = (uint256)(CHashWriter(SER_GETHASH, 0) << 1).GetHash(); uint256 nKey2 = (uint256)(CHashWriter(SER_GETHASH, 0) << 2).GetHash(); - std::vector asmap = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8); - // Test: Make sure the buckets are what we expect - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), 795); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, asmap), 795); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, ngm_asmap), 795); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, source1, ngm_asmap), 795); // Test: Make sure key actually randomizes bucket placement. A fail on // this test could be a security issue. - BOOST_CHECK(info1.GetNewBucket(nKey1, asmap) != info1.GetNewBucket(nKey2, asmap)); + BOOST_CHECK(info1.GetNewBucket(nKey1, ngm_asmap) != info1.GetNewBucket(nKey2, ngm_asmap)); // Test: Ports should not affect bucket placement in the addr AddrInfo info2 = AddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); - BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, asmap), info2.GetNewBucket(nKey1, asmap)); + BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1, ngm_asmap), info2.GetNewBucket(nKey1, ngm_asmap)); std::set buckets; for (int i = 0; i < 255; i++) { AddrInfo infoi = AddrInfo( CAddress(ResolveService("250.1.1." + ToString(i)), NODE_NONE), ResolveIP("250.1.1." + ToString(i))); - int bucket = infoi.GetNewBucket(nKey1, asmap); + int bucket = infoi.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the same /16 prefix @@ -721,7 +566,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) ResolveService( ToString(250 + (j / 255)) + "." + ToString(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the same source /16 prefix should not map to more @@ -733,7 +578,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("101." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different source /16 prefixes usually map to MORE @@ -745,85 +590,84 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) AddrInfo infoj = AddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), ResolveIP("250." + ToString(p) + ".1.1")); - int bucket = infoj.GetNewBucket(nKey1, asmap); + int bucket = infoj.GetNewBucket(nKey1, ngm_asmap); buckets.insert(bucket); } // Test: IP addresses in the different source /16 prefixes sometimes map to NO MORE // than 1 bucket. BOOST_CHECK(buckets.size() == 1); - } BOOST_AUTO_TEST_CASE(addrman_serialization) { std::vector asmap1 = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8); + NetGroupManager netgroupman{asmap1}; + + const auto ratio = GetCheckRatio(m_node); + auto addrman_asmap1 = std::make_unique(netgroupman, DETERMINISTIC, ratio); + auto addrman_asmap1_dup = std::make_unique(netgroupman, DETERMINISTIC, ratio); + auto addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); - 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); CNetAddr default_source; - addrman_asmap1->Add({addr}, default_source); stream << *addrman_asmap1; // serizalizing/deserializing addrman with the same asmap stream >> *addrman_asmap1_dup; - std::pair bucketAndEntry_asmap1 = addrman_asmap1->GetBucketAndEntry(addr); - std::pair bucketAndEntry_asmap1_dup = addrman_asmap1_dup->GetBucketAndEntry(addr); - BOOST_CHECK(bucketAndEntry_asmap1.second != -1); - BOOST_CHECK(bucketAndEntry_asmap1_dup.second != -1); + AddressPosition addr_pos1 = addrman_asmap1->FindAddressEntry(addr).value(); + AddressPosition addr_pos2 = addrman_asmap1_dup->FindAddressEntry(addr).value(); + BOOST_CHECK(addr_pos1.multiplicity != 0); + BOOST_CHECK(addr_pos2.multiplicity != 0); - BOOST_CHECK(bucketAndEntry_asmap1.first == bucketAndEntry_asmap1_dup.first); - BOOST_CHECK(bucketAndEntry_asmap1.second == bucketAndEntry_asmap1_dup.second); + BOOST_CHECK(addr_pos1 == addr_pos2); // deserializing asmaped peers.dat to non-asmaped addrman stream << *addrman_asmap1; stream >> *addrman_noasmap; - std::pair bucketAndEntry_noasmap = addrman_noasmap->GetBucketAndEntry(addr); - BOOST_CHECK(bucketAndEntry_noasmap.second != -1); - BOOST_CHECK(bucketAndEntry_asmap1.first != bucketAndEntry_noasmap.first); - BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second); + AddressPosition addr_pos3 = addrman_noasmap->FindAddressEntry(addr).value(); + BOOST_CHECK(addr_pos3.multiplicity != 0); + BOOST_CHECK(addr_pos1.bucket != addr_pos3.bucket); + BOOST_CHECK(addr_pos1.position != addr_pos3.position); // 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(netgroupman, DETERMINISTIC, ratio); + addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); addrman_noasmap->Add({addr}, default_source); stream << *addrman_noasmap; stream >> *addrman_asmap1; - std::pair bucketAndEntry_asmap1_deser = addrman_asmap1->GetBucketAndEntry(addr); - BOOST_CHECK(bucketAndEntry_asmap1_deser.second != -1); - BOOST_CHECK(bucketAndEntry_asmap1_deser.first != bucketAndEntry_noasmap.first); - BOOST_CHECK(bucketAndEntry_asmap1_deser.first == bucketAndEntry_asmap1_dup.first); - BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second); + + AddressPosition addr_pos4 = addrman_asmap1->FindAddressEntry(addr).value(); + BOOST_CHECK(addr_pos4.multiplicity != 0); + BOOST_CHECK(addr_pos4.bucket != addr_pos3.bucket); + BOOST_CHECK(addr_pos4 == addr_pos2); // 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(netgroupman, DETERMINISTIC, ratio); + addrman_noasmap = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, ratio); 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); - std::pair bucketAndEntry_noasmap_addr1 = addrman_noasmap->GetBucketAndEntry(addr1); - std::pair bucketAndEntry_noasmap_addr2 = addrman_noasmap->GetBucketAndEntry(addr2); - BOOST_CHECK(bucketAndEntry_noasmap_addr1.first != bucketAndEntry_noasmap_addr2.first); - BOOST_CHECK(bucketAndEntry_noasmap_addr1.second != bucketAndEntry_noasmap_addr2.second); + AddressPosition addr_pos5 = addrman_noasmap->FindAddressEntry(addr1).value(); + AddressPosition addr_pos6 = addrman_noasmap->FindAddressEntry(addr2).value(); + BOOST_CHECK(addr_pos5.bucket != addr_pos6.bucket); stream << *addrman_noasmap; stream >> *addrman_asmap1; - std::pair bucketAndEntry_asmap1_deser_addr1 = addrman_asmap1->GetBucketAndEntry(addr1); - std::pair bucketAndEntry_asmap1_deser_addr2 = addrman_asmap1->GetBucketAndEntry(addr2); - BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.first == bucketAndEntry_asmap1_deser_addr2.first); - BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.second != bucketAndEntry_asmap1_deser_addr2.second); + AddressPosition addr_pos7 = addrman_asmap1->FindAddressEntry(addr1).value(); + AddressPosition addr_pos8 = addrman_asmap1->FindAddressEntry(addr2).value(); + BOOST_CHECK(addr_pos7.bucket == addr_pos8.bucket); + BOOST_CHECK(addr_pos7.position != addr_pos8.position); } BOOST_AUTO_TEST_CASE(remove_invalid) { // Confirm that invalid addresses are ignored in unserialization. - auto addrman = std::make_unique(); + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; @@ -855,156 +699,169 @@ 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(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); stream >> *addrman; BOOST_CHECK_EQUAL(addrman->size(), 2); } BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); - BOOST_CHECK(addrman.size() == 0); + BOOST_CHECK(addrman->size() == 0); // Empty addrman should return blank addrman info. - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); // Add twenty two addresses. CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+ToString(i)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); + CService addr = ResolveService("250.1.1." + ToString(i)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); - // No collisions yet. - BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + // No collisions in tried. + BOOST_CHECK(addrman->Good(addr)); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); } // Ensure Good handles duplicates well. + // If an address is a duplicate, Good will return false but will not count it as a collision. for (unsigned int i = 1; i < 23; i++) { - CService addr = ResolveService("250.1.1."+ToString(i)); - addrman.Good(addr); + CService addr = ResolveService("250.1.1." + ToString(i)); - BOOST_CHECK(addrman.size() == 22); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + // Unable to add duplicate address to tried table. + BOOST_CHECK(!addrman->Good(addr)); + + // Verify duplicate address not marked as a collision. + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); } - } BOOST_AUTO_TEST_CASE(addrman_noevict) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); // Add 35 addresses. CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 36; i++) { - CService addr = ResolveService("250.1.1."+ToString(i)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); + CService addr = ResolveService("250.1.1." + ToString(i)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); // No collision yet. - BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + BOOST_CHECK(addrman->Good(addr)); } - // Collision between 36 and 19. + // Collision in tried table between 36 and 19. CService addr36 = ResolveService("250.1.1.36"); - BOOST_CHECK(addrman.Add({CAddress(addr36, NODE_NONE)}, source)); - addrman.Good(addr36); - - BOOST_CHECK(addrman.size() == 36); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.19:0"); + BOOST_CHECK(addrman->Add({CAddress(addr36, NODE_NONE)}, source)); + BOOST_CHECK(!addrman->Good(addr36)); + 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().first.ToString() == "[::]:0"); + // This means we keep 19 in the tried table and + // 36 stays in the new table. + addrman->ResolveCollisions(); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); // Lets create two collisions. for (unsigned int i = 37; i < 59; i++) { - CService addr = ResolveService("250.1.1."+ToString(i)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); - - BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + CService addr = ResolveService("250.1.1." + ToString(i)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); + BOOST_CHECK(addrman->Good(addr)); } - // Cause a collision. + // Cause a collision in the tried table. CService addr59 = ResolveService("250.1.1.59"); - BOOST_CHECK(addrman.Add({CAddress(addr59, NODE_NONE)}, source)); - addrman.Good(addr59); - BOOST_CHECK(addrman.size() == 59); + BOOST_CHECK(addrman->Add({CAddress(addr59, NODE_NONE)}, source)); + BOOST_CHECK(!addrman->Good(addr59)); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.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); + // Cause a second collision in the new table. + BOOST_CHECK(!addrman->Add({CAddress(addr36, NODE_NONE)}, source)); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() != "[::]:0"); - addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + // 36 still cannot be moved from new to tried due to colliding with 19 + BOOST_CHECK(!addrman->Good(addr36)); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() != "[::]:0"); + + // Resolve all collisions. + addrman->ResolveCollisions(); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); } BOOST_AUTO_TEST_CASE(addrman_evictionworks) { - AddrManTest addrman; + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); - BOOST_CHECK(addrman.size() == 0); + BOOST_CHECK(addrman->size() == 0); // Empty addrman should return blank addrman info. - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); // Add 35 addresses CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 36; i++) { - CService addr = ResolveService("250.1.1."+ToString(i)); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); + CService addr = ResolveService("250.1.1." + ToString(i)); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); // No collision yet. - BOOST_CHECK(addrman.size() == i); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + BOOST_CHECK(addrman->Good(addr)); } // Collision between 36 and 19. CService addr = ResolveService("250.1.1.36"); - BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); + BOOST_CHECK(addrman->Add({CAddress(addr, NODE_NONE)}, source)); + BOOST_CHECK(!addrman->Good(addr)); - BOOST_CHECK_EQUAL(addrman.size(), 36); - auto info = addrman.SelectTriedCollision().first; + 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. - addrman.SimConnFail(info); + // Update entry in tried by setting last good connection in the deep past. + BOOST_CHECK(!addrman->Good(info, /*nTime=*/1)); + addrman->Attempt(info, /*fCountFailure=*/false, /*nTime=*/GetAdjustedTime() - 61); // Should swap 36 for 19. - addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); + addrman->ResolveCollisions(); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); + AddressPosition addr_pos{addrman->FindAddressEntry(CAddress(addr, NODE_NONE)).value()}; + BOOST_CHECK(addr_pos.tried); - // If 36 was swapped for 19, then this should cause no collisions. - BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source)); - addrman.Good(addr); + // If 36 was swapped for 19, then adding 36 to tried should fail because we + // are attempting to add a duplicate. + // We check this by verifying Good() returns false and also verifying that + // we have no collisions. + BOOST_CHECK(!addrman->Good(addr)); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); - - // If we insert 19 it should collide with 36 + // 19 should fail as a collision (not a duplicate) if we now attempt to move + // it to the tried table. CService addr19 = ResolveService("250.1.1.19"); - BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source)); - addrman.Good(addr19); + BOOST_CHECK(!addrman->Good(addr19)); + BOOST_CHECK_EQUAL(addrman->SelectTriedCollision().first.ToString(), "250.1.1.36:0"); - BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.36:0"); + // Eviction is also successful if too much time has passed since last try + SetMockTime(GetTime() + 4 * 60 *60); + addrman->ResolveCollisions(); + BOOST_CHECK(addrman->SelectTriedCollision().first.ToString() == "[::]:0"); + //Now 19 is in tried again, and 36 back to new + AddressPosition addr_pos19{addrman->FindAddressEntry(CAddress(addr19, NODE_NONE)).value()}; + BOOST_CHECK(addr_pos19.tried); + AddressPosition addr_pos36{addrman->FindAddressEntry(CAddress(addr, NODE_NONE)).value()}; + BOOST_CHECK(!addr_pos36.tried); +} - addrman.ResolveCollisions(); - BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); +static CDataStream AddrmanToStream(const AddrMan& addrman) +{ + CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); + ssPeersIn << Params().MessageStart(); + ssPeersIn << addrman; + return ssPeersIn; } BOOST_AUTO_TEST_CASE(load_addrman) { - AddrManUncorrupted addrmanUncorrupted; + AddrMan addrman{EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)}; CService addr1, addr2, addr3; BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); @@ -1017,13 +874,13 @@ BOOST_AUTO_TEST_CASE(load_addrman) CService source; BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false)); std::vector addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)}; - BOOST_CHECK(addrmanUncorrupted.Add(addresses, source)); - BOOST_CHECK(addrmanUncorrupted.size() == 3); + BOOST_CHECK(addrman.Add(addresses, source)); + BOOST_CHECK(addrman.size() == 3); // Test that the de-serialization does not throw an exception. - CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); + CDataStream ssPeers1 = AddrmanToStream(addrman); bool exceptionThrown = false; - AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman1{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman1.size() == 0); try { @@ -1038,23 +895,47 @@ BOOST_AUTO_TEST_CASE(load_addrman) BOOST_CHECK(exceptionThrown == false); // Test that ReadFromStream creates an addrman with the correct number of addrs. - CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); + CDataStream ssPeers2 = AddrmanToStream(addrman); - AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman2{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman2.size() == 0); ReadFromStream(addrman2, ssPeers2); BOOST_CHECK(addrman2.size() == 3); } +// Produce a corrupt peers.dat that claims 20 addrs when it only has one addr. +static CDataStream MakeCorruptPeersDat() +{ + CDataStream s(SER_DISK, CLIENT_VERSION); + s << ::Params().MessageStart(); + + unsigned char nVersion = 1; + s << nVersion; + s << ((unsigned char)32); + s << uint256::ONE; + s << 10; // nNew + s << 10; // nTried + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + + CService serv; + BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); + CAddress addr = CAddress(serv, NODE_NONE); + CNetAddr resolved; + BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); + AddrInfo info = AddrInfo(addr, resolved); + s << info; + + return s; +} BOOST_AUTO_TEST_CASE(load_addrman_corrupted) { - AddrManCorrupted addrmanCorrupted; - - // Test that the de-serialization of corrupted addrman throws an exception. - CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); + // Test that the de-serialization of corrupted peers.dat throws an exception. + CDataStream ssPeers1 = MakeCorruptPeersDat(); bool exceptionThrown = false; - AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman1{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; @@ -1068,11 +949,42 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) BOOST_CHECK(exceptionThrown); // Test that ReadFromStream fails if peers.dat is corrupt - CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); + CDataStream ssPeers2 = MakeCorruptPeersDat(); - AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); + AddrMan addrman2{EMPTY_NETGROUPMAN, !DETERMINISTIC, GetCheckRatio(m_node)}; BOOST_CHECK(addrman2.size() == 0); BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure); } +BOOST_AUTO_TEST_CASE(addrman_update_address) +{ + // Tests updating nTime via Connected() and nServices via SetServices() + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); + CNetAddr source{ResolveIP("252.2.2.2")}; + CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)}; + + int64_t start_time{GetAdjustedTime() - 10000}; + addr.nTime = start_time; + BOOST_CHECK(addrman->Add({addr}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 1U); + + // Updating an addrman entry with a different port doesn't change it + CAddress addr_diff_port{CAddress(ResolveService("250.1.1.1", 8334), NODE_NONE)}; + addr_diff_port.nTime = start_time; + addrman->Connected(addr_diff_port); + addrman->SetServices(addr_diff_port, NODE_NETWORK_LIMITED); + std::vector vAddr1{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr1.size(), 1U); + BOOST_CHECK_EQUAL(vAddr1.at(0).nTime, start_time); + BOOST_CHECK_EQUAL(vAddr1.at(0).nServices, NODE_NONE); + + // Updating an addrman entry with the correct port is successful + addrman->Connected(addr); + addrman->SetServices(addr, NODE_NETWORK_LIMITED); + std::vector vAddr2 = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt); + BOOST_CHECK_EQUAL(vAddr2.size(), 1U); + BOOST_CHECK(vAddr2.at(0).nTime >= start_time + 10000); + BOOST_CHECK_EQUAL(vAddr2.at(0).nServices, NODE_NETWORK_LIMITED); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 8c5cee5371..d34f15b78e 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { NodeId id{0}; const CChainParams& chainparams = Params(); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman, @@ -217,7 +217,7 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction) { NodeId id{0}; const CChainParams& chainparams = Params(); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman, @@ -284,7 +284,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) const CChainParams& chainparams = Params(); auto banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman, @@ -389,7 +389,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) const CChainParams& chainparams = Params(); auto banman = std::make_unique(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman, diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index b6d75ba80d..0eafe61f93 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -11,8 +11,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -20,16 +22,34 @@ #include #include +namespace { +const BasicTestingSetup* g_setup; + +int32_t GetCheckRatio() +{ + return std::clamp(g_setup->m_node.args->GetArg("-checkaddrman", 0), 0, 1000000); +} +} // namespace + void initialize_addrman() { - SelectParams(CBaseChainParams::REGTEST); + static const auto testing_setup = MakeNoLogFileContext<>(CBaseChainParams::REGTEST); + g_setup = testing_setup.get(); +} + +[[nodiscard]] NetGroupManager ConsumeNetGroupManager(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(asmap, 128)) asmap.clear(); + return NetGroupManager(asmap); } FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); - AddrMan addr_man(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + AddrMan addr_man(netgroupman, /*deterministic=*/false, GetCheckRatio()); try { ReadFromStream(addr_man, data_stream); } catch (const std::exception&) { @@ -112,8 +132,8 @@ void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider) class AddrManDeterministic : public AddrMan { public: - explicit AddrManDeterministic(std::vector asmap, FuzzedDataProvider& fuzzed_data_provider) - : AddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0) + explicit AddrManDeterministic(const NetGroupManager& netgroupman, FuzzedDataProvider& fuzzed_data_provider) + : AddrMan(netgroupman, /*deterministic=*/true, GetCheckRatio()) { WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } @@ -211,19 +231,12 @@ public: } }; -[[nodiscard]] inline std::vector ConsumeAsmap(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap, 128)) asmap.clear(); - return asmap; -} - 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); + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + auto addr_man_ptr = std::make_unique(netgroupman, 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,7 +245,7 @@ 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(netgroupman, fuzzed_data_provider); } } AddrManDeterministic& addr_man = *addr_man_ptr; @@ -304,9 +317,9 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - std::vector asmap = ConsumeAsmap(fuzzed_data_provider); - AddrManDeterministic addr_man1{asmap, fuzzed_data_provider}; - AddrManDeterministic addr_man2{asmap, fuzzed_data_provider}; + NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; + AddrManDeterministic addr_man1{netgroupman, fuzzed_data_provider}; + AddrManDeterministic addr_man2{netgroupman, fuzzed_data_provider}; CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp index c5e9c56049..80e5178866 100644 --- a/src/test/fuzz/asmap.cpp +++ b/src/test/fuzz/asmap.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -56,5 +57,6 @@ FUZZ_TARGET(asmap) memcpy(&ipv4, addr_data, addr_size); net_addr.SetIP(CNetAddr{ipv4}); } - (void)net_addr.GetMappedAS(asmap); + NetGroupManager netgroupman{asmap}; + (void)netgroupman.GetMappedAS(net_addr); } diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index dcbf31809a..95f1b6516e 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -11,22 +11,31 @@ #include #include #include +#include #include #include #include +namespace { +const TestingSetup* g_setup; +} // namespace + void initialize_connman() { - static const auto testing_setup = MakeNoLogFileContext<>(); + static const auto testing_setup = MakeNoLogFileContext(); + g_setup = testing_setup.get(); } FUZZ_TARGET_INIT(connman, initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - AddrMan addrman(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); - CConnman connman{fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeIntegral(), addrman}; + CConnman connman{fuzzed_data_provider.ConsumeIntegral(), + fuzzed_data_provider.ConsumeIntegral(), + *g_setup->m_node.addrman, + *g_setup->m_node.netgroupman, + fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); CSubNet random_subnet; diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 4bd4a55c83..b60abddaf5 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -15,13 +15,16 @@ #include #include #include +#include #include #include #include #include #include