backport: Merge bitcoin#22217, 23025, 22362

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/bench/bench.cpp

@@ -4,15 +4,20 @@
 
 #include <bench/bench.h>
 
-#include <chainparams.h>
 #include <fs.h>
 #include <test/util/setup_common.h>
-#include <validation.h>
 
 #include <algorithm>
+#include <chrono>
 #include <fstream>
+#include <functional>
 #include <iostream>
+#include <map>
 #include <regex>
+#include <string>
+#include <vector>
+
+using namespace std::chrono_literals;
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
@@ -66,6 +71,12 @@ void benchmark::BenchRunner::RunAll(const Args& args)
 
         Bench bench;
         bench.name(p.first);
+        if (args.min_time > 0ms) {
+            // convert to nanos before dividing to reduce rounding errors
+            std::chrono::nanoseconds min_time_ns = args.min_time;
+            bench.minEpochTime(min_time_ns / bench.epochs());
+        }
+
         if (args.asymptote.empty()) {
             p.second(bench);
         } else {

src/bench/bench.h

@@ -42,11 +42,12 @@ using ankerl::nanobench::Bench;
 typedef std::function<void(Bench&)> BenchFunction;
 
 struct Args {
-    std::string regex_filter;
     bool is_list_only;
+    std::chrono::milliseconds min_time;
     std::vector<double> asymptote;
     fs::path output_csv;
     fs::path output_json;
+    std::string regex_filter;
 };
 
 class BenchRunner

src/bench/bench_bitcoin.cpp

@@ -4,23 +4,31 @@
 
 #include <bench/bench.h>
 
+#include <clientversion.h>
 #include <crypto/sha256.h>
 #include <fs.h>
 #include <stacktraces.h>
 #include <util/strencodings.h>
 #include <util/system.h>
 
 #include <bls/bls.h>
+#include <chrono>
+#include <cstdint>
+#include <iostream>
+#include <sstream>
+#include <vector>
 
 static const char* DEFAULT_BENCH_FILTER = ".*";
+static constexpr int64_t DEFAULT_MIN_TIME_MS{10};
 
 static void SetupBenchArgs(ArgsManager& argsman)
 {
     SetupHelpOptions(argsman);
 
-    argsman.AddArg("-asymptote=n1,n2,n3,...", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-asymptote=<n1,n2,n3,...>", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
+    argsman.AddArg("-min_time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS);
     argsman.AddArg("-output_csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-output_json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 }
@@ -50,16 +58,62 @@ int main(int argc, char** argv)
     }
 
     if (HelpRequested(argsman)) {
-        std::cout << argsman.GetHelpMessage();
+        std::cout << "Usage:  bench_dash [options]\n"
+                     "\n"
+                  << argsman.GetHelpMessage()
+                  << "Description:\n"
+                     "\n"
+                     "  bench_dash executes microbenchmarks. The quality of the benchmark results\n"
+                     "  highly depend on the stability of the machine. It can sometimes be difficult\n"
+                     "  to get stable, repeatable results, so here are a few tips:\n"
+                     "\n"
+                     "  * Use pyperf [1] to disable frequency scaling, turbo boost etc. For best\n"
+                     "    results, use CPU pinning and CPU isolation (see [2]).\n"
+                     "\n"
+                     "  * Each call of run() should do exactly the same work. E.g. inserting into\n"
+                     "    a std::vector doesn't do that as it will reallocate on certain calls. Make\n"
+                     "    sure each run has exactly the same preconditions.\n"
+                     "\n"
+                     "  * If results are still not reliable, increase runtime with e.g.\n"
+                     "    -min_time=5000 to let a benchmark run for at least 5 seconds.\n"
+                     "\n"
+                     "  * bench_dash uses nanobench [3] for which there is extensive\n"
+                     "    documentation available online.\n"
+                     "\n"
+                     "Environment Variables:\n"
+                     "\n"
+                     "  To attach a profiler you can run a benchmark in endless mode. This can be\n"
+                     "  done with the environment variable NANOBENCH_ENDLESS. E.g. like so:\n"
+                     "\n"
+                     "    NANOBENCH_ENDLESS=MuHash ./bench_dash -filter=MuHash\n"
+                     "\n"
+                     "  In rare cases it can be useful to suppress stability warnings. This can be\n"
+                     "  done with the environment variable NANOBENCH_SUPPRESS_WARNINGS, e.g:\n"
+                     "\n"
+                     "    NANOBENCH_SUPPRESS_WARNINGS=1 ./bench_dash\n"
+                     "\n"
+                     "Notes:\n"
+                     "\n"
+                     "  1. pyperf\n"
+                     "     https://github.com/psf/pyperf\n"
+                     "\n"
+                     "  2. CPU pinning & isolation\n"
+                     "     https://pyperf.readthedocs.io/en/latest/system.html\n"
+                     "\n"
+                     "  3. nanobench\n"
+                     "     https://github.com/martinus/nanobench\n"
+                     "\n";
+
         return EXIT_SUCCESS;
     }
 
     benchmark::Args args;
-    args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
-    args.is_list_only = argsman.GetBoolArg("-list", false);
     args.asymptote = parseAsymptote(argsman.GetArg("-asymptote", ""));
+    args.is_list_only = argsman.GetBoolArg("-list", false);
+    args.min_time = std::chrono::milliseconds(argsman.GetArg("-min_time", DEFAULT_MIN_TIME_MS));
     args.output_csv = fs::PathFromString(argsman.GetArg("-output_csv", ""));
     args.output_json = fs::PathFromString(argsman.GetArg("-output_json", ""));
+    args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
 
     benchmark::BenchRunner::RunAll(args);
 

src/bench/crypto_hash.cpp

@@ -249,9 +249,9 @@ static void MuHash(benchmark::Bench& bench)
 {
     MuHash3072 acc;
     unsigned char key[32] = {0};
-    int i = 0;
+    uint32_t i = 0;
     bench.run([&] {
-        key[0] = ++i;
+        key[0] = ++i & 0xFF;
         acc *= MuHash3072(key);
     });
 }
@@ -273,10 +273,6 @@ static void MuHashDiv(benchmark::Bench& bench)
     FastRandomContext rng(true);
     MuHash3072 muhash{rng.randbytes(32)};
 
-    for (size_t i = 0; i < bench.epochIterations(); ++i) {
-        acc *= muhash;
-    }
-
     bench.run([&] {
         acc /= muhash;
     });

src/bench/peer_eviction.cpp

@@ -20,19 +20,17 @@ static void EvictionProtectionCommon(
 {
     using Candidates = std::vector<NodeEvictionCandidate>;
     FastRandomContext random_context{true};
-    bench.warmup(100).epochIterations(1100);
 
     Candidates candidates{GetRandomNodeEvictionCandidates(num_candidates, random_context)};
     for (auto& c : candidates) {
         candidate_setup_fn(c);
     }
 
-    std::vector<Candidates> copies{
-        static_cast<size_t>(bench.epochs() * bench.epochIterations()), candidates};
-    size_t i{0};
+
     bench.run([&] {
-        ProtectEvictionCandidatesByRatio(copies.at(i));
-        ++i;
+        // creating a copy has an overhead of about 3%, so it does not influence the benchmark results much.
+        auto copy = candidates;
+        ProtectEvictionCandidatesByRatio(copy);
     });
 }
 

src/bench/rollingbloom.cpp

@@ -13,16 +13,16 @@ static void RollingBloom(benchmark::Bench& bench)
     uint32_t count = 0;
     bench.run([&] {
         count++;
-        data[0] = count;
-        data[1] = count >> 8;
-        data[2] = count >> 16;
-        data[3] = count >> 24;
+        data[0] = count & 0xFF;
+        data[1] = (count >> 8) & 0xFF;
+        data[2] = (count >> 16) & 0xFF;
+        data[3] = (count >> 24) & 0xFF;
         filter.insert(data);
 
-        data[0] = count >> 24;
-        data[1] = count >> 16;
-        data[2] = count >> 8;
-        data[3] = count;
+        data[0] = (count >> 24) & 0xFF;
+        data[1] = (count >> 16) & 0xFF;
+        data[2] = (count >> 8) & 0xFF;
+        data[3] = count & 0xFF;
         filter.contains(data);
     });
 }

src/interfaces/chain.h

@@ -285,11 +285,18 @@ class Chain
     //! Run function after given number of seconds. Cancel any previous calls with same name.
     virtual void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) = 0;
 
+    //! Get settings value.
+    virtual util::SettingsValue getSetting(const std::string& arg) = 0;
+
+    //! Get list of settings values.
+    virtual std::vector<util::SettingsValue> getSettingsList(const std::string& arg) = 0;
+
     //! Return <datadir>/settings.json setting value.
     virtual util::SettingsValue getRwSetting(const std::string& name) = 0;
 
-    //! Write a setting to <datadir>/settings.json.
-    virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0;
+    //! Write a setting to <datadir>/settings.json. Optionally just update the
+    //! setting in memory and do not write the file.
+    virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write=true) = 0;
 
     //! Synchronously send transactionAddedToMempool notifications about all
     //! current mempool transactions to the specified handler and return after

src/net_types.cpp

@@ -4,20 +4,24 @@
 
 #include <net_types.h>
 
+#include <logging.h>
 #include <netaddress.h>
 #include <netbase.h>
 #include <univalue.h>
 
+static const char* BANMAN_JSON_VERSION_KEY{"version"};
+
 CBanEntry::CBanEntry(const UniValue& json)
-    : nVersion(json["version"].get_int()), nCreateTime(json["ban_created"].get_int64()),
+    : nVersion(json[BANMAN_JSON_VERSION_KEY].get_int()),
+      nCreateTime(json["ban_created"].get_int64()),
       nBanUntil(json["banned_until"].get_int64())
 {
 }
 
 UniValue CBanEntry::ToJson() const
 {
     UniValue json(UniValue::VOBJ);
-    json.pushKV("version", nVersion);
+    json.pushKV(BANMAN_JSON_VERSION_KEY, nVersion);
     json.pushKV("ban_created", nCreateTime);
     json.pushKV("banned_until", nBanUntil);
     return json;
@@ -54,11 +58,16 @@ UniValue BanMapToJson(const banmap_t& bans)
 void BanMapFromJson(const UniValue& bans_json, banmap_t& bans)
 {
     for (const auto& ban_entry_json : bans_json.getValues()) {
+        const int version{ban_entry_json[BANMAN_JSON_VERSION_KEY].get_int()};
+        if (version != CBanEntry::CURRENT_VERSION) {
+            LogPrintf("Dropping entry with unknown version (%s) from ban list\n", version);
+            continue;
+        }
         CSubNet subnet;
         const auto& subnet_str = ban_entry_json[BANMAN_JSON_ADDR_KEY].get_str();
         if (!LookupSubNet(subnet_str, subnet)) {
-            throw std::runtime_error(
-                strprintf("Cannot parse banned address or subnet: %s", subnet_str));
+            LogPrintf("Dropping entry with unparseable address or subnet (%s) from ban list\n", subnet_str);
+            continue;
         }
         bans.insert_or_assign(subnet, CBanEntry{ban_entry_json});
     }

src/node/interfaces.cpp

@@ -971,6 +971,14 @@ class ChainImpl : public Chain
     {
         RPCRunLater(name, std::move(fn), seconds);
     }
+    util::SettingsValue getSetting(const std::string& name) override
+    {
+        return gArgs.GetSetting(name);
+    }
+    std::vector<util::SettingsValue> getSettingsList(const std::string& name) override
+    {
+        return gArgs.GetSettingsList(name);
+    }
     util::SettingsValue getRwSetting(const std::string& name) override
     {
         util::SettingsValue result;
@@ -981,7 +989,7 @@ class ChainImpl : public Chain
         });
         return result;
     }
-    bool updateRwSetting(const std::string& name, const util::SettingsValue& value) override
+    bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write) override
     {
         gArgs.LockSettings([&](util::Settings& settings) {
             if (value.isNull()) {
@@ -990,7 +998,7 @@ class ChainImpl : public Chain
                 settings.rw_settings[name] = value;
             }
         });
-        return gArgs.WriteSettingsFile();
+        return !write || gArgs.WriteSettingsFile();
     }
     void requestMempoolTransactions(Notifications& notifications) override
     {

src/test/banman_tests.cpp

@@ -0,0 +1,43 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <banman.h>
+#include <chainparams.h>
+#include <netbase.h>
+#include <streams.h>
+#include <test/util/logging.h>
+#include <test/util/setup_common.h>
+#include <util/readwritefile.h>
+
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(banman_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(file)
+{
+    SetMockTime(777s);
+    const fs::path banlist_path{m_args.GetDataDirBase() / "banlist_test"};
+    {
+        const std::string entries_write{
+            "{ \"banned_nets\": ["
+            "  { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"aaaaaaaaa\" },"
+            "  { \"version\": 2, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"bbbbbbbbb\" },"
+            "  { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"1.0.0.0/8\" }"
+            "] }",
+        };
+        assert(WriteBinaryFile(banlist_path + ".json", entries_write));
+        {
+            // The invalid entries will be dropped, but the valid one remains
+            ASSERT_DEBUG_LOG("Dropping entry with unparseable address or subnet (aaaaaaaaa) from ban list");
+            ASSERT_DEBUG_LOG("Dropping entry with unknown version (2) from ban list");
+            BanMan banman{banlist_path, /*client_interface=*/nullptr, /*default_ban_time=*/0};
+            banmap_t entries_read;
+            banman.GetBanned(entries_read);
+            assert(entries_read.size() == 1);
+        }
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/util/system.h

@@ -225,6 +225,7 @@ class ArgsManager
      */
     bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
 
+ public:
     /**
      * Get setting value.
      *
@@ -239,7 +240,6 @@ class ArgsManager
      */
     std::vector<util::SettingsValue> GetSettingsList(const std::string& arg) const;
 
-public:
     ArgsManager();
     ~ArgsManager();