Merge dashpay#6034: backport: merge bitcoin#21261, bitcoin#20877, bitcoin#21832, bitcoin#22547, bitcoin#22544, bitcoin#22959, bitcoin#23324, partial bitcoin#20764 (cli backports: part 2)

bde72a4 merge bitcoin#23324: print peer counts for all reachable networks in -netinfo (Kittywhiskers Van Gogh)
4b24544 merge bitcoin#22959: Display all proxies in -getinfo (Kittywhiskers Van Gogh)
30b0fcf merge bitcoin#22544: drop torv2; torv3 becomes onion per GetNetworkName() (Kittywhiskers Van Gogh)
b6ca36e merge bitcoin#22547: Add progress bar for -getinfo (Kittywhiskers Van Gogh)
1f89bfd merge bitcoin#21832: Implement human readable -getinfo (Kittywhiskers Van Gogh)
2200b78 merge bitcoin#20877: user help and argument parsing improvements (Kittywhiskers Van Gogh)
bd934c7 partial bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh)
b2d8656 merge bitcoin#21261: update inbound eviction protection for multiple networks, add I2P peers (Kittywhiskers Van Gogh)
0b16b50 cli: fix loop counter comparison in `ProcessReply` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for dashpay#6035

  * Dependency for dashpay#6031

  * In [dash#5904](dashpay#5904) ([bitcoin#21595](bitcoin#21595)), one of the loops in `ProcessReply` is supposed to iterate `rows.size()` times (which at the time was hardcoded to `3`), the backport erroneously set the value to `m_networks.size()` (which also evaluated to `3`) as part of increasing `m_networks.size()` usage.

    As this pull request includes [bitcoin#23324](bitcoin#23324), which changes it over to  `rows.size()`, the above has been corrected in a separate commit for documentation purposes.

  * `-addrinfo` output

    ![dash-cli addrinfo output](https://github.com/dashpay/dash/assets/63189531/24db46be-729e-4fa8-a268-87f2497cff9a)

  * `-getinfo` output (diamonds are due to rendering limitations of my terminal and are not indicative of the symbols used)

    ![dash-cli getinfo output](https://github.com/dashpay/dash/assets/63189531/626fe67f-f505-4a04-931a-76e75146e5a0)

  * `-netinfo` output

    ![dash-cli netinfo output](https://github.com/dashpay/dash/assets/63189531/afbff3d0-7127-44e2-bfe7-81b08c0e214e)

  ## Breaking Changes

  * CLI `-addrinfo` now returns a single field for the number of `onion` addresses known to the node instead of separate `torv2` and `torv3` fields, as support for TorV2 addresses was removed from Dash Core in 18.0.

  * `-getinfo` has been updated to return data in a user-friendly format that also reduces vertical space.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK bde72a4

Tree-SHA512: 921cb45b7e243a321a32c835eb23d5ba8df610ff234a548a9051436a2c21845ce70097fb9a9bb812b77b04373f9f0a9f90264168d97b08da1890be06bfd9f99c

Author: PastaPastaPasta

doc/release-notes-21832.md

@@ -0,0 +1,4 @@
+Tools and Utilities
+-------------------
+
+- Update `-getinfo` to return data in a user-friendly format that also reduces vertical space.

doc/release-notes-22544.md

@@ -0,0 +1,6 @@
+Tools and Utilities
+-------------------
+
+- CLI `-addrinfo` now returns a single field for the number of `onion` addresses
+  known to the node instead of separate `torv2` and `torv3` fields, as support
+  for Tor V2 addresses was removed from Dash Core in 18.0.

doc/tor.md

@@ -18,10 +18,9 @@ There are several ways to see your local onion address in Dash Core:
 You may set the `-debug=tor` config logging option to have additional
 information in the debug log about your Tor configuration.
 
-CLI `-addrinfo` returns the number of addresses known to your node per network
-type, including Tor v2 and v3. This is useful to see how many onion addresses
-are known to your node for `-onlynet=onion` and how many Tor v3 addresses it
-knows when upgrading to current and future Tor releases that support Tor v3 only.
+CLI `-addrinfo` returns the number of addresses known to your node per
+network. This can be useful to see how many onion peers your node knows,
+e.g. for `-onlynet=onion`.
 
 ## 1. Run Dash Core behind a Tor proxy
 

src/bitcoin-cli.cpp

@@ -62,6 +62,7 @@
 #endif
 
 #include <algorithm>
+#include <array>
 #include <cstdint>
 #include <functional>
 #include <unordered_map>
@@ -913,18 +914,6 @@ static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate& a, cons
     return a.nTimeConnected > b.nTimeConnected;
 }
 
-static bool CompareLocalHostTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
-{
-    if (a.m_is_local != b.m_is_local) return b.m_is_local;
-    return a.nTimeConnected > b.nTimeConnected;
-}
-
-static bool CompareOnionTimeConnected(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b)
-{
-    if (a.m_is_onion != b.m_is_onion) return b.m_is_onion;
-    return a.nTimeConnected > b.nTimeConnected;
-}
-
 static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) {
     return a.nKeyedNetGroup < b.nKeyedNetGroup;
 }
@@ -955,6 +944,26 @@ static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const
     return a.nTimeConnected > b.nTimeConnected;
 }
 
+/**
+ * Sort eviction candidates by network/localhost and connection uptime.
+ * Candidates near the beginning are more likely to be evicted, and those
+ * near the end are more likely to be protected, e.g. less likely to be evicted.
+ * - First, nodes that are not `is_local` and that do not belong to `network`,
+ *   sorted by increasing uptime (from most recently connected to connected longer).
+ * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime.
+ */
+struct CompareNodeNetworkTime {
+    const bool m_is_local;
+    const Network m_network;
+    CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {}
+    bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const
+    {
+        if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local;
+        if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network;
+        return a.nTimeConnected > b.nTimeConnected;
+    };
+};
+
 //! Sort an array by the specified comparator, then erase the last K elements where predicate is true.
 template <typename T, typename Comparator>
 static void EraseLastKElements(
@@ -966,40 +975,72 @@ static void EraseLastKElements(
     elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end());
 }
 
-void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates)
+void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& eviction_candidates)
 {
     // Protect the half of the remaining nodes which have been connected the longest.
     // This replicates the non-eviction implicit behavior, and precludes attacks that start later.
-    // To favorise the diversity of our peer connections, reserve up to (half + 2) of
-    // these protected spots for onion and localhost peers, if any, even if they're not
-    // longest uptime overall. This helps protect tor peers, which tend to be otherwise
+    // To favorise the diversity of our peer connections, reserve up to half of these protected
+    // spots for Tor/onion, localhost and I2P peers, even if they're not longest uptime overall.
+    // This helps protect these higher-latency peers that tend to be otherwise
     // disadvantaged under our eviction criteria.
-    const size_t initial_size = vEvictionCandidates.size();
-    size_t total_protect_size = initial_size / 2;
-    const size_t onion_protect_size = total_protect_size / 2;
-
-    if (onion_protect_size) {
-        // Pick out up to 1/4 peers connected via our onion service, sorted by longest uptime.
-        EraseLastKElements(vEvictionCandidates, CompareOnionTimeConnected, onion_protect_size,
-                           [](const NodeEvictionCandidate& n) { return n.m_is_onion; });
-    }
-
-    const size_t localhost_min_protect_size{2};
-    if (onion_protect_size >= localhost_min_protect_size) {
-        // Allocate any remaining slots of the 1/4, or minimum 2 additional slots,
-        // to localhost peers, sorted by longest uptime, as manually configured
-        // hidden services not using `-bind=addr[:port]=onion` will not be detected
-        // as inbound onion connections.
-        const size_t remaining_tor_slots{onion_protect_size - (initial_size - vEvictionCandidates.size())};
-        const size_t localhost_protect_size{std::max(remaining_tor_slots, localhost_min_protect_size)};
-        EraseLastKElements(vEvictionCandidates, CompareLocalHostTimeConnected, localhost_protect_size,
-                           [](const NodeEvictionCandidate& n) { return n.m_is_local; });
+    const size_t initial_size = eviction_candidates.size();
+    const size_t total_protect_size{initial_size / 2};
+
+    // Disadvantaged networks to protect: I2P, localhost, Tor/onion. In case of equal counts, earlier
+    // array members have first opportunity to recover unused slots from the previous iteration.
+    struct Net { bool is_local; Network id; size_t count; };
+    std::array<Net, 3> networks{
+        {{false, NET_I2P, 0}, {/* localhost */ true, NET_MAX, 0}, {false, NET_ONION, 0}}};
+
+    // Count and store the number of eviction candidates per network.
+    for (Net& n : networks) {
+        n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(),
+                                [&n](const NodeEvictionCandidate& c) {
+                                    return n.is_local ? c.m_is_local : c.m_network == n.id;
+                                });
+    }
+    // Sort `networks` by ascending candidate count, to give networks having fewer candidates
+    // the first opportunity to recover unused protected slots from the previous iteration.
+    std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; });
+
+    // Protect up to 25% of the eviction candidates by disadvantaged network.
+    const size_t max_protect_by_network{total_protect_size / 2};
+    size_t num_protected{0};
+
+    while (num_protected < max_protect_by_network) {
+        const size_t disadvantaged_to_protect{max_protect_by_network - num_protected};
+        const size_t protect_per_network{
+            std::max(disadvantaged_to_protect / networks.size(), static_cast<size_t>(1))};
+
+        // Early exit flag if there are no remaining candidates by disadvantaged network.
+        bool protected_at_least_one{false};
+
+        for (const Net& n : networks) {
+            if (n.count == 0) continue;
+            const size_t before = eviction_candidates.size();
+            EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id),
+                               protect_per_network, [&n](const NodeEvictionCandidate& c) {
+                                   return n.is_local ? c.m_is_local : c.m_network == n.id;
+                               });
+            const size_t after = eviction_candidates.size();
+            if (before > after) {
+                protected_at_least_one = true;
+                num_protected += before - after;
+                if (num_protected >= max_protect_by_network) {
+                    break;
+                }
+            }
+        }
+        if (!protected_at_least_one) {
+            break;
+        }
     }
 
     // Calculate how many we removed, and update our total number of peers that
     // we want to protect based on uptime accordingly.
-    total_protect_size -= initial_size - vEvictionCandidates.size();
-    EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, total_protect_size);
+    assert(num_protected == initial_size - eviction_candidates.size());
+    const size_t remaining_to_protect{total_protect_size - num_protected};
+    EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect);
 }
 
 [[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates)
@@ -1016,8 +1057,7 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvict
     // An attacker cannot manipulate this metric without performing useful work.
     EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4);
     // Protect up to 8 non-tx-relay peers that have sent us novel blocks.
-    const size_t erase_size = std::min(size_t(8), vEvictionCandidates.size());
-    EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, erase_size,
+    EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8,
                        [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; });
 
     // Protect 4 nodes that most recently sent us novel blocks.
@@ -1109,7 +1149,7 @@ bool CConnman::AttemptToEvictConnection()
                                                HasAllDesirableServiceFlags(node->nServices),
                                                node->m_relays_txs.load(), node->m_bloom_filter_loaded.load(),
                                                node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(),
-                                               node->m_inbound_onion};
+                                               node->ConnectedThroughNetwork()};
             vEvictionCandidates.push_back(candidate);
         }
     }

src/net.cpp

@@ -1583,7 +1583,7 @@ struct NodeEvictionCandidate
     uint64_t nKeyedNetGroup;
     bool prefer_evict;
     bool m_is_local;
-    bool m_is_onion;
+    Network m_network;
 };
 
 /**
@@ -1613,20 +1613,20 @@ size_t GetRequestedObjectCount(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
  * longest, to replicate the non-eviction implicit behavior and preclude attacks
  * that start later.
  *
- * Half of these protected spots (1/4 of the total) are reserved for onion peers
- * connected via our tor control service, if any, sorted by longest uptime, even
- * if they're not longest uptime overall. Any remaining slots of the 1/4 are
- * then allocated to protect localhost peers, if any (or up to 2 localhost peers
- * if no slots remain and 2 or more onion peers were protected), sorted by
- * longest uptime, as manually configured hidden services not using
- * `-bind=addr[:port]=onion` will not be detected as inbound onion connections.
+ * Half of these protected spots (1/4 of the total) are reserved for the
+ * following categories of peers, sorted by longest uptime, even if they're not
+ * longest uptime overall:
+ *
+ * - onion peers connected via our tor control service
+ *
+ * - localhost peers, as manually configured hidden services not using
+ *   `-bind=addr[:port]=onion` will not be detected as inbound onion connections
  *
- * This helps protect onion peers, which tend to be otherwise disadvantaged
- * under our eviction criteria for their higher min ping times relative to IPv4
- * and IPv6 peers, and favorise the diversity of peer connections.
+ * - I2P peers
  *
- * This function was extracted from SelectNodeToEvict() to be able to test the
- * ratio-based protection logic deterministically.
+ * This helps protect these privacy network peers, which tend to be otherwise
+ * disadvantaged under our eviction criteria for their higher min ping times
+ * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections.
  */
 void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates);
 

src/net.h

@@ -31,7 +31,7 @@ FUZZ_TARGET(node_eviction)
             /* nKeyedNetGroup */ fuzzed_data_provider.ConsumeIntegral<uint64_t>(),
             /* prefer_evict */ fuzzed_data_provider.ConsumeBool(),
             /* m_is_local */ fuzzed_data_provider.ConsumeBool(),
-            /* m_is_onion */ fuzzed_data_provider.ConsumeBool(),
+            /* m_network */ fuzzed_data_provider.PickValueInArray(ALL_NETWORKS),
         });
     }
     // Make a copy since eviction_candidates may be in some valid but otherwise