[addrman] Remove RemoveInvalid()

Instead of deserializing addresses, placing them in the buckets, and
then removing them if they're invalid, check first and don't place in
the buckets if they're invalid.

Author: jnewbery

src/addrman.cpp

@@ -77,38 +77,6 @@ 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);

src/addrman.h

@@ -365,7 +365,8 @@ class CAddrMan
             s >> info;
             int nKBucket = info.GetTriedBucket(nKey, m_asmap);
             int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket);
-            if (vvTried[nKBucket][nKBucketPos] == -1) {
+            if (info.IsValid()
+                && vvTried[nKBucket][nKBucketPos] == -1) {
                 info.nRandomPos = vRandom.size();
                 info.fInTried = true;
                 vRandom.push_back(nIdCount);
@@ -419,6 +420,9 @@ class CAddrMan
             const int entry_index{bucket_entry.second};
             CAddrInfo& info = mapInfo[entry_index];
 
+            // Don't store the entry in the new bucket if it's not a valid address for our addrman
+            if (!info.IsValid()) continue;
+
             // The entry shouldn't appear in more than
             // ADDRMAN_NEW_BUCKETS_PER_ADDRESS. If it has already, just skip
             // this bucket_entry.
@@ -441,7 +445,7 @@ class CAddrMan
             }
         }
 
-        // Prune new entries with refcount 0 (as a result of collisions).
+        // Prune new entries with refcount 0 (as a result of collisions or invalid address).
         int nLostUnk = 0;
         for (auto it = mapInfo.cbegin(); it != mapInfo.cend(); ) {
             if (it->second.fInTried == false && it->second.nRefCount == 0) {
@@ -453,11 +457,9 @@ class CAddrMan
             }
         }
         if (nLost + nLostUnk > 0) {
-            LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions\n", nLostUnk, nLost);
+            LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost);
         }
 
-        RemoveInvalid();
-
         Check();
     }
 
@@ -770,9 +772,6 @@ 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;
 };