refactor: Add ChainstateManager::ActivateBestChains() method

Deduplicate code looping over chainstate objects and calling
ActivateBestChain() and avoid need for code outside ChainstateManager to use
the GetAll() method.

src/kernel/bitcoinkernel.cpp

@@ -982,13 +982,9 @@ btck_ChainstateManager* btck_chainstate_manager_create(
             LogError("Failed to verify loaded chain state from your datadir: %s", chainstate_err.original);
             return nullptr;
         }
-
+        if (auto result = chainman->ActivateBestChains(); !result) {
-        for (Chainstate* chainstate : WITH_LOCK(chainman->GetMutex(), return chainman->GetAll())) {
+            LogError("%s", util::ErrorString(result).original);
-            BlockValidationState state;
+            return nullptr;
-            if (!chainstate->ActivateBestChain(state, nullptr)) {
-                LogError("Failed to connect best block: %s", state.ToString());
-                return nullptr;
-            }
         }
     } catch (const std::exception& e) {
         LogError("Failed to load chainstate: %s", e.what());

src/node/blockstorage.cpp

@@ -1270,16 +1270,8 @@ void ImportBlocks(ChainstateManager& chainman, std::span<const fs::path> import_
     }
 
     // scan for better chains in the block chain database, that are not yet connected in the active best chain
-
+    if (auto result = chainman.ActivateBestChains(); !result) {
-    // We can't hold cs_main during ActivateBestChain even though we're accessing
+        chainman.GetNotifications().fatalError(util::ErrorString(result));
-    // the chainman unique_ptrs since ABC requires us not to be holding cs_main, so retrieve
-    // the relevant pointers before the ABC call.
-    for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) {
-        BlockValidationState state;
-        if (!chainstate->ActivateBestChain(state, nullptr)) {
-            chainman.GetNotifications().fatalError(strprintf(_("Failed to connect best block (%s)."), state.ToString()));
-            return;
-        }
     }
     // End scope of ImportingNow
 }

src/validation.cpp

@@ -5137,16 +5137,8 @@ void ChainstateManager::LoadExternalBlockFile(
                     // until after all of the block files are loaded. ActivateBestChain can be
                     // called by concurrent network message processing. but, that is not
                     // reliable for the purpose of pruning while importing.
-                    bool activation_failure = false;
+                    if (auto result{ActivateBestChains()}; !result) {
-                    for (auto c : GetAll()) {
+                        LogDebug(BCLog::REINDEX, "%s\n", util::ErrorString(result).original);
-                        BlockValidationState state;
-                        if (!c->ActivateBestChain(state, pblock)) {
-                            LogDebug(BCLog::REINDEX, "failed to activate chain (%s)\n", state.ToString());
-                            activation_failure = true;
-                            break;
-                        }
-                    }
-                    if (activation_failure) {
                         break;
                     }
                 }
@@ -6446,3 +6438,19 @@ std::optional<std::pair<const CBlockIndex*, const CBlockIndex*>> ChainstateManag
     if (!chainstate) return {};
     return std::make_pair(chainstate->m_chain.Tip(), chainstate->TargetBlock());
 }
+
+util::Result<void> ChainstateManager::ActivateBestChains()
+{
+    // We can't hold cs_main during ActivateBestChain even though we're accessing
+    // the chainman unique_ptrs since ABC requires us not to be holding cs_main, so retrieve
+    // the relevant pointers before the ABC call.
+    AssertLockNotHeld(cs_main);
+    for (Chainstate* chainstate : GetAll()) {
+        BlockValidationState state;
+        if (!chainstate->ActivateBestChain(state, nullptr)) {
+            LOCK(GetMutex());
+            return util::Error{Untranslated(strprintf("%s Failed to connect best block (%s)", chainstate->ToString(), state.ToString()))};
+        }
+    }
+    return {};
+}

src/validation.h

@@ -1337,6 +1337,9 @@ public:
     //! Get range of historical blocks to download.
     std::optional<std::pair<const CBlockIndex*, const CBlockIndex*>> GetHistoricalBlockRange() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    //! Call ActivateBestChain() on every chainstate.
+    util::Result<void> ActivateBestChains() LOCKS_EXCLUDED(::cs_main);
+
     //! If, due to invalidation / reconsideration of blocks, the previous
     //! best header is no longer valid / guaranteed to be the most-work
     //! header in our block-index not known to be invalid, recalculate it.