merge bitcoin#22697: Remove CAddrMan::Clear() function

This commit is contained in:
Kittywhiskers Van Gogh 2024-05-27 11:38:47 +00:00
parent 77d8f6c918
commit d4e79aa377
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
8 changed files with 98 additions and 140 deletions

View File

@ -245,12 +245,7 @@ bool CAddrDB::Read(CAddrMan& addr)
bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)
{
bool ret = DeserializeDB(ssPeers, addr, false);
if (!ret) {
// Ensure addrman is left in a clean state
addr.Clear();
}
return ret;
return DeserializeDB(ssPeers, addr, false);
}
void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors)

View File

@ -79,6 +79,23 @@ double CAddrInfo::GetChance(int64_t nNow) const
return fChance;
}
CAddrMan::CAddrMan(bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts)
: insecure_rand{deterministic}
, nKey{deterministic ? uint256{1} : insecure_rand.rand256()}
, m_consistency_check_ratio{consistency_check_ratio}
, discriminatePorts{_discriminatePorts}
{
for (auto& bucket : vvNew) {
for (auto& entry : bucket) {
entry = -1;
}
}
for (auto& bucket : vvTried) {
for (auto& entry : bucket) {
entry = -1;
}
}
}
CAddrInfo* CAddrMan::Find(const CService& addr, int* pnId)
{

View File

@ -473,39 +473,7 @@ public:
Check();
}
void Clear()
EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
std::vector<int>().swap(vRandom);
nKey = insecure_rand.rand256();
for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) {
for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) {
vvNew[bucket][entry] = -1;
}
}
for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; bucket++) {
for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) {
vvTried[bucket][entry] = -1;
}
}
nIdCount = 0;
nTried = 0;
nNew = 0;
nLastGood = 1; //Initially at 1 so that "never" is strictly worse.
mapInfo.clear();
mapAddr.clear();
}
explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts = false) :
insecure_rand{deterministic},
m_consistency_check_ratio{consistency_check_ratio},
discriminatePorts(_discriminatePorts)
{
Clear();
if (deterministic) nKey = uint256{1};
}
explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio, bool _discriminatePorts = false);
~CAddrMan()
{
@ -638,17 +606,17 @@ public:
}
return addrRet;
}
protected:
//! secret key to randomize bucket select with
uint256 nKey;
private:
//! A mutex to protect the inner data structures.
mutable Mutex cs;
private:
//! Source of random numbers for randomization in inner loops
mutable FastRandomContext insecure_rand GUARDED_BY(cs);
//! secret key to randomize bucket select with
uint256 nKey;
//! Serialization versions.
enum Format : uint8_t {
V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88
@ -672,7 +640,7 @@ private:
static constexpr uint8_t INCOMPATIBILITY_BASE = 32;
//! last used nId
int nIdCount GUARDED_BY(cs);
int nIdCount GUARDED_BY(cs){0};
//! table with information about all nIds
std::unordered_map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
@ -686,19 +654,19 @@ private:
mutable std::vector<int> vRandom GUARDED_BY(cs);
// number of "tried" entries
int nTried GUARDED_BY(cs);
int nTried GUARDED_BY(cs){0};
//! list of "tried" buckets
int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
//! number of (unique) "new" entries
int nNew GUARDED_BY(cs);
int nNew GUARDED_BY(cs){0};
//! list of "new" buckets
int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
//! last time Good was called (memory only)
int64_t nLastGood GUARDED_BY(cs);
//! last time Good was called (memory only). Initially set to 1 so that "never" is strictly worse.
int64_t nLastGood GUARDED_BY(cs){1};
//! 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;

View File

@ -71,11 +71,9 @@ static void AddrManAdd(benchmark::Bench& bench)
{
CreateAddresses();
CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0);
bench.run([&] {
CAddrMan addrman{/* deterministic */ false, /* consistency_check_ratio */ 0};
AddAddressesToAddrMan(addrman);
addrman.Clear();
});
}

View File

@ -1666,6 +1666,20 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
assert(!node.addrman);
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);
{
// Load addresses from peers.dat
uiInterface.InitMessage(_("Loading P2P addresses…").translated);
int64_t nStart = GetTimeMillis();
CAddrDB adb;
if (adb.Read(*node.addrman)) {
LogPrintf("Loaded %i addresses from peers.dat %dms\n", node.addrman->size(), GetTimeMillis() - nStart);
} else {
// Addrman can be in an inconsistent state after failure, reset it
node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman);
LogPrintf("Recreating peers.dat\n");
adb.Write(*node.addrman);
}
}
assert(!node.banman);
node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
assert(!node.connman);

View File

@ -3383,22 +3383,6 @@ bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_met
AddAddrFetch(strDest);
}
if (clientInterface) {
clientInterface->InitMessage(_("Loading P2P addresses…").translated);
}
// Load addresses from peers.dat
int64_t nStart = GetTimeMillis();
{
CAddrDB adb;
if (adb.Read(addrman))
LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman.size(), GetTimeMillis() - nStart);
else {
addrman.Clear(); // Addrman can be in an inconsistent state after failure, reset it
LogPrintf("Recreating peers.dat\n");
DumpAddresses();
}
}
if (m_use_addrman_outgoing) {
// Load addresses from anchors.dat
m_anchors = ReadAnchors(GetDataDir() / ANCHORS_DATABASE_FILENAME);

View File

@ -46,7 +46,7 @@ public:
unsigned char nVersion = 1;
s << nVersion;
s << ((unsigned char)32);
s << nKey;
s << uint256::ONE;
s << 10; // nNew
s << 10; // nTried
@ -131,16 +131,6 @@ public:
int64_t nLastTry = GetAdjustedTime()-61;
Attempt(addr, count_failure, nLastTry);
}
void Clear()
{
CAddrMan::Clear();
if (deterministic) {
LOCK(cs);
nKey = uint256{1};
insecure_rand = FastRandomContext(true);
}
}
};
static CNetAddr ResolveIP(const std::string& ip)
@ -174,27 +164,27 @@ BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(addrman_simple)
{
CAddrManTest addrman;
auto addrman = std::make_unique<CAddrManTest>();
CNetAddr source = ResolveIP("252.2.2.2");
// Test: Does Addrman respond correctly when empty.
BOOST_CHECK_EQUAL(addrman.size(), 0U);
CAddrInfo addr_null = addrman.Select();
BOOST_CHECK_EQUAL(addrman->size(), 0U);
CAddrInfo addr_null = addrman->Select();
BOOST_CHECK_EQUAL(addr_null.ToString(), "[::]:0");
// Test: Does Addrman::Add work as expected.
CService addr1 = ResolveService("250.1.1.1", 8333);
BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source));
BOOST_CHECK_EQUAL(addrman.size(), 1U);
CAddrInfo addr_ret1 = addrman.Select();
BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source));
BOOST_CHECK_EQUAL(addrman->size(), 1U);
CAddrInfo addr_ret1 = addrman->Select();
BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
// Test: Does IP address deduplication work correctly.
// Expected dup IP should not be added.
CService addr1_dup = ResolveService("250.1.1.1", 8333);
BOOST_CHECK(!addrman.Add({CAddress(addr1_dup, NODE_NONE)}, source));
BOOST_CHECK_EQUAL(addrman.size(), 1U);
BOOST_CHECK(!addrman->Add({CAddress(addr1_dup, NODE_NONE)}, source));
BOOST_CHECK_EQUAL(addrman->size(), 1U);
// Test: New table has one addr and we add a diff addr we should
@ -204,21 +194,16 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
// success.
CService addr2 = ResolveService("250.1.1.2", 8333);
BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source));
BOOST_CHECK(addrman.size() >= 1);
BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, source));
BOOST_CHECK(addrman->size() >= 1);
// Test: AddrMan::Clear() should empty the new table.
addrman.Clear();
BOOST_CHECK_EQUAL(addrman.size(), 0U);
CAddrInfo addr_null2 = addrman.Select();
BOOST_CHECK_EQUAL(addr_null2.ToString(), "[::]:0");
// Test: AddrMan::Add multiple addresses works as expected
// Test: reset addrman and test AddrMan::Add multiple addresses works as expected
addrman = std::make_unique<CAddrManTest>();
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));
BOOST_CHECK(addrman.Add(vAddr, source));
BOOST_CHECK(addrman.size() >= 1);
BOOST_CHECK(addrman->Add(vAddr, source));
BOOST_CHECK(addrman->size() >= 1);
}
BOOST_AUTO_TEST_CASE(addrman_ports)
@ -773,23 +758,23 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
{
std::vector<bool> asmap1 = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8);
CAddrManTest addrman_asmap1(true, asmap1);
CAddrManTest addrman_asmap1_dup(true, asmap1);
CAddrManTest addrman_noasmap;
auto addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1);
auto addrman_asmap1_dup = std::make_unique<CAddrManTest>(true, asmap1);
auto addrman_noasmap = std::make_unique<CAddrManTest>();
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
CAddress addr = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
CNetAddr default_source;
addrman_asmap1.Add({addr}, default_source);
addrman_asmap1->Add({addr}, default_source);
stream << addrman_asmap1;
stream << *addrman_asmap1;
// serizalizing/deserializing addrman with the same asmap
stream >> addrman_asmap1_dup;
stream >> *addrman_asmap1_dup;
std::pair<int, int> bucketAndEntry_asmap1 = addrman_asmap1.GetBucketAndEntry(addr);
std::pair<int, int> bucketAndEntry_asmap1_dup = addrman_asmap1_dup.GetBucketAndEntry(addr);
std::pair<int, int> bucketAndEntry_asmap1 = addrman_asmap1->GetBucketAndEntry(addr);
std::pair<int, int> bucketAndEntry_asmap1_dup = addrman_asmap1_dup->GetBucketAndEntry(addr);
BOOST_CHECK(bucketAndEntry_asmap1.second != -1);
BOOST_CHECK(bucketAndEntry_asmap1_dup.second != -1);
@ -797,39 +782,39 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
BOOST_CHECK(bucketAndEntry_asmap1.second == bucketAndEntry_asmap1_dup.second);
// deserializing asmaped peers.dat to non-asmaped addrman
stream << addrman_asmap1;
stream >> addrman_noasmap;
std::pair<int, int> bucketAndEntry_noasmap = addrman_noasmap.GetBucketAndEntry(addr);
stream << *addrman_asmap1;
stream >> *addrman_noasmap;
std::pair<int, int> bucketAndEntry_noasmap = addrman_noasmap->GetBucketAndEntry(addr);
BOOST_CHECK(bucketAndEntry_noasmap.second != -1);
BOOST_CHECK(bucketAndEntry_asmap1.first != bucketAndEntry_noasmap.first);
BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second);
// deserializing non-asmaped peers.dat to asmaped addrman
addrman_asmap1.Clear();
addrman_noasmap.Clear();
addrman_noasmap.Add({addr}, default_source);
stream << addrman_noasmap;
stream >> addrman_asmap1;
std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1.GetBucketAndEntry(addr);
addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1);
addrman_noasmap = std::make_unique<CAddrManTest>();
addrman_noasmap->Add({addr}, default_source);
stream << *addrman_noasmap;
stream >> *addrman_asmap1;
std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1->GetBucketAndEntry(addr);
BOOST_CHECK(bucketAndEntry_asmap1_deser.second != -1);
BOOST_CHECK(bucketAndEntry_asmap1_deser.first != bucketAndEntry_noasmap.first);
BOOST_CHECK(bucketAndEntry_asmap1_deser.first == bucketAndEntry_asmap1_dup.first);
BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second);
// used to map to different buckets, now maps to the same bucket.
addrman_asmap1.Clear();
addrman_noasmap.Clear();
addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1);
addrman_noasmap = std::make_unique<CAddrManTest>();
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);
std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap.GetBucketAndEntry(addr1);
std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap.GetBucketAndEntry(addr2);
addrman_noasmap->Add({addr, addr2}, default_source);
std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap->GetBucketAndEntry(addr1);
std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap->GetBucketAndEntry(addr2);
BOOST_CHECK(bucketAndEntry_noasmap_addr1.first != bucketAndEntry_noasmap_addr2.first);
BOOST_CHECK(bucketAndEntry_noasmap_addr1.second != bucketAndEntry_noasmap_addr2.second);
stream << addrman_noasmap;
stream >> addrman_asmap1;
std::pair<int, int> bucketAndEntry_asmap1_deser_addr1 = addrman_asmap1.GetBucketAndEntry(addr1);
std::pair<int, int> bucketAndEntry_asmap1_deser_addr2 = addrman_asmap1.GetBucketAndEntry(addr2);
stream << *addrman_noasmap;
stream >> *addrman_asmap1;
std::pair<int, int> bucketAndEntry_asmap1_deser_addr1 = addrman_asmap1->GetBucketAndEntry(addr1);
std::pair<int, int> bucketAndEntry_asmap1_deser_addr2 = addrman_asmap1->GetBucketAndEntry(addr2);
BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.first == bucketAndEntry_asmap1_deser_addr2.first);
BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.second != bucketAndEntry_asmap1_deser_addr2.second);
}
@ -838,7 +823,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
{
// Confirm that invalid addresses are ignored in unserialization.
CAddrManTest addrman;
auto addrman = std::make_unique<CAddrManTest>();
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE};
@ -846,12 +831,12 @@ BOOST_AUTO_TEST_CASE(remove_invalid)
const CAddress tried1{ResolveService("7.7.7.7"), NODE_NONE};
const CAddress tried2{ResolveService("8.8.8.8"), NODE_NONE};
addrman.Add({new1, tried1, new2, tried2}, CNetAddr{});
addrman.Good(tried1);
addrman.Good(tried2);
BOOST_REQUIRE_EQUAL(addrman.size(), 4);
addrman->Add({new1, tried1, new2, tried2}, CNetAddr{});
addrman->Good(tried1);
addrman->Good(tried2);
BOOST_REQUIRE_EQUAL(addrman->size(), 4);
stream << addrman;
stream << *addrman;
const std::string str{stream.str()};
size_t pos;
@ -870,9 +855,9 @@ 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.Clear();
stream >> addrman;
BOOST_CHECK_EQUAL(addrman.size(), 2);
addrman = std::make_unique<CAddrManTest>();
stream >> *addrman;
BOOST_CHECK_EQUAL(addrman->size(), 2);
}
BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
@ -1083,13 +1068,12 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
BOOST_CHECK(addrman1.size() == 1);
BOOST_CHECK(exceptionThrown);
// Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails.
// Test that CAddrDB::Read fails if peers.dat is corrupt
CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100);
BOOST_CHECK(addrman2.size() == 0);
BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2));
BOOST_CHECK(addrman2.size() == 0);
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -228,24 +228,22 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
SetMockTime(ConsumeTime(fuzzed_data_provider));
CAddrManDeterministic addr_man{fuzzed_data_provider};
auto addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider);
if (fuzzed_data_provider.ConsumeBool()) {
const std::vector<uint8_t> serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)};
CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION);
const auto ser_version{fuzzed_data_provider.ConsumeIntegral<int32_t>()};
ds.SetVersion(ser_version);
try {
ds >> addr_man;
ds >> *addr_man_ptr;
} catch (const std::ios_base::failure&) {
addr_man.Clear();
addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider);
}
}
CAddrManDeterministic& addr_man = *addr_man_ptr;
while (fuzzed_data_provider.ConsumeBool()) {
CallOneOf(
fuzzed_data_provider,
[&] {
addr_man.Clear();
},
[&] {
addr_man.ResolveCollisions();
},