From ebae59eedfd9abcfbbb3cdb7be97eb730d62e0da Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 15:43:07 +0000
Subject: [PATCH 01/15] fix: make sure we flush our committed best block in
 no-upgrade cases

This was spotted when working on `feature_txindex_compatibility.py`,
attempting to load old databases would fail because `MigrateDBIfNeeded()`
would delete `DB_OLD_BEST_BLOCK`, write `EVODB_BEST_BLOCK`, commit it
but never flush it.

So when `MigrateDBIfNeeded2()` would read it again, it would note the lack
of `DB_OLD_BEST_BLOCK` and conclude it was a failed run and exit. This
is solved by actually flushing our new best block, which would prevent
`MigrateDBIfNeeded2` from raising an objection.
---
 src/evo/deterministicmns.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp
index ac167a298bb3..495ca8a03b36 100644
--- a/src/evo/deterministicmns.cpp
+++ b/src/evo/deterministicmns.cpp
@@ -1238,6 +1238,10 @@ bool CDeterministicMNManager::MigrateDBIfNeeded()
         auto dbTx = m_evoDb.BeginTransaction();
         m_evoDb.WriteBestBlock(m_chainstate.m_chain.Tip()->GetBlockHash());
         dbTx->Commit();
+        if (!m_evoDb.CommitRootTransaction()) {
+            LogPrintf("CDeterministicMNManager::%s -- failed to commit to evoDB\n", __func__);
+            return false;
+        }
         return true;
     }
 
@@ -1349,6 +1353,10 @@ bool CDeterministicMNManager::MigrateDBIfNeeded2()
         auto dbTx = m_evoDb.BeginTransaction();
         m_evoDb.WriteBestBlock(m_chainstate.m_chain.Tip()->GetBlockHash());
         dbTx->Commit();
+        if (!m_evoDb.CommitRootTransaction()) {
+            LogPrintf("CDeterministicMNManager::%s -- failed to commit to evoDB\n", __func__);
+            return false;
+        }
         return true;
     }
 

From b218f123b7c86da0c483d6e54c25468968505c52 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Tue, 15 Oct 2024 06:58:49 +0000
Subject: [PATCH 02/15] merge bitcoin#23046: Add txindex migration test

---
 .../feature_txindex_compatibility.py          | 97 +++++++++++++++++++
 test/functional/test_runner.py                |  1 +
 2 files changed, 98 insertions(+)
 create mode 100755 test/functional/feature_txindex_compatibility.py

diff --git a/test/functional/feature_txindex_compatibility.py b/test/functional/feature_txindex_compatibility.py
new file mode 100755
index 000000000000..f4a8e7cf5537
--- /dev/null
+++ b/test/functional/feature_txindex_compatibility.py
@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 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 that legacy txindex will be disabled on upgrade.
+
+Previous releases are required by this test, see test/README.md.
+"""
+
+import os
+import shutil
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.wallet import MiniWallet
+
+
+class MempoolCompatibilityTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 3
+        self.extra_args = [
+            ["-reindex", "-txindex"],
+            ["-txindex=0"],
+            ["-txindex=0"],
+        ]
+
+    def skip_test_if_missing_module(self):
+        self.skip_if_no_previous_releases()
+
+    def setup_network(self):
+        self.add_nodes(
+            self.num_nodes,
+            self.extra_args,
+            versions=[
+                170003,   # Last release with legacy txindex
+                None,     # For MiniWallet, without migration code
+                18020200, # Any release with migration code (0.18.x - 21.x)
+                          # We are using v18.2.2 to avoid MigrateDBIfNeeded(2) routines as
+                          # they don't handle a no-upgrade case correctly
+            ],
+        )
+        # Delete v18.2.2 cached datadir to avoid making a legacy version try to
+        # make sense of our current database formats
+        shutil.rmtree(os.path.join(self.nodes[2].datadir, self.chain))
+        self.start_nodes()
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 2)
+
+    def run_test(self):
+        mini_wallet = MiniWallet(self.nodes[1])
+        mini_wallet.rescan_utxos()
+        spend_utxo = mini_wallet.get_utxo()
+        mini_wallet.send_self_transfer(from_node=self.nodes[1], utxo_to_spend=spend_utxo)
+        self.generate(self.nodes[1], 1)
+
+        self.log.info("Check legacy txindex")
+        self.nodes[0].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
+
+        self.stop_nodes()
+        legacy_chain_dir = os.path.join(self.nodes[0].datadir, self.chain)
+
+        self.log.info("Migrate legacy txindex")
+        migrate_chain_dir = os.path.join(self.nodes[2].datadir, self.chain)
+        shutil.rmtree(migrate_chain_dir)
+        shutil.copytree(legacy_chain_dir, migrate_chain_dir)
+        with self.nodes[2].assert_debug_log([
+                "Upgrading txindex database...",
+                "txindex is enabled at height 200",
+        ]):
+            self.start_node(2, extra_args=["-txindex"])
+        self.nodes[2].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
+
+        self.log.info("Drop legacy txindex")
+        drop_index_chain_dir = os.path.join(self.nodes[1].datadir, self.chain)
+        shutil.rmtree(drop_index_chain_dir)
+        shutil.copytree(legacy_chain_dir, drop_index_chain_dir)
+        self.nodes[1].assert_start_raises_init_error(
+            extra_args=["-txindex"],
+            expected_msg="Error: The block index db contains a legacy 'txindex'. To clear the occupied disk space, run a full -reindex, otherwise ignore this error. This error message will not be displayed again.",
+        )
+        # Build txindex from scratch and check there is no error this time
+        self.start_node(1, extra_args=["-txindex"])
+        self.nodes[2].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
+
+        self.stop_nodes()
+
+        self.log.info("Check migrated txindex can not be read by legacy node")
+        err_msg = f": You need to rebuild the database using -reindex to change -txindex.{os.linesep}Please restart with -reindex or -reindex-chainstate to recover."
+        shutil.rmtree(legacy_chain_dir)
+        shutil.copytree(migrate_chain_dir, legacy_chain_dir)
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg)
+        shutil.rmtree(legacy_chain_dir)
+        shutil.copytree(drop_index_chain_dir, legacy_chain_dir)
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg)
+
+
+if __name__ == "__main__":
+    MempoolCompatibilityTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 5e0cffdf2d70..90450452367d 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -346,6 +346,7 @@
     'rpc_deriveaddresses.py --usecli',
     'p2p_ping.py',
     'rpc_scantxoutset.py',
+    'feature_txindex_compatibility.py',
     'feature_logging.py',
     'feature_anchors.py',
     'feature_coinstatsindex.py',

From 1caaa85716f526c285c0c1ced91225de46448b6b Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sat, 12 Oct 2024 05:57:17 +0000
Subject: [PATCH 03/15] merge bitcoin#24138: Commit MuHash and best block
 together for coinstatsindex

Continuation of c21f23db from dash#5501
---
 src/test/coinstatsindex_tests.cpp | 42 +++++++++++++++++++++++++++++++
 src/test/util/validation.cpp      |  6 +++++
 src/test/util/validation.h        |  8 ++++++
 src/validationinterface.h         |  1 +
 4 files changed, 57 insertions(+)

diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp
index 6ba80397a31e..653790f7d9d3 100644
--- a/src/test/coinstatsindex_tests.cpp
+++ b/src/test/coinstatsindex_tests.cpp
@@ -2,9 +2,11 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <chainparams.h>
 #include <index/coinstatsindex.h>
 #include <test/util/index.h>
 #include <test/util/setup_common.h>
+#include <test/util/validation.h>
 #include <util/time.h>
 #include <validation.h>
 
@@ -74,4 +76,44 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
     coin_stats_index.Stop();
 }
 
+// Test shutdown between BlockConnected and ChainStateFlushed notifications,
+// make sure index is not corrupted and is able to reload.
+BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
+{
+    CChainState& chainstate = Assert(m_node.chainman)->ActiveChainstate();
+    const CChainParams& params = Params();
+    {
+        CoinStatsIndex index{1 << 20};
+        BOOST_REQUIRE(index.Start(chainstate));
+        IndexWaitSynced(index);
+        std::shared_ptr<const CBlock> new_block;
+        CBlockIndex* new_block_index = nullptr;
+        {
+            const CScript script_pub_key{CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG};
+            const CBlock block = this->CreateBlock({}, script_pub_key, chainstate);
+
+            new_block = std::make_shared<CBlock>(block);
+
+            LOCK(cs_main);
+            BlockValidationState state;
+            BOOST_CHECK(CheckBlock(block, state, params.GetConsensus()));
+            BOOST_CHECK(chainstate.AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr));
+            CCoinsViewCache view(&chainstate.CoinsTip());
+            BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view));
+        }
+        // Send block connected notification, then stop the index without
+        // sending a chainstate flushed notification. Prior to #24138, this
+        // would cause the index to be corrupted and fail to reload.
+        ValidationInterfaceTest::BlockConnected(index, new_block, new_block_index);
+        index.Stop();
+    }
+
+    {
+        CoinStatsIndex index{1 << 20};
+        // Make sure the index can be loaded.
+        BOOST_REQUIRE(index.Start(chainstate));
+        index.Stop();
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp
index 1aed492c3c03..49535855f988 100644
--- a/src/test/util/validation.cpp
+++ b/src/test/util/validation.cpp
@@ -7,6 +7,7 @@
 #include <util/check.h>
 #include <util/time.h>
 #include <validation.h>
+#include <validationinterface.h>
 
 void TestChainState::ResetIbd()
 {
@@ -20,3 +21,8 @@ void TestChainState::JumpOutOfIbd()
     m_cached_finished_ibd = true;
     Assert(!IsInitialBlockDownload());
 }
+
+void ValidationInterfaceTest::BlockConnected(CValidationInterface& obj, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex)
+{
+    obj.BlockConnected(block, pindex);
+}
diff --git a/src/test/util/validation.h b/src/test/util/validation.h
index b13aa0be60d5..b0bc717b6c22 100644
--- a/src/test/util/validation.h
+++ b/src/test/util/validation.h
@@ -7,6 +7,8 @@
 
 #include <validation.h>
 
+class CValidationInterface;
+
 struct TestChainState : public CChainState {
     /** Reset the ibd cache to its initial state */
     void ResetIbd();
@@ -14,4 +16,10 @@ struct TestChainState : public CChainState {
     void JumpOutOfIbd();
 };
 
+class ValidationInterfaceTest
+{
+public:
+    static void BlockConnected(CValidationInterface& obj, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex);
+};
+
 #endif // BITCOIN_TEST_UTIL_VALIDATION_H
diff --git a/src/validationinterface.h b/src/validationinterface.h
index c860f7fb862a..a3b45dcf9d62 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -200,6 +200,7 @@ class CValidationInterface {
      * has been received and connected to the headers tree, though not validated yet */
     virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& block) {};
     friend class CMainSignals;
+    friend class ValidationInterfaceTest;
 };
 
 struct MainSignalsInstance;

From 10203560f5367acd9cd96b4857b8b4a223c73550 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sat, 9 Apr 2022 02:14:02 +0200
Subject: [PATCH 04/15] merge bitcoin#24812: Add CHECK_NONFATAL identity
 function and NONFATAL_UNREACHABLE macro

---
 src/rpc/blockchain.cpp      | 25 +++++++----------
 src/rpc/coinjoin.cpp        | 16 ++++-------
 src/rpc/evo.cpp             | 53 +++++++++++--------------------------
 src/rpc/governance.cpp      | 42 +++++++++++------------------
 src/rpc/masternode.cpp      | 25 +++++++----------
 src/rpc/mining.cpp          | 10 +++----
 src/rpc/misc.cpp            |  3 +--
 src/rpc/quorums.cpp         | 15 ++++++-----
 src/rpc/rawtransaction.cpp  |  5 ++--
 src/rpc/util.cpp            | 15 ++++++-----
 src/util/check.h            | 41 +++++++++++++++++++---------
 src/wallet/rpcdump.cpp      |  2 +-
 test/functional/rpc_misc.py |  2 +-
 13 files changed, 110 insertions(+), 144 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 6bf41d7ba870..94426f7da48b 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -39,6 +39,7 @@
 #include <sync.h>
 #include <txmempool.h>
 #include <undo.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>
@@ -1359,8 +1360,7 @@ static RPCHelpMan pruneblockchain()
     }
 
     PruneBlockFilesManual(active_chainstate, height);
-    const CBlockIndex* block = active_chain.Tip();
-    CHECK_NONFATAL(block);
+    const CBlockIndex* block = CHECK_NONFATAL(active_chain.Tip());
     while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
         block = block->pprev;
     }
@@ -1635,14 +1635,13 @@ static RPCHelpMan verifychain()
     const int check_depth{request.params[1].isNull() ? DEFAULT_CHECKBLOCKS : request.params[1].get_int()};
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    CHECK_NONFATAL(node.evodb);
 
     ChainstateManager& chainman = EnsureChainman(node);
     LOCK(cs_main);
 
     CChainState& active_chainstate = chainman.ActiveChainstate();
     return CVerifyDB().VerifyDB(
-        active_chainstate, Params(), active_chainstate.CoinsTip(), *node.evodb, check_level, check_depth);
+        active_chainstate, Params(), active_chainstate.CoinsTip(), *CHECK_NONFATAL(node.evodb), check_level, check_depth);
 },
     };
 }
@@ -1782,13 +1781,11 @@ RPCHelpMan getblockchaininfo()
 
     LOCK(cs_main);
     CChainState& active_chainstate = chainman.ActiveChainstate();
-    const CBlockIndex* tip = active_chainstate.m_chain.Tip();
 
-    CHECK_NONFATAL(tip);
+    const CBlockIndex* tip = CHECK_NONFATAL(active_chainstate.m_chain.Tip());
     const int height = tip->nHeight;
 
-    CHECK_NONFATAL(node.mnhf_manager);
-    const auto ehfSignals = node.mnhf_manager->GetSignalsStage(tip);
+    const auto ehfSignals = CHECK_NONFATAL(node.mnhf_manager)->GetSignalsStage(tip);
 
     UniValue obj(UniValue::VOBJ);
     if (args.IsArgSet("-devnet")) {
@@ -1808,8 +1805,7 @@ RPCHelpMan getblockchaininfo()
     obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage());
     obj.pushKV("pruned", fPruneMode);
     if (fPruneMode) {
-        const CBlockIndex* block = tip;
-        CHECK_NONFATAL(block);
+        const CBlockIndex* block = CHECK_NONFATAL(tip);
         while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
             block = block->pprev;
         }
@@ -2863,10 +2859,8 @@ static RPCHelpMan scantxoutset()
             LOCK(cs_main);
             CChainState& active_chainstate = chainman.ActiveChainstate();
             active_chainstate.ForceFlushStateToDisk();
-            pcursor = active_chainstate.CoinsDB().Cursor();
-            CHECK_NONFATAL(pcursor);
-            tip = active_chainstate.m_chain.Tip();
-            CHECK_NONFATAL(tip);
+            pcursor = CHECK_NONFATAL(active_chainstate.CoinsDB().Cursor());
+            tip = CHECK_NONFATAL(active_chainstate.m_chain.Tip());
         }
         bool res = FindScriptPubKey(g_scan_progress, g_should_abort_scan, count, pcursor.get(), needles, coins, node.rpc_interruption_point);
         result.pushKV("success", res);
@@ -3061,8 +3055,7 @@ UniValue CreateUTXOSnapshot(NodeContext& node, CChainState& chainstate, CAutoFil
         }
 
         pcursor = chainstate.CoinsDB().Cursor();
-        tip = chainstate.m_blockman.LookupBlockIndex(stats.hashBlock);
-        CHECK_NONFATAL(tip);
+        tip = CHECK_NONFATAL(chainstate.m_blockman.LookupBlockIndex(stats.hashBlock));
     }
 
     SnapshotMetadata metadata{tip->GetBlockHash(), stats.coins_count, tip->nChainTx};
diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp
index 0ce1bffff9a8..8e477ce8ceeb 100644
--- a/src/rpc/coinjoin.cpp
+++ b/src/rpc/coinjoin.cpp
@@ -9,6 +9,7 @@
 #include <rpc/blockchain.h>
 #include <rpc/server.h>
 #include <rpc/server_util.h>
+#include <util/check.h>
 #include <rpc/util.h>
 #include <util/strencodings.h>
 
@@ -83,11 +84,8 @@ static RPCHelpMan coinjoin_reset()
 
     ValidateCoinJoinArguments();
 
-    CHECK_NONFATAL(node.coinjoin_loader);
-    auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
-
-    CHECK_NONFATAL(cj_clientman);
-    cj_clientman->ResetPool();
+    auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->walletman().Get(wallet->GetName());
+    CHECK_NONFATAL(cj_clientman)->ResetPool();
 
     return "Mixing was reset";
 },
@@ -126,10 +124,7 @@ static RPCHelpMan coinjoin_start()
             throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please unlock wallet for mixing with walletpassphrase first.");
     }
 
-    CHECK_NONFATAL(node.coinjoin_loader);
-    auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
-
-    CHECK_NONFATAL(cj_clientman);
+    auto cj_clientman = CHECK_NONFATAL(CHECK_NONFATAL(node.coinjoin_loader)->walletman().Get(wallet->GetName()));
     if (!cj_clientman->StartMixing()) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already.");
     }
@@ -450,8 +445,7 @@ static RPCHelpMan getcoinjoininfo()
         return obj;
     }
 
-    auto manager = node.coinjoin_loader->walletman().Get(wallet->GetName());
-    CHECK_NONFATAL(manager != nullptr);
+    auto* manager = CHECK_NONFATAL(node.coinjoin_loader->walletman().Get(wallet->GetName()));
     manager->GetJsonInfo(obj);
 
     std::string warning_msg{""};
diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp
index 30fdab04b734..b584f888abd8 100644
--- a/src/rpc/evo.cpp
+++ b/src/rpc/evo.cpp
@@ -26,6 +26,7 @@
 #include <rpc/server.h>
 #include <rpc/server_util.h>
 #include <rpc/util.h>
+#include <util/check.h>
 #include <util/moneystr.h>
 #include <util/translation.h>
 #include <validation.h>
@@ -628,8 +629,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request,
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.chain_helper);
-    CChainstateHelper& chain_helper = *node.chain_helper;
+    CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
 
     const bool isEvoRequested = mnType == MnType::Evo;
 
@@ -855,8 +855,7 @@ static RPCHelpMan protx_register_submit()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.chain_helper);
-    CChainstateHelper& chain_helper = *node.chain_helper;
+    CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
 
     std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
     if (!wallet) return NullUniValue;
@@ -954,11 +953,8 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.chain_helper);
-    CChainstateHelper& chain_helper = *node.chain_helper;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
 
     const bool isEvoRequested = mnType == MnType::Evo;
     std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
@@ -1092,11 +1088,8 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.chain_helper);
-    CChainstateHelper& chain_helper = *node.chain_helper;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
 
     std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
     if (!wallet) return NullUniValue;
@@ -1207,12 +1200,8 @@ static RPCHelpMan protx_revoke()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.chain_helper);
-    CChainstateHelper& chain_helper = *node.chain_helper;
-
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    CChainstateHelper& chain_helper = *CHECK_NONFATAL(node.chain_helper);
 
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return NullUniValue;
@@ -1370,11 +1359,8 @@ static RPCHelpMan protx_list()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.mn_metaman);
-    CMasternodeMetaMan& mn_metaman = *node.mn_metaman;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    CMasternodeMetaMan& mn_metaman = *CHECK_NONFATAL(node.mn_metaman);
 
     std::shared_ptr<CWallet> wallet{nullptr};
 #ifdef ENABLE_WALLET
@@ -1486,11 +1472,8 @@ static RPCHelpMan protx_info()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.mn_metaman);
-    CMasternodeMetaMan& mn_metaman = *node.mn_metaman;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    CMasternodeMetaMan& mn_metaman = *CHECK_NONFATAL(node.mn_metaman);
 
     std::shared_ptr<CWallet> wallet{nullptr};
 #ifdef ENABLE_WALLET
@@ -1560,11 +1543,8 @@ static RPCHelpMan protx_diff()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
-
-    CHECK_NONFATAL(node.llmq_ctx);
-    const LLMQContext& llmq_ctx = *node.llmq_ctx;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
+    const LLMQContext& llmq_ctx = *CHECK_NONFATAL(node.llmq_ctx);
 
     LOCK(cs_main);
     uint256 baseBlockHash = ParseBlock(request.params[0], chainman, "baseBlock");
@@ -1621,8 +1601,7 @@ static RPCHelpMan protx_listdiff()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
 
-    CHECK_NONFATAL(node.dmnman);
-    CDeterministicMNManager& dmnman = *node.dmnman;
+    CDeterministicMNManager& dmnman = *CHECK_NONFATAL(node.dmnman);
 
     LOCK(cs_main);
     UniValue ret(UniValue::VOBJ);
diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp
index 2137e91a7b5d..5123945c3f32 100644
--- a/src/rpc/governance.cpp
+++ b/src/rpc/governance.cpp
@@ -21,6 +21,7 @@
 #include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <timedata.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <validation.h>
@@ -194,12 +195,11 @@ static RPCHelpMan gobject_prepare()
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
 
     {
         LOCK(cs_main);
         std::string strError = "";
-        if (!govobj.IsValidLocally(node.dmnman->GetListAtChainTip(), chainman, strError, false))
+        if (!govobj.IsValidLocally(CHECK_NONFATAL(node.dmnman)->GetListAtChainTip(), chainman, strError, false))
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Governance object is not valid - " + govobj.GetHash().ToString() + " - " + strError);
     }
 
@@ -303,14 +303,12 @@ static RPCHelpMan gobject_submit()
 {
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
-    CHECK_NONFATAL(node.govman);
 
     if(!node.mn_sync->IsBlockchainSynced()) {
         throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Must wait for client to sync with masternode network. Try again in a minute or so.");
     }
 
-    auto mnList = node.dmnman->GetListAtChainTip();
+    auto mnList = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
 
     if (node.mn_activeman) {
         const bool fMnFound = mnList.HasValidMNByCollateral(node.mn_activeman->GetOutPoint());
@@ -373,7 +371,6 @@ static RPCHelpMan gobject_submit()
         }
 
         LOCK2(cs_main, mempool.cs);
-        CHECK_NONFATAL(node.dmnman);
 
         std::string strError;
         if (!govobj.IsValidLocally(node.dmnman->GetListAtChainTip(), chainman, strError, fMissingConfirmations, true) && !fMissingConfirmations) {
@@ -384,7 +381,7 @@ static RPCHelpMan gobject_submit()
 
     // RELAY THIS OBJECT
     // Reject if rate check fails but don't update buffer
-    if (!node.govman->MasternodeRateCheck(govobj)) {
+    if (!CHECK_NONFATAL(node.govman)->MasternodeRateCheck(govobj)) {
         LogPrintf("gobject(submit) -- Object submission rejected because of rate check failure - hash = %s\n", strHash);
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Object creation rate limit exceeded");
     }
@@ -393,9 +390,8 @@ static RPCHelpMan gobject_submit()
 
     PeerManager& peerman = EnsurePeerman(node);
     if (fMissingConfirmations) {
-        CHECK_NONFATAL(node.mn_sync);
         node.govman->AddPostponedObject(govobj);
-        govobj.Relay(peerman, *node.mn_sync);
+        govobj.Relay(peerman, *CHECK_NONFATAL(node.mn_sync));
     } else {
         node.govman->AddGovernanceObject(govobj, peerman);
     }
@@ -452,7 +448,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet
     int nSuccessful = 0;
     int nFailed = 0;
 
-    auto mnList = node.dmnman->GetListAtChainTip();
+    auto mnList = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
 
     UniValue resultsObj(UniValue::VOBJ);
 
@@ -529,7 +525,6 @@ static RPCHelpMan gobject_vote_many()
     if (!wallet) return NullUniValue;
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    CHECK_NONFATAL(node.dmnman);
 
     uint256 hash(ParseHashV(request.params[0], "Object hash"));
     std::string strVoteSignal = request.params[1].get_str();
@@ -551,7 +546,7 @@ static RPCHelpMan gobject_vote_many()
 
     std::map<uint256, CKeyID> votingKeys;
 
-    auto mnList = node.dmnman->GetListAtChainTip();
+    auto mnList = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
     mnList.ForEachMN(true, [&](auto& dmn) {
         const bool is_mine = CheckWalletOwnsKey(*wallet, dmn.pdmnState->keyIDVoting);
         if (is_mine) {
@@ -583,7 +578,6 @@ static RPCHelpMan gobject_vote_alias()
     if (!wallet) return NullUniValue;
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    CHECK_NONFATAL(node.dmnman);
 
     uint256 hash(ParseHashV(request.params[0], "Object hash"));
     std::string strVoteSignal = request.params[1].get_str();
@@ -604,7 +598,7 @@ static RPCHelpMan gobject_vote_alias()
     EnsureWalletIsUnlocked(*wallet);
 
     uint256 proTxHash(ParseHashV(request.params[3], "protx-hash"));
-    auto dmn = node.dmnman->GetListAtChainTip().GetValidMN(proTxHash);
+    auto dmn = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetValidMN(proTxHash);
     if (!dmn) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid or unknown proTxHash");
     }
@@ -718,11 +712,10 @@ static RPCHelpMan gobject_list_helper(const bool make_a_diff)
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
-    CHECK_NONFATAL(node.govman);
 
     const int64_t last_time = make_a_diff ? node.govman->GetLastDiffTime() : 0;
-    return ListObjects(*node.govman, node.dmnman->GetListAtChainTip(), chainman, strCachedSignal, strType, last_time);
+    return ListObjects(*CHECK_NONFATAL(node.govman), CHECK_NONFATAL(node.dmnman)->GetListAtChainTip(), chainman,
+                       strCachedSignal, strType, last_time);
 },
     };
 }
@@ -759,8 +752,8 @@ static RPCHelpMan gobject_get()
     // FIND THE GOVERNANCE OBJECT THE USER IS LOOKING FOR
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman && node.govman);
 
+    CHECK_NONFATAL(node.govman);
     LOCK2(cs_main, node.govman->cs);
     const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash);
 
@@ -785,7 +778,7 @@ static RPCHelpMan gobject_get()
     // SHOW (MUCH MORE) INFORMATION ABOUT VOTES FOR GOVERNANCE OBJECT (THAN LIST/DIFF ABOVE)
     // -- FUNDING VOTING RESULTS
 
-    auto tip_mn_list = node.dmnman->GetListAtChainTip();
+    auto tip_mn_list = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
 
     UniValue objFundingResult(UniValue::VOBJ);
     objFundingResult.pushKV("AbsoluteYesCount",  pGovObj->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING));
@@ -858,8 +851,8 @@ static RPCHelpMan gobject_getcurrentvotes()
 
     // FIND OBJECT USER IS LOOKING FOR
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    CHECK_NONFATAL(node.govman);
 
+    CHECK_NONFATAL(node.govman);
     LOCK(node.govman->cs);
     const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash);
 
@@ -872,10 +865,9 @@ static RPCHelpMan gobject_getcurrentvotes()
     UniValue bResult(UniValue::VOBJ);
 
     // GET MATCHING VOTES BY HASH, THEN SHOW USERS VOTE INFORMATION
-    CHECK_NONFATAL(node.dmnman);
     std::vector<CGovernanceVote> vecVotes = node.govman->GetCurrentVotes(hash, mnCollateralOutpoint);
     for (const auto& vote : vecVotes) {
-        bResult.pushKV(vote.GetHash().ToString(), vote.ToString(node.dmnman->GetListAtChainTip()));
+        bResult.pushKV(vote.GetHash().ToString(), vote.ToString(CHECK_NONFATAL(node.dmnman)->GetListAtChainTip()));
     }
 
     return bResult;
@@ -975,8 +967,7 @@ static RPCHelpMan voteraw()
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding");
     }
 
-    CHECK_NONFATAL(node.dmnman);
-    const auto tip_mn_list = node.dmnman->GetListAtChainTip();
+    const auto tip_mn_list = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
     auto dmn = tip_mn_list.GetValidMNByCollateral(outpoint);
 
     if (!dmn) {
@@ -1034,7 +1025,6 @@ static RPCHelpMan getgovernanceinfo()
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureAnyChainman(request.context);
-    CHECK_NONFATAL(node.dmnman);
 
     const auto* pindex = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip());
     int nBlockHeight = pindex->nHeight;
@@ -1048,7 +1038,7 @@ static RPCHelpMan getgovernanceinfo()
     obj.pushKV("superblockmaturitywindow", Params().GetConsensus().nSuperblockMaturityWindow);
     obj.pushKV("lastsuperblock", nLastSuperblock);
     obj.pushKV("nextsuperblock", nNextSuperblock);
-    obj.pushKV("fundingthreshold", int(node.dmnman->GetListAtChainTip().GetValidWeightedMNsCount() / 10));
+    obj.pushKV("fundingthreshold", int(CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetValidWeightedMNsCount() / 10));
     obj.pushKV("governancebudget", ValueFromAmount(CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nNextSuperblock)));
 
     return obj;
diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp
index 5b30f0944be8..ee5fcbdea08c 100644
--- a/src/rpc/masternode.cpp
+++ b/src/rpc/masternode.cpp
@@ -19,6 +19,7 @@
 #include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <univalue.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <validation.h>
 #include <wallet/coincontrol.h>
@@ -138,8 +139,7 @@ static RPCHelpMan masternode_winner()
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
-    return GetNextMasternodeForPayment(chainman.ActiveChain(), *node.dmnman, 10);
+    return GetNextMasternodeForPayment(chainman.ActiveChain(), *CHECK_NONFATAL(node.dmnman), 10);
 },
     };
 }
@@ -159,8 +159,7 @@ static RPCHelpMan masternode_current()
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
-    return GetNextMasternodeForPayment(chainman.ActiveChain(), *node.dmnman, 1);
+    return GetNextMasternodeForPayment(chainman.ActiveChain(), *CHECK_NONFATAL(node.dmnman), 1);
 },
     };
 }
@@ -212,7 +211,7 @@ static RPCHelpMan masternode_status()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    CHECK_NONFATAL(node.dmnman);
+
     if (!node.mn_activeman) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "This node does not run an active masternode.");
     }
@@ -221,7 +220,7 @@ static RPCHelpMan masternode_status()
     // keep compatibility with legacy status for now (might get deprecated/removed later)
     mnObj.pushKV("outpoint", node.mn_activeman->GetOutPoint().ToStringShort());
     mnObj.pushKV("service", node.mn_activeman->GetService().ToStringAddrPort());
-    CDeterministicMNCPtr dmn = node.dmnman->GetListAtChainTip().GetMN(node.mn_activeman->GetProTxHash());
+    CDeterministicMNCPtr dmn = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetMN(node.mn_activeman->GetProTxHash());
     if (dmn) {
         mnObj.pushKV("proTxHash", dmn->proTxHash.ToString());
         mnObj.pushKV("type", std::string(GetMnType(dmn->nType).description));
@@ -243,12 +242,12 @@ static std::string GetRequiredPaymentsString(CGovernanceManager& govman, const C
     if (payee) {
         CTxDestination dest;
         if (!ExtractDestination(payee->pdmnState->scriptPayout, dest)) {
-            CHECK_NONFATAL(false);
+            NONFATAL_UNREACHABLE();
         }
         strPayments = EncodeDestination(dest);
         if (payee->nOperatorReward != 0 && payee->pdmnState->scriptOperatorPayout != CScript()) {
             if (!ExtractDestination(payee->pdmnState->scriptOperatorPayout, dest)) {
-                CHECK_NONFATAL(false);
+                NONFATAL_UNREACHABLE();
             }
             strPayments += ", " + EncodeDestination(dest);
         }
@@ -310,14 +309,11 @@ static RPCHelpMan masternode_winners()
     int nChainTipHeight = pindexTip->nHeight;
     int nStartHeight = std::max(nChainTipHeight - nCount, 1);
 
-    CHECK_NONFATAL(node.dmnman);
-    CHECK_NONFATAL(node.govman);
-
-    const auto tip_mn_list = node.dmnman->GetListAtChainTip();
+    const auto tip_mn_list = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
     for (int h = nStartHeight; h <= nChainTipHeight; h++) {
         const CBlockIndex* pIndex = pindexTip->GetAncestor(h - 1);
         auto payee = node.dmnman->GetListForBlock(pIndex).GetMNPayee(pIndex);
-        std::string strPayments = GetRequiredPaymentsString(*node.govman, tip_mn_list, h, payee);
+        std::string strPayments = GetRequiredPaymentsString(*CHECK_NONFATAL(node.govman), tip_mn_list, h, payee);
         if (strFilter != "" && strPayments.find(strFilter) == std::string::npos) continue;
         obj.pushKV(strprintf("%d", h), strPayments);
     }
@@ -558,11 +554,10 @@ static RPCHelpMan masternodelist_helper(bool is_composite)
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
-    CHECK_NONFATAL(node.dmnman);
 
     UniValue obj(UniValue::VOBJ);
 
-    const auto mnList = node.dmnman->GetListAtChainTip();
+    const auto mnList = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip();
     const auto dmnToStatus = [&](const auto& dmn) {
         if (mnList.IsMNValid(dmn)) {
             return "ENABLED";
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index af51ecc9f80d..0444fc5a9145 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -36,6 +36,7 @@
 #include <spork.h>
 #include <txmempool.h>
 #include <univalue.h>
+#include <util/check.h>
 #include <util/fees.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -391,12 +392,11 @@ static RPCHelpMan generateblock()
     block.vtx.insert(block.vtx.end(), txs.begin(), txs.end());
 
     {
-        CHECK_NONFATAL(node.evodb);
-
         LOCK(cs_main);
 
         BlockValidationState state;
-        if (!TestBlockValidity(state, *llmq_ctx.clhandler, *node.evodb, chainparams, active_chainstate, block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) {
+        if (!TestBlockValidity(state, *llmq_ctx.clhandler, *CHECK_NONFATAL(node.evodb), chainparams, active_chainstate,
+                               block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) {
             throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.GetRejectReason()));
         }
     }
@@ -710,14 +710,14 @@ static RPCHelpMan getblocktemplate()
             }
 
             LLMQContext& llmq_ctx = EnsureLLMQContext(node);
-            CHECK_NONFATAL(node.evodb);
 
             CBlockIndex* const pindexPrev = active_chain.Tip();
             // TestBlockValidity only supports blocks built on the current Tip
             if (block.hashPrevBlock != pindexPrev->GetBlockHash())
                 return "inconclusive-not-best-prevblk";
             BlockValidationState state;
-            TestBlockValidity(state, *llmq_ctx.clhandler, *node.evodb, Params(), active_chainstate, block, pindexPrev, false, true);
+            TestBlockValidity(state, *llmq_ctx.clhandler, *CHECK_NONFATAL(node.evodb), Params(), active_chainstate,
+                              block, pindexPrev, false, true);
             return BIP22ValidationResult(state);
         }
 
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 0250036d0464..40ed64f82194 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -1194,9 +1194,8 @@ static RPCHelpMan mockscheduler()
         throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)");
     }
 
-    auto* node_context = GetContext<NodeContext>(request.context);
+    auto* node_context = CHECK_NONFATAL(GetContext<NodeContext>(request.context));
     // protect against null pointer dereference
-    CHECK_NONFATAL(node_context);
     CHECK_NONFATAL(node_context->scheduler);
     node_context->scheduler->MockForward(std::chrono::seconds(delta_seconds));
 
diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp
index f6c27f4c73f3..3aa5de1a5acd 100644
--- a/src/rpc/quorums.cpp
+++ b/src/rpc/quorums.cpp
@@ -11,6 +11,7 @@
 #include <rpc/server.h>
 #include <rpc/server_util.h>
 #include <rpc/util.h>
+#include <util/check.h>
 #include <validation.h>
 
 #include <masternode/node.h>
@@ -282,7 +283,6 @@ static RPCHelpMan quorum_dkgstatus()
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
     const CConnman& connman = EnsureConnman(node);
-    CHECK_NONFATAL(node.dmnman);
     CHECK_NONFATAL(node.sporkman);
 
     int detailLevel = 0;
@@ -296,7 +296,7 @@ static RPCHelpMan quorum_dkgstatus()
     llmq::CDKGDebugStatus status;
     llmq_ctx.dkg_debugman->GetLocalDebugStatus(status);
 
-    auto ret = status.ToJson(*node.dmnman, chainman, detailLevel);
+    auto ret = status.ToJson(*CHECK_NONFATAL(node.dmnman), chainman, detailLevel);
 
     CBlockIndex* pindexTip = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip());
     int tipHeight = pindexTip->nHeight;
@@ -385,7 +385,6 @@ static RPCHelpMan quorum_memberof()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
-    CHECK_NONFATAL(node.dmnman);
 
     uint256 protxHash(ParseHashV(request.params[0], "proTxHash"));
     int scanQuorumsCount = -1;
@@ -397,7 +396,7 @@ static RPCHelpMan quorum_memberof()
     }
 
     const CBlockIndex* pindexTip = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip());
-    auto mnList = node.dmnman->GetListForBlock(pindexTip);
+    auto mnList = CHECK_NONFATAL(node.dmnman)->GetListForBlock(pindexTip);
     auto dmn = mnList.GetMN(protxHash);
     if (!dmn) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "masternode not found");
@@ -842,7 +841,7 @@ static RPCHelpMan quorum_rotationinfo()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
-    CHECK_NONFATAL(node.dmnman);
+    ;
 
     llmq::CGetQuorumRotationInfo cmd;
     llmq::CQuorumRotationInfo quorumRotationInfoRet;
@@ -859,7 +858,8 @@ static RPCHelpMan quorum_rotationinfo()
 
     LOCK(cs_main);
 
-    if (!BuildQuorumRotationInfo(*node.dmnman, chainman, *llmq_ctx.qman, *llmq_ctx.quorum_block_processor, cmd, quorumRotationInfoRet, strError)) {
+    if (!BuildQuorumRotationInfo(*CHECK_NONFATAL(node.dmnman), chainman, *llmq_ctx.qman, *llmq_ctx.quorum_block_processor,
+                                 cmd, quorumRotationInfoRet, strError)) {
         throw JSONRPCError(RPC_INVALID_REQUEST, strError);
     }
 
@@ -1073,7 +1073,8 @@ static RPCHelpMan verifyislock()
     auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend;
     const auto llmq_params_opt = Params().GetLLMQ(llmqType);
     CHECK_NONFATAL(llmq_params_opt.has_value());
-    return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, txid, sig);
+    return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *CHECK_NONFATAL(llmq_ctx.qman),
+                                           signHeight, id, txid, sig);
 },
     };
 }
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 1b01eb3195d9..e06382711e0b 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -38,6 +38,7 @@
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/bip32.h>
+#include <util/check.h>
 #include <util/moneystr.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -495,8 +496,6 @@ static RPCHelpMan getassetunlockstatuses()
         throw JSONRPCError(RPC_INTERNAL_ERROR, "No blocks in chain");
     }
 
-    CHECK_NONFATAL(node.cpoolman);
-
     std::optional<CCreditPool> poolCL{std::nullopt};
     std::optional<CCreditPool> poolOnTip{std::nullopt};
     std::optional<int> nSpecificCoreHeight{std::nullopt};
@@ -506,7 +505,7 @@ static RPCHelpMan getassetunlockstatuses()
         if (nSpecificCoreHeight.value() < 0 || nSpecificCoreHeight.value() > chainman.ActiveChain().Height()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range");
         }
-        poolCL = std::make_optional(node.cpoolman->GetCreditPool(chainman.ActiveChain()[nSpecificCoreHeight.value()], Params().GetConsensus()));
+        poolCL = std::make_optional(CHECK_NONFATAL(node.cpoolman)->GetCreditPool(chainman.ActiveChain()[nSpecificCoreHeight.value()], Params().GetConsensus()));
     }
     else {
         const auto pBlockIndexBestCL = [&]() -> const CBlockIndex* {
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 727fa5e694ce..4a63378b84f6 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -10,6 +10,7 @@
 #include <script/descriptor.h>
 #include <script/signingprovider.h>
 #include <tinyformat.h>
+#include <util/check.h>
 #include <util/system.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -494,7 +495,7 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP
                 // Null values are accepted in all arguments
                 break;
             default:
-                CHECK_NONFATAL(false);
+                NONFATAL_UNREACHABLE();
                 break;
             }
         }
@@ -743,7 +744,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
         return;
     }
     case Type::ANY: {
-        CHECK_NONFATAL(false); // Only for testing
+        NONFATAL_UNREACHABLE(); // Only for testing
     }
     case Type::NONE: {
         sections.PushSection({indent + "null" + maybe_separator, Description("json null")});
@@ -807,7 +808,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
         return;
     }
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 bool RPCResult::MatchesType(const UniValue& result) const
@@ -843,7 +844,7 @@ bool RPCResult::MatchesType(const UniValue& result) const
         return UniValue::VOBJ == result.getType();
     }
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 std::string RPCArg::ToStringObj(const bool oneline) const
@@ -878,9 +879,9 @@ std::string RPCArg::ToStringObj(const bool oneline) const
     case Type::OBJ:
     case Type::OBJ_USER_KEYS:
         // Currently unused, so avoid writing dead code
-        CHECK_NONFATAL(false);
+        NONFATAL_UNREACHABLE();
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 std::string RPCArg::ToString(const bool oneline) const
@@ -915,7 +916,7 @@ std::string RPCArg::ToString(const bool oneline) const
         return "[" + res + "...]";
     }
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 static std::pair<int64_t, int64_t> ParseRange(const UniValue& value)
diff --git a/src/util/check.h b/src/util/check.h
index cd6663f25ce4..247d89bdb6e3 100644
--- a/src/util/check.h
+++ b/src/util/check.h
@@ -18,8 +18,24 @@ class NonFatalCheckError : public std::runtime_error
     using std::runtime_error::runtime_error;
 };
 
+#define format_internal_error(msg, file, line, func, report)                                    \
+    strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\nPlease report this issue here: %s\n", \
+              msg, file, line, func, report)
+
+/** Helper for CHECK_NONFATAL() */
+template <typename T>
+T&& inline_check_non_fatal(T&& val, const char* file, int line, const char* func, const char* assertion)
+{
+    if (!(val)) {
+        throw NonFatalCheckError(
+            format_internal_error(assertion, file, line, func, PACKAGE_BUGREPORT));
+    }
+
+    return std::forward<T>(val);
+}
+
 /**
- * Throw a NonFatalCheckError when the condition evaluates to false
+ * Identity function. Throw a NonFatalCheckError when the condition evaluates to false
  *
  * This should only be used
  * - where the condition is assumed to be true, not for error handling or validating user input
@@ -29,18 +45,8 @@ class NonFatalCheckError : public std::runtime_error
  * asserts or recoverable logic errors. A NonFatalCheckError in RPC code is caught and passed as a string to the RPC
  * caller, which can then report the issue to the developers.
  */
-#define CHECK_NONFATAL(condition)                                 \
-    do {                                                          \
-        if (!(condition)) {                                       \
-            throw NonFatalCheckError(                             \
-                strprintf("Internal bug detected: '%s'\n"         \
-                          "%s:%d (%s)\n"                          \
-                          "You may report this issue here: %s\n", \
-                    (#condition),                                 \
-                    __FILE__, __LINE__, __func__,                 \
-                    PACKAGE_BUGREPORT));                          \
-        }                                                         \
-    } while (false)
+#define CHECK_NONFATAL(condition) \
+    inline_check_non_fatal(condition, __FILE__, __LINE__, __func__, #condition)
 
 #if defined(NDEBUG)
 #error "Cannot compile without assertions!"
@@ -80,4 +86,13 @@ T&& inline_assertion_check(T&& val, [[maybe_unused]] const char* file, [[maybe_u
  */
 #define Assume(val) inline_assertion_check<false>(val, __FILE__, __LINE__, __func__, #val)
 
+/**
+ * NONFATAL_UNREACHABLE() is a macro that is used to mark unreachable code. It throws a NonFatalCheckError.
+ * This is used to mark code that is not yet implemented or is not yet reachable.
+ */
+#define NONFATAL_UNREACHABLE()                                        \
+    throw NonFatalCheckError(                                         \
+        format_internal_error("Unreachable code reached (non-fatal)", \
+                              __FILE__, __LINE__, __func__, PACKAGE_BUGREPORT))
+
 #endif // BITCOIN_UTIL_CHECK_H
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 4d75d2ae1c76..409944bdad62 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -1135,7 +1135,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d
     case TxoutType::NONSTANDARD:
         return "unrecognized script";
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<CKeyID>& ordered_pubkeys)
diff --git a/test/functional/rpc_misc.py b/test/functional/rpc_misc.py
index c5aeabd8ca90..9e2663349f42 100755
--- a/test/functional/rpc_misc.py
+++ b/test/functional/rpc_misc.py
@@ -27,7 +27,7 @@ def run_test(self):
         self.log.info("test CHECK_NONFATAL")
         assert_raises_rpc_error(
             -1,
-            'Internal bug detected: \'request.params[9].get_str() != "trigger_internal_bug"\'',
+            'Internal bug detected: "request.params[9].get_str() != "trigger_internal_bug""',
             lambda: node.echo(arg9='trigger_internal_bug'),
         )
 

From bcd24a25e3e107802840cca19b145ed197bfee8e Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 15:42:59 +0000
Subject: [PATCH 05/15] fix: push activation height for forks ahead, fix
 `feature_pruning.py`

We need to push DIP-0001's activation height ahead as post-DIP-0001
consensus rejects transactions greater than `MAX_STANDARD_TX_SIZE`
(`bad-txns-oversize`) and this test generates transactions nearly 9.5x
that.

We also need to push v20 activation ahead to avoid different subsidy
calculation logic (`bad-cb-amount`).
---
 test/functional/feature_pruning.py | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 67173dada75c..2f62ae005849 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -33,6 +33,13 @@
 # compatible with pruning based on key creation time.
 TIMESTAMP_WINDOW = 2 * 60 * 60
 
+DEPLOYMENT_ARGS = [
+    "-dip3params=2000:2000",
+    "-testactivationheight=dip0001@2000",
+    "-testactivationheight=dip0008@2000",
+    "-testactivationheight=v20@2000",
+]
+
 def mine_large_blocks(node, n):
     # Make a large scriptPubKey for the coinbase transaction. This is OP_RETURN
     # followed by 950k of OP_NOP. This would be non-standard in a non-coinbase
@@ -87,16 +94,16 @@ def set_test_params(self):
 
         # Create nodes 0 and 1 to mine.
         # Create node 2 to test pruning.
-        self.full_node_default_args = ["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"]
+        self.full_node_default_args = ["-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"] + DEPLOYMENT_ARGS
         # Create nodes 3 and 4 to test manual pruning (they will be re-started with manual pruning later)
         # Create nodes 5 to test wallet in prune mode, but do not connect
         self.extra_args = [
             self.full_node_default_args,
             self.full_node_default_args,
-            ["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-prune=550"],
-            ["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-blockmaxsize=999000"],
-            ["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-blockmaxsize=999000"],
-            ["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance","-txindex=0","-prune=550"],
+            ["-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-prune=550"] + DEPLOYMENT_ARGS,
+            ["-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-blockmaxsize=999000"] + DEPLOYMENT_ARGS,
+            ["-disablegovernance","-txindex=0","-maxreceivebuffer=20000","-blockmaxsize=999000"] + DEPLOYMENT_ARGS,
+            ["-disablegovernance","-txindex=0","-prune=550"] + DEPLOYMENT_ARGS,
         ]
         self.rpc_timeout = 120
 
@@ -213,7 +220,7 @@ def reorg_test(self):
         self.log.info("New best height: %d" % self.nodes[1].getblockcount())
 
         # Mine one block to avoid automatic recovery from forks on restart
-        self.generate(self.nodes[1], 1)
+        self.generate(self.nodes[1], 1, sync_fun=self.no_op)
         # Disconnect node1 and generate the new chain
         self.disconnect_nodes(0, 1)
         self.disconnect_nodes(1, 2)
@@ -283,13 +290,13 @@ def reorg_back(self):
 
     def manual_test(self, node_number, use_timestamp):
         # at this point, node has 995 blocks and has not yet run in prune mode
-        self.start_node(node_number, extra_args=["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance", "-txindex=0"])
+        self.start_node(node_number, extra_args=["-disablegovernance", "-txindex=0"] + DEPLOYMENT_ARGS)
         node = self.nodes[node_number]
         assert_equal(node.getblockcount(), 995)
         assert_raises_rpc_error(-1, "Cannot prune blocks because node is not in prune mode", node.pruneblockchain, 500)
 
         # now re-start in manual pruning mode
-        self.restart_node(node_number, extra_args=["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance", "-txindex=0", "-prune=1"], expected_stderr=EXPECTED_STDERR_NO_GOV)
+        self.restart_node(node_number, extra_args=["-disablegovernance", "-txindex=0", "-prune=1"] + DEPLOYMENT_ARGS, expected_stderr=EXPECTED_STDERR_NO_GOV)
         node = self.nodes[node_number]
         assert_equal(node.getblockcount(), 995)
 
@@ -358,14 +365,14 @@ def has_block(index):
         assert not has_block(3), "blk00003.dat is still there, should be pruned by now"
 
         # stop node, start back up with auto-prune at 550 MiB, make sure still runs
-        self.restart_node(node_number, extra_args=["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance", "-txindex=0", "-prune=550"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+        self.restart_node(node_number, extra_args=["-disablegovernance", "-txindex=0", "-prune=550"] + DEPLOYMENT_ARGS, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
 
         self.log.info("Success")
 
     def wallet_test(self):
         # check that the pruning node's wallet is still in good shape
         self.log.info("Stop and start pruning node to trigger wallet rescan")
-        self.restart_node(2, extra_args=["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance", "-txindex=0", "-prune=550"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+        self.restart_node(2, extra_args=["-disablegovernance", "-txindex=0", "-prune=550"] + DEPLOYMENT_ARGS, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
         self.log.info("Success")
 
         # check that wallet loads successfully when restarting a pruned node after IBD.
@@ -374,7 +381,7 @@ def wallet_test(self):
         self.connect_nodes(0, 5)
         nds = [self.nodes[0], self.nodes[5]]
         self.sync_blocks(nds, wait=5, timeout=300)
-        self.restart_node(5, extra_args=["-dip3params=2000:2000", "-testactivationheight=dip0008@2000", "-disablegovernance", "-txindex=0", "-prune=550"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) # restart to trigger rescan
+        self.restart_node(5, extra_args=["-disablegovernance", "-txindex=0", "-prune=550"] + DEPLOYMENT_ARGS, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) # restart to trigger rescan
         self.log.info("Success")
 
     def run_test(self):

From c65ec190c562eba578b2c17ec9809bd6e2f0225d Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Mon, 21 Mar 2022 13:13:49 +0100
Subject: [PATCH 06/15] merge bitcoin#24626: disallow reindex-chainstate when
 pruning

---
 src/init.cpp                       | 4 +++-
 test/functional/feature_pruning.py | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/init.cpp b/src/init.cpp
index 16c7f048600c..2887cfc1eab2 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1156,12 +1156,14 @@ bool AppInitParameterInteraction(const ArgsManager& args)
         nLocalServices = ServiceFlags(nLocalServices | NODE_COMPACT_FILTERS);
     }
 
-    // if using block pruning, then disallow txindex, coinstatsindex and require disabling governance validation
     if (args.GetArg("-prune", 0)) {
         if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX))
             return InitError(_("Prune mode is incompatible with -txindex."));
         if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX))
             return InitError(_("Prune mode is incompatible with -coinstatsindex."));
+        if (args.GetBoolArg("-reindex-chainstate", false)) {
+            return InitError(_("Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead."));
+        }
         if (!args.GetBoolArg("-disablegovernance", !DEFAULT_GOVERNANCE_ENABLE)) {
             return InitError(_("Prune mode is incompatible with -disablegovernance=false."));
         }
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 2f62ae005849..e96584f0cb53 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -153,6 +153,10 @@ def test_invalid_command_line_options(self):
             expected_msg='Error: Prune mode is incompatible with -coinstatsindex.',
             extra_args=['-prune=550', '-coinstatsindex'],
         )
+        self.nodes[0].assert_start_raises_init_error(
+            expected_msg='Error: Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead.',
+            extra_args=['-prune=550', '-reindex-chainstate'],
+        )
         self.nodes[0].assert_start_raises_init_error(
             expected_msg='Error: Prune mode is incompatible with -disablegovernance=false.',
             extra_args=['-prune=550', '-disablegovernance=false'],

From baf6e26eed4af9d98f1707f292a3c6bdb5e41c56 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 16:53:27 +0000
Subject: [PATCH 07/15] merge bitcoin#21726: Improve Indices on pruned nodes
 via prune blockers

---
 src/index/base.cpp                            |  33 ++--
 src/index/base.h                              |   6 +
 src/index/blockfilterindex.h                  |   2 +
 src/index/coinstatsindex.h                    |   2 +
 src/index/txindex.h                           |   2 +
 src/init.cpp                                  |   4 +-
 src/node/blockstorage.cpp                     |  16 ++
 src/node/blockstorage.h                       |  19 +++
 src/rpc/blockchain.cpp                        |  13 +-
 src/validation.cpp                            |  41 ++++-
 .../feature_blockfilterindex_prune.py         |  80 ---------
 test/functional/feature_index_prune.py        | 161 ++++++++++++++++++
 test/functional/feature_pruning.py            |   4 -
 test/functional/test_runner.py                |   2 +-
 test/lint/lint-circular-dependencies.sh       |   2 -
 15 files changed, 269 insertions(+), 118 deletions(-)
 delete mode 100755 test/functional/feature_blockfilterindex_prune.py
 create mode 100755 test/functional/feature_index_prune.py

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 539041143261..0aa4b8cd58d3 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -62,9 +62,9 @@ bool BaseIndex::Init()
     LOCK(cs_main);
     CChain& active_chain = m_chainstate->m_chain;
     if (locator.IsNull()) {
-        m_best_block_index = nullptr;
+        SetBestBlockIndex(nullptr);
     } else {
-        m_best_block_index = m_chainstate->FindForkInGlobalIndex(locator);
+        SetBestBlockIndex(m_chainstate->FindForkInGlobalIndex(locator));
     }
 
     // Note: this will latch to true immediately if the user starts up with an empty
@@ -76,11 +76,7 @@ bool BaseIndex::Init()
         if (!m_best_block_index) {
             // index is not built yet
             // make sure we have all block data back to the genesis
-            const CBlockIndex* block = active_chain.Tip();
-            while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-                block = block->pprev;
-            }
-            prune_violation = block != active_chain.Genesis();
+            prune_violation = GetFirstStoredBlock(active_chain.Tip()) != active_chain.Genesis();
         }
         // in case the index has a best block set and is not fully synced
         // check if we have the required blocks to continue building the index
@@ -138,7 +134,7 @@ void BaseIndex::ThreadSync()
         std::chrono::steady_clock::time_point last_locator_write_time{0s};
         while (true) {
             if (m_interrupt) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 // No need to handle errors in Commit. If it fails, the error will be already be
                 // logged. The best way to recover is to continue, as index cannot be corrupted by
                 // a missed commit to disk for an advanced index state.
@@ -150,7 +146,7 @@ void BaseIndex::ThreadSync()
                 LOCK(cs_main);
                 const CBlockIndex* pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain);
                 if (!pindex_next) {
-                    m_best_block_index = pindex;
+                    SetBestBlockIndex(pindex);
                     m_synced = true;
                     // No need to handle errors in Commit. See rationale above.
                     Commit();
@@ -172,7 +168,7 @@ void BaseIndex::ThreadSync()
             }
 
             if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 last_locator_write_time = current_time;
                 // No need to handle errors in Commit. See rationale above.
                 Commit();
@@ -230,10 +226,10 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
     // out of sync may be possible but a users fault.
     // In case we reorg beyond the pruned depth, ReadBlockFromDisk would
     // throw and lead to a graceful shutdown
-    m_best_block_index = new_tip;
+    SetBestBlockIndex(new_tip);
     if (!Commit()) {
         // If commit fails, revert the best block index to avoid corruption.
-        m_best_block_index = current_tip;
+        SetBestBlockIndex(current_tip);
         return false;
     }
 
@@ -274,7 +270,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const
     }
 
     if (WriteBlock(*block, pindex)) {
-        m_best_block_index = pindex;
+        SetBestBlockIndex(pindex);
     } else {
         FatalError("%s: Failed to write block %s to index",
                    __func__, pindex->GetBlockHash().ToString());
@@ -381,3 +377,14 @@ IndexSummary BaseIndex::GetSummary() const
     summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0;
     return summary;
 }
+
+void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
+    assert(!fPruneMode || AllowPrune());
+
+    m_best_block_index = block;
+    if (AllowPrune() && block) {
+        PruneLockInfo prune_lock;
+        prune_lock.height_first = block->nHeight;
+        WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
+    }
+}
diff --git a/src/index/base.h b/src/index/base.h
index 403c8c87b1b2..323e4e9bafed 100644
--- a/src/index/base.h
+++ b/src/index/base.h
@@ -81,6 +81,9 @@ class BaseIndex : public CValidationInterface
     /// to a chain reorganization), the index must halt until Commit succeeds or else it could end up
     /// getting corrupted.
     bool Commit();
+
+    virtual bool AllowPrune() const = 0;
+
 protected:
     CChainState* m_chainstate{nullptr};
 
@@ -109,6 +112,9 @@ class BaseIndex : public CValidationInterface
     /// Get the name of the index for display in logs.
     virtual const char* GetName() const = 0;
 
+    /// Update the internal best block index as well as the prune lock.
+    void SetBestBlockIndex(const CBlockIndex* block);
+
 public:
     /// Destructor interrupts sync thread if running and blocks until it exits.
     virtual ~BaseIndex();
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
index 760a71234901..06b107d111df 100644
--- a/src/index/blockfilterindex.h
+++ b/src/index/blockfilterindex.h
@@ -39,6 +39,8 @@ class BlockFilterIndex final : public BaseIndex
     /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */
     std::unordered_map<uint256, uint256, FilterHeaderHasher> m_headers_cache GUARDED_BY(m_cs_headers_cache);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 
diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h
index ed0c8673f85e..8cfb87ee8b08 100644
--- a/src/index/coinstatsindex.h
+++ b/src/index/coinstatsindex.h
@@ -36,6 +36,8 @@ class CoinStatsIndex final : public BaseIndex
 
     bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 
diff --git a/src/index/txindex.h b/src/index/txindex.h
index 23a1aef1e4a0..3fe3628f0a70 100644
--- a/src/index/txindex.h
+++ b/src/index/txindex.h
@@ -20,6 +20,8 @@ class TxIndex final : public BaseIndex
 private:
     const std::unique_ptr<DB> m_db;
 
+    bool AllowPrune() const override { return false; }
+
 protected:
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 
diff --git a/src/init.cpp b/src/init.cpp
index 2887cfc1eab2..c2cf16b1a249 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -538,7 +538,7 @@ void SetupServerArgs(ArgsManager& argsman)
         -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -coinstatsindex, -rescan and -disablegovernance=false. "
+    argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -rescan and -disablegovernance=false. "
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
             "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-settings=<file>", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -1159,8 +1159,6 @@ bool AppInitParameterInteraction(const ArgsManager& args)
     if (args.GetArg("-prune", 0)) {
         if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX))
             return InitError(_("Prune mode is incompatible with -txindex."));
-        if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX))
-            return InitError(_("Prune mode is incompatible with -coinstatsindex."));
         if (args.GetBoolArg("-reindex-chainstate", false)) {
             return InitError(_("Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead."));
         }
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index d8e59b88ab0d..960759396f7e 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -24,6 +24,7 @@
 #include <walletinitinterface.h>
 
 #include <map>
+#include <unordered_map>
 
 std::atomic_bool fImporting(false);
 std::atomic_bool fReindex(false);
@@ -249,6 +250,11 @@ void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPr
            nLastBlockWeCanPrune, count);
 }
 
+void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) {
+    AssertLockHeld(::cs_main);
+    m_prune_locks[name] = lock_info;
+}
+
 CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash)
 {
     AssertLockHeld(cs_main);
@@ -421,6 +427,16 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex)
     return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0);
 }
 
+const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) {
+    AssertLockHeld(::cs_main);
+    assert(start_block);
+    const CBlockIndex* last_block = start_block;
+    while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) {
+        last_block = last_block->pprev;
+    }
+    return last_block;
+}
+
 // If we're using -prune with -reindex, then delete block files that will be ignored by the
 // reindex.  Since reindexing works by starting at block file 0 and looping until a blockfile
 // is missing, do the same here to delete any later block files after a gap.  Also delete all
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index ae7319d5eab4..7f671fd5f8de 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -12,6 +12,7 @@
 #include <txdb.h>
 
 #include <cstdint>
+#include <unordered_map>
 #include <vector>
 
 extern RecursiveMutex cs_main;
@@ -77,6 +78,10 @@ struct CBlockIndexHeightOnlyComparator {
     bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const;
 };
 
+struct PruneLockInfo {
+    int height_first{std::numeric_limits<int>::max()}; //! Height of earliest block that should be kept and not pruned
+};
+
 /**
  * Maintains a tree of blocks (stored in `m_block_index`) which is consulted
  * to determine where the most-work tip is.
@@ -137,6 +142,14 @@ class BlockManager
     /** Dirty block file entries. */
     std::set<int> m_dirty_fileinfo;
 
+    /**
+     * Map from external index name to oldest block that must not be pruned.
+     *
+     * @note Internally, only blocks at height (height_first - PRUNE_LOCK_BUFFER - 1) and
+     * below will be pruned, but callers should avoid assuming any particular buffer size.
+     */
+    std::unordered_map<std::string, PruneLockInfo> m_prune_locks GUARDED_BY(::cs_main);
+
 public:
     BlockMap m_block_index GUARDED_BY(cs_main);
     PrevBlockMap m_prev_block_index GUARDED_BY(cs_main);
@@ -185,8 +198,14 @@ class BlockManager
 
     //! Check whether the block associated with this index entry is pruned or not.
     bool IsBlockPruned(const CBlockIndex* pblockindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+    //! Create or update a prune lock identified by its name
+    void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 };
 
+//! Find the first block that is not pruned
+const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
 void CleanupBlockRevFiles();
 
 /** Open a block file (blk?????.dat) */
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 94426f7da48b..15ae8e26a54d 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1361,10 +1361,9 @@ static RPCHelpMan pruneblockchain()
 
     PruneBlockFilesManual(active_chainstate, height);
     const CBlockIndex* block = CHECK_NONFATAL(active_chain.Tip());
-    while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-        block = block->pprev;
-    }
-    return uint64_t(block->nHeight);
+    const CBlockIndex* last_block = GetFirstStoredBlock(block);
+
+    return static_cast<uint64_t>(last_block->nHeight);
 },
     };
 }
@@ -1806,11 +1805,7 @@ RPCHelpMan getblockchaininfo()
     obj.pushKV("pruned", fPruneMode);
     if (fPruneMode) {
         const CBlockIndex* block = CHECK_NONFATAL(tip);
-        while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
-            block = block->pprev;
-        }
-
-        obj.pushKV("pruneheight",        block->nHeight);
+        obj.pushKV("pruneheight", GetFirstStoredBlock(block)->nHeight);
 
         // if 0, execution bypasses the whole if block.
         bool automatic_pruning{args.GetArg("-prune", 0) != 1};
diff --git a/src/validation.cpp b/src/validation.cpp
index 38a030ace1c6..efe4e2cfe2f7 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -19,7 +19,6 @@
 #include <deploymentstatus.h>
 #include <flatfile.h>
 #include <hash.h>
-#include <index/blockfilterindex.h>
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
@@ -93,6 +92,12 @@ const std::vector<std::string> CHECKLEVEL_DOC {
     "level 4 tries to reconnect the blocks",
     "each level includes the checks of the previous levels",
 };
+/** The number of blocks to keep below the deepest prune lock.
+ *  There is nothing special about this number. It is higher than what we
+ *  expect to see in regular mainnet reorgs, but not so high that it would
+ *  noticeably interfere with the pruning mechanism.
+ * */
+static constexpr int PRUNE_LOCK_BUFFER{10};
 
 /**
  * Mutex to guard access to validation specific variables, such as reading
@@ -2368,12 +2373,24 @@ bool CChainState::FlushStateToDisk(
         CoinsCacheSizeState cache_state = GetCoinsCacheSizeState();
         LOCK(m_blockman.cs_LastBlockFile);
         if (fPruneMode && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && !fReindex) {
-            // make sure we don't prune above the blockfilterindexes bestblocks
+            // make sure we don't prune above any of the prune locks bestblocks
             // pruning is height-based
-            int last_prune = m_chain.Height(); // last height we can prune
-            ForEachBlockFilterIndex([&](BlockFilterIndex& index) {
-               last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
-            });
+            int last_prune{m_chain.Height()}; // last height we can prune
+            std::optional<std::string> limiting_lock; // prune lock that actually was the limiting factor, only used for logging
+
+            for (const auto& prune_lock : m_blockman.m_prune_locks) {
+                if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
+                // Remove the buffer and one additional block here to get actual height that is outside of the buffer
+                const int lock_height{prune_lock.second.height_first - PRUNE_LOCK_BUFFER - 1};
+                last_prune = std::max(1, std::min(last_prune, lock_height));
+                if (last_prune == lock_height) {
+                    limiting_lock = prune_lock.first;
+                }
+            }
+
+            if (limiting_lock) {
+                LogPrint(BCLog::PRUNE, "%s limited pruning to height %d\n", limiting_lock.value(), last_prune);
+            }
 
             if (nManualPruneHeight > 0) {
                 LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCHMARK);
@@ -2628,6 +2645,18 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr
         dbTx->Commit();
     }
     LogPrint(BCLog::BENCHMARK, "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * MILLI);
+
+    {
+        // Prune locks that began at or after the tip should be moved backward so they get a chance to reorg
+        const int max_height_first{pindexDelete->nHeight - 1};
+        for (auto& prune_lock : m_blockman.m_prune_locks) {
+            if (prune_lock.second.height_first <= max_height_first) continue;
+
+            prune_lock.second.height_first = max_height_first;
+            LogPrint(BCLog::PRUNE, "%s prune lock moved back to %d\n", prune_lock.first, max_height_first);
+        }
+    }
+
     // Write the chain state to disk, if necessary.
     if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) {
         return false;
diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py
deleted file mode 100755
index 361a9cce01f1..000000000000
--- a/test/functional/feature_blockfilterindex_prune.py
+++ /dev/null
@@ -1,80 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (c) 2020 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 blockfilterindex in conjunction with prune."""
-from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (
-    assert_equal,
-    assert_greater_than,
-    assert_raises_rpc_error,
-)
-from test_framework.governance import EXPECTED_STDERR_NO_GOV_PRUNE
-
-
-class FeatureBlockfilterindexPruneTest(BitcoinTestFramework):
-    def set_test_params(self):
-        self.num_nodes = 1
-        self.extra_args = [["-fastprune", "-prune=1", "-blockfilterindex=1", "-testactivationheight=v20@2000"]]
-
-    def sync_index(self, height):
-        expected = {'basic block filter index': {'synced': True, 'best_block_height': height}}
-        self.wait_until(lambda: self.nodes[0].getindexinfo() == expected)
-
-    def run_test(self):
-        self.log.info("check if we can access a blockfilter when pruning is enabled but no blocks are actually pruned")
-        self.sync_index(height=200)
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0)
-        self.generate(self.nodes[0], 500)
-        self.sync_index(height=700)
-
-        self.log.info("prune some blocks")
-        pruneheight = self.nodes[0].pruneblockchain(400)
-        # the prune heights used here and below are magic numbers that are determined by the
-        # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
-        assert_equal(pruneheight, 366)
-
-        self.log.info("check if we can access the tips blockfilter when we have pruned some blocks")
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0)
-
-        self.log.info("check if we can access the blockfilter of a pruned block")
-        assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getblockhash(2))['filter']), 0)
-
-        # mine and sync index up to a height that will later be the pruneheight
-        self.generate(self.nodes[0], 298)
-        self.sync_index(height=998)
-
-        self.log.info("start node without blockfilterindex")
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
-
-        self.log.info("make sure accessing the blockfilters throws an error")
-        assert_raises_rpc_error(-1, "Index is not enabled for filtertype basic", self.nodes[0].getblockfilter, self.nodes[0].getblockhash(2))
-        self.generate(self.nodes[0], 502)
-
-        self.log.info("prune exactly up to the blockfilterindexes best block while blockfilters are disabled")
-        pruneheight_2 = self.nodes[0].pruneblockchain(1000)
-        assert_equal(pruneheight_2, 932)
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
-        self.log.info("make sure that we can continue with the partially synced index after having pruned up to the index height")
-        self.sync_index(height=1500)
-
-        self.log.info("prune below the blockfilterindexes best block while blockfilters are disabled")
-        self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
-        self.generate(self.nodes[0], 1000)
-        pruneheight_3 = self.nodes[0].pruneblockchain(2000)
-        assert_greater_than(pruneheight_3, pruneheight_2)
-        self.stop_node(0, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
-
-        self.log.info("make sure we get an init error when starting the node again with block filters")
-        self.nodes[0].assert_start_raises_init_error(
-            extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"],
-            expected_msg=f"{EXPECTED_STDERR_NO_GOV_PRUNE}\nError: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)",
-        )
-
-        self.log.info("make sure the node starts again with the -reindex arg")
-        self.start_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex", "-reindex"])
-        self.stop_nodes(expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
-
-
-if __name__ == '__main__':
-    FeatureBlockfilterindexPruneTest().main()
diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py
new file mode 100755
index 000000000000..5ec80ced6ecb
--- /dev/null
+++ b/test/functional/feature_index_prune.py
@@ -0,0 +1,161 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 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 indices in conjunction with prune."""
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    assert_raises_rpc_error,
+    p2p_port,
+)
+from test_framework.governance import EXPECTED_STDERR_NO_GOV_PRUNE
+
+DEPLOYMENT_ARG = "-testactivationheight=v20@3000"
+
+class FeatureIndexPruneTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 4
+        self.extra_args = [
+            ["-fastprune", "-prune=1", "-blockfilterindex=1", DEPLOYMENT_ARG],
+            ["-fastprune", "-prune=1", "-coinstatsindex=1", DEPLOYMENT_ARG],
+            ["-fastprune", "-prune=1", "-blockfilterindex=1", "-coinstatsindex=1", DEPLOYMENT_ARG],
+            [DEPLOYMENT_ARG]
+        ]
+
+    def sync_index(self, height):
+        expected_filter = {
+            'basic block filter index': {'synced': True, 'best_block_height': height},
+        }
+        self.wait_until(lambda: self.nodes[0].getindexinfo() == expected_filter)
+
+        expected_stats = {
+            'coinstatsindex': {'synced': True, 'best_block_height': height}
+        }
+        self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats)
+
+        expected = {**expected_filter, **expected_stats}
+        self.wait_until(lambda: self.nodes[2].getindexinfo() == expected)
+
+    def reconnect_nodes(self):
+        self.connect_nodes(0,1)
+        self.connect_nodes(0,2)
+        self.connect_nodes(0,3)
+
+    def mine_batches(self, blocks):
+        n = blocks // 250
+        for _ in range(n):
+            self.generate(self.nodes[0], 250)
+        self.generate(self.nodes[0], blocks % 250)
+        self.sync_blocks()
+
+    def restart_without_indices(self):
+        for i in range(3):
+            self.restart_node(i, extra_args=["-fastprune", "-prune=1", DEPLOYMENT_ARG], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+        self.reconnect_nodes()
+
+    def run_test(self):
+        filter_nodes = [self.nodes[0], self.nodes[2]]
+        stats_nodes = [self.nodes[1], self.nodes[2]]
+
+        self.log.info("check if we can access blockfilters and coinstats when pruning is enabled but no blocks are actually pruned")
+        self.sync_index(height=200)
+        tip = self.nodes[0].getbestblockhash()
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash'])
+
+        self.mine_batches(500)
+        self.sync_index(height=700)
+
+        self.log.info("prune some blocks")
+        for node in self.nodes[:2]:
+            with node.assert_debug_log(['limited pruning to height 689']):
+                pruneheight_new = node.pruneblockchain(400)
+                # the prune heights used here and below are magic numbers that are determined by the
+                # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
+                assert_equal(pruneheight_new, 366)
+
+        self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks")
+        tip = self.nodes[0].getbestblockhash()
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash'])
+
+        self.log.info("check if we can access the blockfilter and coinstats of a pruned block")
+        height_hash = self.nodes[0].getblockhash(2)
+        for node in filter_nodes:
+            assert_greater_than(len(node.getblockfilter(height_hash)['filter']), 0)
+        for node in stats_nodes:
+            assert(node.gettxoutsetinfo(hash_type="muhash", hash_or_height=height_hash)['muhash'])
+
+        # mine and sync index up to a height that will later be the pruneheight
+        self.generate(self.nodes[0], 298)
+        self.sync_index(height=998)
+
+        self.restart_without_indices()
+
+        self.log.info("make sure trying to access the indices throws errors")
+        for node in filter_nodes:
+            msg = "Index is not enabled for filtertype basic"
+            assert_raises_rpc_error(-1, msg, node.getblockfilter, height_hash)
+        for node in stats_nodes:
+            msg = "Querying specific block heights requires coinstatsindex"
+            assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash)
+
+        self.mine_batches(502)
+
+        self.log.info("prune exactly up to the indices best blocks while the indices are disabled")
+        for i in range(3):
+            pruneheight_2 = self.nodes[i].pruneblockchain(1000)
+            assert_equal(pruneheight_2, 932)
+            # Restart the nodes again with the indices activated
+            self.restart_node(i, extra_args=self.extra_args[i], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+
+        self.log.info("make sure that we can continue with the partially synced indices after having pruned up to the index height")
+        self.sync_index(height=1500)
+
+        self.log.info("prune further than the indices best blocks while the indices are disabled")
+        self.restart_without_indices()
+        self.mine_batches(1000)
+
+        for i in range(3):
+            pruneheight_3 = self.nodes[i].pruneblockchain(2000)
+            assert_greater_than(pruneheight_3, pruneheight_2)
+            self.stop_node(i, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+
+        self.log.info("make sure we get an init error when starting the nodes again with the indices")
+        filter_msg = "Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"
+        stats_msg = "Error: coinstatsindex best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"
+        for i, msg in enumerate([filter_msg, stats_msg, filter_msg]):
+            self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=f"{EXPECTED_STDERR_NO_GOV_PRUNE}\n{msg}")
+
+        self.log.info("make sure the nodes start again with the indices and an additional -reindex arg")
+        ip_port = "127.0.0.1:" + str(p2p_port(3))
+        for i in range(3):
+            # The nodes need to be reconnected to the non-pruning node upon restart, otherwise they will be stuck
+            restart_args = self.extra_args[i]+["-reindex", f"-connect={ip_port}"]
+            self.restart_node(i, extra_args=restart_args, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+
+        self.sync_blocks(timeout=300)
+
+        for node in self.nodes[:2]:
+            with node.assert_debug_log(['limited pruning to height 2489']):
+                pruneheight_new = node.pruneblockchain(2500)
+                assert_equal(pruneheight_new, 2197)
+
+        self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario")
+        with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']):
+            self.nodes[3].invalidateblock(self.nodes[0].getblockhash(2480))
+            self.generate(self.nodes[3], 30)
+            self.sync_blocks()
+
+        for idx in range(self.num_nodes):
+            self.nodes[idx].stop_node(expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE if idx != 3 else "")
+
+
+if __name__ == '__main__':
+    FeatureIndexPruneTest().main()
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index e96584f0cb53..6f67fcbcf193 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -149,10 +149,6 @@ def test_invalid_command_line_options(self):
             expected_msg='Error: Prune mode is incompatible with -txindex.',
             extra_args=['-prune=550', '-txindex'],
         )
-        self.nodes[0].assert_start_raises_init_error(
-            expected_msg='Error: Prune mode is incompatible with -coinstatsindex.',
-            extra_args=['-prune=550', '-coinstatsindex'],
-        )
         self.nodes[0].assert_start_raises_init_error(
             expected_msg='Error: Prune mode is incompatible with -reindex-chainstate. Use full -reindex instead.',
             extra_args=['-prune=550', '-reindex-chainstate'],
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 90450452367d..8f9a10339671 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -87,6 +87,7 @@
     # Longest test should go first, to favor running tests in parallel
     'feature_pruning.py', # NOTE: Prune mode is incompatible with -txindex, should work with governance validation disabled though.
     'feature_dbcrash.py',
+    'feature_index_prune.py',
 ]
 
 BASE_SCRIPTS = [
@@ -365,7 +366,6 @@
     'rpc_help.py',
     'feature_dirsymlinks.py',
     'feature_help.py',
-    'feature_blockfilterindex_prune.py'
     # Don't append tests at the end to avoid merge conflicts
     # Put them in a random line within the section that fits their approximate run-time
 ]
diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh
index 9bbe54a0a00d..90eb1376882c 100755
--- a/test/lint/lint-circular-dependencies.sh
+++ b/test/lint/lint-circular-dependencies.sh
@@ -11,8 +11,6 @@ export LC_ALL=C
 EXPECTED_CIRCULAR_DEPENDENCIES=(
     "chainparamsbase -> util/system -> chainparamsbase"
     "node/blockstorage -> validation -> node/blockstorage"
-    "index/blockfilterindex -> node/blockstorage -> validation -> index/blockfilterindex"
-    "index/base -> validation -> index/blockfilterindex -> index/base"
     "index/coinstatsindex -> node/coinstats -> index/coinstatsindex"
     "policy/fees -> txmempool -> policy/fees"
     "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel"

From e6867a35cee0f9e53c8f6c24da3c47ecac2bc553 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Fri, 13 May 2022 00:51:48 +0200
Subject: [PATCH 08/15] merge bitcoin#25123: Fix race condition in index prune
 test

---
 test/functional/feature_index_prune.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py
index 5ec80ced6ecb..0222de33e3eb 100755
--- a/test/functional/feature_index_prune.py
+++ b/test/functional/feature_index_prune.py
@@ -8,7 +8,6 @@
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    p2p_port,
 )
 from test_framework.governance import EXPECTED_STDERR_NO_GOV_PRUNE
 
@@ -134,11 +133,11 @@ def run_test(self):
             self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=f"{EXPECTED_STDERR_NO_GOV_PRUNE}\n{msg}")
 
         self.log.info("make sure the nodes start again with the indices and an additional -reindex arg")
-        ip_port = "127.0.0.1:" + str(p2p_port(3))
         for i in range(3):
-            # The nodes need to be reconnected to the non-pruning node upon restart, otherwise they will be stuck
-            restart_args = self.extra_args[i]+["-reindex", f"-connect={ip_port}"]
+            restart_args = self.extra_args[i]+["-reindex"]
             self.restart_node(i, extra_args=restart_args, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
+            # The nodes need to be reconnected to the non-pruning node upon restart, otherwise they will be stuck
+            self.connect_nodes(i, 3)
 
         self.sync_blocks(timeout=300)
 

From a86109a017577891382fc1f366e553506e1a0a28 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Fri, 6 May 2022 14:07:51 +0200
Subject: [PATCH 09/15] merge bitcoin#25074: During sync, commit best block
 after indexing

---
 src/index/base.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 0aa4b8cd58d3..376d0314ca43 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -168,7 +168,7 @@ void BaseIndex::ThreadSync()
             }
 
             if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) {
-                SetBestBlockIndex(pindex);
+                SetBestBlockIndex(pindex->pprev);
                 last_locator_write_time = current_time;
                 // No need to handle errors in Commit. See rationale above.
                 Commit();

From e507a51323088541c901d477c6a75df414662b03 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 16:19:57 +0000
Subject: [PATCH 10/15] fix: avoid `mandatory-script-verify-flag-failed` crash
 in bench test

Aberrant behaviour was first introduced in 5d10b413 (dash#6152), changes
meant for SegWit made it in, reverting them fixes the problem.
---
 src/bench/block_assemble.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp
index 56afa0132f3c..2d2c7c97ac8d 100644
--- a/src/bench/block_assemble.cpp
+++ b/src/bench/block_assemble.cpp
@@ -32,7 +32,7 @@ static void AssembleBlock(benchmark::Bench& bench)
     std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
     for (size_t b{0}; b < NUM_BLOCKS; ++b) {
         CMutableTransaction tx;
-        tx.vin.push_back(MineBlock(test_setup->m_node, P2SH_OP_TRUE));
+        tx.vin.push_back(MineBlock(test_setup->m_node, SCRIPT_PUB));
         tx.vin.back().scriptSig = scriptSig;
         tx.vout.emplace_back(1337, SCRIPT_PUB);
         if (NUM_BLOCKS - b >= COINBASE_MATURITY)
@@ -48,7 +48,7 @@ static void AssembleBlock(benchmark::Bench& bench)
     }
 
     bench.minEpochIterations(700).run([&] {
-        PrepareBlock(test_setup->m_node, P2SH_OP_TRUE);
+        PrepareBlock(test_setup->m_node, SCRIPT_PUB);
     });
 }
 

From 3bd584c845e7aae4fbc94a0f1456dca56a3a5629 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 16:16:07 +0000
Subject: [PATCH 11/15] merge bitcoin#24832: Verify the block filter hash when
 reading the filter from disk

---
 src/bench/gcs_filter.cpp       | 80 ++++++++++++++++++++++++++--------
 src/blockfilter.cpp            |  8 ++--
 src/blockfilter.h              |  6 +--
 src/index/blockfilterindex.cpp | 13 ++++--
 src/index/blockfilterindex.h   |  2 +-
 5 files changed, 81 insertions(+), 28 deletions(-)

diff --git a/src/bench/gcs_filter.cpp b/src/bench/gcs_filter.cpp
index 3dd53bab1ba6..7f08cbec681b 100644
--- a/src/bench/gcs_filter.cpp
+++ b/src/bench/gcs_filter.cpp
@@ -5,38 +5,84 @@
 #include <bench/bench.h>
 #include <blockfilter.h>
 
-static void ConstructGCSFilter(benchmark::Bench& bench)
+static const GCSFilter::ElementSet GenerateGCSTestElements()
 {
     GCSFilter::ElementSet elements;
-    for (int i = 0; i < 10000; ++i) {
+
+    // Testing the benchmarks with different number of elements show that a filter
+    // with at least 100,000 elements results in benchmarks that have the same
+    // ns/op. This makes it easy to reason about how long (in nanoseconds) a single
+    // filter element takes to process.
+    for (int i = 0; i < 100000; ++i) {
         GCSFilter::Element element(32);
         element[0] = static_cast<unsigned char>(i);
         element[1] = static_cast<unsigned char>(i >> 8);
         elements.insert(std::move(element));
     }
 
+    return elements;
+}
+
+static void GCSBlockFilterGetHash(benchmark::Bench& bench)
+{
+    auto elements = GenerateGCSTestElements();
+
+    GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
+    BlockFilter block_filter(BlockFilterType::BASIC_FILTER, {}, filter.GetEncoded(), /*skip_decode_check=*/false);
+
+    bench.run([&] {
+        block_filter.GetHash();
+    });
+}
+
+static void GCSFilterConstruct(benchmark::Bench& bench)
+{
+    auto elements = GenerateGCSTestElements();
+
     uint64_t siphash_k0 = 0;
-    bench.batch(elements.size()).unit("elem").run([&] {
-        GCSFilter filter({siphash_k0, 0, 20, 1 << 20}, elements);
+    bench.run([&]{
+        GCSFilter filter({siphash_k0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
+
         siphash_k0++;
     });
 }
 
-static void MatchGCSFilter(benchmark::Bench& bench)
+static void GCSFilterDecode(benchmark::Bench& bench)
 {
-    GCSFilter::ElementSet elements;
-    for (int i = 0; i < 10000; ++i) {
-        GCSFilter::Element element(32);
-        element[0] = static_cast<unsigned char>(i);
-        element[1] = static_cast<unsigned char>(i >> 8);
-        elements.insert(std::move(element));
-    }
-    GCSFilter filter({0, 0, 20, 1 << 20}, elements);
+    auto elements = GenerateGCSTestElements();
 
-    bench.unit("elem").run([&] {
-        filter.Match(GCSFilter::Element());
+    GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
+    auto encoded = filter.GetEncoded();
+
+    bench.run([&] {
+        GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/false);
+    });
+}
+
+static void GCSFilterDecodeSkipCheck(benchmark::Bench& bench)
+{
+    auto elements = GenerateGCSTestElements();
+
+    GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
+    auto encoded = filter.GetEncoded();
+
+    bench.run([&] {
+        GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/true);
     });
 }
 
-BENCHMARK(ConstructGCSFilter);
-BENCHMARK(MatchGCSFilter);
+static void GCSFilterMatch(benchmark::Bench& bench)
+{
+    auto elements = GenerateGCSTestElements();
+
+    GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
+
+    bench.run([&] {
+        filter.Match(GCSFilter::Element());
+    });
+}
+BENCHMARK(GCSBlockFilterGetHash);
+BENCHMARK(GCSFilterConstruct);
+BENCHMARK(GCSFilterDecode);
+BENCHMARK(GCSFilterDecodeSkipCheck);
+BENCHMARK(GCSFilterMatch);
diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp
index 25ca14c002ec..e7cc29afda70 100644
--- a/src/blockfilter.cpp
+++ b/src/blockfilter.cpp
@@ -47,7 +47,7 @@ GCSFilter::GCSFilter(const Params& params)
     : m_params(params), m_N(0), m_F(0), m_encoded{0}
 {}
 
-GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter)
+GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check)
     : m_params(params), m_encoded(std::move(encoded_filter))
 {
     SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0};
@@ -59,6 +59,8 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi
     }
     m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
 
+    if (skip_decode_check) return;
+
     // Verify that the encoded filter contains exactly N elements. If it has too much or too little
     // data, a std::ios_base::failure exception will be raised.
     BitStreamReader<SpanReader> bitreader{stream};
@@ -219,14 +221,14 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block,
 }
 
 BlockFilter::BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
-                         std::vector<unsigned char> filter)
+                         std::vector<unsigned char> filter, bool skip_decode_check)
     : m_filter_type(filter_type), m_block_hash(block_hash)
 {
     GCSFilter::Params params;
     if (!BuildParams(params)) {
         throw std::invalid_argument("unknown filter_type");
     }
-    m_filter = GCSFilter(params, std::move(filter));
+    m_filter = GCSFilter(params, std::move(filter), skip_decode_check);
 }
 
 BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo)
diff --git a/src/blockfilter.h b/src/blockfilter.h
index 8bdd789dd50b..081e35ed9f20 100644
--- a/src/blockfilter.h
+++ b/src/blockfilter.h
@@ -60,7 +60,7 @@ class GCSFilter
     explicit GCSFilter(const Params& params = Params());
 
     /** Reconstructs an already-created filter from an encoding. */
-    GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter);
+    GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check);
 
     /** Builds a new filter from the params and set of elements. */
     GCSFilter(const Params& params, const ElementSet& elements);
@@ -123,7 +123,7 @@ class BlockFilter
 
     //! Reconstruct a BlockFilter from parts.
     BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
-                std::vector<unsigned char> filter);
+                std::vector<unsigned char> filter, bool skip_decode_check);
 
     //! Construct a new BlockFilter of the specified type from a block.
     BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo);
@@ -165,7 +165,7 @@ class BlockFilter
         if (!BuildParams(params)) {
             throw std::ios_base::failure("unknown filter_type");
         }
-        m_filter = GCSFilter(params, std::move(encoded_filter));
+        m_filter = GCSFilter(params, std::move(encoded_filter), /*skip_decode_check=*/false);
     }
 };
 
diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp
index 75a190fdb864..2e84934eba91 100644
--- a/src/index/blockfilterindex.cpp
+++ b/src/index/blockfilterindex.cpp
@@ -5,6 +5,7 @@
 #include <map>
 
 #include <dbwrapper.h>
+#include <hash.h>
 #include <index/blockfilterindex.h>
 #include <node/blockstorage.h>
 #include <serialize.h>
@@ -146,18 +147,22 @@ bool BlockFilterIndex::CommitInternal(CDBBatch& batch)
     return BaseIndex::CommitInternal(batch);
 }
 
-bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const
+bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const
 {
     CAutoFile filein(m_filter_fileseq->Open(pos, true), SER_DISK, CLIENT_VERSION);
     if (filein.IsNull()) {
         return false;
     }
 
+    // Check that the hash of the encoded_filter matches the one stored in the db.
     uint256 block_hash;
     std::vector<uint8_t> encoded_filter;
     try {
         filein >> block_hash >> encoded_filter;
-        filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter));
+        uint256 result;
+        CHash256().Write(encoded_filter).Finalize(result);
+        if (result != hash) return error("Checksum mismatch in filter decode.");
+        filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true);
     }
     catch (const std::exception& e) {
         return error("%s: Failed to deserialize block filter from disk: %s", __func__, e.what());
@@ -384,7 +389,7 @@ bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter&
         return false;
     }
 
-    return ReadFilterFromDisk(entry.pos, filter_out);
+    return ReadFilterFromDisk(entry.pos, entry.hash, filter_out);
 }
 
 bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out)
@@ -428,7 +433,7 @@ bool BlockFilterIndex::LookupFilterRange(int start_height, const CBlockIndex* st
     filters_out.resize(entries.size());
     auto filter_pos_it = filters_out.begin();
     for (const auto& entry : entries) {
-        if (!ReadFilterFromDisk(entry.pos, *filter_pos_it)) {
+        if (!ReadFilterFromDisk(entry.pos, entry.hash, *filter_pos_it)) {
             return false;
         }
         ++filter_pos_it;
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
index 06b107d111df..38754ead04ad 100644
--- a/src/index/blockfilterindex.h
+++ b/src/index/blockfilterindex.h
@@ -32,7 +32,7 @@ class BlockFilterIndex final : public BaseIndex
     FlatFilePos m_next_filter_pos;
     std::unique_ptr<FlatFileSeq> m_filter_fileseq;
 
-    bool ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const;
+    bool ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const;
     size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter);
 
     Mutex m_cs_headers_cache;

From 9e75b99c5309767b1e8c7f995f4e8d1976a22f9c Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Fri, 30 Sep 2022 08:44:04 -0400
Subject: [PATCH 12/15] merge bitcoin#26215: Improve
 BaseIndex::BlockUntilSyncedToCurrentChain reliability

---
 src/index/base.cpp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 376d0314ca43..ba403e8002cf 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -270,6 +270,10 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const
     }
 
     if (WriteBlock(*block, pindex)) {
+        // Setting the best block index is intentionally the last step of this
+        // function, so BlockUntilSyncedToCurrentChain callers waiting for the
+        // best block index to be updated can rely on the block being fully
+        // processed, and the index object being safe to delete.
         SetBestBlockIndex(pindex);
     } else {
         FatalError("%s: Failed to write block %s to index",
@@ -381,10 +385,17 @@ IndexSummary BaseIndex::GetSummary() const
 void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
     assert(!fPruneMode || AllowPrune());
 
-    m_best_block_index = block;
     if (AllowPrune() && block) {
         PruneLockInfo prune_lock;
         prune_lock.height_first = block->nHeight;
         WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
     }
+
+    // Intentionally set m_best_block_index as the last step in this function,
+    // after updating prune locks above, and after making any other references
+    // to *this, so the BlockUntilSyncedToCurrentChain function (which checks
+    // m_best_block_index as an optimization) can be used to wait for the last
+    // BlockConnected notification and safely assume that prune locks are
+    // updated and that the index object is safe to delete.
+    m_best_block_index = block;
 }

From b04b71a9570912b08ff2e3da9c38e5db737a79e4 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 16:36:33 +0000
Subject: [PATCH 13/15] merge bitcoin#24858: incorrect blk file size
 calculation during reindex results in recoverable blk file corruption

---
 src/Makefile.test.include       |  1 +
 src/node/blockstorage.cpp       | 13 +++++++----
 src/node/blockstorage.h         |  4 ++++
 src/test/blockmanager_tests.cpp | 39 +++++++++++++++++++++++++++++++++
 src/validation.cpp              | 13 ++++++++++-
 5 files changed, 65 insertions(+), 5 deletions(-)
 create mode 100644 src/test/blockmanager_tests.cpp

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 839224e9a1c3..a9b24c0007fd 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -89,6 +89,7 @@ BITCOIN_TESTS =\
   test/blockencodings_tests.cpp \
   test/blockfilter_tests.cpp \
   test/blockfilter_index_tests.cpp \
+  test/blockmanager_tests.cpp \
   test/bloom_tests.cpp \
   test/bls_tests.cpp \
   test/bswap_tests.cpp \
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 960759396f7e..2c8cacccef8f 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -790,19 +790,24 @@ bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus
     return true;
 }
 
-/** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */
 FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CChain& active_chain, const CChainParams& chainparams, const FlatFilePos* dbp)
 {
     unsigned int nBlockSize = ::GetSerializeSize(block, CLIENT_VERSION);
     FlatFilePos blockPos;
-    if (dbp != nullptr) {
+    const auto position_known {dbp != nullptr};
+    if (position_known) {
         blockPos = *dbp;
+    } else {
+        // when known, blockPos.nPos points at the offset of the block data in the blk file. that already accounts for
+        // the serialization header present in the file (the 4 magic message start bytes + the 4 length bytes = 8 bytes = BLOCK_SERIALIZATION_HEADER_SIZE).
+        // we add BLOCK_SERIALIZATION_HEADER_SIZE only for new blocks since they will have the serialization header added when written to disk.
+        nBlockSize += static_cast<unsigned int>(BLOCK_SERIALIZATION_HEADER_SIZE);
     }
-    if (!FindBlockPos(blockPos, nBlockSize + 8, nHeight, active_chain, block.GetBlockTime(), dbp != nullptr)) {
+    if (!FindBlockPos(blockPos, nBlockSize, nHeight, active_chain, block.GetBlockTime(), position_known)) {
         error("%s: FindBlockPos failed", __func__);
         return FlatFilePos();
     }
-    if (dbp == nullptr) {
+    if (!position_known) {
         if (!WriteBlockToDisk(block, blockPos, chainparams.MessageStart())) {
             AbortNode("Failed to write block");
             return FlatFilePos();
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 7f671fd5f8de..1b919df908e2 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -47,6 +47,9 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
 /** The maximum size of a blk?????.dat file (since 0.8) */
 static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
 
+/** Size of header written by WriteBlockToDisk before a serialized CBlock */
+static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int);
+
 extern std::atomic_bool fImporting;
 extern std::atomic_bool fReindex;
 /** Pruning-related variables and constants */
@@ -185,6 +188,7 @@ class BlockManager
     bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex* pindex, const CChainParams& chainparams)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    /** Store block on disk. If dbp is not nullptr, then it provides the known position of the block within a block file on disk. */
     FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, CChain& active_chain, const CChainParams& chainparams, const FlatFilePos* dbp);
 
     /** Calculate the amount of disk space the block & undo files currently use */
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
new file mode 100644
index 000000000000..a9ce570a3df5
--- /dev/null
+++ b/src/test/blockmanager_tests.cpp
@@ -0,0 +1,39 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <chainparams.h>
+#include <node/blockstorage.h>
+#include <node/context.h>
+#include <validation.h>
+
+#include <boost/test/unit_test.hpp>
+#include <test/util/setup_common.h>
+
+// use BasicTestingSetup here for the data directory configuration, setup, and cleanup
+BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
+{
+    const auto params {CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)};
+    BlockManager blockman {};
+    CChain chain {};
+    // simulate adding a genesis block normally
+    BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, *params, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE);
+    // simulate what happens during reindex
+    // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file
+    // the block is found at offset 8 because there is an 8 byte serialization header
+    // consisting of 4 magic bytes + 4 length bytes before each block in a well-formed blk file.
+    FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE};
+    BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, *params, &pos).nPos, BLOCK_SERIALIZATION_HEADER_SIZE);
+    // now simulate what happens after reindex for the first new block processed
+    // the actual block contents don't matter, just that it's a block.
+    // verify that the write position is at offset 0x12d.
+    // this is a check to make sure that https://github.com/bitcoin/bitcoin/issues/21379 does not recur
+    // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293
+    // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301
+    FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1, chain, *params, nullptr)};
+    BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(params->GenesisBlock(), CLIENT_VERSION) + BLOCK_SERIALIZATION_HEADER_SIZE);
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/validation.cpp b/src/validation.cpp
index efe4e2cfe2f7..844ad7eb6e2b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4705,7 +4705,18 @@ void CChainState::LoadExternalBlockFile(
                     }
                 }
             } catch (const std::exception& e) {
-                LogPrintf("%s: Deserialize or I/O error - %s\n", __func__, e.what());
+                // historical bugs added extra data to the block files that does not deserialize cleanly.
+                // commonly this data is between readable blocks, but it does not really matter. such data is not fatal to the import process.
+                // the code that reads the block files deals with invalid data by simply ignoring it.
+                // it continues to search for the next {4 byte magic message start bytes + 4 byte length + block} that does deserialize cleanly
+                // and passes all of the other block validation checks dealing with POW and the merkle root, etc...
+                // we merely note with this informational log message when unexpected data is encountered.
+                // we could also be experiencing a storage system read error, or a read of a previous bad write. these are possible, but
+                // less likely scenarios. we don't have enough information to tell a difference here.
+                // the reindex process is not the place to attempt to clean and/or compact the block files. if so desired, a studious node operator
+                // may use knowledge of the fact that the block files are not entirely pristine in order to prepare a set of pristine, and
+                // perhaps ordered, block files for later reindexing.
+                LogPrint(BCLog::REINDEX, "%s: unexpected data at file offset 0x%x - %s. continuing\n", __func__, (nRewind - 1), e.what());
             }
         }
     } catch (const std::runtime_error& e) {

From e531dff5f71aee68a0cbfc029419af463e7fd87d Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Fri, 28 Oct 2022 15:24:22 -0400
Subject: [PATCH 14/15] merge bitcoin#26417: fix intermittent failure in
 feature_index_prune.py

---
 test/functional/feature_index_prune.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py
index 0222de33e3eb..dcb649362f82 100755
--- a/test/functional/feature_index_prune.py
+++ b/test/functional/feature_index_prune.py
@@ -140,6 +140,7 @@ def run_test(self):
             self.connect_nodes(i, 3)
 
         self.sync_blocks(timeout=300)
+        self.sync_index(height=2500)
 
         for node in self.nodes[:2]:
             with node.assert_debug_log(['limited pruning to height 2489']):

From 7d9ff96091312885a5dc5471c55e3b8afdbdb632 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Sun, 13 Oct 2024 16:46:11 +0000
Subject: [PATCH 15/15] merge bitcoin#16981: Improve runtime performance of
 --reindex

---
 src/Makefile.bench.include         |  1 +
 src/bench/load_external.cpp        | 63 ++++++++++++++++++++++++++++
 src/streams.h                      | 48 ++++++++++++++-------
 src/test/streams_tests.cpp         | 67 ++++++++++++++++++++++++++++--
 src/validation.cpp                 | 46 ++++++++++++--------
 test/functional/feature_reindex.py | 50 ++++++++++++++++++++++
 6 files changed, 238 insertions(+), 37 deletions(-)
 create mode 100644 src/bench/load_external.cpp

diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 133bc282ef74..6e13fd173b9c 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -35,6 +35,7 @@ bench_bench_dash_SOURCES = \
   bench/ccoins_caching.cpp \
   bench/gcs_filter.cpp \
   bench/hashpadding.cpp \
+  bench/load_external.cpp \
   bench/merkle_root.cpp \
   bench/mempool_eviction.cpp \
   bench/mempool_stress.cpp \
diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp
new file mode 100644
index 000000000000..6bc359a408a8
--- /dev/null
+++ b/src/bench/load_external.cpp
@@ -0,0 +1,63 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <bench/data.h>
+#include <chainparams.h>
+#include <test/util/setup_common.h>
+#include <validation.h>
+
+/**
+ * The LoadExternalBlockFile() function is used during -reindex and -loadblock.
+ *
+ * Create a test file that's similar to a datadir/blocks/blk?????.dat file,
+ * It contains around 134 copies of the same block (typical size of real block files).
+ * For each block in the file, LoadExternalBlockFile() won't find its parent,
+ * and so will skip the block. (In the real system, it will re-read the block
+ * from disk later when it encounters its parent.)
+ *
+ * This benchmark measures the performance of deserializing the block (or just
+ * its header, beginning with PR 16981).
+ */
+static void LoadExternalBlockFile(benchmark::Bench& bench)
+{
+    const auto testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};
+
+    // Create a single block as in the blocks files (magic bytes, block size,
+    // block data) as a stream object.
+    const fs::path blkfile{testing_setup.get()->m_path_root / "blk.dat"};
+    CDataStream ss(SER_DISK, 0);
+    auto params{Params()};
+    ss << params.MessageStart();
+    ss << static_cast<uint32_t>(benchmark::data::block813851.size());
+    // We can't use the streaming serialization (ss << benchmark::data::block813851)
+    // because that first writes a compact size.
+    ss.write(MakeByteSpan(benchmark::data::block813851));
+
+    // Create the test file.
+    {
+        // "wb+" is "binary, O_RDWR | O_CREAT | O_TRUNC".
+        FILE* file{fsbridge::fopen(blkfile, "wb+")};
+        // Make the test block file about 128 MB in length.
+        for (size_t i = 0; i < MAX_BLOCKFILE_SIZE / ss.size(); ++i) {
+            if (fwrite(ss.data(), 1, ss.size(), file) != ss.size()) {
+                throw std::runtime_error("write to test file failed\n");
+            }
+        }
+        fclose(file);
+    }
+
+    CChainState& chainstate{testing_setup->m_node.chainman->ActiveChainstate()};
+    std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent;
+    FlatFilePos pos;
+    bench.run([&] {
+        // "rb" is "binary, O_RDONLY", positioned to the start of the file.
+        // The file will be closed by LoadExternalBlockFile().
+        FILE* file{fsbridge::fopen(blkfile, "rb")};
+        chainstate.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
+    });
+    fs::remove(blkfile);
+}
+
+BENCHMARK(LoadExternalBlockFile);
diff --git a/src/streams.h b/src/streams.h
index ced2b1f76fe2..76feda569ff0 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -641,7 +641,6 @@ class CBufferedFile
     uint64_t nRewind;     //!< how many bytes we guarantee to rewind
     std::vector<std::byte> vchBuf; //!< the buffer
 
-protected:
     //! read data from the source to fill the buffer
     bool Fill() {
         unsigned int pos = nSrcPos % vchBuf.size();
@@ -659,6 +658,28 @@ class CBufferedFile
         return true;
     }
 
+    //! Advance the stream's read pointer (m_read_pos) by up to 'length' bytes,
+    //! filling the buffer from the file so that at least one byte is available.
+    //! Return a pointer to the available buffer data and the number of bytes
+    //! (which may be less than the requested length) that may be accessed
+    //! beginning at that pointer.
+    std::pair<std::byte*, size_t> AdvanceStream(size_t length)
+    {
+        assert(m_read_pos <= nSrcPos);
+        if (m_read_pos + length > nReadLimit) {
+            throw std::ios_base::failure("Attempt to position past buffer limit");
+        }
+        // If there are no bytes available, read from the file.
+        if (m_read_pos == nSrcPos && length > 0) Fill();
+
+        size_t buffer_offset{static_cast<size_t>(m_read_pos % vchBuf.size())};
+        size_t buffer_available{static_cast<size_t>(vchBuf.size() - buffer_offset)};
+        size_t bytes_until_source_pos{static_cast<size_t>(nSrcPos - m_read_pos)};
+        size_t advance{std::min({length, buffer_available, bytes_until_source_pos})};
+        m_read_pos += advance;
+        return std::make_pair(&vchBuf[buffer_offset], advance);
+    }
+
 public:
     CBufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn)
         : nType(nTypeIn), nVersion(nVersionIn), nSrcPos(0), m_read_pos(0), nReadLimit(std::numeric_limits<uint64_t>::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0})
@@ -696,24 +717,21 @@ class CBufferedFile
     //! read a number of bytes
     void read(Span<std::byte> dst)
     {
-        if (dst.size() + m_read_pos > nReadLimit) {
-            throw std::ios_base::failure("Read attempted past buffer limit");
-        }
         while (dst.size() > 0) {
-            if (m_read_pos == nSrcPos)
-                Fill();
-            unsigned int pos = m_read_pos % vchBuf.size();
-            size_t nNow = dst.size();
-            if (nNow + pos > vchBuf.size())
-                nNow = vchBuf.size() - pos;
-            if (nNow + m_read_pos > nSrcPos)
-                nNow = nSrcPos - m_read_pos;
-            memcpy(dst.data(), &vchBuf[pos], nNow);
-            m_read_pos += nNow;
-            dst = dst.subspan(nNow);
+            auto [buffer_pointer, length]{AdvanceStream(dst.size())};
+            memcpy(dst.data(), buffer_pointer, length);
+            dst = dst.subspan(length);
         }
     }
 
+    //! Move the read position ahead in the stream to the given position.
+    //! Use SetPos() to back up in the stream, not SkipTo().
+    void SkipTo(const uint64_t file_pos)
+    {
+        assert(file_pos >= m_read_pos);
+        while (m_read_pos < file_pos) AdvanceStream(file_pos - m_read_pos);
+    }
+
     //! return the current reading position
     uint64_t GetPos() const {
         return m_read_pos;
diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
index 2eae80bf8b79..2f8b76d59262 100644
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -251,7 +251,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
         BOOST_CHECK(false);
     } catch (const std::exception& e) {
         BOOST_CHECK(strstr(e.what(),
-                        "Read attempted past buffer limit") != nullptr);
+                           "Attempt to position past buffer limit") != nullptr);
     }
     // The default argument removes the limit completely.
     BOOST_CHECK(bf.SetLimit());
@@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
     BOOST_CHECK(!bf.SetPos(0));
     // But we should now be positioned at least as far back as allowed
     // by the rewind window (relative to our farthest read position, 40).
-    BOOST_CHECK(bf.GetPos() <= 30);
+    BOOST_CHECK(bf.GetPos() <= 30U);
 
     // We can explicitly close the file, or the destructor will do it.
     bf.fclose();
@@ -328,6 +328,55 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
     fs::remove("streams_test_tmp");
 }
 
+BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
+{
+    fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
+    FILE* file = fsbridge::fopen(streams_test_filename, "w+b");
+    // The value at each offset is the byte offset (e.g. byte 1 in the file has the value 0x01).
+    for (uint8_t j = 0; j < 40; ++j) {
+        fwrite(&j, 1, 1, file);
+    }
+    rewind(file);
+
+    // The buffer is 25 bytes, allow rewinding 10 bytes.
+    CBufferedFile bf(file, 25, 10, 222, 333);
+
+    uint8_t i;
+    // This is like bf >> (7-byte-variable), in that it will cause data
+    // to be read from the file into memory, but it's not copied to us.
+    bf.SkipTo(7);
+    BOOST_CHECK_EQUAL(bf.GetPos(), 7U);
+    bf >> i;
+    BOOST_CHECK_EQUAL(i, 7);
+
+    // The bytes in the buffer up to offset 7 are valid and can be read.
+    BOOST_CHECK(bf.SetPos(0));
+    bf >> i;
+    BOOST_CHECK_EQUAL(i, 0);
+    bf >> i;
+    BOOST_CHECK_EQUAL(i, 1);
+
+    bf.SkipTo(11);
+    bf >> i;
+    BOOST_CHECK_EQUAL(i, 11);
+
+    // SkipTo() honors the transfer limit; we can't position beyond the limit.
+    bf.SetLimit(13);
+    try {
+        bf.SkipTo(14);
+        BOOST_CHECK(false);
+    } catch (const std::exception& e) {
+        BOOST_CHECK(strstr(e.what(), "Attempt to position past buffer limit") != nullptr);
+    }
+
+    // We can position exactly to the transfer limit.
+    bf.SkipTo(13);
+    BOOST_CHECK_EQUAL(bf.GetPos(), 13U);
+
+    bf.fclose();
+    fs::remove(streams_test_filename);
+}
+
 BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
 {
     // Make this test deterministic.
@@ -358,7 +407,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
             // sizes; the boundaries of the objects can interact arbitrarily
             // with the CBufferFile's internal buffer. These first three
             // cases simulate objects of various sizes (1, 2, 5 bytes).
-            switch (InsecureRandRange(5)) {
+            switch (InsecureRandRange(6)) {
             case 0: {
                 uint8_t a[1];
                 if (currentPos + 1 > fileSize)
@@ -396,6 +445,16 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
                 break;
             }
             case 3: {
+                // SkipTo is similar to the "read" cases above, except
+                // we don't receive the data.
+                size_t skip_length{static_cast<size_t>(InsecureRandRange(5))};
+                if (currentPos + skip_length > fileSize) continue;
+                bf.SetLimit(currentPos + skip_length);
+                bf.SkipTo(currentPos + skip_length);
+                currentPos += skip_length;
+                break;
+            }
+            case 4: {
                 // Find a byte value (that is at or ahead of the current position).
                 size_t find = currentPos + InsecureRandRange(8);
                 if (find >= fileSize)
@@ -412,7 +471,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
                 currentPos++;
                 break;
             }
-            case 4: {
+            case 5: {
                 size_t requestPos = InsecureRandRange(maxPos + 4);
                 bool okay = bf.SetPos(requestPos);
                 // The new position may differ from the requested position
diff --git a/src/validation.cpp b/src/validation.cpp
index 844ad7eb6e2b..0e9ca1b60967 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4603,6 +4603,8 @@ void CChainState::LoadExternalBlockFile(
         unsigned int nMaxBlockSize = MaxBlockSize();
         // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor
         CBufferedFile blkdat(fileIn, 2*nMaxBlockSize, nMaxBlockSize+8, SER_DISK, CLIENT_VERSION);
+        // nRewind indicates where to resume scanning in case something goes wrong,
+        // such as a block fails to deserialize.
         uint64_t nRewind = blkdat.GetPos();
         while (!blkdat.eof()) {
             if (ShutdownRequested()) return;
@@ -4626,28 +4628,30 @@ void CChainState::LoadExternalBlockFile(
                     continue;
             } catch (const std::exception&) {
                 // no valid block header found; don't complain
+                // (this happens at the end of every blk.dat file)
                 break;
             }
             try {
-                // read block
-                uint64_t nBlockPos = blkdat.GetPos();
+                // read block header
+                const uint64_t nBlockPos{blkdat.GetPos()};
                 if (dbp)
                     dbp->nPos = nBlockPos;
                 blkdat.SetLimit(nBlockPos + nSize);
-                std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>();
-                CBlock& block = *pblock;
-                blkdat >> block;
-                nRewind = blkdat.GetPos();
-
-                uint256 hash = block.GetHash();
+                CBlockHeader header;
+                blkdat >> header;
+                const uint256 hash{header.GetHash()};
+                // Skip the rest of this block (this may read from disk into memory); position to the marker before the
+                // next block, but it's still possible to rewind to the start of the current block (without a disk read).
+                nRewind = nBlockPos + nSize;
+                blkdat.SkipTo(nRewind);
                 {
                     LOCK(cs_main);
                     // detect out of order blocks, and store them for later
-                    if (hash != m_params.GetConsensus().hashGenesisBlock && !m_blockman.LookupBlockIndex(block.hashPrevBlock)) {
+                    if (hash != m_params.GetConsensus().hashGenesisBlock && !m_blockman.LookupBlockIndex(header.hashPrevBlock)) {
                         LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(),
-                                block.hashPrevBlock.ToString());
+                                 header.hashPrevBlock.ToString());
                         if (dbp && blocks_with_unknown_parent) {
-                            blocks_with_unknown_parent->emplace(block.hashPrevBlock, *dbp);
+                            blocks_with_unknown_parent->emplace(header.hashPrevBlock, *dbp);
                         }
                         continue;
                     }
@@ -4655,13 +4659,19 @@ void CChainState::LoadExternalBlockFile(
                     // process in case the block isn't known yet
                     const CBlockIndex* pindex = m_blockman.LookupBlockIndex(hash);
                     if (!pindex || (pindex->nStatus & BLOCK_HAVE_DATA) == 0) {
-                      BlockValidationState state;
-                      if (AcceptBlock(pblock, state, nullptr, true, dbp, nullptr)) {
-                          nLoaded++;
-                      }
-                      if (state.IsError()) {
-                          break;
-                      }
+                        // This block can be processed immediately; rewind to its start, read and deserialize it.
+                        blkdat.SetPos(nBlockPos);
+                        std::shared_ptr<CBlock> pblock{std::make_shared<CBlock>()};
+                        blkdat >> *pblock;
+                        nRewind = blkdat.GetPos();
+
+                        BlockValidationState state;
+                        if (AcceptBlock(pblock, state, nullptr, true, dbp, nullptr)) {
+                            nLoaded++;
+                        }
+                        if (state.IsError()) {
+                            break;
+                        }
                     } else if (hash != m_params.GetConsensus().hashGenesisBlock && pindex->nHeight % 1000 == 0) {
                         LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), pindex->nHeight);
                     }
diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py
index 8c15a74e2ff4..588d8766ad30 100755
--- a/test/functional/feature_reindex.py
+++ b/test/functional/feature_reindex.py
@@ -7,9 +7,12 @@
 - Start a single node and generate 3 blocks.
 - Stop the node and restart it with -reindex. Verify that the node has reindexed up to block 3.
 - Stop the node and restart it with -reindex-chainstate. Verify that the node has reindexed up to block 3.
+- Verify that out-of-order blocks are correctly processed, see LoadExternalBlockFile()
 """
 
+import os
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.p2p import MAGIC_BYTES
 from test_framework.util import assert_equal
 
 
@@ -27,6 +30,50 @@ def reindex(self, justchainstate=False, txindex=0):
         assert_equal(self.nodes[0].getblockcount(), blockcount)  # start_node is blocking on reindex
         self.log.info("Success")
 
+    # Check that blocks can be processed out of order
+    def out_of_order(self):
+        # The previous test created 24 blocks
+        assert_equal(self.nodes[0].getblockcount(), 24)
+        self.stop_nodes()
+
+        # In this test environment, blocks will always be in order (since
+        # we're generating them rather than getting them from peers), so to
+        # test out-of-order handling, swap blocks 1 and 2 on disk.
+        blk0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00000.dat')
+        with open(blk0, 'r+b') as bf:
+            # Read at least the first few blocks (including genesis)
+            b = bf.read(2000)
+
+            # Find the offsets of blocks 2, 3, and 4 (the first 3 blocks beyond genesis)
+            # by searching for the regtest marker bytes (see pchMessageStart).
+            def find_block(b, start):
+                return b.find(MAGIC_BYTES["regtest"], start)+4
+
+            genesis_start = find_block(b, 0)
+            assert_equal(genesis_start, 4)
+            b2_start = find_block(b, genesis_start)
+            b3_start = find_block(b, b2_start)
+            b4_start = find_block(b, b3_start)
+
+            # Blocks 2 and 3 should be the same size.
+            assert_equal(b3_start-b2_start, b4_start-b3_start)
+
+            # Swap the second and third blocks (don't disturb the genesis block).
+            bf.seek(b2_start)
+            bf.write(b[b3_start:b4_start])
+            bf.write(b[b2_start:b3_start])
+
+        # The reindexing code should detect and accommodate out of order blocks.
+        with self.nodes[0].assert_debug_log([
+            'LoadExternalBlockFile: Out of order block',
+            'LoadExternalBlockFile: Processing out of order child',
+        ]):
+            extra_args = [["-reindex"]]
+            self.start_nodes(extra_args)
+
+        # All blocks should be accepted and processed.
+        assert_equal(self.nodes[0].getblockcount(), 24)
+
     def run_test(self):
         for txindex in [0, 1]:
             self.reindex(False, txindex)
@@ -34,5 +81,8 @@ def run_test(self):
             self.reindex(False, txindex)
             self.reindex(True, txindex)
 
+        self.out_of_order()
+
+
 if __name__ == '__main__':
     ReindexTest().main()
