merge bitcoin#23373: Parse command line arguments from unit and fuzz …

…tests, make addrman consistency check ratio easier to change
dashpay · Sep 3, 2024 · cdcaf22 · cdcaf22
1 parent b30f0fa
commit cdcaf22
Show file tree

Hide file tree

Showing 13 changed files with 169 additions and 55 deletions.
diff --git a/doc/fuzzing.md b/doc/fuzzing.md
@@ -64,6 +64,15 @@ block^@M-^?M-^?M-^?M-^?M-^?nM-^?M-^?
 
 In this case the fuzzer managed to create a `block` message which when passed to `ProcessMessage(...)` increased coverage.
 
+It is possible to specify `dashd` arguments to the `fuzz` executable.
+Depending on the test, they may be ignored or consumed and alter the behavior
+of the test. Just make sure to use double-dash to distinguish them from the
+fuzzer's own arguments:
+
+```sh
+$ FUZZ=address_deserialize_v2 src/test/fuzz/fuzz -runs=1 fuzz_seed_corpus/address_deserialize_v2 --checkaddrman=5 --printtoconsole=1
+```
+
 ## Fuzzing corpora
 
 The project's collection of seed corpora is found in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo.

diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp
@@ -14,6 +14,9 @@
 static constexpr size_t NUM_SOURCES = 64;
 static constexpr size_t NUM_ADDRESSES_PER_SOURCE = 256;
 
+static const std::vector<bool> EMPTY_ASMAP;
+static constexpr uint32_t ADDRMAN_CONSISTENCY_CHECK_RATIO{0};
+
 static std::vector<CAddress> g_sources;
 static std::vector<std::vector<CAddress>> g_addresses;
 
@@ -72,14 +75,14 @@ static void AddrManAdd(benchmark::Bench& bench)
     CreateAddresses();
 
     bench.run([&] {
-        AddrMan addrman{/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0};
+        AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
         AddAddressesToAddrMan(addrman);
     });
 }
 
 static void AddrManSelect(benchmark::Bench& bench)
 {
-    AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
+    AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
 
     FillAddrMan(addrman);
 
@@ -91,7 +94,7 @@ static void AddrManSelect(benchmark::Bench& bench)
 
 static void AddrManGetAddr(benchmark::Bench& bench)
 {
-    AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
+    AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
 
     FillAddrMan(addrman);
 
@@ -120,7 +123,7 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
         //
         // This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in
         // AddrMan::Good() will still be noticeable.
-        AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
+        AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
         AddAddressesToAddrMan(addrman);
 
         markSomeAsGood(addrman);

diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp
@@ -16,6 +16,8 @@
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
+const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{};
+
 namespace {
 
 void GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>& benchmarkResults, const fs::path& file, const char* tpl)

diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp
@@ -23,6 +23,7 @@
 #include <QApplication>
 #include <QObject>
 #include <QTest>
+#include <functional>
 
 #if defined(QT_STATICPLUGIN)
 #include <QtPlugin>
@@ -40,6 +41,8 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin);
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
+const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{};
+
 // This is all you need to run all the tests
 int main(int argc, char* argv[])
 {

diff --git a/src/test/README.md b/src/test/README.md
@@ -33,19 +33,31 @@ the `src/qt/test/test_main.cpp` file.
 
 ### Running individual tests
 
-`test_dash` has some built-in command-line arguments; for
-example, to run just the `getarg_tests` verbosely:
+`test_dash` accepts the command line arguments from the boost framework.
+For example, to run just the `getarg_tests` suite of tests:
 
-    test_dash --log_level=all --run_test=getarg_tests -- DEBUG_LOG_OUT
+```bash
+test_dash --log_level=all --run_test=getarg_tests
+```
 
 `log_level` controls the verbosity of the test framework, which logs when a
-test case is entered, for example. The `DEBUG_LOG_OUT` after the two dashes
-redirects the debug log, which would normally go to a file in the test datadir
+test case is entered, for example. `test_dash` also accepts the command
+line arguments accepted by `dashd`. Use `--` to separate both types of
+arguments:
+
+```bash
+test_dash --log_level=all --run_test=getarg_tests -- -printtoconsole=1
+```
+
+The `-printtoconsole=1` after the two dashes redirects the debug log, which
+would normally go to a file in the test datadir
 (`BasicTestingSetup::m_path_root`), to the standard terminal output.
 
 ... or to run just the doubledash test:
 
-    test_dash --run_test=getarg_tests/doubledash
+```bash
+test_dash --run_test=getarg_tests/doubledash
+```
 
 Run `test_dash --help` for the full list.
 
@@ -68,7 +80,7 @@ on failure. For running individual tests verbosely, refer to the section
 To write to logs from unit tests you need to use specific message methods
 provided by Boost. The simplest is `BOOST_TEST_MESSAGE`.
 
-For debugging you can launch the `test_dash` executable with `gdb`or `lldb` and
+For debugging you can launch the `test_dash` executable with `gdb` or `lldb` and
 start debugging, just like you would with any other program:
 
 ```bash
@@ -95,7 +107,7 @@ Running the tests and hitting a segmentation fault should now produce a file cal
 `/proc/sys/kernel/core_pattern`).
 
 You can then explore the core dump using
-``` bash
+```bash
 gdb src/test/test_dash core
 
 (gbd) bt  # produce a backtrace for where a segfault occurred

diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
@@ -9,6 +9,7 @@
 #include <clientversion.h>
 #include <hash.h>
 #include <netbase.h>
+#include <node/context.h>
 #include <random.h>
 #include <test/data/asmap.raw.h>
 #include <test/util/setup_common.h>
@@ -20,6 +21,14 @@
 
 using namespace std::literals;
 
+static const std::vector<bool> EMPTY_ASMAP;
+static const bool DETERMINISTIC{true};
+
+static int32_t GetCheckRatio(const NodeContext& node_ctx)
+{
+    return std::clamp<int32_t>(node_ctx.args->GetArg("-checkaddrman", 100), 0, 1000000);
+}
+
 static CNetAddr ResolveIP(const std::string& ip)
 {
     CNetAddr addr;
@@ -47,17 +56,11 @@ static std::vector<bool> FromBytes(const unsigned char* source, int vector_size)
     return result;
 }
 
-/* Utility function to create a deterministic addrman, as used in most tests */
-static std::unique_ptr<AddrMan> TestAddrMan(std::vector<bool> asmap = std::vector<bool>())
-{
-    return std::make_unique<AddrMan>(asmap, /*deterministic=*/true, /*consistency_check_ratio=*/100);
-}
-
 BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(addrman_simple)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     CNetAddr source = ResolveIP("252.2.2.2");
 
@@ -91,7 +94,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
     BOOST_CHECK(addrman->size() >= 1);
 
     // Test: reset addrman and test AddrMan::Add multiple addresses works as expected
-    addrman = TestAddrMan();
+    addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
     std::vector<CAddress> vAddr;
     vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE));
     vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE));
@@ -101,7 +104,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
 
 BOOST_AUTO_TEST_CASE(addrman_ports)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     CNetAddr source = ResolveIP("252.2.2.2");
 
@@ -129,7 +132,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
 
 BOOST_AUTO_TEST_CASE(addrman_select)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     CNetAddr source = ResolveIP("252.2.2.2");
 
@@ -188,7 +191,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
 
 BOOST_AUTO_TEST_CASE(addrman_new_collisions)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     CNetAddr source = ResolveIP("252.2.2.2");
 
@@ -217,7 +220,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
 
 BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
     CAddress addr{CAddress(ResolveService("253.3.3.3", 8333), NODE_NONE)};
     int64_t start_time{GetAdjustedTime()};
     addr.nTime = start_time;
@@ -249,7 +252,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
 
 BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     CNetAddr source = ResolveIP("252.2.2.2");
 
@@ -280,7 +283,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
 
 BOOST_AUTO_TEST_CASE(addrman_getaddr)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     // Test: Sanity check, GetAddr should never return anything if addrman
     //  is empty.
@@ -601,9 +604,11 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
 {
     std::vector<bool> asmap1 = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8);
 
-    auto addrman_asmap1 = TestAddrMan(asmap1);
-    auto addrman_asmap1_dup = TestAddrMan(asmap1);
-    auto addrman_noasmap = TestAddrMan();
+    const auto ratio = GetCheckRatio(m_node);
+    auto addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
+    auto addrman_asmap1_dup = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
+    auto addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
+
     CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
 
     CAddress addr = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
@@ -631,8 +636,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     BOOST_CHECK(addr_pos1.position != addr_pos3.position);
 
     // deserializing non-asmaped peers.dat to asmaped addrman
-    addrman_asmap1 = TestAddrMan(asmap1);
-    addrman_noasmap = TestAddrMan();
+    addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
+    addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
     addrman_noasmap->Add({addr}, default_source);
     stream << *addrman_noasmap;
     stream >> *addrman_asmap1;
@@ -643,8 +648,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     BOOST_CHECK(addr_pos4 == addr_pos2);
 
     // used to map to different buckets, now maps to the same bucket.
-    addrman_asmap1 = TestAddrMan(asmap1);
-    addrman_noasmap = TestAddrMan();
+    addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
+    addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
     CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
     CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE);
     addrman_noasmap->Add({addr, addr2}, default_source);
@@ -663,7 +668,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
 {
     // Confirm that invalid addresses are ignored in unserialization.
 
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
     CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
 
     const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE};
@@ -695,14 +700,14 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
     BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size());
     memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement));
 
-    addrman = TestAddrMan();
+    addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
     stream >> *addrman;
     BOOST_CHECK_EQUAL(addrman->size(), 2);
 }
 
 BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     BOOST_CHECK(addrman->size() == 0);
 
@@ -735,7 +740,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
 
 BOOST_AUTO_TEST_CASE(addrman_noevict)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     // Add 35 addresses.
     CNetAddr source = ResolveIP("252.2.2.2");
@@ -787,7 +792,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
 
 BOOST_AUTO_TEST_CASE(addrman_evictionworks)
 {
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
 
     BOOST_CHECK(addrman->size() == 0);
 
@@ -857,8 +862,7 @@ static CDataStream AddrmanToStream(const AddrMan& addrman)
 
 BOOST_AUTO_TEST_CASE(load_addrman)
 {
-    AddrMan addrman{/*asmap=*/ std::vector<bool>(), /*deterministic=*/ true,
-                    /*consistency_check_ratio=*/ 100};
+    AddrMan addrman{EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)};
 
     CService addr1, addr2, addr3;
     BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
@@ -877,7 +881,7 @@ BOOST_AUTO_TEST_CASE(load_addrman)
     // Test that the de-serialization does not throw an exception.
     CDataStream ssPeers1 = AddrmanToStream(addrman);
     bool exceptionThrown = false;
-    AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
+    AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
 
     BOOST_CHECK(addrman1.size() == 0);
     try {
@@ -894,7 +898,7 @@ BOOST_AUTO_TEST_CASE(load_addrman)
     // Test that ReadFromStream creates an addrman with the correct number of addrs.
     CDataStream ssPeers2 = AddrmanToStream(addrman);
 
-    AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
+    AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
     BOOST_CHECK(addrman2.size() == 0);
     ReadFromStream(addrman2, ssPeers2);
     BOOST_CHECK(addrman2.size() == 3);
@@ -932,7 +936,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
     // Test that the de-serialization of corrupted peers.dat throws an exception.
     CDataStream ssPeers1 = MakeCorruptPeersDat();
     bool exceptionThrown = false;
-    AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
+    AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
     BOOST_CHECK(addrman1.size() == 0);
     try {
         unsigned char pchMsgTmp[4];
@@ -948,15 +952,15 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
     // Test that ReadFromStream fails if peers.dat is corrupt
     CDataStream ssPeers2 = MakeCorruptPeersDat();
 
-    AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
+    AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
     BOOST_CHECK(addrman2.size() == 0);
     BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure);
 }
 
 BOOST_AUTO_TEST_CASE(addrman_update_address)
 {
     // Tests updating nTime via Connected() and nServices via SetServices()
-    auto addrman = TestAddrMan();
+    auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
     CNetAddr source{ResolveIP("252.2.2.2")};
     CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)};