Skip to content

Commit 92617b7

Browse files
sipasipa
committed
Make AddrMan support multiple ports per IP
1 parent ff65b69 commit 92617b7

File tree

5 files changed

+53
-62
lines changed

5 files changed

+53
-62
lines changed

‎src/addrman.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ void AddrManImpl::Unserialize(Stream& s_)
401401
}
402402
}
403403

404-
AddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId)
404+
AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId)
405405
{
406406
AssertLockHeld(cs);
407407

@@ -556,10 +556,6 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT
556556

557557
AddrInfo& info = *pinfo;
558558

559-
// check whether we are talking about the exact same CService (including same port)
560-
if (info != addr)
561-
return;
562-
563559
// update info
564560
info.nLastSuccess = nTime;
565561
info.nLastTry = nTime;
@@ -683,10 +679,6 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi
683679

684680
AddrInfo& info = *pinfo;
685681

686-
// check whether we are talking about the exact same CService (including same port)
687-
if (info != addr)
688-
return;
689-
690682
// update info
691683
info.nLastTry = nTime;
692684
if (fCountFailure && info.nLastCountAttempt < nLastGood) {
@@ -796,10 +788,6 @@ void AddrManImpl::Connected_(const CService& addr, int64_t nTime)
796788

797789
AddrInfo& info = *pinfo;
798790

799-
// check whether we are talking about the exact same CService (including same port)
800-
if (info != addr)
801-
return;
802-
803791
// update info
804792
int64_t nUpdateInterval = 20 * 60;
805793
if (nTime - info.nTime > nUpdateInterval)
@@ -818,10 +806,6 @@ void AddrManImpl::SetServices_(const CService& addr, ServiceFlags nServices)
818806

819807
AddrInfo& info = *pinfo;
820808

821-
// check whether we are talking about the exact same CService (including same port)
822-
if (info != addr)
823-
return;
824-
825809
// update info
826810
info.nServices = nServices;
827811
}

‎src/addrman_impl.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ class AddrManImpl
179179
//! table with information about all nIds
180180
std::unordered_map<int, AddrInfo> mapInfo GUARDED_BY(cs);
181181

182-
//! find an nId based on its network address
183-
std::unordered_map<CNetAddr, int, CNetAddrHash> mapAddr GUARDED_BY(cs);
182+
//! find an nId based on its network address and port.
183+
std::unordered_map<CService, int, CServiceHash> mapAddr GUARDED_BY(cs);
184184

185185
//! randomly-ordered vector of all nIds
186186
//! This is mutable because it is unobservable outside the class, so any
@@ -225,7 +225,7 @@ class AddrManImpl
225225
const std::vector<bool> m_asmap;
226226

227227
//! Find an entry.
228-
AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
228+
AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
229229

230230
//! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom.
231231
AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);

‎src/netaddress.h

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ class CNetAddr
253253
}
254254
}
255255

256-
friend class CNetAddrHash;
257256
friend class CSubNet;
258257

259258
private:
@@ -467,22 +466,6 @@ class CNetAddr
467466
}
468467
};
469468

470-
class CNetAddrHash
471-
{
472-
public:
473-
size_t operator()(const CNetAddr& a) const noexcept
474-
{
475-
CSipHasher hasher(m_salt_k0, m_salt_k1);
476-
hasher.Write(a.m_net);
477-
hasher.Write(a.m_addr.data(), a.m_addr.size());
478-
return static_cast<size_t>(hasher.Finalize());
479-
}
480-
481-
private:
482-
const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
483-
const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
484-
};
485-
486469
class CSubNet
487470
{
488471
protected:
@@ -565,6 +548,25 @@ class CService : public CNetAddr
565548
READWRITEAS(CNetAddr, obj);
566549
READWRITE(Using<BigEndianFormatter<2>>(obj.port));
567550
}
551+
552+
friend class CServiceHash;
553+
};
554+
555+
class CServiceHash
556+
{
557+
public:
558+
size_t operator()(const CService& a) const noexcept
559+
{
560+
CSipHasher hasher(m_salt_k0, m_salt_k1);
561+
hasher.Write(a.m_net);
562+
hasher.Write(a.port);
563+
hasher.Write(a.m_addr.data(), a.m_addr.size());
564+
return static_cast<size_t>(hasher.Finalize());
565+
}
566+
567+
private:
568+
const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
569+
const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
568570
};
569571

570572
#endif // BITCOIN_NETADDRESS_H

‎src/test/addrman_tests.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class AddrManTest : public AddrMan
8989
deterministic = makeDeterministic;
9090
}
9191

92-
AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr)
92+
AddrInfo* Find(const CService& addr, int* pnId = nullptr)
9393
{
9494
LOCK(m_impl->cs);
9595
return m_impl->Find(addr, pnId);
@@ -222,15 +222,15 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
222222
BOOST_CHECK_EQUAL(addrman.size(), 1U);
223223

224224
CService addr1_port = ResolveService("250.1.1.1", 8334);
225-
BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
226-
BOOST_CHECK_EQUAL(addrman.size(), 1U);
225+
BOOST_CHECK(addrman.Add({CAddress(addr1_port, NODE_NONE)}, source));
226+
BOOST_CHECK_EQUAL(addrman.size(), 2U);
227227
auto addr_ret2 = addrman.Select().first;
228-
BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333");
228+
BOOST_CHECK(addr_ret2.ToString() == "250.1.1.1:8333" || addr_ret2.ToString() == "250.1.1.1:8334");
229229

230-
// Test: Add same IP but diff port to tried table, it doesn't get added.
231-
// Perhaps this is not ideal behavior but it is the current behavior.
230+
// Test: Add same IP but diff port to tried table; this converts the entry with
231+
// the specified port to tried, but not the other.
232232
addrman.Good(CAddress(addr1_port, NODE_NONE));
233-
BOOST_CHECK_EQUAL(addrman.size(), 1U);
233+
BOOST_CHECK_EQUAL(addrman.size(), 2U);
234234
bool newOnly = true;
235235
auto addr_ret3 = addrman.Select(newOnly).first;
236236
BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333");
@@ -369,18 +369,18 @@ BOOST_AUTO_TEST_CASE(addrman_find)
369369
CNetAddr source2 = ResolveIP("250.1.2.2");
370370

371371
BOOST_CHECK(addrman.Add({addr1}, source1));
372-
BOOST_CHECK(!addrman.Add({addr2}, source2));
372+
BOOST_CHECK(addrman.Add({addr2}, source2));
373373
BOOST_CHECK(addrman.Add({addr3}, source1));
374374

375-
// Test: ensure Find returns an IP matching what we searched on.
375+
// Test: ensure Find returns an IP/port matching what we searched on.
376376
AddrInfo* info1 = addrman.Find(addr1);
377377
BOOST_REQUIRE(info1);
378378
BOOST_CHECK_EQUAL(info1->ToString(), "250.1.2.1:8333");
379379

380-
// Test 18; Find does not discriminate by port number.
380+
// Test; Find discriminates by port number.
381381
AddrInfo* info2 = addrman.Find(addr2);
382382
BOOST_REQUIRE(info2);
383-
BOOST_CHECK_EQUAL(info2->ToString(), info1->ToString());
383+
BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:9999");
384384

385385
// Test: Find returns another IP matching what we searched on.
386386
AddrInfo* info3 = addrman.Find(addr3);

‎src/test/fuzz/addrman.cpp

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -137,24 +137,29 @@ class AddrManDeterministic : public AddrMan
137137
// Check that all values in `mapInfo` are equal to all values in `other.mapInfo`.
138138
// Keys may be different.
139139

140-
using AddrInfoHasher = std::function<size_t(const AddrInfo&)>;
141-
using AddrInfoEq = std::function<bool(const AddrInfo&, const AddrInfo&)>;
142-
143-
CNetAddrHash netaddr_hasher;
144-
145-
AddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const AddrInfo& a) {
146-
return netaddr_hasher(static_cast<CNetAddr>(a)) ^ netaddr_hasher(a.source) ^
147-
a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried;
140+
auto addrinfo_hasher = [](const AddrInfo& a) {
141+
CSipHasher hasher(0, 0);
142+
auto addr_key = a.GetKey();
143+
auto source_key = a.source.GetAddrBytes();
144+
hasher.Write(a.nLastSuccess);
145+
hasher.Write(a.nAttempts);
146+
hasher.Write(a.nRefCount);
147+
hasher.Write(a.fInTried);
148+
hasher.Write(a.GetNetwork());
149+
hasher.Write(a.source.GetNetwork());
150+
hasher.Write(addr_key.size());
151+
hasher.Write(source_key.size());
152+
hasher.Write(addr_key.data(), addr_key.size());
153+
hasher.Write(source_key.data(), source_key.size());
154+
return (size_t)hasher.Finalize();
148155
};
149156

150-
AddrInfoEq addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) {
151-
return static_cast<CNetAddr>(lhs) == static_cast<CNetAddr>(rhs) &&
152-
lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess &&
153-
lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount &&
154-
lhs.fInTried == rhs.fInTried;
157+
auto addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) {
158+
return std::tie(static_cast<const CService&>(lhs), lhs.source, lhs.nLastSuccess, lhs.nAttempts, lhs.nRefCount, lhs.fInTried) ==
159+
std::tie(static_cast<const CService&>(rhs), rhs.source, rhs.nLastSuccess, rhs.nAttempts, rhs.nRefCount, rhs.fInTried);
155160
};
156161

157-
using Addresses = std::unordered_set<AddrInfo, AddrInfoHasher, AddrInfoEq>;
162+
using Addresses = std::unordered_set<AddrInfo, decltype(addrinfo_hasher), decltype(addrinfo_eq)>;
158163

159164
const size_t num_addresses{m_impl->mapInfo.size()};
160165

0 commit comments

Comments
 (0)