Skip to content

Commit c4fc899

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#22950: [p2p] Pimpl AddrMan to abstract implementation details
021f869 [style] Run changed files through clang formatter. (Amiti Uttarwar) 3757503 scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar) dd8f7f2 scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar) 3c263d3 [includes] Fix up included files (Amiti Uttarwar) 29727c2 [doc] Update comments (Amiti Uttarwar) 14f9e00 [refactor] Update GetAddr_() function signature (Amiti Uttarwar) 40acd6f [move-only] Move constants to test-only header (Amiti Uttarwar) 7cf41bb [addrman] Change CAddrInfo access (Amiti Uttarwar) e3f1ea6 [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar) 7cba9d5 [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar) 8af5b54 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar) f2e5f38 [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar) 5faa7dd [move-only] Move CAddrMan function definitions to cpp (Amiti Uttarwar) Pull request description: Introduce the pimpl pattern for AddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics. Since the unit & fuzz tests currently rely on accessing AddrMan internals, this PR introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files. ACKs for top commit: jnewbery: ACK 021f869 GeneFerneau: utACK [021f869](bitcoin/bitcoin@021f869) mzumsande: ACK 021f869 rajarshimaitra: Concept + Code Review ACK bitcoin/bitcoin@021f869 theuni: ACK 021f869 Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
2 parents c79d9fb + 021f869 commit c4fc899

20 files changed

+857
-704
lines changed

src/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ endif
117117
BITCOIN_CORE_H = \
118118
addrdb.h \
119119
addrman.h \
120+
addrman_impl.h \
120121
attributes.h \
121122
banman.h \
122123
base58.h \

src/addrdb.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,21 +170,21 @@ bool CBanDB::Read(banmap_t& banSet)
170170
return true;
171171
}
172172

173-
bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr)
173+
bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr)
174174
{
175175
const auto pathAddr = args.GetDataDirNet() / "peers.dat";
176176
return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION);
177177
}
178178

179-
void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers)
179+
void ReadFromStream(AddrMan& addr, CDataStream& ssPeers)
180180
{
181181
DeserializeDB(ssPeers, addr, false);
182182
}
183183

184-
std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman)
184+
std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman)
185185
{
186186
auto check_addrman = std::clamp<int32_t>(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
187-
addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
187+
addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
188188

189189
int64_t nStart = GetTimeMillis();
190190
const auto path_addr{args.GetDataDirNet() / "peers.dat"};
@@ -193,7 +193,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A
193193
LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart);
194194
} catch (const DbNotFoundError&) {
195195
// Addrman can be in an inconsistent state after failure, reset it
196-
addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
196+
addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
197197
LogPrintf("Creating peers.dat because the file was not found (%s)\n", path_addr);
198198
DumpPeerAddresses(args, *addrman);
199199
} catch (const std::exception& e) {

src/addrdb.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@
1414
#include <vector>
1515

1616
class ArgsManager;
17-
class CAddrMan;
17+
class AddrMan;
1818
class CAddress;
1919
class CDataStream;
2020
struct bilingual_str;
2121

22-
bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr);
22+
bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr);
2323
/** Only used by tests. */
24-
void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers);
24+
void ReadFromStream(AddrMan& addr, CDataStream& ssPeers);
2525

2626
/** Access to the banlist database (banlist.json) */
2727
class CBanDB
@@ -48,7 +48,7 @@ class CBanDB
4848
};
4949

5050
/** Returns an error string on failure */
51-
std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman);
51+
std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman);
5252

5353
/**
5454
* Dump the anchor IP address database (anchors.dat)

0 commit comments

Comments
 (0)