Merge #6024: backport: Merge bitcoin#22496, 21985

c3f8acf Merge bitcoin#21985: net: Return IPv6 scope id in `CNetAddr::ToStringIP()` (W. J. van der Laan)
28daf0d Merge bitcoin#22496: addrman: Remove addrman hotfixes (fanquake)

Pull request description:

  bitcoin backports

Top commit has no ACKs.

Tree-SHA512: e208aee446ce5d4b29b43b9f86236b6f8ac1805572edcdf6355246c186947a5e783ecf77869525249436aa7fa2e10ae00e6b13aba7f6f04c8543e8fd382b9c8d

src/addrman.cpp

@@ -79,37 +79,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 CService& addr, int* pnId)
 {

src/addrman.h

@@ -371,7 +371,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);
@@ -425,6 +426,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.
@@ -447,7 +451,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) {
@@ -459,10 +463,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();
     }
@@ -795,8 +798,6 @@ class CAddrMan
     //! Get address info for address
     CAddrInfo GetAddressInfo_(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    //! Remove invalid addresses.
-    void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     friend class CAddrManTest;
     friend class CAddrManDeterministic;

src/netaddress.cpp

@@ -544,7 +544,7 @@ static std::string IPv4ToString(Span<const uint8_t> a)
 
 // Return an IPv6 address text representation with zero compression as described in RFC 5952
 // ("A Recommendation for IPv6 Address Text Representation").
-static std::string IPv6ToString(Span<const uint8_t> a)
+static std::string IPv6ToString(Span<const uint8_t> a, uint32_t scope_id)
 {
     assert(a.size() == ADDR_IPV6_SIZE);
     const std::array groups{
@@ -592,6 +592,10 @@ static std::string IPv6ToString(Span<const uint8_t> a)
         r += strprintf("%s%x", ((!r.empty() && r.back() != ':') ? ":" : ""), groups[i]);
     }
 
+    if (scope_id != 0) {
+        r += strprintf("%%%u", scope_id);
+    }
+
     return r;
 }
 
@@ -612,14 +616,14 @@ std::string CNetAddr::ToStringIP() const
     case NET_IPV4:
         return IPv4ToString(m_addr);
     case NET_IPV6: {
-        return IPv6ToString(m_addr);
+        return IPv6ToString(m_addr, m_scope_id);
     }
     case NET_ONION:
         return OnionToString(m_addr);
     case NET_I2P:
         return EncodeBase32(m_addr, false /* don't pad with = */) + ".b32.i2p";
     case NET_CJDNS:
-        return IPv6ToString(m_addr);
+        return IPv6ToString(m_addr, 0);
     case NET_INTERNAL:
         return EncodeBase32(m_addr) + ".internal";
     case NET_UNROUTABLE: // m_net is never and should not be set to NET_UNROUTABLE

src/test/net_tests.cpp

@@ -302,13 +302,17 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
 
     // IPv6, scoped/link-local. See https://tools.ietf.org/html/rfc4007
     // We support non-negative decimal integers (uint32_t) as zone id indices.
-    // Test with a fairly-high value, e.g. 32, to avoid locally reserved ids.
+    // Normal link-local scoped address functionality is to append "%" plus the
+    // zone id, for example, given a link-local address of "fe80::1" and a zone
+    // id of "32", return the address as "fe80::1%32".
     const std::string link_local{"fe80::1"};
     const std::string scoped_addr{link_local + "%32"};
     BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
     BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), scoped_addr);
+
     // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
     BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
     BOOST_REQUIRE(addr.IsValid());