diff --git a/src/addrman.cpp b/src/addrman.cpp index 8f702b5a8c0..8192b4eba66 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -77,6 +77,38 @@ double CAddrInfo::GetChance(int64_t nNow) const return fChance; } +void CAddrMan::RemoveInvalid() +{ + for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) { + for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + const auto id = vvNew[bucket][i]; + if (id != -1 && !mapInfo[id].IsValid()) { + ClearNew(bucket, i); + } + } + } + + for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; ++bucket) { + for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + const auto id = vvTried[bucket][i]; + if (id == -1) { + continue; + } + const auto& addr_info = mapInfo[id]; + if (addr_info.IsValid()) { + continue; + } + vvTried[bucket][i] = -1; + --nTried; + SwapRandom(addr_info.nRandomPos, vRandom.size() - 1); + vRandom.pop_back(); + mapAddr.erase(addr_info); + mapInfo.erase(id); + m_tried_collisions.erase(id); + } + } +} + CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId) { AssertLockHeld(cs); diff --git a/src/addrman.h b/src/addrman.h index 665e2531926..c2f425f2fa8 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -450,6 +450,8 @@ public: LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost); } + RemoveInvalid(); + Check(); } @@ -762,6 +764,9 @@ private: //! Update an entry's service bits. void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); + //! Remove invalid addresses. + void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs); + friend class CAddrManTest; }; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 1ea3969978d..e7b3377475b 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -489,7 +489,7 @@ bool CNetAddr::IsValid() const */ bool CNetAddr::IsRoutable() const { - return IsValid() && !(IsRFC1918() || IsRFC2544() || IsRFC3927() || IsRFC4862() || IsRFC6598() || IsRFC5737() || (IsRFC4193() && !IsTor()) || IsRFC4843() || IsRFC7343() || IsLocal() || IsInternal()); + return IsValid() && !(IsRFC1918() || IsRFC2544() || IsRFC3927() || IsRFC4862() || IsRFC6598() || IsRFC5737() || IsRFC4193() || IsRFC4843() || IsRFC7343() || IsLocal() || IsInternal()); } /** diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index eb5c37b34dd..1103292c1a8 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -783,6 +783,46 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.second != bucketAndEntry_asmap1_deser_addr2.second); } +BOOST_AUTO_TEST_CASE(remove_invalid) +{ + // Confirm that invalid addresses are ignored in unserialization. + + CAddrManTest addrman; + CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); + + const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; + const CAddress new2{ResolveService("6.6.6.6"), NODE_NONE}; + const CAddress tried1{ResolveService("7.7.7.7"), NODE_NONE}; + const CAddress tried2{ResolveService("8.8.8.8"), NODE_NONE}; + + addrman.Add({new1, tried1, new2, tried2}, CNetAddr{}); + addrman.Good(tried1); + addrman.Good(tried2); + BOOST_REQUIRE_EQUAL(addrman.size(), 4); + + stream << addrman; + + const std::string str{stream.str()}; + size_t pos; + + const char new2_raw[]{6, 6, 6, 6}; + const uint8_t new2_raw_replacement[]{0, 0, 0, 0}; // 0.0.0.0 is !IsValid() + pos = str.find(new2_raw, 0, sizeof(new2_raw)); + BOOST_REQUIRE(pos != std::string::npos); + BOOST_REQUIRE(pos + sizeof(new2_raw_replacement) <= stream.size()); + memcpy(stream.data() + pos, new2_raw_replacement, sizeof(new2_raw_replacement)); + + const char tried2_raw[]{8, 8, 8, 8}; + const uint8_t tried2_raw_replacement[]{255, 255, 255, 255}; // 255.255.255.255 is !IsValid() + pos = str.find(tried2_raw, 0, sizeof(tried2_raw)); + BOOST_REQUIRE(pos != std::string::npos); + BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size()); + memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement)); + + addrman.Clear(); + stream >> addrman; + BOOST_CHECK_EQUAL(addrman.size(), 2); +} BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) { diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index f9d8129ca9e..6cb81901cb3 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -54,7 +54,7 @@ FUZZ_TARGET(netaddress) (void)net_addr.IsRFC3927(); (void)net_addr.IsRFC3964(); if (net_addr.IsRFC4193()) { - assert(net_addr.GetNetwork() == Network::NET_ONION || net_addr.GetNetwork() == Network::NET_INTERNAL || net_addr.GetNetwork() == Network::NET_UNROUTABLE); + assert(net_addr.GetNetwork() == Network::NET_INTERNAL || net_addr.GetNetwork() == Network::NET_UNROUTABLE); } (void)net_addr.IsRFC4380(); (void)net_addr.IsRFC4843();