merge bitcoin#27823: return error when block index is non-contiguous, fix feature_init.py file perturbation

Author: kwvg

src/node/blockstorage.cpp

@@ -289,8 +289,13 @@ bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params)
     std::sort(vSortedByHeight.begin(), vSortedByHeight.end(),
               CBlockIndexHeightOnlyComparator());
 
+    CBlockIndex* previous_index{nullptr};
     for (CBlockIndex* pindex : vSortedByHeight) {
         if (ShutdownRequested()) return false;
+        if (previous_index && pindex->nHeight > previous_index->nHeight + 1) {
+            return error("%s: block index is non-contiguous, index of height %d missing", __func__, previous_index->nHeight + 1);
+        }
+        previous_index = pindex;
         pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex);
         pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime);
 

test/functional/feature_init.py

@@ -5,6 +5,7 @@
 """Stress tests related to node initialization."""
 import os
 from pathlib import Path
+import shutil
 
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
 from test_framework.test_node import ErrorMatch
@@ -44,7 +45,7 @@ def sigterm_node():
 
         def start_expecting_error(err_fragment):
             node.assert_start_raises_init_error(
-                extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1'],
+                extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1', '-checkblocks=200', '-checklevel=4'],
                 expected_msg=err_fragment,
                 match=ErrorMatch.PARTIAL_REGEX,
             )
@@ -99,9 +100,9 @@ def check_clean_start():
         }
 
         files_to_perturb = {
-            'blocks/index/*.ldb': 'Error opening block database.',
+            'blocks/index/*.ldb': 'Error loading block database.',
             'chainstate/*.ldb': 'Error opening block database.',
-            'blocks/blk*.dat': 'Error opening block database.',
+            'blocks/blk*.dat': 'Corrupted block database detected.',
         }
 
         for file_patt, err_fragment in files_to_delete.items():
@@ -122,18 +123,31 @@ def check_clean_start():
             check_clean_start()
             self.stop_node(0)
 
+        self.log.info("Test startup errors after perturbing certain essential files")
         for file_patt, err_fragment in files_to_perturb.items():
+            shutil.copytree(node.chain_path / "blocks", node.chain_path / "blocks_bak")
+            shutil.copytree(node.chain_path / "chainstate", node.chain_path / "chainstate_bak")
             target_files = list(node.chain_path.glob(file_patt))
 
             for target_file in target_files:
                 self.log.info(f"Perturbing file to ensure failure {target_file}")
-                with open(target_file, "rb") as tf_read, open(target_file, "wb") as tf_write:
+                with open(target_file, "rb") as tf_read:
                     contents = tf_read.read()
                     tweaked_contents = bytearray(contents)
-                    tweaked_contents[50:250] = b'1' * 200
+                    # Since the genesis block is not checked by -checkblocks, the
+                    # perturbation window must be chosen such that a higher block
+                    # in blk*.dat is affected.
+                    tweaked_contents[150:350] = b'1' * 200
+                with open(target_file, "wb") as tf_write:
                     tf_write.write(bytes(tweaked_contents))
 
             start_expecting_error(err_fragment)
 
+            shutil.rmtree(node.chain_path / "blocks")
+            shutil.rmtree(node.chain_path / "chainstate")
+            shutil.move(node.chain_path / "blocks_bak", node.chain_path / "blocks")
+            shutil.move(node.chain_path / "chainstate_bak", node.chain_path / "chainstate")
+
+
 if __name__ == '__main__':
     InitStressTest().main()