Skip to content

Commit cdcaf22

Browse files
kwvgkwvg
committed
merge bitcoin#23373: Parse command line arguments from unit and fuzz tests, make addrman consistency check ratio easier to change
1 parent b30f0fa commit cdcaf22

File tree

13 files changed

+169
-55
lines changed

13 files changed

+169
-55
lines changed

‎doc/fuzzing.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ block^@M-^?M-^?M-^?M-^?M-^?nM-^?M-^?
6464
6565
In this case the fuzzer managed to create a `block` message which when passed to `ProcessMessage(...)` increased coverage.
6666
67+
It is possible to specify `dashd` arguments to the `fuzz` executable.
68+
Depending on the test, they may be ignored or consumed and alter the behavior
69+
of the test. Just make sure to use double-dash to distinguish them from the
70+
fuzzer's own arguments:
71+
72+
```sh
73+
$ FUZZ=address_deserialize_v2 src/test/fuzz/fuzz -runs=1 fuzz_seed_corpus/address_deserialize_v2 --checkaddrman=5 --printtoconsole=1
74+
```
75+
6776
## Fuzzing corpora
6877
6978
The project's collection of seed corpora is found in the [`bitcoin-core/qa-assets`](https://github.com/bitcoin-core/qa-assets) repo.

‎src/bench/addrman.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
static constexpr size_t NUM_SOURCES = 64;
1515
static constexpr size_t NUM_ADDRESSES_PER_SOURCE = 256;
1616

17+
static const std::vector<bool> EMPTY_ASMAP;
18+
static constexpr uint32_t ADDRMAN_CONSISTENCY_CHECK_RATIO{0};
19+
1720
static std::vector<CAddress> g_sources;
1821
static std::vector<std::vector<CAddress>> g_addresses;
1922

@@ -72,14 +75,14 @@ static void AddrManAdd(benchmark::Bench& bench)
7275
CreateAddresses();
7376

7477
bench.run([&] {
75-
AddrMan addrman{/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0};
78+
AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
7679
AddAddressesToAddrMan(addrman);
7780
});
7881
}
7982

8083
static void AddrManSelect(benchmark::Bench& bench)
8184
{
82-
AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
85+
AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
8386

8487
FillAddrMan(addrman);
8588

@@ -91,7 +94,7 @@ static void AddrManSelect(benchmark::Bench& bench)
9194

9295
static void AddrManGetAddr(benchmark::Bench& bench)
9396
{
94-
AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
97+
AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
9598

9699
FillAddrMan(addrman);
97100

@@ -120,7 +123,7 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
120123
//
121124
// This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in
122125
// AddrMan::Good() will still be noticeable.
123-
AddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
126+
AddrMan addrman{EMPTY_ASMAP, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
124127
AddAddressesToAddrMan(addrman);
125128

126129
markSomeAsGood(addrman);

‎src/bench/bench.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
1818

19+
const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{};
20+
1921
namespace {
2022

2123
void GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>& benchmarkResults, const fs::path& file, const char* tpl)

‎src/qt/test/test_main.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <QApplication>
2424
#include <QObject>
2525
#include <QTest>
26+
#include <functional>
2627

2728
#if defined(QT_STATICPLUGIN)
2829
#include <QtPlugin>
@@ -40,6 +41,8 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin);
4041

4142
const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
4243

44+
const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{};
45+
4346
// This is all you need to run all the tests
4447
int main(int argc, char* argv[])
4548
{

‎src/test/README.md‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,31 @@ the `src/qt/test/test_main.cpp` file.
3333

3434
### Running individual tests
3535

36-
`test_dash` has some built-in command-line arguments; for
37-
example, to run just the `getarg_tests` verbosely:
36+
`test_dash` accepts the command line arguments from the boost framework.
37+
For example, to run just the `getarg_tests` suite of tests:
3838

39-
test_dash --log_level=all --run_test=getarg_tests -- DEBUG_LOG_OUT
39+
```bash
40+
test_dash --log_level=all --run_test=getarg_tests
41+
```
4042

4143
`log_level` controls the verbosity of the test framework, which logs when a
42-
test case is entered, for example. The `DEBUG_LOG_OUT` after the two dashes
43-
redirects the debug log, which would normally go to a file in the test datadir
44+
test case is entered, for example. `test_dash` also accepts the command
45+
line arguments accepted by `dashd`. Use `--` to separate both types of
46+
arguments:
47+
48+
```bash
49+
test_dash --log_level=all --run_test=getarg_tests -- -printtoconsole=1
50+
```
51+
52+
The `-printtoconsole=1` after the two dashes redirects the debug log, which
53+
would normally go to a file in the test datadir
4454
(`BasicTestingSetup::m_path_root`), to the standard terminal output.
4555

4656
... or to run just the doubledash test:
4757

48-
test_dash --run_test=getarg_tests/doubledash
58+
```bash
59+
test_dash --run_test=getarg_tests/doubledash
60+
```
4961

5062
Run `test_dash --help` for the full list.
5163

@@ -68,7 +80,7 @@ on failure. For running individual tests verbosely, refer to the section
6880
To write to logs from unit tests you need to use specific message methods
6981
provided by Boost. The simplest is `BOOST_TEST_MESSAGE`.
7082

71-
For debugging you can launch the `test_dash` executable with `gdb`or `lldb` and
83+
For debugging you can launch the `test_dash` executable with `gdb` or `lldb` and
7284
start debugging, just like you would with any other program:
7385

7486
```bash
@@ -95,7 +107,7 @@ Running the tests and hitting a segmentation fault should now produce a file cal
95107
`/proc/sys/kernel/core_pattern`).
96108

97109
You can then explore the core dump using
98-
``` bash
110+
```bash
99111
gdb src/test/test_dash core
100112

101113
(gbd) bt # produce a backtrace for where a segfault occurred

‎src/test/addrman_tests.cpp‎

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <clientversion.h>
1010
#include <hash.h>
1111
#include <netbase.h>
12+
#include <node/context.h>
1213
#include <random.h>
1314
#include <test/data/asmap.raw.h>
1415
#include <test/util/setup_common.h>
@@ -20,6 +21,14 @@
2021

2122
using namespace std::literals;
2223

24+
static const std::vector<bool> EMPTY_ASMAP;
25+
static const bool DETERMINISTIC{true};
26+
27+
static int32_t GetCheckRatio(const NodeContext& node_ctx)
28+
{
29+
return std::clamp<int32_t>(node_ctx.args->GetArg("-checkaddrman", 100), 0, 1000000);
30+
}
31+
2332
static CNetAddr ResolveIP(const std::string& ip)
2433
{
2534
CNetAddr addr;
@@ -47,17 +56,11 @@ static std::vector<bool> FromBytes(const unsigned char* source, int vector_size)
4756
return result;
4857
}
4958

50-
/* Utility function to create a deterministic addrman, as used in most tests */
51-
static std::unique_ptr<AddrMan> TestAddrMan(std::vector<bool> asmap = std::vector<bool>())
52-
{
53-
return std::make_unique<AddrMan>(asmap, /*deterministic=*/true, /*consistency_check_ratio=*/100);
54-
}
55-
5659
BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
5760

5861
BOOST_AUTO_TEST_CASE(addrman_simple)
5962
{
60-
auto addrman = TestAddrMan();
63+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
6164

6265
CNetAddr source = ResolveIP("252.2.2.2");
6366

@@ -91,7 +94,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
9194
BOOST_CHECK(addrman->size() >= 1);
9295

9396
// Test: reset addrman and test AddrMan::Add multiple addresses works as expected
94-
addrman = TestAddrMan();
97+
addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
9598
std::vector<CAddress> vAddr;
9699
vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE));
97100
vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE));
@@ -101,7 +104,7 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
101104

102105
BOOST_AUTO_TEST_CASE(addrman_ports)
103106
{
104-
auto addrman = TestAddrMan();
107+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
105108

106109
CNetAddr source = ResolveIP("252.2.2.2");
107110

@@ -129,7 +132,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
129132

130133
BOOST_AUTO_TEST_CASE(addrman_select)
131134
{
132-
auto addrman = TestAddrMan();
135+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
133136

134137
CNetAddr source = ResolveIP("252.2.2.2");
135138

@@ -188,7 +191,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
188191

189192
BOOST_AUTO_TEST_CASE(addrman_new_collisions)
190193
{
191-
auto addrman = TestAddrMan();
194+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
192195

193196
CNetAddr source = ResolveIP("252.2.2.2");
194197

@@ -217,7 +220,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)
217220

218221
BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
219222
{
220-
auto addrman = TestAddrMan();
223+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
221224
CAddress addr{CAddress(ResolveService("253.3.3.3", 8333), NODE_NONE)};
222225
int64_t start_time{GetAdjustedTime()};
223226
addr.nTime = start_time;
@@ -249,7 +252,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
249252

250253
BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
251254
{
252-
auto addrman = TestAddrMan();
255+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
253256

254257
CNetAddr source = ResolveIP("252.2.2.2");
255258

@@ -280,7 +283,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions)
280283

281284
BOOST_AUTO_TEST_CASE(addrman_getaddr)
282285
{
283-
auto addrman = TestAddrMan();
286+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
284287

285288
// Test: Sanity check, GetAddr should never return anything if addrman
286289
// is empty.
@@ -601,9 +604,11 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
601604
{
602605
std::vector<bool> asmap1 = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8);
603606

604-
auto addrman_asmap1 = TestAddrMan(asmap1);
605-
auto addrman_asmap1_dup = TestAddrMan(asmap1);
606-
auto addrman_noasmap = TestAddrMan();
607+
const auto ratio = GetCheckRatio(m_node);
608+
auto addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
609+
auto addrman_asmap1_dup = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
610+
auto addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
611+
607612
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
608613

609614
CAddress addr = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
@@ -631,8 +636,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
631636
BOOST_CHECK(addr_pos1.position != addr_pos3.position);
632637

633638
// deserializing non-asmaped peers.dat to asmaped addrman
634-
addrman_asmap1 = TestAddrMan(asmap1);
635-
addrman_noasmap = TestAddrMan();
639+
addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
640+
addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
636641
addrman_noasmap->Add({addr}, default_source);
637642
stream << *addrman_noasmap;
638643
stream >> *addrman_asmap1;
@@ -643,8 +648,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
643648
BOOST_CHECK(addr_pos4 == addr_pos2);
644649

645650
// used to map to different buckets, now maps to the same bucket.
646-
addrman_asmap1 = TestAddrMan(asmap1);
647-
addrman_noasmap = TestAddrMan();
651+
addrman_asmap1 = std::make_unique<AddrMan>(asmap1, DETERMINISTIC, ratio);
652+
addrman_noasmap = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, ratio);
648653
CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
649654
CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE);
650655
addrman_noasmap->Add({addr, addr2}, default_source);
@@ -663,7 +668,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
663668
{
664669
// Confirm that invalid addresses are ignored in unserialization.
665670

666-
auto addrman = TestAddrMan();
671+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
667672
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
668673

669674
const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE};
@@ -695,14 +700,14 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
695700
BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size());
696701
memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement));
697702

698-
addrman = TestAddrMan();
703+
addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
699704
stream >> *addrman;
700705
BOOST_CHECK_EQUAL(addrman->size(), 2);
701706
}
702707

703708
BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
704709
{
705-
auto addrman = TestAddrMan();
710+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
706711

707712
BOOST_CHECK(addrman->size() == 0);
708713

@@ -735,7 +740,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
735740

736741
BOOST_AUTO_TEST_CASE(addrman_noevict)
737742
{
738-
auto addrman = TestAddrMan();
743+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
739744

740745
// Add 35 addresses.
741746
CNetAddr source = ResolveIP("252.2.2.2");
@@ -787,7 +792,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
787792

788793
BOOST_AUTO_TEST_CASE(addrman_evictionworks)
789794
{
790-
auto addrman = TestAddrMan();
795+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
791796

792797
BOOST_CHECK(addrman->size() == 0);
793798

@@ -857,8 +862,7 @@ static CDataStream AddrmanToStream(const AddrMan& addrman)
857862

858863
BOOST_AUTO_TEST_CASE(load_addrman)
859864
{
860-
AddrMan addrman{/*asmap=*/ std::vector<bool>(), /*deterministic=*/ true,
861-
/*consistency_check_ratio=*/ 100};
865+
AddrMan addrman{EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node)};
862866

863867
CService addr1, addr2, addr3;
864868
BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
@@ -877,7 +881,7 @@ BOOST_AUTO_TEST_CASE(load_addrman)
877881
// Test that the de-serialization does not throw an exception.
878882
CDataStream ssPeers1 = AddrmanToStream(addrman);
879883
bool exceptionThrown = false;
880-
AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
884+
AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
881885

882886
BOOST_CHECK(addrman1.size() == 0);
883887
try {
@@ -894,7 +898,7 @@ BOOST_AUTO_TEST_CASE(load_addrman)
894898
// Test that ReadFromStream creates an addrman with the correct number of addrs.
895899
CDataStream ssPeers2 = AddrmanToStream(addrman);
896900

897-
AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
901+
AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
898902
BOOST_CHECK(addrman2.size() == 0);
899903
ReadFromStream(addrman2, ssPeers2);
900904
BOOST_CHECK(addrman2.size() == 3);
@@ -932,7 +936,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
932936
// Test that the de-serialization of corrupted peers.dat throws an exception.
933937
CDataStream ssPeers1 = MakeCorruptPeersDat();
934938
bool exceptionThrown = false;
935-
AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
939+
AddrMan addrman1{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
936940
BOOST_CHECK(addrman1.size() == 0);
937941
try {
938942
unsigned char pchMsgTmp[4];
@@ -948,15 +952,15 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
948952
// Test that ReadFromStream fails if peers.dat is corrupt
949953
CDataStream ssPeers2 = MakeCorruptPeersDat();
950954

951-
AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
955+
AddrMan addrman2{EMPTY_ASMAP, !DETERMINISTIC, GetCheckRatio(m_node)};
952956
BOOST_CHECK(addrman2.size() == 0);
953957
BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure);
954958
}
955959

956960
BOOST_AUTO_TEST_CASE(addrman_update_address)
957961
{
958962
// Tests updating nTime via Connected() and nServices via SetServices()
959-
auto addrman = TestAddrMan();
963+
auto addrman = std::make_unique<AddrMan>(EMPTY_ASMAP, DETERMINISTIC, GetCheckRatio(m_node));
960964
CNetAddr source{ResolveIP("252.2.2.2")};
961965
CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)};
962966

0 commit comments

Comments
 (0)