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

Merge bitcoin/bitcoin#22496: addrman: Remove addrman hotfixes

Browse Source 
65332b1178 [addrman] Remove RemoveInvalid() (John Newbery)

Pull request description:

  PRs #22179 and #22112 (EDIT: later reverted in #22497) added hotfix code to addrman to remove invalid addresses and mutate the ports of I2P entries after entering into addrman. Those hotfixes included at least two addrman data corruption bugs:

  - #22467 (Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()' failed)
  - #22470 (Changing I2P ports in addrman may wronly skip some entries from "new" buckets)

  Hotfixing addrman is inherently dangerous. There are many members that have implicit assumptions on each others' state, and mutating those directly can lead to violating addrman's internal invariants.

  Instead of trying to hotfix addrman, just don't insert any invalid addresses. For now, those are addresses which fail `CNetAddr::IsValid()`.

ACKs for top commit:
  sipa:
    utACK 65332b1178. I tried to reason through scenarios that could introduce inconsistencies with this code, but can't find any.
  fanquake:
    ACK 65332b1178 - Skipping the addition of invalid addresses (this code was initially added for Tor addrs) rather than adding all the invalids then removing them all when finishing unserializing seems like an improvement. Especially if it can be achieved with less code.

Tree-SHA512: 023113764cb475572f15da7bf9824b62b79e10a7e359af2eee59017df354348d2aeed88de0fd4ad7a9f89a0dad10827f99d70af6f1cb20abb0eca2714689c8d7
This commit is contained in:
fanquake
2021-08-03 15:33:51 +08:00
parent 71797beec5 65332b1178
commit 5cf28d5203
2 changed files with 7 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
src/addrman.cpp
View File

@@ -77,38 +77,6 @@ double CAddrInfo::GetChance(int64_t nNow) const
return fChance;
}
void CAddrMan::RemoveInvalid()
{
for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) {
for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
const auto id = vvNew[bucket][i];
if (id != -1 && !mapInfo[id].IsValid()) {
ClearNew(bucket, i);
}
}
}
for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; ++bucket) {
for (size_t i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
const auto id = vvTried[bucket][i];
if (id == -1) {
continue;
}
const auto& addr_info = mapInfo[id];
if (addr_info.IsValid()) {
continue;
}
vvTried[bucket][i] = -1;
--nTried;
SwapRandom(addr_info.nRandomPos, vRandom.size() - 1);
vRandom.pop_back();
mapAddr.erase(addr_info);
mapInfo.erase(id);
m_tried_collisions.erase(id);
}
}
}
CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
{
AssertLockHeld(cs);