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):
This code block now duplicates `InvalidBlockFound()`, minus the `setBlockIndex` erasure - but I think that should be a (fast) no-op, perhaps it's worth deduplicating this?

<details>
<summary>git diff on ed172d3002</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 31a0f54c11..3312849024 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4516,11 +4516,8 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock,
 
     if (!CheckBlock(block, state, params.GetConsensus()) ||
         !ContextualCheckBlock(block, state, *this, pindex->pprev)) {
-        if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
-            pindex->nStatus |= BLOCK_FAILED_VALID;
-            m_blockman.m_dirty_blockindex.insert(pindex);
-            ActiveChainstate().InvalidChainFound(pindex);
-        }
+        Assume(state.IsInvalid());
+        ActiveChainstate().InvalidBlockFound(pindex, state);
         LogError("%s: %s\n", __func__, state.ToString());
         return false;
     }

```
</details>

From my (limited) understanding,  think `state.IsInvalid()` should always evaluate to `true` and thus seems like more of an assertion, but it's orthogonal to my suggestion.

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;
}