Merge bitcoin/bitcoin#23826: test: Make AddrMan unit tests use public interface, extend coverage

ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866 test: Cover eviction by timeout in addrman_evictionworks (Martin Zumsande)
4f1bb467b556ec93c9b8f758783fda4d050da491 test: Add test for multiplicity in addrman new tables (Martin Zumsande)
e880bb7836dab2018049390884220177c6db9b92 test: Add test for updating addrman entries (Martin Zumsande)
f02eee8c8784dfc8db80a21ab6508f7c99298255 test: introduce utility function to retrieve an addrman (Martin Zumsande)
f0e5efb82493f7a14580335ce719d5be81c8713e test: Remove unused AddrManTest class (Martin Zumsande)
b696d7870b29232057600df5ddd8351888253b95 test: Remove tests for internal helper functions (Martin Zumsande)
0538520091bf2982a029a0298835400f5afbdc15 test: use AddrMan instead of AddrManTest where possible (Martin Zumsande)
1c65d427bbf61bb558cf7e18f7aff99b19f68508 test: Inline SimConnFail function (Martin Zumsande)
5b7aac34f2363822c3a1cfafda8ffc9528905058 test: delete unused GetBucketAndEntry function (Amiti Uttarwar)
2ba1e74e59a325ca6cb140757067dd5e0c7c249b test: Update addrman_serialization unit test to use AddrMan's interface (Amiti Uttarwar)
dad5f760211df314d650999e0a76edb0151b4fe1 addrman: 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:
    ACK ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866
  josibake:
    reACK https://github.com/bitcoin/bitcoin/pull/23826/commits/ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866

Tree-SHA512: c2d4ec8bdc62ffd6055ddcd37dea85ec08c76889e9e417e8d7c62a96cf68a8bcbe8c67bec3344d91fa7d3c499f6d9f810962da1dddd38e70966186b10b8ab447

1 files changed, 34 insertions, 0 deletions

diff --git a/src/addrman.h b/src/addrman.h
index 7f0936be8c..0646ef368d 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -22,6 +22,31 @@ class AddrManImpl;
 /** Default for -checkaddrman */
 static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0};
 
+/** Test-only struct, capturing info about an address in AddrMan */
+struct AddressPosition {
+    // Whether the address is in the new or tried table
+    const bool tried;
+
+    // Addresses in the tried table should always have a multiplicity of 1.
+    // Addresses in the new table can have multiplicity between 1 and
+    // ADDRMAN_NEW_BUCKETS_PER_ADDRESS
+    const int multiplicity;
+
+    // If the address is in the new table, the bucket and position are
+    // populated based on the first source who sent the address.
+    // In certain edge cases, this may not be where the address is currently
+    // located.
+    const int bucket;
+    const int position;
+
+    bool operator==(AddressPosition other) {
+        return std::tie(tried, multiplicity, bucket, position) ==
+               std::tie(other.tried, other.multiplicity, other.bucket, other.position);
+    }
+    explicit AddressPosition(bool tried_in, int multiplicity_in, int bucket_in, int position_in)
+        : tried{tried_in}, multiplicity{multiplicity_in}, bucket{bucket_in}, position{position_in} {}
+};
+
 /** Stochastic address manager
  *
  * Design goals:
@@ -142,6 +167,15 @@ public:
     void SetServices(const CService& addr, ServiceFlags nServices);
 
     const std::vector<bool>& GetAsmap() const;
+
+    /** Test-only function
+     * Find the address record in AddrMan and return information about its
+     * position.
+     * @param[in] addr       The address record to look up.
+     * @return               Information about the address record in AddrMan
+     *                       or nullopt if address is not found.
+     */
+    std::optional<AddressPosition> FindAddressEntry(const CAddress& addr);
 };
 
 #endif // BITCOIN_ADDRMAN_H