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

Code before:
        // Pruned nodes may have deleted the block.
        if (pindex->nTx > 0) {
            if (pindex->pprev) {
                if (m_snapshot_height && pindex->nHeight == *m_snapshot_height &&
                        pindex->GetBlockHash() == *snapshot_blockhash) {
                    // Should have been set above; don't disturb it with code below.
                    Assert(pindex->m_chain_tx_count > 0);
                } else if (pindex->pprev->m_chain_tx_count > 0) {
                    pindex->m_chain_tx_count = pindex->pprev->m_chain_tx_count + pindex->nTx;
                } else {
                    pindex->m_chain_tx_count = 0;
                    m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex));
                }
            } else {
                pindex->m_chain_tx_count = pindex->nTx;
            }
        }

        if (pindex->nStatus & BLOCK_FAILED_CHILD) {
            // BLOCK_FAILED_CHILD is deprecated, but may still exist on disk. Replace it with BLOCK_FAILED_VALID.
            pindex->nStatus = (pindex->nStatus & ~BLOCK_FAILED_CHILD) | BLOCK_FAILED_VALID;
            m_dirty_blockindex.insert(pindex);
        }
        if (!(pindex->nStatus & BLOCK_FAILED_VALID) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_VALID)) {
            // All descendants of invalid blocks are invalid too.
            pindex->nStatus = BLOCK_FAILED_VALID;
            m_dirty_blockindex.insert(pindex);
        }

        if (pindex->pprev) {
            pindex->BuildSkip();
        }
    }

    return true;
}

void BlockManager::WriteBlockIndexDB()
{
    AssertLockHeld(::cs_main);
    std::vector<std::pair<int, const CBlockFileInfo*>> vFiles;
    vFiles.reserve(m_dirty_fileinfo.size());
    for (std::set<int>::iterator it = m_dirty_fileinfo.begin(); it != m_dirty_fileinfo.end();) {
        vFiles.emplace_back(*it, &m_blockfile_info[*it]);
        m_dirty_fileinfo.erase(it++);
    }
    std::vector<const CBlockIndex*> vBlocks;
    vBlocks.reserve(m_dirty_blockindex.size());
    for (std::set<CBlockIndex*>::iterator it = m_dirty_blockindex.begin(); it != m_dirty_blockindex.end();) {
        vBlocks.push_back(*it);
        m_dirty_blockindex.erase(it++);

Review comment (bug):
I don't think we mean to override the entire `nStatus` here, just bitflip? (sorry for getting this wrong in my [suggestion](https://github.com/bitcoin/bitcoin/pull/32950/changes#r2405502780) earlier)
```suggestion
            pindex->nStatus |= BLOCK_FAILED_VALID;
```

The below test change fails without this fix, and passes with.

<details>
<summary>git diff on 16636cc5b9</summary>

```diff
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 45c72f5af8..218ee222ef 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -495,7 +495,7 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
         }
         if (!(pindex->nStatus & BLOCK_FAILED_VALID) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_VALID)) {
             // All descendants of invalid blocks are invalid too.
-            pindex->nStatus = BLOCK_FAILED_VALID;
+            pindex->nStatus |= BLOCK_FAILED_VALID;
             m_dirty_blockindex.insert(pindex);
         }
 
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 40f99690ce..64aac67ccb 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -601,6 +601,7 @@ BOOST_FIXTURE_TEST_CASE(loadblockindex_invalid_descendants, TestChain100Setup)
     //   - child: not invalid
     // Test that when the block index is loaded, all blocks are marked as BLOCK_FAILED_VALID
     auto* child{m_node.chainman->ActiveChain().Tip()};
+    BOOST_CHECK(child->nStatus & BLOCK_HAVE_DATA);
     auto* parent{child->pprev};
     auto* grand_parent{parent->pprev};
     grand_parent->nStatus = (grand_parent->nStatus | BLOCK_FAILED_VALID);
@@ -614,6 +615,8 @@ BOOST_FIXTURE_TEST_CASE(loadblockindex_invalid_descendants, TestChain100Setup)
     BOOST_CHECK(grand_parent->nStatus & BLOCK_FAILED_VALID);
     BOOST_CHECK(parent->nStatus & BLOCK_FAILED_VALID);
     BOOST_CHECK(child->nStatus & BLOCK_FAILED_VALID);
+
+    BOOST_CHECK(child->nStatus & BLOCK_HAVE_DATA);
 }
 
 //! Ensure that snapshot chainstate can be loaded when found on disk after a

```
</details>


Code after:
        // Pruned nodes may have deleted the block.
        if (pindex->nTx > 0) {
            if (pindex->pprev) {
                if (m_snapshot_height && pindex->nHeight == *m_snapshot_height &&
                        pindex->GetBlockHash() == *snapshot_blockhash) {
                    // Should have been set above; don't disturb it with code below.
                    Assert(pindex->m_chain_tx_count > 0);
                } else if (pindex->pprev->m_chain_tx_count > 0) {
                    pindex->m_chain_tx_count = pindex->pprev->m_chain_tx_count + pindex->nTx;
                } else {
                    pindex->m_chain_tx_count = 0;
                    m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex));
                }
            } else {
                pindex->m_chain_tx_count = pindex->nTx;
            }
        }

        if (pindex->nStatus & BLOCK_FAILED_CHILD) {
            // BLOCK_FAILED_CHILD is deprecated, but may still exist on disk. Replace it with BLOCK_FAILED_VALID.
            pindex->nStatus = (pindex->nStatus & ~BLOCK_FAILED_CHILD) | BLOCK_FAILED_VALID;
            m_dirty_blockindex.insert(pindex);
        }
        if (!(pindex->nStatus & BLOCK_FAILED_VALID) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_VALID)) {
            // All descendants of invalid blocks are invalid too.
            pindex->nStatus |= BLOCK_FAILED_VALID;
            m_dirty_blockindex.insert(pindex);
        }

        if (pindex->pprev) {
            pindex->BuildSkip();
        }
    }

    return true;
}

void BlockManager::WriteBlockIndexDB()
{
    AssertLockHeld(::cs_main);
    std::vector<std::pair<int, const CBlockFileInfo*>> vFiles;
    vFiles.reserve(m_dirty_fileinfo.size());
    for (std::set<int>::iterator it = m_dirty_fileinfo.begin(); it != m_dirty_fileinfo.end();) {
        vFiles.emplace_back(*it, &m_blockfile_info[*it]);
        m_dirty_fileinfo.erase(it++);
    }
    std::vector<const CBlockIndex*> vBlocks;
    vBlocks.reserve(m_dirty_blockindex.size());
    for (std::set<CBlockIndex*>::iterator it = m_dirty_blockindex.begin(); it != m_dirty_blockindex.end();) {
        vBlocks.push_back(*it);
        m_dirty_blockindex.erase(it++);