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

Code before:
    // Header is valid/has work, merkle tree and segwit merkle tree are good...RELAY NOW
    // (but if it does not build on our best tip, let the SendMessages loop relay it)
    if (!IsInitialBlockDownload() && ActiveTip() == pindex->pprev && m_options.signals) {
        m_options.signals->NewPoWValidBlock(pindex, pblock);
    }

    // Write block to history file
    if (fNewBlock) *fNewBlock = true;
    try {
        FlatFilePos blockPos{};
        if (dbp) {
            blockPos = *dbp;
            m_blockman.UpdateBlockInfo(block, pindex->nHeight, blockPos);
        } else {
            blockPos = m_blockman.WriteBlock(block, pindex->nHeight);
            if (blockPos.IsNull()) {
                state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__));
                return false;
            }
        }
        ReceivedBlockTransactions(block, pindex, blockPos);
    } catch (const std::runtime_error& e) {
        return FatalError(GetNotifications(), state, strprintf(_("System error while saving block to disk: %s"), e.what()));
    }

    // TODO: FlushStateToDisk() handles flushing of both block and chainstate
    // data, so we should move this to ChainstateManager so that we can be more
    // intelligent about how we flush.
    // For now, since FlushStateMode::NONE is used, all that can happen is that
    // the block files may be pruned, so we can just call this on one
    // chainstate (particularly if we haven't implemented pruning with
    // background validation yet).
    ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE);

    CheckBlockIndex();

    return true;
}

bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock>& block, bool force_processing, bool min_pow_checked, bool* new_block)
{
    AssertLockNotHeld(cs_main);

    {
        CBlockIndex *pindex = nullptr;
        if (new_block) *new_block = false;
        BlockValidationState state;

        // CheckBlock() does not support multi-threaded block validation because CBlock::fChecked can cause data race.
        // Therefore, the following critical section must include the CheckBlock() call as well.
        LOCK(cs_main);

Review comment (bug):
In commit "validation: call InvalidChainfound also from AcceptBlock" (65de18836e5ddbdb8388e3bbdbcebee84c3b4c59):

Commit description mentions "at the cost of looping over the entire block index" which is referring to the loop in `Chainstate::SetBlockFailureFlags`. Probably not important in this context, but that function does seem like it could trigger unnecessary work because it is [adding blocks to m_dirty_blockindex](https://github.com/bitcoin/bitcoin/blob/1c6602399be6de0148938a3fd7b51e6c48b985d2/src/validation.cpp#L3818) regardless of whether flags have actually changed, which is different than most of the other code updating `m_dirty_blockindex`. Maybe it doesn't matter because the function has no callers that will mark already invalid blocks invalid, but not checking for changes does seem like a footgun that could lead to unnecessary database writes if the function is called other places.

Just noting in case this could be a potential followup, or in case I'm missing something.

EDIT: It seems like this behavior was changed recently in aac5488909f72f8c5a91ca7f12398069b7cd9ce4 from #31835. Before that commit there was an incorrect check to see if m_dirty_blockindex should be updated, which the commit removed instead of replacing.

Code after:

    const CChainParams& params{GetParams()};

    if (!CheckBlock(block, state, params.GetConsensus()) ||
        !ContextualCheckBlock(block, state, *this, pindex->pprev)) {
        if (Assume(state.IsInvalid())) {
            ActiveChainstate().InvalidBlockFound(pindex, state);
        }
        LogError("%s: %s\n", __func__, state.ToString());
        return false;
    }

    // Header is valid/has work, merkle tree and segwit merkle tree are good...RELAY NOW
    // (but if it does not build on our best tip, let the SendMessages loop relay it)
    if (!IsInitialBlockDownload() && ActiveTip() == pindex->pprev && m_options.signals) {
        m_options.signals->NewPoWValidBlock(pindex, pblock);
    }

    // Write block to history file
    if (fNewBlock) *fNewBlock = true;
    try {
        FlatFilePos blockPos{};
        if (dbp) {
            blockPos = *dbp;
            m_blockman.UpdateBlockInfo(block, pindex->nHeight, blockPos);
        } else {
            blockPos = m_blockman.WriteBlock(block, pindex->nHeight);
            if (blockPos.IsNull()) {
                state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__));
                return false;
            }
        }
        ReceivedBlockTransactions(block, pindex, blockPos);
    } catch (const std::runtime_error& e) {
        return FatalError(GetNotifications(), state, strprintf(_("System error while saving block to disk: %s"), e.what()));
    }

    // TODO: FlushStateToDisk() handles flushing of both block and chainstate
    // data, so we should move this to ChainstateManager so that we can be more
    // intelligent about how we flush.
    // For now, since FlushStateMode::NONE is used, all that can happen is that
    // the block files may be pruned, so we can just call this on one
    // chainstate (particularly if we haven't implemented pruning with
    // background validation yet).
    ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE);

    CheckBlockIndex();

    return true;
}