Merge bitcoin/bitcoin#22497: scripted-diff: remove ResetI2PPorts() (revert e0a2b390c14)

d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 scripted-diff: remove ResetI2PPorts() (revert e0a2b390c14) (Vasil Dimov)

Pull request description:

  `CAddrMan::ResetI2PPorts()` was temporary. Remove it:
  * it has partially achieved its goal: probably ran on about half of the
    I2P nodes
  * it is hackish, deemed risky and two bugs where found in it:
    https://github.com/bitcoin/bitcoin/issues/22467
    https://github.com/bitcoin/bitcoin/issues/22470

  -BEGIN VERIFY SCRIPT-
  git show e0a2b390c144e123e2fc8a289fdff36815476964 |git apply -R
  -END VERIFY SCRIPT-

  Fixes https://github.com/bitcoin/bitcoin/issues/22467
  Fixes https://github.com/bitcoin/bitcoin/issues/22470

ACKs for top commit:
  laanwj:
    ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678
  MarcoFalke:
    review ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 😲
  jonatack:
    ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 per IRC discussions https://www.erisian.com.au/bitcoin-core-dev/log-2021-07-16.html#l-212 and https://www.erisian.com.au/bitcoin-core-dev/log-2021-07-19.html#l-210

Tree-SHA512: 60d8f0ea0f66a8fcedfcb9c8944a419b974b15509b54ddfeec58db49ae9418e6916df712bba3fbd6b29497d85f7951fb9aa2e48eb9c59f88d09435685bd00b4c

3 files changed, 0 insertions, 225 deletions

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 389d106164..8192b4eba6 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -6,7 +6,6 @@
 #include <addrman.h>
 
 #include <hash.h>
-#include <i2p.h>
 #include <logging.h>
 #include <netaddress.h>
 #include <serialize.h>
@@ -732,100 +731,3 @@ std::vector<bool> CAddrMan::DecodeAsmap(fs::path path)
     }
     return bits;
 }
-
-void CAddrMan::ResetI2PPorts()
-{
-    for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) {
-        for (int i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
-            const auto id = vvNew[bucket][i];
-            if (id == -1) {
-                continue;
-            }
-            auto it = mapInfo.find(id);
-            if (it == mapInfo.end()) {
-                return;
-            }
-            auto& addr_info = it->second;
-            if (!addr_info.IsI2P() || addr_info.GetPort() == I2P_SAM31_PORT) {
-                continue;
-            }
-
-            auto addr_info_newport = addr_info;
-            // The below changes addr_info_newport.GetKey(), which is used in finding a
-            // bucket and a position within that bucket. So a re-bucketing may be necessary.
-            addr_info_newport.port = I2P_SAM31_PORT;
-
-            // Reposition entries of vvNew within the same bucket because we don't know the source
-            // address which led to the decision to store the entry in vvNew[bucket] so we can't
-            // re-evaluate that decision, but even if we could, CAddrInfo::GetNewBucket() does not
-            // use CAddrInfo::GetKey() so it would end up in the same bucket as before the port
-            // change.
-            const auto i_target = addr_info_newport.GetBucketPosition(nKey, true, bucket);
-
-            if (i_target == i) { // No need to re-position.
-                addr_info = addr_info_newport;
-                continue;
-            }
-
-            // Reposition from i to i_target, removing the entry from i_target (if any).
-            ClearNew(bucket, i_target);
-            vvNew[bucket][i_target] = id;
-            vvNew[bucket][i] = -1;
-            addr_info = addr_info_newport;
-        }
-    }
-
-    for (int bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; ++bucket) {
-        for (int i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
-            const auto id = vvTried[bucket][i];
-            if (id == -1) {
-                continue;
-            }
-            auto it = mapInfo.find(id);
-            if (it == mapInfo.end()) {
-                return;
-            }
-            auto& addr_info = it->second;
-            if (!addr_info.IsI2P() || addr_info.GetPort() == I2P_SAM31_PORT) {
-                continue;
-            }
-
-            auto addr_info_newport = addr_info;
-            // The below changes addr_info_newport.GetKey(), which is used in finding a
-            // bucket and a position within that bucket. So a re-bucketing may be necessary.
-            addr_info_newport.port = I2P_SAM31_PORT;
-
-            const auto bucket_target = addr_info_newport.GetTriedBucket(nKey, m_asmap);
-            const auto i_target = addr_info_newport.GetBucketPosition(nKey, false, bucket_target);
-
-            if (bucket_target == bucket && i_target == i) { // No need to re-position.
-                addr_info = addr_info_newport;
-                continue;
-            }
-
-            // Reposition from (bucket, i) to (bucket_target, i_target). If the latter is
-            // occupied, then move the entry from there to vvNew.
-
-            const auto old_target_id = vvTried[bucket_target][i_target];
-            if (old_target_id != -1) {
-                CAddrInfo& old_target_info = mapInfo[old_target_id];
-
-                old_target_info.fInTried = false;
-                vvTried[bucket_target][i_target] = -1;
-                --nTried;
-
-                const auto new_bucket = old_target_info.GetNewBucket(nKey, m_asmap);
-                const auto new_bucket_i = old_target_info.GetBucketPosition(nKey, true, new_bucket);
-                ClearNew(new_bucket, new_bucket_i);
-
-                old_target_info.nRefCount = 1;
-                vvNew[new_bucket][new_bucket_i] = old_target_id;
-                ++nNew;
-            }
-
-            vvTried[bucket_target][i_target] = id;
-            vvTried[bucket][i] = -1;
-            addr_info = addr_info_newport;
-        }
-    }
-}
diff --git a/src/addrman.h b/src/addrman.h
index 5de90653c1..6f081b8dc1 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -458,8 +458,6 @@ public:
 
         RemoveInvalid();
 
-        ResetI2PPorts();
-
         Check();
     }
 
@@ -775,14 +773,6 @@ private:
     //! Remove invalid addresses.
     void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    /**
-     * Reset the ports of I2P peers to 0.
-     * This is needed as a temporary measure because now we enforce port 0 and
-     * only connect to I2P hosts if the port is 0, but in the early days some
-     * I2P addresses with port 8333 were rumoured and persisted into addrmans.
-     */
-    void ResetI2PPorts() EXCLUSIVE_LOCKS_REQUIRED(cs);
-
     friend class CAddrManTest;
 };
 
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index f2eed956cd..1103292c1a 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -2,7 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 #include <addrman.h>
-#include <i2p.h>
 #include <test/data/asmap.raw.h>
 #include <test/util/setup_common.h>
 #include <util/asmap.h>
@@ -967,121 +966,5 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
 }
 
-BOOST_AUTO_TEST_CASE(reset_i2p_ports)
-{
-    CAddrManTest addrman1;
-    CAddrManTest addrman2;
-    const uint32_t good_time{static_cast<uint32_t>(GetAdjustedTime())};
-    constexpr uint16_t port = 8333;
-
-    // Has its port changed, will be re-positioned within the same bucket in vvNew.
-    const CAddress i2p_new1{
-        ResolveService("72l3ucjkuscrbiiepoehuwqgknyzgo7zuix5ty4puwrkyhtmnsga.b32.i2p", port),
-        NODE_NONE,
-        good_time};
-
-    // Has its port changed, will not be re-positioned in vvNew because ports 0 and 10075 result in
-    // the same bucket position.
-    const CAddress i2p_new2{
-        ResolveService("gehtac45oaghz54ypyopim64mql7oad2bqclla74l6tfeolzmodq.b32.i2p", 10075),
-        NODE_NONE,
-        good_time};
-
-    // Remains unchanged, port is already as it should be.
-    const CAddress i2p_new3{
-        ResolveService("c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p",
-                       I2P_SAM31_PORT),
-        NODE_NONE,
-        good_time};
-
-    // Has its port changed, re-positioning in vvNew will cause i2p_new3 to be evicted.
-    const CAddress i2p_new4{
-        ResolveService("c4cbbkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p", port),
-        NODE_NONE,
-        good_time};
-
-    // Remains unchanged.
-    const CAddress ipv4_new{ResolveService("1.2.3.4", port), NODE_NONE, good_time};
-
-    // Has its port changed, will be re-positioned in vvTried.
-    const CAddress i2p_tried1{
-        ResolveService("h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p", port),
-        NODE_NONE,
-        good_time};
-
-    // Has its port changed, will not be re-positioned in vvTried because ports 0 and 10537
-    // result in the same position (bucket, i).
-    const CAddress i2p_tried2{
-        ResolveService("pjs7or2ctvteeo5tu4bwyrtydeuhqhvdprtujn4daxr75jpebjxa.b32.i2p", 10537),
-        NODE_NONE,
-        good_time};
-
-    // Remains unchanged, port is already as it should be.
-    const CAddress i2p_tried3{
-        ResolveService("hnbbyjpxx54623l555sta7pocy3se4sdgmuebi5k6reesz5rjp6q.b32.i2p",
-                       I2P_SAM31_PORT),
-        NODE_NONE,
-        good_time};
-
-    // Has its port changed, re-positioning in vvTried will cause i2p_tried3 to be moved to vvNew.
-    const CAddress i2p_tried4{
-        ResolveService("hna37nqr3ahkqv62cuqfwgtneekvvpnuc4i4f6yo7tpoqjswvcwa.b32.i2p", port),
-        NODE_NONE,
-        good_time};
-
-    // Remains unchanged.
-    const CAddress ipv4_tried{ResolveService("2.3.4.5", port), NODE_NONE, good_time};
-
-    const CNetAddr source;
-
-    CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
-
-    addrman1.Add(i2p_new1, source);
-    addrman1.Add(i2p_new2, source);
-    addrman1.Add(i2p_new3, source);
-    addrman1.Add(i2p_new4, source);
-    addrman1.Add(ipv4_new, source);
-
-    addrman1.Add(i2p_tried1, source);
-    addrman1.Good(i2p_tried1);
-    addrman1.Add(i2p_tried2, source);
-    addrman1.Good(i2p_tried2);
-    addrman1.Add(i2p_tried3, source);
-    addrman1.Good(i2p_tried3);
-    addrman1.Add(i2p_tried4, source);
-    addrman1.Good(i2p_tried4);
-    addrman1.Add(ipv4_tried, source);
-    addrman1.Good(ipv4_tried);
-
-    stream << addrman1;
-    stream >> addrman2;
-
-    const size_t max_addresses{0};
-    const size_t max_pct{0};
-
-    auto addresses = addrman2.GetAddr(max_addresses, max_pct, NET_I2P);
-    BOOST_REQUIRE_EQUAL(addresses.size(), 7UL);
-    std::sort(addresses.begin(), addresses.end()); // Just some deterministic order.
-    BOOST_CHECK_EQUAL(addresses[0].ToStringIP(), i2p_new4.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[0].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[1].ToStringIP(), i2p_new2.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[1].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[2].ToStringIP(), i2p_tried4.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[2].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[3].ToStringIP(), i2p_tried3.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[3].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[4].ToStringIP(), i2p_tried1.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[4].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[5].ToStringIP(), i2p_tried2.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[5].GetPort(), I2P_SAM31_PORT);
-    BOOST_CHECK_EQUAL(addresses[6].ToStringIP(), i2p_new1.ToStringIP());
-    BOOST_CHECK_EQUAL(addresses[6].GetPort(), I2P_SAM31_PORT);
-
-    addresses = addrman2.GetAddr(max_addresses, max_pct, NET_IPV4);
-    BOOST_REQUIRE_EQUAL(addresses.size(), 2UL);
-    std::sort(addresses.begin(), addresses.end()); // Just some deterministic order.
-    BOOST_CHECK_EQUAL(addresses[0].ToStringIPPort(), ipv4_new.ToStringIPPort());
-    BOOST_CHECK_EQUAL(addresses[1].ToStringIPPort(), ipv4_tried.ToStringIPPort());
-}
 
 BOOST_AUTO_TEST_SUITE_END()