Merge branch 'develop' of https://github.com/dashpay/dash into develop

.github/workflows/semantic-pull-request.yml

@@ -47,3 +47,4 @@ jobs:
             utils
             wallet
             zmq
+            stats

CONTRIBUTING.md

@@ -132,6 +132,7 @@ include:
   - *wallet* for changes to the wallet code
   - *zmq* for changes to the ZMQ APIs
   - *guix* for changes to the GUIX reproducible builds
+  - *stats* for changes to reporting of statistics
 
 Examples:
 

configure.ac

@@ -1171,13 +1171,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>
  [ AC_MSG_RESULT(no)]
 )
 
-AC_MSG_CHECKING(for getentropy)
-AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>]],
- [[ getentropy(nullptr, 32) ]])],
- [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_GETENTROPY, 1,[Define this symbol if the BSD getentropy system call is available]) ],
- [ AC_MSG_RESULT(no)]
-)
-
 AC_MSG_CHECKING(for getentropy via random.h)
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>
  #include <sys/random.h>]],

src/Makefile.test.include

@@ -77,6 +77,7 @@ BITCOIN_TESTS =\
   test/addrman_tests.cpp \
   test/amount_tests.cpp \
   test/allocator_tests.cpp \
+  test/banman_tests.cpp \
   test/base32_tests.cpp \
   test/base58_tests.cpp \
   test/base64_tests.cpp \

src/addrdb.cpp

@@ -192,7 +192,7 @@ std::optional<bilingual_str> LoadAddrman(const NetGroupManager& netgroupman, con
     const auto path_addr{gArgs.GetDataDirNet() / "peers.dat"};
     try {
         DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION);
-        LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->size(), GetTimeMillis() - nStart);
+        LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->Size(), GetTimeMillis() - nStart);
     } catch (const DbNotFoundError&) {
         // Addrman can be in an inconsistent state after failure, reset it
         addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);

src/addrman.h

@@ -110,8 +110,14 @@ class AddrMan
     template <typename Stream>
     void Unserialize(Stream& s_);
 
-    //! Return the number of (unique) addresses in all tables.
-    size_t size() const;
+    /**
+    * Return size information about addrman.
+    *
+    * @param[in] net              Select addresses only from specified network (nullopt = all)
+    * @param[in] in_new           Select addresses only from one table (true = new, false = tried, nullopt = both)
+    * @return                     Number of unique addresses that match specified options.
+    */
+    size_t Size(std::optional<Network> net = {}, std::optional<bool> in_new = {}) const;
 
     /**
      * Attempt to add one or more addresses to addrman's new table.
@@ -151,11 +157,16 @@ class AddrMan
     /**
      * Choose an address to connect to.
      *
-     * @param[in] newOnly  Whether to only select addresses from the new table.
+     * @param[in] new_only Whether to only select addresses from the new table. Passing `true` returns
+     *                     an address from the new table or an empty pair. Passing `false` will return an
+     *                     empty pair or an address from either the new or tried table (it does not
+     *                     guarantee a tried entry).
+     * @param[in] network  Select only addresses of this network (nullopt = all). Passing a network may
+     *                     slow down the search.
      * @return    CAddress The record for the selected peer.
      *            int64_t  The last time we attempted to connect to that peer.
      */
-    std::pair<CAddress, int64_t> Select(bool newOnly = false) const;
+    std::pair<CAddress, int64_t> Select(bool new_only = false, std::optional<Network> network = std::nullopt) const;
 
     /**
      * Return all or many randomly selected addresses, optionally by network.

src/addrman_impl.h

@@ -88,7 +88,7 @@ class AddrInfo : public CAddress
     }
 
     //! Calculate in which position of a bucket to store this entry.
-    int GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const;
+    int GetBucketPosition(const uint256 &nKey, bool fNew, int bucket) const;
 
     //! Determine whether the statistics about this entry are bad enough so that it can just be deleted
     bool IsTerrible(int64_t nNow = GetAdjustedTime()) const;
@@ -110,7 +110,7 @@ class AddrManImpl
     template <typename Stream>
     void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
-    size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    size_t Size(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)
         EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -125,7 +125,7 @@ class AddrManImpl
 
     std::pair<CAddress, int64_t> SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
-    std::pair<CAddress, int64_t> Select(bool newOnly) const
+    std::pair<CAddress, int64_t> Select(bool new_only, std::optional<Network> network) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
@@ -216,6 +216,14 @@ class AddrManImpl
     /** Reference to the netgroup manager. netgroupman must be constructed before addrman and destructed after. */
     const NetGroupManager& m_netgroupman;
 
+    struct NewTriedCount {
+        size_t n_new;
+        size_t n_tried;
+    };
+
+    /** Number of entries in addrman per network and new/tried table. */
+    std::unordered_map<Network, NewTriedCount> m_network_counts GUARDED_BY(cs);
+
     //! Find an entry.
     AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
@@ -244,7 +252,13 @@ class AddrManImpl
 
     void Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    std::pair<CAddress, int64_t> Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::pair<CAddress, int64_t> Select_(bool new_only, std::optional<Network> network) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+
+    /** Helper to generalize looking up an addrman entry from either table.
+     *
+     *  @return  int The nid of the entry. If the addrman position is empty or not found, returns -1.
+     * */
+    int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     std::vector<CAddress> GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
@@ -260,6 +274,8 @@ class AddrManImpl
 
     std::optional<AddressPosition> FindAddressEntry_(const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
+    size_t Size_(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+
     //! Consistency check, taking into account m_consistency_check_ratio.
     //! Will std::abort if an inconsistency is detected.
     void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);

src/bench/addrman.cpp

@@ -4,6 +4,7 @@
 
 #include <addrman.h>
 #include <bench/bench.h>
+#include <netbase.h>
 #include <netgroup.h>
 #include <random.h>
 #include <util/time.h>
@@ -93,6 +94,41 @@ static void AddrManSelect(benchmark::Bench& bench)
     });
 }
 
+// The worst case performance of the Select() function is when there is only
+// one address on the table, because it linearly searches every position of
+// several buckets before identifying the correct bucket
+static void AddrManSelectFromAlmostEmpty(benchmark::Bench& bench)
+{
+    AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
+
+    // Add one address to the new table
+    CService addr = Lookup("250.3.1.1", 8333, false).value();
+    addrman.Add({CAddress(addr, NODE_NONE)}, addr);
+
+    bench.run([&] {
+        (void)addrman.Select();
+    });
+}
+
+static void AddrManSelectByNetwork(benchmark::Bench& bench)
+{
+    AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
+
+    // add single I2P address to new table
+    CService i2p_service;
+    i2p_service.SetSpecial("udhdrtrcetjm5sxzskjyr5ztpeszydbh4dpl3pl4utgqqw2v4jna.b32.i2p");
+    CAddress i2p_address(i2p_service, NODE_NONE);
+    i2p_address.nTime = GetAdjustedTime();
+    const CNetAddr source{LookupHost("252.2.2.2", false).value()};
+    addrman.Add({i2p_address}, source);
+
+    FillAddrMan(addrman);
+
+    bench.run([&] {
+        (void)addrman.Select(/*new_only=*/false, NET_I2P);
+    });
+}
+
 static void AddrManGetAddr(benchmark::Bench& bench)
 {
     AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
@@ -133,5 +169,7 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
 
 BENCHMARK(AddrManAdd);
 BENCHMARK(AddrManSelect);
+BENCHMARK(AddrManSelectFromAlmostEmpty);
+BENCHMARK(AddrManSelectByNetwork);
 BENCHMARK(AddrManGetAddr);
 BENCHMARK(AddrManAddThenGood);

src/bitcoin-cli.cpp

@@ -52,6 +52,7 @@ static constexpr int DEFAULT_WAIT_CLIENT_TIMEOUT = 0;
 static const bool DEFAULT_NAMED=false;
 static const int CONTINUE_EXECUTION=-1;
 static constexpr int8_t UNKNOWN_NETWORK{-1};
+static constexpr std::array NETWORKS{"ipv4", "ipv6", "onion", "i2p", "cjdns"};
 
 /** Default number of blocks to generate for RPC generatetoaddress. */
 static const std::string DEFAULT_NBLOCKS = "1";
@@ -259,11 +260,10 @@ class BaseRequestHandler
 class AddrinfoRequestHandler : public BaseRequestHandler
 {
 private:
-    static constexpr std::array m_networks{"ipv4", "ipv6", "onion", "i2p"};
     int8_t NetworkStringToId(const std::string& str) const
     {
-        for (size_t i = 0; i < m_networks.size(); ++i) {
-            if (str == m_networks.at(i)) return i;
+        for (size_t i = 0; i < NETWORKS.size(); ++i) {
+            if (str == NETWORKS[i]) return i;
         }
         return UNKNOWN_NETWORK;
     }
@@ -286,7 +286,7 @@ class AddrinfoRequestHandler : public BaseRequestHandler
             throw std::runtime_error("-addrinfo requires dashd server to be running v21.0 and up");
         }
         // Count the number of peers known to our node, by network.
-        std::array<uint64_t, m_networks.size()> counts{{}};
+        std::array<uint64_t, NETWORKS.size()> counts{{}};
         for (const UniValue& node : nodes) {
             std::string network_name{node["network"].get_str()};
             const int8_t network_id{NetworkStringToId(network_name)};
@@ -296,8 +296,8 @@ class AddrinfoRequestHandler : public BaseRequestHandler
         // Prepare result to return to user.
         UniValue result{UniValue::VOBJ}, addresses{UniValue::VOBJ};
         uint64_t total{0}; // Total address count
-        for (size_t i = 0; i < m_networks.size(); ++i) {
-            addresses.pushKV(m_networks.at(i), counts.at(i));
+        for (size_t i = 0; i < NETWORKS.size(); ++i) {
+            addresses.pushKV(NETWORKS[i], counts.at(i));
             total += counts.at(i);
         }
         addresses.pushKV("total", total);
@@ -384,14 +384,13 @@ class NetinfoRequestHandler : public BaseRequestHandler
 {
 private:
     static constexpr uint8_t MAX_DETAIL_LEVEL{4};
-    static constexpr std::array m_networks{"ipv4", "ipv6", "onion", "i2p"};
-    std::array<std::array<uint16_t, m_networks.size() + 1>, 3> m_counts{{{}}}; //!< Peer counts by (in/out/total, networks/total)
+    std::array<std::array<uint16_t, NETWORKS.size() + 1>, 3> m_counts{{{}}}; //!< Peer counts by (in/out/total, networks/total)
     uint8_t m_block_relay_peers_count{0};
     uint8_t m_manual_peers_count{0};
     int8_t NetworkStringToId(const std::string& str) const
     {
-        for (size_t i = 0; i < m_networks.size(); ++i) {
-            if (str == m_networks.at(i)) return i;
+        for (size_t i = 0; i < NETWORKS.size(); ++i) {
+            if (str == NETWORKS[i]) return i;
         }
         return UNKNOWN_NETWORK;
     }
@@ -493,9 +492,9 @@ class NetinfoRequestHandler : public BaseRequestHandler
             const bool is_block_relay{peer["relaytxes"].isNull() ? false : !peer["relaytxes"].get_bool()};
             const std::string conn_type{peer["connection_type"].get_str()};
             ++m_counts.at(is_outbound).at(network_id);        // in/out by network
-            ++m_counts.at(is_outbound).at(m_networks.size()); // in/out overall
+            ++m_counts.at(is_outbound).at(NETWORKS.size());   // in/out overall
             ++m_counts.at(2).at(network_id);                  // total by network
-            ++m_counts.at(2).at(m_networks.size());           // total overall
+            ++m_counts.at(2).at(NETWORKS.size());             // total overall
             if (is_block_relay) ++m_block_relay_peers_count;
             if (conn_type == "manual") ++m_manual_peers_count;
             if (DetailsRequested()) {
@@ -592,7 +591,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
             for (int8_t n : reachable_networks) {
                 result += strprintf("%8i", m_counts.at(i).at(n)); // network peers count
             }
-            result += strprintf("   %5i", m_counts.at(i).at(m_networks.size())); // total peers count
+            result += strprintf("   %5i", m_counts.at(i).at(NETWORKS.size())); // total peers count
             if (i == 1) { // the outbound row has two extra columns for block relay and manual peer counts
                 result += strprintf("   %5i", m_block_relay_peers_count);
                 if (m_manual_peers_count) result += strprintf("   %5i", m_manual_peers_count);