phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-11 04:12:36 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

[net, addrman] Remove external dependencies on CAddrInfo objects

Browse source 
CAddrInfo objects are an implementation detail of how AddrMan manages and adds
metadata to different records. Encapsulate this logic by updating Select &
SelectTriedCollision to return the additional info that the callers need.
This commit is contained in:
Amiti Uttarwar 2021-08-25 15:40:59 -07:00
parent 8af5b54f97
commit 7cba9d5618
6 changed files with 70 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
src/addrman.cpp
View file

@ -694,15 +694,13 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi
}
}
CAddrInfo AddrManImpl::Select_(bool newOnly) const
std::pair<CAddress, int64_t> AddrManImpl::Select_(bool newOnly) const
{
AssertLockHeld(cs);
if (vRandom.empty())
return CAddrInfo();
if (vRandom.empty()) return {};
if (newOnly && nNew == 0)
return CAddrInfo();
if (newOnly && nNew == 0) return {};
// Use a 50% chance for choosing between tried and new table entries.
if (!newOnly &&
@ -720,8 +718,9 @@ CAddrInfo AddrManImpl::Select_(bool newOnly) const
const auto it_found{mapInfo.find(nId)};
assert(it_found != mapInfo.end());
const CAddrInfo& info{it_found->second};
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30))
return info;
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
return {info, info.nLastTry};
}
fChanceFactor *= 1.2;
}
} else {
@ -738,8 +737,9 @@ CAddrInfo AddrManImpl::Select_(bool newOnly) const
const auto it_found{mapInfo.find(nId)};
assert(it_found != mapInfo.end());
const CAddrInfo& info{it_found->second};
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30))
return info;
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
return {info, info.nLastTry};
}
fChanceFactor *= 1.2;
}
}
@ -883,11 +883,11 @@ void AddrManImpl::ResolveCollisions_()
}
}
CAddrInfo AddrManImpl::SelectTriedCollision_()
std::pair<CAddress, int64_t> AddrManImpl::SelectTriedCollision_()
{
AssertLockHeld(cs);
if (m_tried_collisions.size() == 0) return CAddrInfo();
if (m_tried_collisions.size() == 0) return {};
std::set<int>::iterator it = m_tried_collisions.begin();
@ -898,7 +898,7 @@ CAddrInfo AddrManImpl::SelectTriedCollision_()
// 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();
return {};
}
const CAddrInfo& newInfo = mapInfo[id_new];
@ -907,9 +907,8 @@ CAddrInfo AddrManImpl::SelectTriedCollision_()
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];
const CAddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]];
return {info_old, info_old.nLastTry};
}
void AddrManImpl::Check() const
@ -1059,20 +1058,20 @@ void AddrManImpl::ResolveCollisions()
Check();
}
CAddrInfo AddrManImpl::SelectTriedCollision()
std::pair<CAddress, int64_t> AddrManImpl::SelectTriedCollision()
{
LOCK(cs);
Check();
const CAddrInfo ret = SelectTriedCollision_();
const auto ret = SelectTriedCollision_();
Check();
return ret;
}
CAddrInfo AddrManImpl::Select(bool newOnly) const
std::pair<CAddress, int64_t> AddrManImpl::Select(bool newOnly) const
{
LOCK(cs);
Check();
const CAddrInfo addrRet = Select_(newOnly);
const auto addrRet = Select_(newOnly);
Check();
return addrRet;
}
@ -1159,12 +1158,12 @@ void CAddrMan::ResolveCollisions()
m_impl->ResolveCollisions();
}
CAddrInfo CAddrMan::SelectTriedCollision()
std::pair<CAddress, int64_t> CAddrMan::SelectTriedCollision()
{
return m_impl->SelectTriedCollision();
}
CAddrInfo CAddrMan::Select(bool newOnly) const
std::pair<CAddress, int64_t> CAddrMan::Select(bool newOnly) const
{
return m_impl->Select(newOnly);
}

16
src/addrman.h
View file

@ -171,13 +171,23 @@ public:
//! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
void ResolveCollisions();
//! Randomly select an address in tried that another address is attempting to evict.
CAddrInfo SelectTriedCollision();
/**
* Randomly select an address in the tried table that another address is
* attempting to evict.
*
* @return CAddress The record for the selected tried peer.
* int64_t The last time we attempted to connect to that peer.
*/
std::pair<CAddress, int64_t> SelectTriedCollision();
/**
* Choose an address to connect to.
*
* @param[in] newOnly Whether to only select addresses from the new table.
* @return CAddress The record for the selected peer.
* int64_t The last time we attempted to connect to that peer.
*/
CAddrInfo Select(bool newOnly = false) const;
std::pair<CAddress, int64_t> Select(bool newOnly = false) const;
/**
* Return all or many randomly selected addresses, optionally by network.

8
src/addrman_impl.h
View file

@ -31,9 +31,9 @@ public:
void ResolveCollisions() EXCLUSIVE_LOCKS_REQUIRED(!cs);
CAddrInfo SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs);
std::pair<CAddress, int64_t> SelectTriedCollision() EXCLUSIVE_LOCKS_REQUIRED(!cs);
CAddrInfo Select(bool newOnly) const
std::pair<CAddress, int64_t> Select(bool newOnly) const
EXCLUSIVE_LOCKS_REQUIRED(!cs);
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
@ -161,7 +161,7 @@ private:
void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
CAddrInfo Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs);
std::pair<CAddress, int64_t> Select_(bool newOnly) const EXCLUSIVE_LOCKS_REQUIRED(cs);
/**
* Return all or many randomly selected addresses, optionally by network.
@ -193,7 +193,7 @@ private:
void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Return a random to-be-evicted tried table address.
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
std::pair<CAddress, int64_t> SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected.
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);

2
src/bench/addrman.cpp
View file

@ -87,7 +87,7 @@ static void AddrManSelect(benchmark::Bench& bench)
bench.run([&] {
const auto& address = addrman.Select();
assert(address.GetPort() > 0);
assert(address.first.GetPort() > 0);
});
}

13
src/net.cpp
View file

@ -2006,17 +2006,18 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
if (nTries > 100)
break;
CAddrInfo addr;
CAddress addr;
int64_t addr_last_try{0};
if (fFeeler) {
// First, try to get a tried table collision address. This returns
// an empty (invalid) address if there are no collisions to try.
addr = addrman.SelectTriedCollision();
std::tie(addr, addr_last_try) = addrman.SelectTriedCollision();
if (!addr.IsValid()) {
// No tried table collisions. Select a new table address
// for our feeler.
addr = addrman.Select(true);
std::tie(addr, addr_last_try) = addrman.Select(true);
} else if (AlreadyConnectedToAddress(addr)) {
// If test-before-evict logic would have us connect to a
// peer that we're already connected to, just mark that
@ -2025,11 +2026,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
// a currently-connected peer.
addrman.Good(addr);
// Select a new table address for our feeler instead.
addr = addrman.Select(true);
std::tie(addr, addr_last_try) = addrman.Select(true);
}
} else {
// Not a feeler
addr = addrman.Select();
std::tie(addr, addr_last_try) = addrman.Select();
}
// Require outbound connections, other than feelers, to be to distinct network groups
@ -2046,7 +2047,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
continue;
// only consider very recently tried nodes after 30 failed attempts
if (nANow - addr.nLastTry < 600 && nTries < 30)
if (nANow - addr_last_try < 600 && nTries < 30)
continue;
// for non-feelers, require all the services we'll want,

50
src/test/addrman_tests.cpp
View file

@ -172,14 +172,14 @@ BOOST_AUTO_TEST_CASE(addrman_simple)
// Test: Does Addrman respond correctly when empty.
BOOST_CHECK_EQUAL(addrman->size(), 0U);
CAddrInfo addr_null = addrman->Select();
auto addr_null = addrman->Select().first;
BOOST_CHECK_EQUAL(addr_null.ToString(), "[::]:0");
// Test: Does Addrman::Add work as expected.
CService addr1 = ResolveService("250.1.1.1", 8333);
BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source));
BOOST_CHECK_EQUAL(addrman->size(), 1U);
CAddrInfo addr_ret1 = addrman->Select();
auto addr_ret1 = addrman->Select().first;
BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
// Test: Does IP address deduplication work correctly.
@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
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);
CAddrInfo addr_ret2 = addrman.Select();
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.
@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(addrman_ports)
addrman.Good(CAddress(addr1_port, NODE_NONE));
BOOST_CHECK_EQUAL(addrman.size(), 1U);
bool newOnly = true;
CAddrInfo addr_ret3 = addrman.Select(newOnly);
auto addr_ret3 = addrman.Select(newOnly).first;
BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333");
}
@ -249,16 +249,16 @@ BOOST_AUTO_TEST_CASE(addrman_select)
BOOST_CHECK_EQUAL(addrman.size(), 1U);
bool newOnly = true;
CAddrInfo addr_ret1 = addrman.Select(newOnly);
auto addr_ret1 = addrman.Select(newOnly).first;
BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333");
// Test: move addr to tried, select from new expected nothing returned.
addrman.Good(CAddress(addr1, NODE_NONE));
BOOST_CHECK_EQUAL(addrman.size(), 1U);
CAddrInfo addr_ret2 = addrman.Select(newOnly);
auto addr_ret2 = addrman.Select(newOnly).first;
BOOST_CHECK_EQUAL(addr_ret2.ToString(), "[::]:0");
CAddrInfo addr_ret3 = addrman.Select();
auto addr_ret3 = addrman.Select().first;
BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333");
BOOST_CHECK_EQUAL(addrman.size(), 1U);
@ -291,7 +291,7 @@ BOOST_AUTO_TEST_CASE(addrman_select)
// Test: Select pulls from new and tried regardless of port number.
std::set<uint16_t> ports;
for (int i = 0; i < 20; ++i) {
ports.insert(addrman.Select().GetPort());
ports.insert(addrman.Select().first.GetPort());
}
BOOST_CHECK_EQUAL(ports.size(), 3U);
}
@ -869,7 +869,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
BOOST_CHECK(addrman.size() == 0);
// Empty addrman should return blank addrman info.
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
// Add twenty two addresses.
CNetAddr source = ResolveIP("252.2.2.2");
@ -880,7 +880,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
// No collisions yet.
BOOST_CHECK(addrman.size() == i);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
// Ensure Good handles duplicates well.
@ -889,7 +889,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision)
addrman.Good(addr);
BOOST_CHECK(addrman.size() == 22);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
}
@ -907,7 +907,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
// No collision yet.
BOOST_CHECK(addrman.size() == i);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
// Collision between 36 and 19.
@ -916,11 +916,11 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
addrman.Good(addr36);
BOOST_CHECK(addrman.size() == 36);
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.19:0");
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.19:0");
// 36 should be discarded and 19 not evicted.
addrman.ResolveCollisions();
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
// Lets create two collisions.
for (unsigned int i = 37; i < 59; i++) {
@ -929,7 +929,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
addrman.Good(addr);
BOOST_CHECK(addrman.size() == i);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
// Cause a collision.
@ -938,16 +938,16 @@ BOOST_AUTO_TEST_CASE(addrman_noevict)
addrman.Good(addr59);
BOOST_CHECK(addrman.size() == 59);
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.10:0");
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.10:0");
// Cause a second collision.
BOOST_CHECK(!addrman.Add({CAddress(addr36, NODE_NONE)}, source));
addrman.Good(addr36);
BOOST_CHECK(addrman.size() == 59);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() != "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() != "[::]:0");
addrman.ResolveCollisions();
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
BOOST_AUTO_TEST_CASE(addrman_evictionworks)
@ -957,7 +957,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
BOOST_CHECK(addrman.size() == 0);
// Empty addrman should return blank addrman info.
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
// Add 35 addresses
CNetAddr source = ResolveIP("252.2.2.2");
@ -968,7 +968,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
// No collision yet.
BOOST_CHECK(addrman.size() == i);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
// Collision between 36 and 19.
@ -977,7 +977,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
addrman.Good(addr);
BOOST_CHECK_EQUAL(addrman.size(), 36);
CAddrInfo info = addrman.SelectTriedCollision();
auto info = addrman.SelectTriedCollision().first;
BOOST_CHECK_EQUAL(info.ToString(), "250.1.1.19:0");
// Ensure test of address fails, so that it is evicted.
@ -985,23 +985,23 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
// Should swap 36 for 19.
addrman.ResolveCollisions();
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
// If 36 was swapped for 19, then this should cause no collisions.
BOOST_CHECK(!addrman.Add({CAddress(addr, NODE_NONE)}, source));
addrman.Good(addr);
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
// If we insert 19 it should collide with 36
CService addr19 = ResolveService("250.1.1.19");
BOOST_CHECK(!addrman.Add({CAddress(addr19, NODE_NONE)}, source));
addrman.Good(addr19);
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.36:0");
BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().first.ToString(), "250.1.1.36:0");
addrman.ResolveCollisions();
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
}
BOOST_AUTO_TEST_CASE(load_addrman)