Repository: bitcoin/bitcoin
File: src/init.cpp (C++)

Code before:
        .notifications = *node.notifications,
        .signals = node.validation_signals.get(),
    };
    Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction

    BlockManager::Options blockman_opts{
        .chainparams = chainman_opts.chainparams,
        .blocks_dir = args.GetBlocksDirPath(),
        .notifications = chainman_opts.notifications,
        .block_tree_db_params = DBParams {
            .path = args.GetDataDirNet()  / "blocks" / "index",
            .cache_bytes = static_cast<size_t>(cache_sizes.block_tree_db),
            .wipe_data = do_reindex,
        },
    };
    Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction

    // Creating the chainstate manager internally creates a BlockManager, opens
    // the blocks tree db, and wipes existing block files in case of a reindex.
    // The coinsdb is opened at a later point on LoadChainstate.
    try {
        node.chainman = std::make_unique<ChainstateManager>(*Assert(node.shutdown_signal), chainman_opts, blockman_opts);
    } catch (dbwrapper_error& e) {
        LogError("%s\n", e.what());
        return {ChainstateLoadStatus::FAILURE, _("Error opening block database")};
    } catch (std::exception& e) {
        return {ChainstateLoadStatus::FAILURE_FATAL, strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what())};
    }
    ChainstateManager& chainman = *node.chainman;
    if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}};

    // This is defined and set here instead of inline in validation.h to avoid a hard
    // dependency between validation and index/base, since the latter is not in
    // libbitcoinkernel.
    chainman.snapshot_download_completed = [&node]() {
        if (!node.chainman->m_blockman.IsPruneMode()) {
            LogPrintf("[snapshot] re-enabling NODE_NETWORK services\n");
            node.connman->AddLocalServices(NODE_NETWORK);
        }
        LogPrintf("[snapshot] restarting indexes\n");
        // Drain the validation interface queue to ensure that the old indexes
        // don't have any pending work.
        Assert(node.validation_signals)->SyncWithValidationInterfaceQueue();
        for (auto* index : node.indexes) {
            index->Interrupt();
            index->Stop();
            if (!(index->Init() && index->StartBackgroundSync())) {
                LogPrintf("[snapshot] WARNING failed to restart index %s on snapshot chain\n", index->GetName());
            }
        }
    };

Review comment (refactor):
In commit "refactor: Remove redundant reindex check" (fd590d5ca1cfb6c2525b7755edb6a3a2cf16c856)

Curious about something: In https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2397922175 you wrote,

> Yes, my eventual goal was to move calling `InitializeChainstate` for the fully validated chainstate into the `ChainstateManager` constructor as well as calling `InitCoinsDB` for it. I wanted to do this in a future pull request if this change is accepted.

So if coins db were intialized in the chainstate constructor, then this dbwrapper_error exception could mean there is an error either from the coins db or the blocks db, not just the blocks db as stated in the current error message.

For this reason, and in any case, it seems fragile for this code to have to know what ChainstateManager constructor is doing internally and what exceptions that code might throw. I think it might be better if instead of letting dbwrapper_error be thrown from the ChainstateManager constructor, code in that constructor or code in the BlockStorage constructor would catch that exception and throw a more generic exception that this code could handle without needing to know about ChainstateManager internals.

Maybe a good candidate for a generic exception type could be the existing `ChainstateLoadResult` type since it can hold an error message and also indicate whether the error is fatal or not (i.e. whether a reindex should be attempted or not).

Code after:
        .datadir = args.GetDataDirNet(),
        .notifications = *node.notifications,
        .signals = node.validation_signals.get(),
    };
    Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction

    BlockManager::Options blockman_opts{
        .chainparams = chainman_opts.chainparams,
        .blocks_dir = args.GetBlocksDirPath(),
        .notifications = chainman_opts.notifications,
        .block_tree_db_params = DBParams{
            .path = args.GetDataDirNet() / "blocks" / "index",
            .cache_bytes = cache_sizes.block_tree_db,
            .wipe_data = do_reindex,
        },
    };
    Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction

    // Creating the chainstate manager internally creates a BlockManager, opens
    // the blocks tree db, and wipes existing block files in case of a reindex.
    // The coinsdb is opened at a later point on LoadChainstate.
    try {
        node.chainman = std::make_unique<ChainstateManager>(*Assert(node.shutdown_signal), chainman_opts, blockman_opts);
    } catch (dbwrapper_error& e) {
        LogError("%s", e.what());
        return {ChainstateLoadStatus::FAILURE, _("Error opening block database")};
    } catch (std::exception& e) {
        return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated(strprintf("Failed to initialize ChainstateManager: %s", e.what()))};
    }
    ChainstateManager& chainman = *node.chainman;
    if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}};

    // This is defined and set here instead of inline in validation.h to avoid a hard
    // dependency between validation and index/base, since the latter is not in
    // libbitcoinkernel.
    chainman.snapshot_download_completed = [&node]() {
        if (!node.chainman->m_blockman.IsPruneMode()) {
            LogPrintf("[snapshot] re-enabling NODE_NETWORK services\n");
            node.connman->AddLocalServices(NODE_NETWORK);
        }
        LogPrintf("[snapshot] restarting indexes\n");
        // Drain the validation interface queue to ensure that the old indexes
        // don't have any pending work.
        Assert(node.validation_signals)->SyncWithValidationInterfaceQueue();
        for (auto* index : node.indexes) {
            index->Interrupt();
            index->Stop();
            if (!(index->Init() && index->StartBackgroundSync())) {
                LogPrintf("[snapshot] WARNING failed to restart index %s on snapshot chain\n", index->GetName());
            }
        }