aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/addrman.cpp18
-rw-r--r--src/addrman_impl.h6
-rw-r--r--src/netaddress.h36
-rw-r--r--src/test/addrman_tests.cpp22
-rw-r--r--src/test/fuzz/addrman.cpp33
5 files changed, 53 insertions, 62 deletions
diff --git a/src/addrman.cpp b/src/addrman.cpp
index cf6e847b04..86f56052c1 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -395,7 +395,7 @@ void AddrManImpl::Unserialize(Stream& s_)
}
}
-AddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId)
+AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId)
{
AssertLockHeld(cs);
@@ -553,10 +553,6 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT
AddrInfo& info = *pinfo;
- // check whether we are talking about the exact same CService (including same port)
- if (info != addr)
- return;
-
// update info
info.nLastSuccess = nTime;
info.nLastTry = nTime;
@@ -685,10 +681,6 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi
AddrInfo& info = *pinfo;
- // check whether we are talking about the exact same CService (including same port)
- if (info != addr)
- return;
-
// update info
info.nLastTry = nTime;
if (fCountFailure && info.nLastCountAttempt < nLastGood) {
@@ -816,10 +808,6 @@ void AddrManImpl::Connected_(const CService& addr, int64_t nTime)
AddrInfo& info = *pinfo;
- // check whether we are talking about the exact same CService (including same port)
- if (info != addr)
- return;
-
// update info
int64_t nUpdateInterval = 20 * 60;
if (nTime - info.nTime > nUpdateInterval)
@@ -838,10 +826,6 @@ void AddrManImpl::SetServices_(const CService& addr, ServiceFlags nServices)
AddrInfo& info = *pinfo;
- // check whether we are talking about the exact same CService (including same port)
- if (info != addr)
- return;
-
// update info
info.nServices = nServices;
}
diff --git a/src/addrman_impl.h b/src/addrman_impl.h
index 1dc7f25f9c..f8191d6b85 100644
--- a/src/addrman_impl.h
+++ b/src/addrman_impl.h
@@ -179,8 +179,8 @@ private:
//! table with information about all nIds
std::unordered_map<int, AddrInfo> mapInfo GUARDED_BY(cs);
- //! find an nId based on its network address
- std::unordered_map<CNetAddr, int, CNetAddrHash> mapAddr GUARDED_BY(cs);
+ //! find an nId based on its network address and port.
+ std::unordered_map<CService, int, CServiceHash> mapAddr GUARDED_BY(cs);
//! randomly-ordered vector of all nIds
//! This is mutable because it is unobservable outside the class, so any
@@ -225,7 +225,7 @@ private:
const std::vector<bool> m_asmap;
//! Find an entry.
- AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
+ AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom.
AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
diff --git a/src/netaddress.h b/src/netaddress.h
index 66c8c48f08..57eb8bc72f 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -253,7 +253,6 @@ public:
}
}
- friend class CNetAddrHash;
friend class CSubNet;
private:
@@ -467,22 +466,6 @@ private:
}
};
-class CNetAddrHash
-{
-public:
- size_t operator()(const CNetAddr& a) const noexcept
- {
- CSipHasher hasher(m_salt_k0, m_salt_k1);
- hasher.Write(a.m_net);
- hasher.Write(a.m_addr.data(), a.m_addr.size());
- return static_cast<size_t>(hasher.Finalize());
- }
-
-private:
- const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
- const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
-};
-
class CSubNet
{
protected:
@@ -565,6 +548,25 @@ public:
READWRITEAS(CNetAddr, obj);
READWRITE(Using<BigEndianFormatter<2>>(obj.port));
}
+
+ friend class CServiceHash;
+};
+
+class CServiceHash
+{
+public:
+ size_t operator()(const CService& a) const noexcept
+ {
+ CSipHasher hasher(m_salt_k0, m_salt_k1);
+ hasher.Write(a.m_net);
+ hasher.Write(a.port);
+ hasher.Write(a.m_addr.data(), a.m_addr.size());
+ return static_cast<size_t>(hasher.Finalize());
+ }
+
+private:
+ const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
+ const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
};
#endif // BITCOIN_NETADDRESS_H
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index bd6f470219..991bfa5efc 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -89,7 +89,7 @@ public:
deterministic = makeDeterministic;
}
- AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr)
+ AddrInfo* Find(const CService& addr, int* pnId = nullptr)
{
LOCK(m_impl->cs);
return m_impl->Find(addr, pnId);
@@ -222,15 +222,15 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
BOOST_CHECK_EQUAL(addrman.size(), 1U);
CService addr1_port = ResolveService("250.1.1.1", 8334);
- BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
- BOOST_CHECK_EQUAL(addrman.size(), 1U);
+ BOOST_CHECK(addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
+ BOOST_CHECK_EQUAL(addrman.size(), 2U);
auto addr_ret2 = addrman.Select().first;
- BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333");
+ BOOST_CHECK(addr_ret2.ToString() == "250.1.1.1:8333" || addr_ret2.ToString() == "250.1.1.1:8334");
- // Test: Add same IP but diff port to tried table, it doesn't get added.
- // Perhaps this is not ideal behavior but it is the current behavior.
+ // Test: Add same IP but diff port to tried table; this converts the entry with
+ // the specified port to tried, but not the other.
addrman.Good(CAddress(addr1_port, NODE_NONE));
- BOOST_CHECK_EQUAL(addrman.size(), 1U);
+ BOOST_CHECK_EQUAL(addrman.size(), 2U);
bool newOnly = true;
auto addr_ret3 = addrman.Select(newOnly).first;
BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333");
@@ -369,18 +369,18 @@ BOOST_AUTO_TEST_CASE(addrman_find)
CNetAddr source2 = ResolveIP("250.1.2.2");
BOOST_CHECK(addrman.Add({addr1}, source1));
- BOOST_CHECK(!addrman.Add({addr2}, source2));
+ BOOST_CHECK(addrman.Add({addr2}, source2));
BOOST_CHECK(addrman.Add({addr3}, source1));
- // Test: ensure Find returns an IP matching what we searched on.
+ // Test: ensure Find returns an IP/port matching what we searched on.
AddrInfo* info1 = addrman.Find(addr1);
BOOST_REQUIRE(info1);
BOOST_CHECK_EQUAL(info1->ToString(), "250.1.2.1:8333");
- // Test 18; Find does not discriminate by port number.
+ // Test; Find discriminates by port number.
AddrInfo* info2 = addrman.Find(addr2);
BOOST_REQUIRE(info2);
- BOOST_CHECK_EQUAL(info2->ToString(), info1->ToString());
+ BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:9999");
// Test: Find returns another IP matching what we searched on.
AddrInfo* info3 = addrman.Find(addr3);
diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp
index 8df3707fc9..c6df6a0e61 100644
--- a/src/test/fuzz/addrman.cpp
+++ b/src/test/fuzz/addrman.cpp
@@ -137,24 +137,29 @@ public:
// Check that all values in `mapInfo` are equal to all values in `other.mapInfo`.
// Keys may be different.
- using AddrInfoHasher = std::function<size_t(const AddrInfo&)>;
- using AddrInfoEq = std::function<bool(const AddrInfo&, const AddrInfo&)>;
-
- CNetAddrHash netaddr_hasher;
-
- AddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const AddrInfo& a) {
- return netaddr_hasher(static_cast<CNetAddr>(a)) ^ netaddr_hasher(a.source) ^
- a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried;
+ auto addrinfo_hasher = [](const AddrInfo& a) {
+ CSipHasher hasher(0, 0);
+ auto addr_key = a.GetKey();
+ auto source_key = a.source.GetAddrBytes();
+ hasher.Write(a.nLastSuccess);
+ hasher.Write(a.nAttempts);
+ hasher.Write(a.nRefCount);
+ hasher.Write(a.fInTried);
+ hasher.Write(a.GetNetwork());
+ hasher.Write(a.source.GetNetwork());
+ hasher.Write(addr_key.size());
+ hasher.Write(source_key.size());
+ hasher.Write(addr_key.data(), addr_key.size());
+ hasher.Write(source_key.data(), source_key.size());
+ return (size_t)hasher.Finalize();
};
- AddrInfoEq addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) {
- return static_cast<CNetAddr>(lhs) == static_cast<CNetAddr>(rhs) &&
- lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess &&
- lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount &&
- lhs.fInTried == rhs.fInTried;
+ auto addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) {
+ return std::tie(static_cast<const CService&>(lhs), lhs.source, lhs.nLastSuccess, lhs.nAttempts, lhs.nRefCount, lhs.fInTried) ==
+ std::tie(static_cast<const CService&>(rhs), rhs.source, rhs.nLastSuccess, rhs.nAttempts, rhs.nRefCount, rhs.fInTried);
};
- using Addresses = std::unordered_set<AddrInfo, AddrInfoHasher, AddrInfoEq>;
+ using Addresses = std::unordered_set<AddrInfo, decltype(addrinfo_hasher), decltype(addrinfo_eq)>;
const size_t num_addresses{m_impl->mapInfo.size()};