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

4445211 [fuzz] Update comment in FillAddrman() (John Newbery)
640476e [fuzz] Make Fill() a free function in fuzz/addrman.cpp (John Newbery)
90ad8ad [fuzz] Make RandAddr() a free function in fuzz/addrman.cpp (John Newbery)
491975c [fuzz] Pass FuzzedDataProvider& into Fill() in addrman fuzz tests (John Newbery)
56303e3 [fuzz] Create a FastRandomContext in addrman fuzz tests (John Newbery)

Pull request description:

  bitcoin#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 4445211. Agree the failure seems unrelated, looks like some startup race.
  mzumsande:
    ACK 4445211
  vasild:
    ACK 4445211

Tree-SHA512: fcf994e1dedd0012b77f632720b6423d51ceda4eb85c9efe572f2a1150117f9e511114a5206738dd94409137287577f3b01a9998f5237de845410d3d96e7cb7f

Author: fanquake

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);