validation: mark blocks building on an invalid block as BLOCK_FAILED_CHILD

Without doing so, header-only chains building on a chain that
will be marked as invalid would still be eligible for m_best_header.
This improves both getblockchaininfo and getchaintips behavior.

While this adds an iteration over the entire block index, it can only be
triggered by the user (invalidateblock) or by others at a cost (the
header needs to be accepted in the first place, so it needs valid PoW).

Co-authored-by: TheCharlatan <seb.kung@gmail.com>

src/validation.cpp

@@ -2045,6 +2045,7 @@ void Chainstate::InvalidChainFound(CBlockIndex* pindexNew)
     if (!m_chainman.m_best_invalid || pindexNew->nChainWork > m_chainman.m_best_invalid->nChainWork) {
         m_chainman.m_best_invalid = pindexNew;
     }
+    SetBlockFailureFlags(pindexNew);
     if (m_chainman.m_best_header != nullptr && m_chainman.m_best_header->GetAncestor(pindexNew->nHeight) == pindexNew) {
         m_chainman.RecalculateBestHeader();
     }
@@ -3785,6 +3786,17 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
     return true;
 }
 
+void Chainstate::SetBlockFailureFlags(CBlockIndex* invalid_block)
+{
+    AssertLockHeld(cs_main);
+
+    for (auto& [_, block_index] : m_blockman.m_block_index) {
+        if (block_index.GetAncestor(invalid_block->nHeight) == invalid_block && !(block_index.nStatus & BLOCK_FAILED_MASK)) {
+            block_index.nStatus |= BLOCK_FAILED_CHILD;
+        }
+    }
+}
+
 void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
     AssertLockHeld(cs_main);
 

src/validation.h

@@ -735,6 +735,9 @@ public:
         EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex)
         LOCKS_EXCLUDED(::cs_main);
 
+    /** Set invalidity status to all descendants of a block */
+    void SetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
     /** Remove invalidity status from a block and its descendants. */
     void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 

test/functional/rpc_invalidateblock.py

@@ -6,6 +6,10 @@
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR
+from test_framework.blocktools import (
+    create_block,
+    create_coinbase,
+)
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
@@ -35,15 +39,34 @@ class InvalidateTest(BitcoinTestFramework):
         self.connect_nodes(0, 1)
         self.sync_blocks(self.nodes[0:2])
         assert_equal(self.nodes[0].getblockcount(), 6)
-        badhash = self.nodes[1].getblockhash(2)
+
+        # Add a header to the tip of node 0 without submitting the block. This shouldn't
+        # affect results since this chain will be invalidated next.
+        tip = self.nodes[0].getbestblockhash()
+        block_time = self.nodes[0].getblock(self.nodes[0].getbestblockhash())['time'] + 1
+        block = create_block(int(tip, 16), create_coinbase(self.nodes[0].getblockcount()), block_time, version=4)
+        block.solve()
+        self.nodes[0].submitheader(block.serialize().hex())
+        assert_equal(self.nodes[0].getblockchaininfo()["headers"], self.nodes[0].getblockchaininfo()["blocks"] + 1)
 
         self.log.info("Invalidate block 2 on node 0 and verify we reorg to node 0's original chain")
+        badhash = self.nodes[1].getblockhash(2)
         self.nodes[0].invalidateblock(badhash)
         assert_equal(self.nodes[0].getblockcount(), 4)
         assert_equal(self.nodes[0].getbestblockhash(), besthash_n0)
         # Should report consistent blockchain info
         assert_equal(self.nodes[0].getblockchaininfo()["headers"], self.nodes[0].getblockchaininfo()["blocks"])
 
+        self.log.info("Reconsider block 6 on node 0 again and verify that the best header is set correctly")
+        self.nodes[0].reconsiderblock(tip)
+        assert_equal(self.nodes[0].getblockchaininfo()["headers"], self.nodes[0].getblockchaininfo()["blocks"] + 1)
+
+        self.log.info("Invalidate block 2 on node 0 and verify we reorg to node 0's original chain again")
+        self.nodes[0].invalidateblock(badhash)
+        assert_equal(self.nodes[0].getblockcount(), 4)
+        assert_equal(self.nodes[0].getbestblockhash(), besthash_n0)
+        assert_equal(self.nodes[0].getblockchaininfo()["headers"], self.nodes[0].getblockchaininfo()["blocks"])
+
         self.log.info("Make sure we won't reorg to a lower work chain:")
         self.connect_nodes(1, 2)
         self.log.info("Sync node 2 to node 1 so both have 6 blocks")