fix: follow-up changes for #7451 [bitcoin#25308 and related changes]

src/bitcoin-chainstate.cpp

@@ -101,32 +101,25 @@ int main(int argc, char* argv[])
 
     std::unique_ptr<LLMQContext> llmq_ctx;
     std::unique_ptr<CChainstateHelper> chain_helper;
+
     node::CacheSizes cache_sizes;
     cache_sizes.block_tree_db = 2 << 20;
     cache_sizes.coins_db = 2 << 22;
     cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
     node::ChainstateLoadOptions options;
-    options.bls_threads = 1;
-    options.worker_count = 1;
-    options.max_recsigs_age = 1;
+    options.mn_metaman = &metaman;
+    options.sporkman = &sporkman;
+    options.chainlocks = &chainlocks;
+    options.mn_sync = &mn_sync;
+    options.data_dir = gArgs.GetDataDirNet();
     options.check_interrupt = [] { return false; };
-    auto [status, error] = node::LoadChainstate(chainman,
-                                   metaman,
-                                   sporkman,
-                                   chainlocks,
-                                   mn_sync,
-                                   chain_helper,
-                                   dmnman,
-                                   evodb,
-                                   llmq_ctx,
-                                   gArgs.GetDataDirNet(),
-                                   cache_sizes,
-                                   options);
+    options.coins_error_cb = [] {};
+    auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options, evodb, dmnman, llmq_ctx, chain_helper);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
         std::cerr << "Failed to load Chain state from your datadir." << std::endl;
         goto epilogue;
     } else {
-        std::tie(status, error) = node::VerifyLoadedChainstate(chainman, *evodb, options);
+        std::tie(status, error) = node::VerifyLoadedChainstate(chainman, options, *evodb);
         if (status != node::ChainstateLoadStatus::SUCCESS) {
             std::cerr << "Failed to verify loaded Chain state from your datadir." << std::endl;
             goto epilogue;
@@ -283,6 +276,8 @@ int main(int argc, char* argv[])
     }
     GetMainSignals().UnregisterBackgroundSignalScheduler();
     // Tear down Dash kernel objects before kernel::~Context().
-    node::DashChainstateSetupClose(chain_helper, dmnman, llmq_ctx, /*mempool=*/nullptr);
+    chain_helper.reset();
+    llmq_ctx.reset();
+    dmnman.reset();
     evodb.reset();
 }

src/init.cpp

@@ -158,7 +158,6 @@ using kernel::DumpMempool;
 
 using node::CacheSizes;
 using node::CalculateCacheSizes;
-using node::DashChainstateSetupClose;
 using node::DEFAULT_PERSIST_MEMPOOL;
 using node::DEFAULT_PRINTPRIORITY;
 using node::DEFAULT_STOPAFTERBLOCKIMPORT;
@@ -430,8 +429,12 @@ void PrepareShutdown(NodeContext& node)
                 chainstate->ResetCoinsViews();
             }
         }
-        DashChainstateSetupClose(node.chain_helper, node.dmnman, node.llmq_ctx,
-                                 Assert(node.mempool.get()));
+        if (node.mempool) {
+            node.mempool->DisconnectManagers();
+        }
+        node.chain_helper.reset();
+        node.llmq_ctx.reset();
+        node.dmnman.reset();
         node.evodb.reset();
     }
     for (const auto& client : node.chain_clients) {
@@ -2021,10 +2024,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
          */
         node.mn_sync = std::make_unique<CMasternodeSync>(std::make_unique<NodeSyncNotifierImpl>(*node.connman, *node.netfulfilledman));
 
-        bilingual_str strLoadError;
-
         node::ChainstateLoadOptions options;
         options.mempool = Assert(node.mempool.get());
+        options.mn_metaman = Assert(node.mn_metaman.get());
+        options.sporkman = Assert(node.sporkman.get());
+        options.chainlocks = Assert(node.chainlocks.get());
+        options.mn_sync = Assert(node.mn_sync.get());
+        options.data_dir = args.GetDataDirNet();
         options.reindex = node::fReindex;
         options.reindex_chainstate = fReindexChainState;
         options.prune = node::fPruneMode;
@@ -2039,7 +2045,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             const int64_t adjusted_threads = std::clamp<int64_t>(threads, 1, int64_t{llmq::MAX_BLSCHECK_THREADS} + 1) - 1;
             return static_cast<int8_t>(adjusted_threads);
         }();
-        options.worker_count = llmq::DEFAULT_WORKER_COUNT;
         options.max_recsigs_age = args.GetIntArg("-maxrecsigsage", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE);
         options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
         options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
@@ -2049,59 +2054,43 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                 _("Error reading from database, shutting down."),
                 "", CClientUIInterface::MSG_ERROR);
         };
-        options.notify_bls_state = [](bool bls_state) {
-            LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls_state);
-        };
 
         uiInterface.InitMessage(_("Loading block index…").translated);
         const auto load_block_index_start_time{SteadyClock::now()};
-        auto catch_exceptions = [](auto&& fn) -> node::ChainstateLoadResult {
+        auto catch_exceptions = [](auto&& f) {
             try {
-                return fn();
+                return f();
             } catch (const std::exception& e) {
                 LogPrintf("%s\n", e.what());
-                return {node::ChainstateLoadStatus::FAILURE, _("Error opening block database")};
+                return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database"));
             }
         };
-        auto [status, error] = catch_exceptions([&] {
-            return LoadChainstate(chainman,
-                                  *node.mn_metaman,
-                                  *node.sporkman,
-                                  *node.chainlocks,
-                                  *node.mn_sync,
-                                  node.chain_helper,
-                                  node.dmnman,
-                                  node.evodb,
-                                  node.llmq_ctx,
-                                  args.GetDataDirNet(),
-                                  cache_sizes,
-                                  options);
-        });
+        auto [status, error] = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options, node.evodb, node.dmnman, node.llmq_ctx, node.chain_helper); });
         if (status == node::ChainstateLoadStatus::SUCCESS) {
             uiInterface.InitMessage(_("Verifying blocks…").translated);
             if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
                 LogWarning("pruned datadir may not have more than %d blocks; only checking available blocks\n",
                            MIN_BLOCKS_TO_KEEP);
             }
-            std::tie(status, error) = catch_exceptions([&] {
-                return VerifyLoadedChainstate(chainman, *Assert(node.evodb), options);
-            });
+            std::tie(status, error) = catch_exceptions([&]{ return VerifyLoadedChainstate(chainman, options, *Assert(node.evodb), [](bool bls_state) {
+                        LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls_state);
+                        });});
+            if (status == node::ChainstateLoadStatus::SUCCESS) {
+                fLoaded = true;
+                LogPrintf(" block index %15dms\n", Ticks<std::chrono::milliseconds>(SteadyClock::now() - load_block_index_start_time));
+            }
         }
-        if (status == node::ChainstateLoadStatus::SUCCESS) {
-            fLoaded = true;
-            LogPrintf(" block index %15dms\n", Ticks<std::chrono::milliseconds>(SteadyClock::now() - load_block_index_start_time));
-        } else if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {
+
+        if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {
             return InitError(error);
-        } else {
-            strLoadError = error;
         }
 
         if (!fLoaded && !ShutdownRequested()) {
             // first suggest a reindex
             if (!options.reindex) {
                 bool fRet = uiInterface.ThreadSafeQuestion(
-                    strLoadError + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
-                    strLoadError.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
+                    error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
+                    error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
                     "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT);
                 if (fRet) {
                     fReindex = true;
@@ -2111,7 +2100,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                     return false;
                 }
             } else {
-                return InitError(strLoadError);
+                return InitError(error);
             }
         }
     }

src/llmq/options.h

@@ -27,6 +27,9 @@ enum class QvvecSyncMode : int8_t {
     OnlyIfTypeMember = 1,
 };
 
+// Keep recovered signatures for a week. This is a "-maxrecsigsage" option default.
+static constexpr int64_t DEFAULT_MAX_RECOVERED_SIGS_AGE{60 * 60 * 24 * 7};
+
 /** -llmq-data-recovery default */
 static constexpr bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY{true};
 /** -watchquorums default, if true, we will connect to all new quorums and watch their communication */

src/llmq/signing.h

@@ -42,9 +42,6 @@ class CQuorumManager;
 class CSigSharesManager;
 class SignHash;
 
-// Keep recovered signatures for a week. This is a "-maxrecsigsage" option default.
-static constexpr int64_t DEFAULT_MAX_RECOVERED_SIGS_AGE{60 * 60 * 24 * 7};
-
 class CSigBase
 {
 protected:

src/node/chainstate.cpp

@@ -35,26 +35,30 @@
 #include <vector>
 
 namespace node {
-ChainstateLoadResult LoadChainstate(ChainstateManager& chainman,
-                                                     CMasternodeMetaMan& mn_metaman,
-                                                     CSporkManager& sporkman,
-                                                     chainlock::Chainlocks& chainlocks,
-                                                     const CMasternodeSync& mn_sync,
-                                                     std::unique_ptr<CChainstateHelper>& chain_helper,
-                                                     std::unique_ptr<CDeterministicMNManager>& dmnman,
-                                                     std::unique_ptr<CEvoDB>& evodb,
-                                                     std::unique_ptr<LLMQContext>& llmq_ctx,
-                                                     const fs::path& data_dir,
-                                                     const CacheSizes& cache_sizes,
-                                                     const ChainstateLoadOptions& options)
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
+                                    const ChainstateLoadOptions& options, std::unique_ptr<CEvoDB>& evodb,
+                                    std::unique_ptr<CDeterministicMNManager>& dmnman, std::unique_ptr<LLMQContext>& llmq_ctx,
+                                    std::unique_ptr<CChainstateHelper>& chain_helper)
 {
+    assert(options.mn_metaman);
+    assert(options.sporkman);
+    assert(options.chainlocks);
+    assert(options.mn_sync);
+
+    const bool to_wipe_data = options.reindex || options.reindex_chainstate;
     auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return to_wipe_data || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
     LOCK(cs_main);
+
     evodb.reset();
-    evodb = std::make_unique<CEvoDB>(util::DbWrapperParams{.path = data_dir, .memory = options.dash_dbs_in_memory, .wipe = options.reindex || options.reindex_chainstate});
+    // TODO: pass DbWrapperParams as options instead multiple params
+    evodb = std::make_unique<CEvoDB>(util::DbWrapperParams{.path = options.data_dir, .memory = options.dash_dbs_in_memory, .wipe = to_wipe_data});
+
+    dmnman.reset();
+    dmnman = std::make_unique<CDeterministicMNManager>(*evodb, *options.mn_metaman);
+
     chainman.m_total_coinstip_cache = cache_sizes.coins;
     chainman.m_total_coinsdb_cache = cache_sizes.coins_db;
 
@@ -70,10 +74,20 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman,
     pblocktree.reset();
     pblocktree.reset(new CBlockTreeDB(cache_sizes.block_tree_db, options.block_tree_db_in_memory, options.reindex));
 
-    DashChainstateSetup(chainman, mn_metaman, sporkman, chainlocks, mn_sync, chain_helper,
-                        dmnman, *evodb, llmq_ctx, options.mempool, data_dir, options.dash_dbs_in_memory,
-                        /*llmq_dbs_wipe=*/options.reindex || options.reindex_chainstate, options.bls_threads, options.worker_count,
-                        options.max_recsigs_age);
+    // Initialize llmq_ctx and connection to mempool
+    llmq_ctx.reset();
+    llmq_ctx = std::make_unique<LLMQContext>(*dmnman, *evodb, *options.sporkman, chainman,
+                                             util::DbWrapperParams{.path = options.data_dir, .memory = options.dash_dbs_in_memory, .wipe = to_wipe_data},
+                                             options.bls_threads, options.worker_count, options.max_recsigs_age);
+    if (options.mempool) {
+        options.mempool->ConnectManagers(dmnman.get(), llmq_ctx->isman.get());
+    }
+
+    // Initialize chain_helper
+    chain_helper.reset();
+    chain_helper = std::make_unique<CChainstateHelper>(*evodb, *dmnman, *options.mn_sync, *(llmq_ctx->isman), *(llmq_ctx->quorum_block_processor),
+                                                       *(llmq_ctx->qsnapman), chainman, chainman.GetConsensus(), *options.chainlocks,
+                                                       *(llmq_ctx->qman));
 
     if (options.reindex) {
         pblocktree->WriteReindexing(true);
@@ -96,6 +110,8 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman,
 
     if (!chainman.BlockIndex().empty() &&
             !chainman.m_blockman.LookupBlockIndex(chainman.GetConsensus().hashGenesisBlock)) {
+        // If the loaded chain has a wrong genesis, bail out immediately
+        // (we're likely using a testnet datadir, or the other way around).
         return {ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB, _("Incorrect or no genesis block found. Wrong datadir for network?")};
     }
 
@@ -190,56 +206,8 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman,
     return {ChainstateLoadStatus::SUCCESS, {}};
 }
 
-void DashChainstateSetup(ChainstateManager& chainman,
-                         CMasternodeMetaMan& mn_metaman,
-                         CSporkManager& sporkman,
-                         chainlock::Chainlocks& chainlocks,
-                         const CMasternodeSync& mn_sync,
-                         std::unique_ptr<CChainstateHelper>& chain_helper,
-                         std::unique_ptr<CDeterministicMNManager>& dmnman,
-                         CEvoDB& evodb,
-                         std::unique_ptr<LLMQContext>& llmq_ctx,
-                         CTxMemPool* mempool,
-                         const fs::path& data_dir,
-                         bool llmq_dbs_in_memory,
-                         bool llmq_dbs_wipe,
-                         int8_t bls_threads,
-                         int16_t worker_count,
-                         int64_t max_recsigs_age)
-{
-    // Same logic as pblocktree
-    dmnman.reset();
-    dmnman = std::make_unique<CDeterministicMNManager>(evodb, mn_metaman);
-
-    llmq_ctx.reset();
-    llmq_ctx = std::make_unique<LLMQContext>(*dmnman, evodb, sporkman, chainman,
-                                             util::DbWrapperParams{.path = data_dir, .memory = llmq_dbs_in_memory, .wipe = llmq_dbs_wipe},
-                                             bls_threads, worker_count, max_recsigs_age);
-    if (mempool) {
-        mempool->ConnectManagers(dmnman.get(), llmq_ctx->isman.get());
-    }
-    chain_helper.reset();
-    chain_helper = std::make_unique<CChainstateHelper>(evodb, *dmnman, mn_sync, *(llmq_ctx->isman), *(llmq_ctx->quorum_block_processor),
-                                                       *(llmq_ctx->qsnapman), chainman, chainman.GetConsensus(), chainlocks,
-                                                       *(llmq_ctx->qman));
-}
-
-void DashChainstateSetupClose(std::unique_ptr<CChainstateHelper>& chain_helper,
-                              std::unique_ptr<CDeterministicMNManager>& dmnman,
-                              std::unique_ptr<LLMQContext>& llmq_ctx,
-                              CTxMemPool* mempool)
-
-{
-    chain_helper.reset();
-    llmq_ctx.reset();
-    if (mempool) {
-        mempool->DisconnectManagers();
-    }
-    dmnman.reset();
-}
-
-ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, CEvoDB& evodb,
-                                            const ChainstateLoadOptions& options)
+ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options, CEvoDB& evodb,
+                                            std::function<void(bool)> notify_bls_state)
 {
     auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
@@ -258,7 +226,8 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, CEvoDB&
             const bool v19active{DeploymentActiveAfter(tip, chainman, Consensus::DEPLOYMENT_V19)};
             if (v19active) {
                 bls::bls_legacy_scheme.store(false);
-                if (options.notify_bls_state) options.notify_bls_state(bls::bls_legacy_scheme.load());
+                // TODO: remove notify_bls_state, it's alien and irrelevant once v19 activated
+                if (notify_bls_state) notify_bls_state(bls::bls_legacy_scheme.load());
             }
 
             if (!CVerifyDB().VerifyDB(
@@ -273,7 +242,7 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, CEvoDB&
             // Make sure we use the right scheme.
             if (v19active && bls::bls_legacy_scheme.load()) {
                 bls::bls_legacy_scheme.store(false);
-                if (options.notify_bls_state) options.notify_bls_state(bls::bls_legacy_scheme.load());
+                if (notify_bls_state) notify_bls_state(bls::bls_legacy_scheme.load());
             }
 
             if (options.check_level >= 3) {