Merge bitcoin/bitcoin#31696: test: Check that reindex with prune wipes blk files

fa9aced test: Check that reindex with prune wipes blk files (MarcoFalke)
fa9593e test: Use high-level python types (MarcoFalke)

Pull request description:

  This adds missing test coverage for `CleanupBlockRevFiles`.

ACKs for top commit:
  TheCharlatan:
    Re-ACK fa9aced
  l0rinc:
    ACK fa9aced
  tdb3:
    re ACK fa9aced

Tree-SHA512: b31ff8a896ce344437715e7fb7efdb8cd7e11470e8465d8972fddfdb58ffd78257786c4060e8596cc53b6278f8ac6a9b6eb05a06e9df58b8b240bdaa719a8e5b

Author: fanquake

test/functional/feature_remove_pruned_files_on_startup.py

@@ -1,12 +1,13 @@
 #!/usr/bin/env python3
-# Copyright (c) 2022 The Bitcoin Core developers
+# Copyright (c) 2022-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test removing undeleted pruned blk files on startup."""
+"""Tests around pruning rev and blk files on startup."""
 
 import platform
-import os
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
 
 class FeatureRemovePrunedFilesOnStartupTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -18,38 +19,54 @@ def mine_batches(self, blocks):
         for _ in range(n):
             self.generate(self.nodes[0], 250)
         self.generate(self.nodes[0], blocks % 250)
-        self.sync_blocks()
 
     def run_test(self):
         blk0 = self.nodes[0].blocks_path / "blk00000.dat"
         rev0 = self.nodes[0].blocks_path / "rev00000.dat"
         blk1 = self.nodes[0].blocks_path / "blk00001.dat"
         rev1 = self.nodes[0].blocks_path / "rev00001.dat"
         self.mine_batches(800)
-        fo1 = os.open(blk0, os.O_RDONLY)
-        fo2 = os.open(rev1, os.O_RDONLY)
-        fd1 = os.fdopen(fo1)
-        fd2 = os.fdopen(fo2)
+
+        self.log.info("Open some files to check that this may delay deletion")
+        fd1 = open(blk0, "rb")
+        fd2 = open(rev1, "rb")
         self.nodes[0].pruneblockchain(600)
 
         # Windows systems will not remove files with an open fd
         if platform.system() != 'Windows':
-            assert not os.path.exists(blk0)
-            assert not os.path.exists(rev0)
-            assert not os.path.exists(blk1)
-            assert not os.path.exists(rev1)
+            assert not blk0.exists()
+            assert not rev0.exists()
+            assert not blk1.exists()
+            assert not rev1.exists()
         else:
-            assert os.path.exists(blk0)
-            assert not os.path.exists(rev0)
-            assert not os.path.exists(blk1)
-            assert os.path.exists(rev1)
+            assert blk0.exists()
+            assert not rev0.exists()
+            assert not blk1.exists()
+            assert rev1.exists()
 
-        # Check that the files are removed on restart once the fds are closed
+        self.log.info("Check that the files are removed on restart once the fds are closed")
         fd1.close()
         fd2.close()
         self.restart_node(0)
-        assert not os.path.exists(blk0)
-        assert not os.path.exists(rev1)
+        assert not blk0.exists()
+        assert not rev1.exists()
+
+        self.log.info("Check that a reindex will wipe all files")
+
+        def ls_files():
+            ls = [
+                entry.name
+                for entry in self.nodes[0].blocks_path.iterdir()
+                if entry.is_file() and any(map(entry.name.startswith, ["rev", "blk"]))
+            ]
+            return sorted(ls)
+
+        assert_equal(len(ls_files()), 4)
+        self.restart_node(0, extra_args=self.extra_args[0] + ["-reindex"])
+        assert_equal(self.nodes[0].getblockcount(), 0)
+        self.stop_node(0)  # Stop node to flush the two newly created files
+        assert_equal(ls_files(), ["blk00000.dat", "rev00000.dat"])
+
 
 if __name__ == '__main__':
     FeatureRemovePrunedFilesOnStartupTest(__file__).main()