Merge bitcoin/bitcoin#32822: fuzz: Make process_message(s) more deterministic

fa1a14a13a fuzz: Reset chainman state in process_message(s) targets (MarcoFalke)
fa9a3de09b fuzz: DisableNextWrite (MarcoFalke)
aeeeeec9f7 fuzz: Reset dirty connman state in process_message(s) targets (MarcoFalke)
fa11eea405 fuzz: Avoid non-determinism in process_message(s) target (PeerMan) (MarcoFalke)

Pull request description:

  `process_message(s)` are the least stable fuzz targets, according to OSS-Fuzz.

  Tracking issue: https://github.com/bitcoin/bitcoin/issues/29018.

  ### Testing

  Needs coverage compilation, as explained in `./contrib/devtools/README.md`. And then, using 32 threads:

  ```
  cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ process_messages 32
  ```

  Each commit can be reverted to see more non-determinism re-appear.

ACKs for top commit:
  marcofleon:
    ReACK fa1a14a13a
  dergoegge:
    reACK fa1a14a13a

Tree-SHA512: 37b5b6dbdde6a39b4f83dc31e92cffb4a62a4b8f5befbf17029d943d0b2fd506f4a0833570dcdbf79a90b42af9caca44e98e838b03213d6bc1c3ecb70a6bb135

src/test/fuzz/p2p_handshake.cpp

@@ -42,14 +42,14 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize)
     SeedRandomStateForTest(SeedRand::ZEROS);
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
-    ConnmanTestMsg& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
+    auto& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
     auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
     SetMockTime(1610000000); // any time to successfully reset ibd
     chainman.ResetIbd();
 
     node::Warnings warnings{};
     NetGroupManager netgroupman{{}};
-    AddrMan addrman{netgroupman, /*deterministic=*/true, 0};
+    AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0};
     auto peerman = PeerManager::make(connman, addrman,
                                      /*banman=*/nullptr, chainman,
                                      *g_setup->m_node.mempool, warnings,

src/test/fuzz/process_message.cpp

@@ -5,6 +5,7 @@
 #include <consensus/consensus.h>
 #include <net.h>
 #include <net_processing.h>
+#include <node/warnings.h>
 #include <primitives/transaction.h>
 #include <protocol.h>
 #include <script/script.h>
@@ -29,8 +30,20 @@
 #include <vector>
 
 namespace {
-const TestingSetup* g_setup;
+TestingSetup* g_setup;
 std::string_view LIMIT_TO_MESSAGE_TYPE{};
+
+void ResetChainman(TestingSetup& setup)
+{
+    SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time());
+    setup.m_node.chainman.reset();
+    setup.m_make_chainman();
+    setup.LoadVerifyActivateChainstate();
+    for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
+        MineBlock(setup.m_node, {});
+    }
+    setup.m_node.validation_signals->SyncWithValidationInterfaceQueue();
+}
 } // namespace
 
 void initialize_process_message()
@@ -40,15 +53,13 @@ void initialize_process_message()
         Assert(std::count(ALL_NET_MESSAGE_TYPES.begin(), ALL_NET_MESSAGE_TYPES.end(), LIMIT_TO_MESSAGE_TYPE)); // Unknown message type passed
     }
 
-    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
+    static const auto testing_setup{
+        MakeNoLogFileContext<TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            {.extra_args = {"-txreconciliation"}});
+            {}),
+    };
     g_setup = testing_setup.get();
-    SetMockTime(WITH_LOCK(g_setup->m_node.chainman->GetMutex(), return g_setup->m_node.chainman->ActiveTip()->Time()));
+    ResetChainman(*g_setup);
-    for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
-        MineBlock(g_setup->m_node, {});
-    }
-    g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
 }
 
 FUZZ_TARGET(process_message, .init = initialize_process_message)
@@ -56,11 +67,27 @@ FUZZ_TARGET(process_message, .init = initialize_process_message)
     SeedRandomStateForTest(SeedRand::ZEROS);
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
-    ConnmanTestMsg& connman = *static_cast<ConnmanTestMsg*>(g_setup->m_node.connman.get());
+    auto& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
+    connman.ResetAddrCache();
+    connman.ResetMaxOutboundCycle();
     auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
+    const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())};
     SetMockTime(1610000000); // any time to successfully reset ibd
     chainman.ResetIbd();
+    chainman.DisableNextWrite();
 
+    node::Warnings warnings{};
+    NetGroupManager netgroupman{{}};
+    AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0};
+    auto peerman = PeerManager::make(connman, addrman,
+                                     /*banman=*/nullptr, chainman,
+                                     *g_setup->m_node.mempool, warnings,
+                                     PeerManager::Options{
+                                         .reconcile_txs = true,
+                                         .deterministic_rng = true,
+                                     });
+
+    connman.SetMsgProc(peerman.get());
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
     const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::MESSAGE_TYPE_SIZE).c_str()};
@@ -93,4 +120,8 @@ FUZZ_TARGET(process_message, .init = initialize_process_message)
     }
     g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
     g_setup->m_node.connman->StopNodes();
+    if (block_index_size != WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())) {
+        // Reuse the global chainman, but reset it when it is dirty
+        ResetChainman(*g_setup);
+    }
 }

src/test/fuzz/process_messages.cpp

@@ -5,6 +5,7 @@
 #include <consensus/consensus.h>
 #include <net.h>
 #include <net_processing.h>
+#include <node/warnings.h>
 #include <protocol.h>
 #include <script/script.h>
 #include <sync.h>
@@ -25,20 +26,30 @@
 #include <vector>
 
 namespace {
-const TestingSetup* g_setup;
+TestingSetup* g_setup;
+
+void ResetChainman(TestingSetup& setup)
+{
+    SetMockTime(setup.m_node.chainman->GetParams().GenesisBlock().Time());
+    setup.m_node.chainman.reset();
+    setup.m_make_chainman();
+    setup.LoadVerifyActivateChainstate();
+    for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
+        MineBlock(setup.m_node, {});
+    }
+    setup.m_node.validation_signals->SyncWithValidationInterfaceQueue();
+}
 } // namespace
 
 void initialize_process_messages()
 {
-    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
+    static const auto testing_setup{
+        MakeNoLogFileContext<TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            {.extra_args = {"-txreconciliation"}});
+            {}),
+    };
     g_setup = testing_setup.get();
-    SetMockTime(WITH_LOCK(g_setup->m_node.chainman->GetMutex(), return g_setup->m_node.chainman->ActiveTip()->Time()));
+    ResetChainman(*g_setup);
-    for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
-        MineBlock(g_setup->m_node, {});
-    }
-    g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
 }
 
 FUZZ_TARGET(process_messages, .init = initialize_process_messages)
@@ -46,10 +57,26 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages)
     SeedRandomStateForTest(SeedRand::ZEROS);
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
-    ConnmanTestMsg& connman = *static_cast<ConnmanTestMsg*>(g_setup->m_node.connman.get());
+    auto& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
+    connman.ResetAddrCache();
+    connman.ResetMaxOutboundCycle();
     auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
+    const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())};
     SetMockTime(1610000000); // any time to successfully reset ibd
     chainman.ResetIbd();
+    chainman.DisableNextWrite();
+
+    node::Warnings warnings{};
+    NetGroupManager netgroupman{{}};
+    AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0};
+    auto peerman = PeerManager::make(connman, addrman,
+                                     /*banman=*/nullptr, chainman,
+                                     *g_setup->m_node.mempool, warnings,
+                                     PeerManager::Options{
+                                         .reconcile_txs = true,
+                                         .deterministic_rng = true,
+                                     });
+    connman.SetMsgProc(peerman.get());
 
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
@@ -93,4 +120,8 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages)
     }
     g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
     g_setup->m_node.connman->StopNodes();
+    if (block_index_size != WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())) {
+        // Reuse the global chainman, but reset it when it is dirty
+        ResetChainman(*g_setup);
+    }
 }

src/test/util/net.cpp

@@ -71,6 +71,15 @@ void ConnmanTestMsg::Handshake(CNode& node,
     }
 }
 
+void ConnmanTestMsg::ResetAddrCache() { m_addr_response_caches = {}; }
+
+void ConnmanTestMsg::ResetMaxOutboundCycle()
+{
+    LOCK(m_total_bytes_sent_mutex);
+    nMaxOutboundCycleStartTime = 0s;
+    nMaxOutboundTotalBytesSentInCycle = 0;
+}
+
 void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, std::span<const uint8_t> msg_bytes, bool& complete) const
 {
     assert(node.ReceiveMsgBytes(msg_bytes, complete));

src/test/util/net.h

@@ -45,6 +45,9 @@ struct ConnmanTestMsg : public CConnman {
         m_peer_connect_timeout = timeout;
     }
 
+    void ResetAddrCache();
+    void ResetMaxOutboundCycle();
+
     std::vector<CNode*> TestNodes()
     {
         LOCK(m_nodes_mutex);

src/test/util/validation.cpp

@@ -9,6 +9,15 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+void TestChainstateManager::DisableNextWrite()
+{
+    struct TestChainstate : public Chainstate {
+        void ResetNextWrite() { m_next_write = NodeClock::time_point::max() - 1s; }
+    };
+    for (auto* cs : GetAll()) {
+        static_cast<TestChainstate*>(cs)->ResetNextWrite();
+    }
+}
 void TestChainstateManager::ResetIbd()
 {
     m_cached_finished_ibd = false;

src/test/util/validation.h

@@ -10,6 +10,8 @@
 class CValidationInterface;
 
 struct TestChainstateManager : public ChainstateManager {
+    /** Disable the next write of all chainstates */
+    void DisableNextWrite();
     /** Reset the ibd cache to its initial state */
     void ResetIbd();
     /** Toggle IsInitialBlockDownload from true to false */

src/validation.h

@@ -785,7 +785,7 @@ public:
         return m_mempool ? &m_mempool->cs : nullptr;
     }
 
-private:
+protected:
     bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
     bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);