addrman: remove invalid addresses when unserializing

The Tor v2 addresses, left over from when Tor v2 was supported will be
unserialized as a dummy, invalid `::` (all zeros) IPv6 address. Remove
them so that they do not take up space in addrman.

Author: vasild

src/addrman.cpp

@@ -75,6 +75,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)
 {
     std::map<CNetAddr, int>::iterator it = mapAddr.find(addr);

src/addrman.h

@@ -448,6 +448,8 @@ class CAddrMan
             LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost);
         }
 
+        RemoveInvalid();
+
         Check();
     }
 
@@ -756,6 +758,9 @@ class CAddrMan
     //! 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;
 };
 

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)
 {