Merge bitcoin/bitcoin#23053: [fuzz] Use public methods in addrman fuzz tests

44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c [fuzz] Update comment in FillAddrman() (John Newbery)
640476eb0e17fd4c64d4840ceab229642f1d79d9 [fuzz] Make Fill() a free function in fuzz/addrman.cpp (John Newbery)
90ad8ad61a38dbb1f247a5f3d5f649a856d9938a [fuzz] Make RandAddr() a free function in fuzz/addrman.cpp (John Newbery)
491975c596ebce93ae8de192c9ef171f002fac7c [fuzz] Pass FuzzedDataProvider& into Fill() in addrman fuzz tests (John Newbery)
56303e382e26ac7096c09152c66894dc3bb4d1fd [fuzz] Create a FastRandomContext in addrman fuzz tests (John Newbery)

Pull request description:

  #22974 improved the performance of `CAddrMan::Good()` significantly so that it could be used directly in the fuzz tests, instead of those tests reaching directly into addrman's internal data structures.

  This PR continues the work of making the fuzz tests only use `CAddrMan`'s public methods and pulls the `Fill()` and `RandAddr()` methods from `CAddrManDeterministic` into free functions, since they no longer need access to `CAddrMan` internals.

ACKs for top commit:
  theuni:
    utACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c. Agree the failure seems unrelated, looks like some startup race.
  mzumsande:
    ACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c
  vasild:
    ACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c

Tree-SHA512: fcf994e1dedd0012b77f632720b6423d51ceda4eb85c9efe572f2a1150117f9e511114a5206738dd94409137287577f3b01a9998f5237de845410d3d96e7cb7f

1 files changed, 71 insertions, 76 deletions

diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp
index cfeab9dcdc..8df3707fc9 100644
--- a/src/test/fuzz/addrman.cpp
+++ b/src/test/fuzz/addrman.cpp
@@ -7,6 +7,7 @@
 #include <addrman_impl.h>
 #include <chainparams.h>
 #include <merkleblock.h>
+#include <random.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
@@ -35,92 +36,86 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman)
     }
 }
 
-class AddrManDeterministic : public AddrMan
+/**
+ * Generate a random address. Always returns a valid address.
+ */
+CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context)
 {
-public:
-    FuzzedDataProvider& m_fuzzed_data_provider;
-
-    explicit AddrManDeterministic(std::vector<bool> asmap, FuzzedDataProvider& fuzzed_data_provider)
-        : AddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0)
-        , m_fuzzed_data_provider(fuzzed_data_provider)
-    {
-        WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)});
-    }
-
-    /**
-     * Generate a random address. Always returns a valid address.
-     */
-    CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs)
-    {
-        CNetAddr addr;
-        if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) {
-            addr = ConsumeNetAddr(m_fuzzed_data_provider);
-        } else {
-            // The networks [1..6] correspond to CNetAddr::BIP155Network (private).
-            static const std::map<uint8_t, uint8_t> net_len_map = {{1, ADDR_IPV4_SIZE},
-                                                                   {2, ADDR_IPV6_SIZE},
-                                                                   {4, ADDR_TORV3_SIZE},
-                                                                   {5, ADDR_I2P_SIZE},
-                                                                   {6, ADDR_CJDNS_SIZE}};
-            uint8_t net = m_impl->insecure_rand.randrange(5) + 1; // [1..5]
-            if (net == 3) {
-                net = 6;
-            }
-
-            CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT);
-
-            s << net;
-            s << m_impl->insecure_rand.randbytes(net_len_map.at(net));
-
-            s >> addr;
-        }
-
-        // Return a dummy IPv4 5.5.5.5 if we generated an invalid address.
-        if (!addr.IsValid()) {
-            in_addr v4_addr = {};
-            v4_addr.s_addr = 0x05050505;
-            addr = CNetAddr{v4_addr};
+    CNetAddr addr;
+    if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) {
+        addr = ConsumeNetAddr(fuzzed_data_provider);
+    } else {
+        // The networks [1..6] correspond to CNetAddr::BIP155Network (private).
+        static const std::map<uint8_t, uint8_t> net_len_map = {{1, ADDR_IPV4_SIZE},
+                                                               {2, ADDR_IPV6_SIZE},
+                                                               {4, ADDR_TORV3_SIZE},
+                                                               {5, ADDR_I2P_SIZE},
+                                                               {6, ADDR_CJDNS_SIZE}};
+        uint8_t net = fast_random_context.randrange(5) + 1; // [1..5]
+        if (net == 3) {
+            net = 6;
         }
 
-        return addr;
-    }
-
-    /**
-     * Fill this addrman with lots of addresses from lots of sources.
-     */
-    void Fill()
-    {
-        LOCK(m_impl->cs);
+        CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT);
 
-        // Add some of the addresses directly to the "tried" table.
+        s << net;
+        s << fast_random_context.randbytes(net_len_map.at(net));
 
-        // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33%
-        const size_t n = m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 3);
+        s >> addr;
+    }
 
-        const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 50);
-        CNetAddr prev_source;
-        // Use insecure_rand inside the loops instead of m_fuzzed_data_provider because when
-        // the latter is exhausted it just returns 0.
-        for (size_t i = 0; i < num_sources; ++i) {
-            const auto source = RandAddr();
-            const size_t num_addresses = m_impl->insecure_rand.randrange(500) + 1; // [1..500]
+    // Return a dummy IPv4 5.5.5.5 if we generated an invalid address.
+    if (!addr.IsValid()) {
+        in_addr v4_addr = {};
+        v4_addr.s_addr = 0x05050505;
+        addr = CNetAddr{v4_addr};
+    }
 
-            for (size_t j = 0; j < num_addresses; ++j) {
-                const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK};
-                const auto time_penalty = m_impl->insecure_rand.randrange(100000001);
-                m_impl->Add_(addr, source, time_penalty);
+    return addr;
+}
 
-                if (n > 0 && m_impl->mapInfo.size() % n == 0) {
-                    m_impl->Good_(addr, false, GetTime());
-                }
+/** Fill addrman with lots of addresses from lots of sources.  */
+void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider)
+{
+    // Add a fraction of the addresses to the "tried" table.
+    // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33%
+    const size_t n = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 3);
+
+    const size_t num_sources = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 50);
+    CNetAddr prev_source;
+    // Generate a FastRandomContext seed to use inside the loops instead of
+    // fuzzed_data_provider. When fuzzed_data_provider is exhausted it
+    // just returns 0.
+    FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)};
+    for (size_t i = 0; i < num_sources; ++i) {
+        const auto source = RandAddr(fuzzed_data_provider, fast_random_context);
+        const size_t num_addresses = fast_random_context.randrange(500) + 1; // [1..500]
+
+        for (size_t j = 0; j < num_addresses; ++j) {
+            const auto addr = CAddress{CService{RandAddr(fuzzed_data_provider, fast_random_context), 8333}, NODE_NETWORK};
+            const auto time_penalty = fast_random_context.randrange(100000001);
+            addrman.Add({addr}, source, time_penalty);
+
+            if (n > 0 && addrman.size() % n == 0) {
+                addrman.Good(addr, GetTime());
+            }
 
-                // Add 10% of the addresses from more than one source.
-                if (m_impl->insecure_rand.randrange(10) == 0 && prev_source.IsValid()) {
-                    m_impl->Add_({addr}, prev_source, time_penalty);
-                }
+            // Add 10% of the addresses from more than one source.
+            if (fast_random_context.randrange(10) == 0 && prev_source.IsValid()) {
+                addrman.Add({addr}, prev_source, time_penalty);
             }
-            prev_source = source;
         }
+        prev_source = source;
+    }
+}
+
+class AddrManDeterministic : public AddrMan
+{
+public:
+    explicit AddrManDeterministic(std::vector<bool> asmap, FuzzedDataProvider& fuzzed_data_provider)
+        : AddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0)
+    {
+        WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)});
     }
 
     /**
@@ -307,7 +302,7 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman)
 
     CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION);
 
-    addr_man1.Fill();
+    FillAddrman(addr_man1, fuzzed_data_provider);
     data_stream << addr_man1;
     data_stream >> addr_man2;
     assert(addr_man1 == addr_man2);