diff --git a/src/addrman.cpp b/src/addrman.cpp index 5664b0559a..f756be4bc3 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -99,12 +99,11 @@ double AddrInfo::GetChance(int64_t nNow) const return fChance; } -AddrManImpl::AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio, bool discriminate_ports) +AddrManImpl::AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio) : insecure_rand{deterministic} , nKey{deterministic ? uint256{1} : insecure_rand.rand256()} , m_consistency_check_ratio{consistency_check_ratio} , m_asmap{std::move(asmap)} - , m_discriminate_ports{discriminate_ports} { for (auto& bucket : vvNew) { for (auto& entry : bucket) { @@ -400,12 +399,7 @@ void AddrManImpl::Unserialize(Stream& s_) AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId) { - CService addr2 = addr; - if (!m_discriminate_ports) { - addr2.SetPort(0); - } - - const auto it = mapAddr.find(addr2); + const auto it = mapAddr.find(addr); if (it == mapAddr.end()) return nullptr; if (pnId) @@ -418,15 +412,11 @@ AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId) AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId) { - CService addr2 = addr; - if (!m_discriminate_ports) { - addr2.SetPort(0); - } AssertLockHeld(cs); int nId = nIdCount++; mapInfo[nId] = AddrInfo(addr, addrSource); - mapAddr[addr2] = nId; + mapAddr[addr] = nId; mapInfo[nId].nRandomPos = vRandom.size(); vRandom.push_back(nId); if (pnId) @@ -467,14 +457,9 @@ void AddrManImpl::Delete(int nId) assert(!info.fInTried); assert(info.nRefCount == 0); - CService addr = info; - if (!m_discriminate_ports) { - addr.SetPort(0); - } - SwapRandom(info.nRandomPos, vRandom.size() - 1); vRandom.pop_back(); - mapAddr.erase(addr); + mapAddr.erase(info); mapInfo.erase(nId); nNew--; } @@ -645,10 +630,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 (!m_discriminate_ports && info != addr) - return; - // update info info.nLastSuccess = nTime; info.nLastTry = nTime; @@ -716,10 +697,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 (!m_discriminate_ports && info != addr) - return; - // update info info.nLastTry = nTime; if (fCountFailure && info.nLastCountAttempt < nLastGood) { @@ -847,10 +824,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 (!m_discriminate_ports && info != addr) - return; - // update info int64_t nUpdateInterval = 20 * 60; if (nTime - info.nTime > nUpdateInterval) @@ -869,10 +842,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 (!m_discriminate_ports && info != addr) - return; - // update info info.nServices = nServices; } @@ -885,12 +854,6 @@ AddrInfo AddrManImpl::GetAddressInfo_(const CService& addr) if (!pinfo) return AddrInfo(); - AddrInfo& info = *pinfo; - - // check whether we are talking about the exact same CService (including same port) - if (!m_discriminate_ports && info != addr) - return AddrInfo(); - return *pinfo; } @@ -1187,8 +1150,8 @@ const std::vector& AddrManImpl::GetAsmap() const return m_asmap; } -AddrMan::AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio, bool discriminate_ports) - : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio, discriminate_ports)) {} +AddrMan::AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio) + : m_impl(std::make_unique(std::move(asmap), deterministic, consistency_check_ratio)) {} AddrMan::~AddrMan() = default; diff --git a/src/addrman.h b/src/addrman.h index 3515d98cae..7b36d8ddd9 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -57,7 +57,7 @@ class AddrMan const std::unique_ptr m_impl; public: - explicit AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio, bool discriminate_ports = false); + explicit AddrMan(std::vector asmap, bool deterministic, int32_t consistency_check_ratio); ~AddrMan(); diff --git a/src/addrman_impl.h b/src/addrman_impl.h index d139c1f554..1e29b69da6 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -99,7 +99,7 @@ public: class AddrManImpl { public: - AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio, bool discriminate_ports); + AddrManImpl(std::vector&& asmap, bool deterministic, int32_t consistency_check_ratio); ~AddrManImpl(); @@ -227,9 +227,6 @@ private: // would be re-bucketed accordingly. const std::vector m_asmap; - //! Discriminate entries based on port. - bool m_discriminate_ports GUARDED_BY(cs); - //! Find an entry. AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 5c0d1b907d..3fb668df6d 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -1056,11 +1056,6 @@ std::string CService::ToString() const return ToStringIPPort(); } -void CService::SetPort(uint16_t portIn) -{ - port = portIn; -} - CSubNet::CSubNet(): valid(false) { diff --git a/src/netaddress.h b/src/netaddress.h index cbb10a2483..6525a0cf4a 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -544,7 +544,6 @@ public: CService(const CNetAddr& ip, uint16_t port); CService(const struct in_addr& ipv4Addr, uint16_t port); explicit CService(const struct sockaddr_in& addr); - void SetPort(uint16_t portIn); uint16_t GetPort() const; bool GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const; bool SetSockAddr(const struct sockaddr* paddr); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index b65c36b92b..05fb06ec0a 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -26,7 +26,7 @@ public: virtual void Serialize(CDataStream& s) const = 0; AddrManSerializationMock() - : AddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100, /* discriminate_ports */ true) + : AddrMan(/* asmap */ std::vector(), /* deterministic */ true, /* consistency_check_ratio */ 100) {} }; @@ -82,9 +82,8 @@ private: public: explicit AddrManTest(bool makeDeterministic = true, - std::vector asmap = std::vector(), - bool discriminatePorts = true) - : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100, discriminatePorts) + std::vector asmap = std::vector()) + : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100) { deterministic = makeDeterministic; } @@ -236,34 +235,6 @@ BOOST_AUTO_TEST_CASE(addrman_ports) BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); } -BOOST_AUTO_TEST_CASE(addrman_ports_nondiscriminate) -{ - AddrManTest addrman(/* deterministic */ true, /* asmap */ std::vector(), /* discriminate */ false); - - CNetAddr source = ResolveIP("252.2.2.2"); - - BOOST_CHECK_EQUAL(addrman.size(), 0U); - - // Test 7; Addr with same IP but diff port does not replace existing addr. - CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); - 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); - auto addr_ret2 = addrman.Select().first; - BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333"); - - // 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. - addrman.Good(CAddress(addr1_port, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); - bool newOnly = true; - auto addr_ret3 = addrman.Select(newOnly).first; - BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); -} - BOOST_AUTO_TEST_CASE(addrman_select) { AddrManTest addrman; @@ -415,39 +386,6 @@ BOOST_AUTO_TEST_CASE(addrman_find) BOOST_CHECK_EQUAL(info3->ToString(), "251.255.2.1:8333"); } -BOOST_AUTO_TEST_CASE(addrman_find_nondiscriminate) -{ - AddrManTest addrman(/* deterministic */ true, /* asmap */ std::vector(), /* discriminate */ false); - - BOOST_CHECK_EQUAL(addrman.size(), 0U); - - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); - CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); - CAddress addr3 = CAddress(ResolveService("251.255.2.1", 8333), NODE_NONE); - - CNetAddr source1 = ResolveIP("250.1.2.1"); - CNetAddr source2 = ResolveIP("250.1.2.2"); - - BOOST_CHECK(addrman.Add({addr1}, source1)); - BOOST_CHECK(!addrman.Add({addr2}, source2)); - BOOST_CHECK(addrman.Add({addr3}, source1)); - - // Test: ensure Find returns an IP 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. - AddrInfo* info2 = addrman.Find(addr2); - BOOST_REQUIRE(info2); - BOOST_CHECK_EQUAL(info2->ToString(), info1->ToString()); - - // Test: Find returns another IP matching what we searched on. - AddrInfo* info3 = addrman.Find(addr3); - BOOST_REQUIRE(info3); - BOOST_CHECK_EQUAL(info3->ToString(), "251.255.2.1:8333"); -} - BOOST_AUTO_TEST_CASE(addrman_create) { AddrManTest addrman;