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

Merge bitcoin/bitcoin#22791: init: Fix asmap/addrman initialization order bug

Browse Source 
724c497562 [fuzz] Add ConsumeAsmap() function (John Newbery)
5840476714 [addrman] Make m_asmap private (John Newbery)
f9002cb5db [net] Rename the copyStats arg from m_asmap to asmap (John Newbery)
f572f2b204 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery)
593247872d [net] Remove CConnMan::SetAsmap() (John Newbery)
50fd77045e [init] Read/decode asmap before constructing addrman (John Newbery)

Pull request description:

  Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat.

  The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer:

  - don't reach into `CAddrMan`'s internal data from `CConnMan`
  - set `m_asmap` in the initializer list and make it const
  - make `m_asmap` private, and access it (as a reference to const) from a getter.

  This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction.

ACKs for top commit:
  mzumsande:
    Tested ACK 724c497562
  amitiuttarwar:
    code review but utACK 724c497562
  naumenkogs:
    utACK 724c497562
  vasild:
    ACK 724c497562
  MarcoFalke:
    review ACK 724c497562 👫

Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
This commit is contained in:
MarcoFalke
2021-09-06 12:41:31 +02:00
parent 2c6707be8b 724c497562
commit a8fdfea77b
13 changed files with 92 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/addrman.h
View File

@@ -149,22 +149,6 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2};
class CAddrMan
{
public:
// Compressed IP->ASN mapping, loaded from a file when a node starts.
// Should be always empty if no file was provided.
// This mapping is then used for bucketing nodes in Addrman.
//
// If asmap is provided, nodes will be bucketed by
// AS they belong to, in order to make impossible for a node
// to connect to several nodes hosted in a single AS.
// This is done in response to Erebus attack, but also to generally
// diversify the connections every node creates,
// especially useful when a large fraction of nodes
// operate under a couple of cloud providers.
//
// If a new asmap was provided, the existing records
// would be re-bucketed accordingly.
std::vector<bool> m_asmap;
// Read asmap from provided binary file
static std::vector<bool> DecodeAsmap(fs::path path);
@@ -174,7 +158,7 @@ public:
template <typename Stream>
void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio);
explicit CAddrMan(std::vector<bool> asmap, bool deterministic, int32_t consistency_check_ratio);
~CAddrMan()
{
@@ -296,6 +280,8 @@ public:
Check();
}
const std::vector<bool>& GetAsmap() const { return m_asmap; }
private:
//! A mutex to protect the inner data structures.
mutable Mutex cs;
@@ -363,6 +349,22 @@ private:
/** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */
const int32_t m_consistency_check_ratio;
// Compressed IP->ASN mapping, loaded from a file when a node starts.
// Should be always empty if no file was provided.
// This mapping is then used for bucketing nodes in Addrman.
//
// If asmap is provided, nodes will be bucketed by
// AS they belong to, in order to make impossible for a node
// to connect to several nodes hosted in a single AS.
// This is done in response to Erebus attack, but also to generally
// diversify the connections every node creates,
// especially useful when a large fraction of nodes
// operate under a couple of cloud providers.
//
// If a new asmap was provided, the existing records
// would be re-bucketed accordingly.
const std::vector<bool> m_asmap;
//! Find an entry.
CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);