From 8339b55df4a053245361af7ada2f913765206629 Mon Sep 17 00:00:00 2001 From: Duke Leto Date: Mon, 1 Mar 2021 17:30:46 -0500 Subject: [PATCH] Test-before-evict in addrman From BTC Core https://github.com/bitcoin/bitcoin/pull/9037/ with modifications to support our asmap. This has a small part of code commented out that depends feeler connection code. --- src/addrman.cpp | 98 ++++++++++++++++++++-- src/addrman.h | 57 ++++++++++--- src/net.cpp | 18 ++-- src/test/addrman_tests.cpp | 163 ++++++++++++++++++++++++++++++++++++- 4 files changed, 315 insertions(+), 21 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 1568deb9f..7016c1882 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -215,8 +215,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) info.fInTried = true; } -void CAddrMan::Good_(const CService& addr, int64_t nTime) -{ +void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime) { int nId; CAddrInfo* pinfo = Find(addr, &nId); @@ -258,12 +257,101 @@ void CAddrMan::Good_(const CService& addr, int64_t nTime) if (nUBucket == -1) return; - LogPrint("addrman", "Moving %s to tried\n", addr.ToString()); + // which tried bucket to move the entry to + int tried_bucket = info.GetTriedBucket(nKey,m_asmap); + int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); - // move nId to the tried tables - MakeTried(info, nId); + // Will moving this address into tried evict another entry? + if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) { + LogPrint("addrman", "Collision inserting element into tried table, moving %s to m_tried_collisions=%d\n", addr.ToString(), m_tried_collisions.size()); + if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) { + m_tried_collisions.insert(nId); + } + } else { + LogPrint("addrman", "Moving %s to tried\n", addr.ToString()); + printf("%s: Moving %s to tried\n", __func__, addr.ToString().c_str() ); + + // move nId to the tried tables + MakeTried(info, nId); + } } +void CAddrMan::ResolveCollisions_() { + for (std::set::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) { + int id_new = *it; + bool erase_collision = false; + + // If id_new not found in mapInfo remove it from m_tried_collisions + if (mapInfo.count(id_new) != 1) { + erase_collision = true; + } else { + CAddrInfo& info_new = mapInfo[id_new]; + + // Which tried bucket to move the entry to. + int tried_bucket = info_new.GetTriedBucket(nKey,m_asmap); + int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket); + if (!info_new.IsValid()) { // id_new may no longer map to a valid address + erase_collision = true; + } else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty + + // Get the to-be-evicted address that is being tested + int id_old = vvTried[tried_bucket][tried_bucket_pos]; + CAddrInfo& info_old = mapInfo[id_old]; + + // Has successfully connected in last X hours + if (GetTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { + erase_collision = true; + } else if (GetTime() - info_old.nLastTry < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { // attempted to connect and failed in last X hours + + // Give address at least 60 seconds to successfully connect + if (GetTime() - info_old.nLastTry > 60) { + LogPrint("addrman", "Swapping %s for %s in tried table\n", info_new.ToString(), info_old.ToString()); + + // Replaces an existing address already in the tried table with the new address + Good_(info_new, false, GetTime()); + erase_collision = true; + } + } + } else { // Collision is not actually a collision anymore + Good_(info_new, false, GetTime()); + erase_collision = true; + } + } + + if (erase_collision) { + m_tried_collisions.erase(it++); + } else { + it++; + } + } +} + +CAddrInfo CAddrMan::SelectTriedCollision_() { + if (m_tried_collisions.size() == 0) return CAddrInfo(); + + std::set::iterator it = m_tried_collisions.begin(); + + // Selects a random element from m_tried_collisions + std::advance(it, GetRandInt(m_tried_collisions.size())); + int id_new = *it; + + // If id_new not found in mapInfo remove it from m_tried_collisions + if (mapInfo.count(id_new) != 1) { + m_tried_collisions.erase(it); + return CAddrInfo(); + } + + CAddrInfo& newInfo = mapInfo[id_new]; + + // which tried bucket to move the entry to + int tried_bucket = newInfo.GetTriedBucket(nKey,m_asmap); + int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); + int id_old = vvTried[tried_bucket][tried_bucket_pos]; + + return mapInfo[id_old]; +} + + bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) { if (!addr.IsRoutable()) diff --git a/src/addrman.h b/src/addrman.h index eb2662536..68419ea61 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -1,8 +1,7 @@ // Copyright (c) 2012 Pieter Wuille -// Copyright (c) 2016-2020 The Hush developers +// Copyright (c) 2016-2021 The Hush developers // Distributed under the GPLv3 software license, see the accompanying // file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html - /****************************************************************************** * Copyright © 2014-2019 The SuperNET Developers. * * * @@ -31,7 +30,6 @@ #include "clientversion.h" #include "hash.h" #include "netbase.h" - #include #include #include @@ -43,7 +41,6 @@ class CAddrInfo : public CAddress { - public: //! last try whatsoever by us (memory only) int64_t nLastTry; @@ -174,12 +171,18 @@ public: //! after how many failed attempts we give up on a new node #define ADDRMAN_RETRIES 3 +//! the maximum number of tried addr collisions to store +#define ADDRMAN_SET_TRIED_COLLISION_SIZE 10 + //! how many successive failures are allowed ... #define ADDRMAN_MAX_FAILURES 10 //! ... in at least this many days #define ADDRMAN_MIN_FAIL_DAYS 7 +//! how recent a successful connection should be before we allow an address to be evicted from tried +#define ADDRMAN_REPLACEMENT_HOURS 4 + //! the maximum percentage of nodes to return in a getaddr call #define ADDRMAN_GETADDR_MAX_PCT 23 @@ -220,6 +223,12 @@ private: //! list of "new" buckets int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE]; + //! last time Good was called (memory only) + int64_t nLastGood; + + //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discpline used to resolve these collisions. + std::set m_tried_collisions; + protected: //! secret key to randomize bucket select with uint256 nKey; @@ -244,7 +253,14 @@ protected: void ClearNew(int nUBucket, int nUBucketPos); //! Mark an entry "good", possibly moving it from "new" to "tried". - void Good_(const CService &addr, int64_t nTime); + void Good_(const CService &addr, bool test_before_evict, int64_t time); + + + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_(); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_(); //! Add an entry to the "new" table. bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty); @@ -583,12 +599,12 @@ public: } //! Mark an entry as accessible. - void Good(const CService &addr, int64_t nTime = GetTime()) + void Good(const CService &addr, bool test_before_evict = true, int64_t nTime = GetTime()) { { LOCK(cs); Check(); - Good_(addr, nTime); + Good_(addr, test_before_evict, nTime); Check(); } } @@ -604,9 +620,30 @@ public: } } - /** - * Choose an address to connect to. - */ + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions() + { + LOCK(cs); + Check(); + ResolveCollisions_(); + Check(); + } + + //! Randomly select an address in tried that another address is attempting to evict. + CAddrInfo SelectTriedCollision() + { + CAddrInfo ret; + { + LOCK(cs); + Check(); + ret = SelectTriedCollision_(); + Check(); + } + return ret; + } + + + // Choose an address to connect to. CAddrInfo Select(bool newOnly = false) { CAddrInfo addrRet; diff --git a/src/net.cpp b/src/net.cpp index 67a9f6cbb..7fc9b59e0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1434,6 +1434,7 @@ void ThreadOpenConnections() } } + // Initiate network connections int64_t nStart = GetTime(); while (true) @@ -1476,12 +1477,19 @@ void ThreadOpenConnections() } } - int64_t nANow = GetTime(); + addrman.ResolveCollisions(); - int nTries = 0; - while (true) - { - CAddrInfo addr = addrman.Select(); + int64_t nANow = GetTime(); + int nTries = 0; + while (true) { + CAddrInfo addr = addrman.SelectTriedCollision(); + + // SelectTriedCollision returns an invalid address if it is empty. + /* TODO: Uncomment when merged with feeler connections + if (!fFeeler || !addr.IsValid()) { + addr = addrman.Select(fFeeler); + } + */ // if we selected an invalid address, restart if (!addr.IsValid() || setConnected.count(addr.GetGroup(addrman.m_asmap)) || IsLocal(addr)) diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index e50842afe..41e896627 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -6,7 +6,6 @@ #include "test/test_bitcoin.h" #include #include - #include "hash.h" #include "random.h" @@ -49,6 +48,17 @@ public: { CAddrMan::Delete(nId); } + + // Simulates connection failure so that we can test eviction of offline nodes + void SimConnFail(CService& addr) + { + int64_t nLastSuccess = 1; + Good_(addr, true, nLastSuccess); // Set last good connection in the deep past. + + bool count_failure = false; + int64_t nLastTry = GetAdjustedTime()-61; + Attempt(addr, count_failure, nLastTry); + } }; BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) @@ -519,4 +529,155 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) // than 64 buckets. BOOST_CHECK(buckets.size() > 64); } +BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + BOOST_CHECK(addrman.size() == 0); + + // Empty addrman should return blank addrman info. + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collisions yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Ensure Good handles duplicates well. + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == 22); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + +} + +BOOST_AUTO_TEST_CASE(addrman_noevict) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collision yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Collision between 23 and 19. + CService addr23 = ResolveService("250.1.1.23"); + addrman.Add(CAddress(addr23, NODE_NONE), source); + addrman.Good(addr23); + + BOOST_CHECK(addrman.size() == 23); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.19:0"); + + // 23 should be discarded and 19 not evicted. + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Lets create two collisions. + for (unsigned int i = 24; i < 33; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Cause a collision. + CService addr33 = ResolveService("250.1.1.33"); + addrman.Add(CAddress(addr33, NODE_NONE), source); + addrman.Good(addr33); + BOOST_CHECK(addrman.size() == 33); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.27:0"); + + // Cause a second collision. + addrman.Add(CAddress(addr23, NODE_NONE), source); + addrman.Good(addr23); + BOOST_CHECK(addrman.size() == 33); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() != "[::]:0"); + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); +} + +BOOST_AUTO_TEST_CASE(addrman_evictionworks) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + BOOST_CHECK(addrman.size() == 0); + + // Empty addrman should return blank addrman info. + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collision yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Collision between 23 and 19. + CService addr = ResolveService("250.1.1.23"); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == 23); + CAddrInfo info = addrman.SelectTriedCollision(); + BOOST_CHECK(info.ToString() == "250.1.1.19:0"); + + // Ensure test of address fails, so that it is evicted. + addrman.SimConnFail(info); + + // Should swap 23 for 19. + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // If 23 was swapped for 19, then this should cause no collisions. + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // If we insert 19 is should collide with 23. + CService addr19 = ResolveService("250.1.1.19"); + addrman.Add(CAddress(addr19, NODE_NONE), source); + addrman.Good(addr19); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.23:0"); + + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); +} + BOOST_AUTO_TEST_SUITE_END()