highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-05-21 01:11:43 +02:00
Code Issues Packages Projects Releases Wiki Activity

[addrman] Make addrman consistency checks a runtime option

Browse Source 
Currently addrman consistency checks are a compile time option, and are not
enabled in our CI. It's unlikely anyone is running these consistency checks.

Make them a runtime option instead, where users can enable addrman
consistency checks every n operations (similar to mempool tests). Update
the addrman unit tests to do internal consistency checks every 100
operations (checking on every operations causes the test runtime to
increase by several seconds).

Also assert on a failed addrman consistency check to terminate program
execution.
This commit is contained in:
John Newbery 2020-10-23 22:03:24 +01:00
parent 10aac24145
commit a4d78546b0
11 changed files with 37 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/addrman.cpp
View File

@ -433,9 +433,12 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const
int CAddrMan::Check_() const
{
#ifdef DEBUG_ADDRMAN
AssertLockHeld(cs);
// Run consistency checks 1 in m_consistency_check_ratio times if enabled
if (m_consistency_check_ratio == 0) return 0;
if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return 0;
std::unordered_set<int> setTried;
std::unordered_map<int, int> mapNew;
@ -514,7 +517,6 @@ int CAddrMan::Check_() const
if (nKey.IsNull())
return -16;
#endif // DEBUG_ADDRMAN
return 0;
}

20
src/addrman.h
View File

@ -26,6 +26,9 @@
#include <unordered_map>
#include <vector>
/** Default for -checkaddrman */
static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0};
/**
* Extended statistics about a CAddress
*/
@ -124,8 +127,8 @@ public:
* attempt was unsuccessful.
* * Bucket selection is based on cryptographic hashing, using a randomly-generated 256-bit key, which should not
* be observable by adversaries.
* * Several indexes are kept for high performance. Defining DEBUG_ADDRMAN will introduce frequent (and expensive)
* consistency checks for the entire data structure.
* * Several indexes are kept for high performance. Setting m_consistency_check_ratio with the -checkaddrman
* configuration option will introduce (expensive) consistency checks for the entire data structure.
*/
//! total number of buckets for tried addresses
@ -493,8 +496,9 @@ public:
mapAddr.clear();
}
explicit CAddrMan(bool deterministic)
: insecure_rand{deterministic}
explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio)
: insecure_rand{deterministic},
m_consistency_check_ratio{consistency_check_ratio}
{
Clear();
if (deterministic) nKey = uint256{1};
@ -700,6 +704,9 @@ private:
//! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions.
std::set<int> m_tried_collisions;
/** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */
const int32_t m_consistency_check_ratio;
//! Find an entry.
CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
@ -737,13 +744,14 @@ private:
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Consistency check
void Check() const
EXCLUSIVE_LOCKS_REQUIRED(cs)
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs)
{
AssertLockHeld(cs);
const int err = Check_();
if (err) {
LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err);
assert(false);
}
}

8
src/bench/addrman.cpp
View File

@ -72,7 +72,7 @@ static void AddrManAdd(benchmark::Bench& bench)
{
CreateAddresses();
CAddrMan addrman(/* deterministic */ false);
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
bench.run([&] {
AddAddressesToAddrMan(addrman);
@ -82,7 +82,7 @@ static void AddrManAdd(benchmark::Bench& bench)
static void AddrManSelect(benchmark::Bench& bench)
{
CAddrMan addrman(/* deterministic */ false);
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(addrman);
@ -94,7 +94,7 @@ static void AddrManSelect(benchmark::Bench& bench)
static void AddrManGetAddr(benchmark::Bench& bench)
{
CAddrMan addrman(/* deterministic */ false);
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(addrman);
@ -116,7 +116,7 @@ static void AddrManGood(benchmark::Bench& bench)
std::vector<std::unique_ptr<CAddrMan>> addrmans(addrman_count);
for (size_t i{0}; i < addrman_count; ++i) {
addrmans[i] = std::make_unique<CAddrMan>(/* deterministic */ false);
addrmans[i] = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0);
FillAddrMan(*addrmans[i]);
}

6
src/init.cpp
View File

@ -501,7 +501,8 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-checkblocks=<n>", strprintf("How many blocks to check at startup (default: %u, 0 = all)", DEFAULT_CHECKBLOCKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checklevel=<n>", strprintf("How thorough the block verification of -checkblocks is: %s (0-4, default: %u)", Join(CHECKLEVEL_DOC, ", "), DEFAULT_CHECKLEVEL), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkmempool=<n>", strprintf("Run checks every <n> transactions (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkaddrman=<n>", strprintf("Run addrman consistency checks every <n> operations. Use 0 to disable. (default: %u)", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkmempool=<n>", strprintf("Run mempool consistency checks every <n> transactions. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-checkpoints", strprintf("Enable rejection of any forks from the known historical chain until block %s (default: %u)", defaultChainParams->Checkpoints().GetHeight(), DEFAULT_CHECKPOINTS_ENABLED), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-deprecatedrpc=<method>", "Allows deprecated RPC method(s) to be used", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", DEFAULT_STOPAFTERBLOCKIMPORT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
@ -1164,7 +1165,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)};
assert(!node.addrman);
node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false);
auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman);
assert(!node.banman);
node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
assert(!node.connman);

2
src/test/addrman_tests.cpp
View File

@ -22,7 +22,7 @@ private:
public:
explicit CAddrManTest(bool makeDeterministic = true,
std::vector<bool> asmap = std::vector<bool>())
: CAddrMan(makeDeterministic)
: CAddrMan(makeDeterministic, /* consistency_check_ratio */ 100)
{
deterministic = makeDeterministic;
m_asmap = asmap;

2
src/test/fuzz/addrman.cpp
View File

@ -29,7 +29,7 @@ public:
FuzzedDataProvider& m_fuzzed_data_provider;
explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider)
: CAddrMan(/* deterministic */ true)
: CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 0)
, m_fuzzed_data_provider(fuzzed_data_provider)
{
WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)});

2
src/test/fuzz/connman.cpp
View File

@ -25,7 +25,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
SetMockTime(ConsumeTime(fuzzed_data_provider));
CAddrMan addrman(/* deterministic */ false);
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), addrman, fuzzed_data_provider.ConsumeBool()};
CNetAddr random_netaddr;
CNode random_node = ConsumeNode(fuzzed_data_provider);

2
src/test/fuzz/data_stream.cpp
View File

@ -21,6 +21,6 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man)
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider);
CAddrMan addr_man(/* deterministic */ false);
CAddrMan addr_man(/* deterministic */ false, /* consistency_check_ratio */ 0);
CAddrDB::Read(addr_man, data_stream);
}

2
src/test/fuzz/deserialize.cpp
View File

@ -188,7 +188,7 @@ FUZZ_TARGET_DESERIALIZE(blockmerkleroot, {
BlockMerkleRoot(block, &mutated);
})
FUZZ_TARGET_DESERIALIZE(addrman_deserialize, {
CAddrMan am(/* deterministic */ false);
CAddrMan am(/* deterministic */ false, /* consistency_check_ratio */ 0);
DeserializeFromFuzzingInput(buffer, am);
})
FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, {

10
src/test/net_tests.cpp
View File

@ -35,7 +35,7 @@ public:
virtual void Serialize(CDataStream& s) const = 0;
CAddrManSerializationMock()
: CAddrMan(/* deterministic */ true)
: CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 100)
{}
};
@ -119,7 +119,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
// Test that the de-serialization does not throw an exception.
CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);
bool exceptionThrown = false;
CAddrMan addrman1(/* deterministic */ false);
CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman1.size() == 0);
try {
@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
// Test that CAddrDB::Read creates an addrman with the correct number of addrs.
CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted);
CAddrMan addrman2(/* deterministic */ false);
CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman2.size() == 0);
BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2));
BOOST_CHECK(addrman2.size() == 3);
@ -150,7 +150,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
// Test that the de-serialization of corrupted addrman throws an exception.
CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted);
bool exceptionThrown = false;
CAddrMan addrman1(/* deterministic */ false);
CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman1.size() == 0);
try {
unsigned char pchMsgTmp[4];
@ -166,7 +166,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
// Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails.
CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
CAddrMan addrman2(/* deterministic */ false);
CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman2.size() == 0);
BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2));
BOOST_CHECK(addrman2.size() == 0);

2
src/test/util/setup_common.cpp
View File

@ -193,7 +193,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString()));
}
m_node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false);
m_node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0);
m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests.
m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman,