diff --git a/src/addrman.h b/src/addrman.h index a9f697f66f..455d84ca71 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -53,6 +53,7 @@ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; */ class AddrMan { +protected: const std::unique_ptr m_impl; public: @@ -135,9 +136,6 @@ public: void SetServices(const CService& addr, ServiceFlags nServices); const std::vector& GetAsmap() const; - - friend class AddrManTest; - friend class AddrManDeterministic; }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index eabc11c467..572b9871dc 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -22,80 +22,20 @@ using namespace std::literals; -class AddrManSerializationMock : public AddrMan -{ -public: - virtual void Serialize(CDataStream& s) const = 0; - - AddrManSerializationMock() - : AddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100) - {} -}; - -class AddrManUncorrupted : public AddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - AddrMan::Serialize(s); - } -}; - -class AddrManCorrupted : public AddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. - unsigned char nVersion = 1; - s << nVersion; - s << ((unsigned char)32); - s << uint256::ONE; - s << 10; // nNew - s << 10; // nTried - - int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); - s << nUBuckets; - - CService serv; - BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); - CAddress addr = CAddress(serv, NODE_NONE); - CNetAddr resolved; - BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); - AddrInfo info = AddrInfo(addr, resolved); - s << info; - } -}; - -static CDataStream AddrmanToStream(const AddrManSerializationMock& _addrman) -{ - CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); - ssPeersIn << Params().MessageStart(); - ssPeersIn << _addrman; - std::string str = ssPeersIn.str(); - std::vector vchData(str.begin(), str.end()); - return CDataStream(vchData, SER_DISK, CLIENT_VERSION); -} - class AddrManTest : public AddrMan { -private: - bool deterministic; public: - explicit AddrManTest(bool makeDeterministic = true, - std::vector asmap = std::vector()) - : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) - { - deterministic = makeDeterministic; - } + explicit AddrManTest(std::vector asmap = std::vector()) + : AddrMan(asmap, /*deterministic=*/true, /* consistency_check_ratio */ 100) + {} - AddrInfo* Find(const CService& addr, int* pnId = nullptr) + AddrInfo* Find(const CService& addr) { LOCK(m_impl->cs); - return m_impl->Find(addr, pnId); + return m_impl->Find(addr); } - AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) + AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) { LOCK(m_impl->cs); return m_impl->Create(addr, addrSource, pnId); @@ -760,8 +700,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) { std::vector asmap1 = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); - auto addrman_asmap1 = std::make_unique(true, asmap1); - auto addrman_asmap1_dup = std::make_unique(true, asmap1); + auto addrman_asmap1 = std::make_unique(asmap1); + auto addrman_asmap1_dup = std::make_unique(asmap1); auto addrman_noasmap = std::make_unique(); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); @@ -792,7 +732,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second); // deserializing non-asmaped peers.dat to asmaped addrman - addrman_asmap1 = std::make_unique(true, asmap1); + addrman_asmap1 = std::make_unique(asmap1); addrman_noasmap = std::make_unique(); addrman_noasmap->Add({addr}, default_source); stream << *addrman_noasmap; @@ -804,7 +744,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second); // used to map to different buckets, now maps to the same bucket. - addrman_asmap1 = std::make_unique(true, asmap1); + addrman_asmap1 = std::make_unique(asmap1); addrman_noasmap = std::make_unique(); CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE); @@ -1004,9 +944,20 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0"); } +static CDataStream AddrmanToStream(const AddrMan& addrman) +{ + CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); + ssPeersIn << Params().MessageStart(); + ssPeersIn << addrman; + std::string str = ssPeersIn.str(); + std::vector vchData(str.begin(), str.end()); + return CDataStream(vchData, SER_DISK, CLIENT_VERSION); +} + BOOST_AUTO_TEST_CASE(load_addrman) { - AddrManUncorrupted addrmanUncorrupted; + AddrMan addrman{/*asmap=*/ std::vector(), /*deterministic=*/ true, + /*consistency_check_ratio=*/ 100}; CService addr1, addr2, addr3; BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); @@ -1019,11 +970,11 @@ BOOST_AUTO_TEST_CASE(load_addrman) CService source; BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false)); std::vector addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)}; - BOOST_CHECK(addrmanUncorrupted.Add(addresses, source)); - BOOST_CHECK(addrmanUncorrupted.size() == 3); + BOOST_CHECK(addrman.Add(addresses, source)); + BOOST_CHECK(addrman.size() == 3); // Test that the de-serialization does not throw an exception. - CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); + CDataStream ssPeers1 = AddrmanToStream(addrman); bool exceptionThrown = false; AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); @@ -1040,7 +991,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) BOOST_CHECK(exceptionThrown == false); // Test that ReadFromStream creates an addrman with the correct number of addrs. - CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); + CDataStream ssPeers2 = AddrmanToStream(addrman); AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); @@ -1048,13 +999,39 @@ BOOST_AUTO_TEST_CASE(load_addrman) BOOST_CHECK(addrman2.size() == 3); } +// Produce a corrupt peers.dat that claims 20 addrs when it only has one addr. +static CDataStream MakeCorruptPeersDat() +{ + CDataStream s(SER_DISK, CLIENT_VERSION); + s << ::Params().MessageStart(); + + unsigned char nVersion = 1; + s << nVersion; + s << ((unsigned char)32); + s << uint256::ONE; + s << 10; // nNew + s << 10; // nTried + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + + CService serv; + BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); + CAddress addr = CAddress(serv, NODE_NONE); + CNetAddr resolved; + BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); + AddrInfo info = AddrInfo(addr, resolved); + s << info; + + std::string str = s.str(); + std::vector vchData(str.begin(), str.end()); + return CDataStream(vchData, SER_DISK, CLIENT_VERSION); +} BOOST_AUTO_TEST_CASE(load_addrman_corrupted) { - AddrManCorrupted addrmanCorrupted; - - // Test that the de-serialization of corrupted addrman throws an exception. - CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); + // Test that the de-serialization of corrupted peers.dat throws an exception. + CDataStream ssPeers1 = MakeCorruptPeersDat(); bool exceptionThrown = false; AddrMan addrman1(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman1.size() == 0); @@ -1070,7 +1047,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) BOOST_CHECK(exceptionThrown); // Test that ReadFromStream fails if peers.dat is corrupt - CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); + CDataStream ssPeers2 = MakeCorruptPeersDat(); AddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0);