mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-11-10 22:18:54 +01:00
Merge bitcoin/bitcoin#23826: test: Make AddrMan unit tests use public interface, extend coverage
ea4c9fd4abtest: Cover eviction by timeout in addrman_evictionworks (Martin Zumsande)4f1bb467b5test: Add test for multiplicity in addrman new tables (Martin Zumsande)e880bb7836test: Add test for updating addrman entries (Martin Zumsande)f02eee8c87test: introduce utility function to retrieve an addrman (Martin Zumsande)f0e5efb824test: Remove unused AddrManTest class (Martin Zumsande)b696d7870btest: Remove tests for internal helper functions (Martin Zumsande)0538520091test: use AddrMan instead of AddrManTest where possible (Martin Zumsande)1c65d427bbtest: Inline SimConnFail function (Martin Zumsande)5b7aac34f2test: delete unused GetBucketAndEntry function (Amiti Uttarwar)2ba1e74e59test: Update addrman_serialization unit test to use AddrMan's interface (Amiti Uttarwar)dad5f76021addrman: Introduce a test-only function to lookup addresses (Amiti Uttarwar) Pull request description: This PR (joint work with Amiti Uttarwar) changes the addrman unit tests such that they only use the public `AddrMan` interface: This has the advantage that the tests are less implementation-dependent, i.e. it would be possible to rewrite the internal addrman implementation (as drafted [here](https://github.com/sipa/bitcoin/tree/202106_multiindex_addrman) for using a multiindex) without having to adjust the tests. This includes the following steps: * Adding a test-only function `FindAddressEntry()` to the public addrman interface which returns info about an address in addrman (e.g. bucket, position, whethe the address is in new or tried). Obviously we want to do this sparingly, but I think a single test-only function is ok (which could also be useful elsewhere, e.g. in fuzz tests). * Removal of the `AddrManTest` subclass which would reach into AddrMan's internals, using `AddrMan` instead * Removal of tests for internal helper functions that are not publicly exposed (these are still tested indirectly via the public functions calling them). * Additional tests for previously untested features such as multiplicity in the new tables, that can be tested with the help of `FindAddressEntry()`. All in all, this PR increases the unit test coverage of AddrMan by a bit. ACKs for top commit: jnewbery: ACKea4c9fd4abjosibake: reACKea4c9fd4abTree-SHA512: c2d4ec8bdc62ffd6055ddcd37dea85ec08c76889e9e417e8d7c62a96cf68a8bcbe8c67bec3344d91fa7d3c499f6d9f810962da1dddd38e70966186b10b8ab447
This commit is contained in:
@@ -930,6 +930,29 @@ std::pair<CAddress, int64_t> AddrManImpl::SelectTriedCollision_()
|
||||
return {info_old, info_old.nLastTry};
|
||||
}
|
||||
|
||||
std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& addr)
|
||||
{
|
||||
AssertLockHeld(cs);
|
||||
|
||||
AddrInfo* addr_info = Find(addr);
|
||||
|
||||
if (!addr_info) return std::nullopt;
|
||||
|
||||
if(addr_info->fInTried) {
|
||||
int bucket{addr_info->GetTriedBucket(nKey, m_asmap)};
|
||||
return AddressPosition(/*tried=*/true,
|
||||
/*multiplicity=*/1,
|
||||
/*bucket=*/bucket,
|
||||
/*position=*/addr_info->GetBucketPosition(nKey, false, bucket));
|
||||
} else {
|
||||
int bucket{addr_info->GetNewBucket(nKey, m_asmap)};
|
||||
return AddressPosition(/*tried=*/false,
|
||||
/*multiplicity=*/addr_info->nRefCount,
|
||||
/*bucket=*/bucket,
|
||||
/*position=*/addr_info->GetBucketPosition(nKey, true, bucket));
|
||||
}
|
||||
}
|
||||
|
||||
void AddrManImpl::Check() const
|
||||
{
|
||||
AssertLockHeld(cs);
|
||||
@@ -1116,6 +1139,15 @@ void AddrManImpl::SetServices(const CService& addr, ServiceFlags nServices)
|
||||
Check();
|
||||
}
|
||||
|
||||
std::optional<AddressPosition> AddrManImpl::FindAddressEntry(const CAddress& addr)
|
||||
{
|
||||
LOCK(cs);
|
||||
Check();
|
||||
auto entry = FindAddressEntry_(addr);
|
||||
Check();
|
||||
return entry;
|
||||
}
|
||||
|
||||
const std::vector<bool>& AddrManImpl::GetAsmap() const
|
||||
{
|
||||
return m_asmap;
|
||||
@@ -1201,3 +1233,8 @@ const std::vector<bool>& AddrMan::GetAsmap() const
|
||||
{
|
||||
return m_impl->GetAsmap();
|
||||
}
|
||||
|
||||
std::optional<AddressPosition> AddrMan::FindAddressEntry(const CAddress& addr)
|
||||
{
|
||||
return m_impl->FindAddressEntry(addr);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user