From c21f23db1b9597ced77a3403c604f65bdfde6774 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Date: Wed, 19 Jul 2023 17:56:35 +0000
Subject: [PATCH] partial bitcoin#24138: Commit MuHash and best block together
 for coinstatsindex

excludes:
- 691d45fdc83ec14f87a400f548553168ac70263f (portions)
---
 src/index/coinstatsindex.cpp      | 17 ++++++++++++-----
 src/index/coinstatsindex.h        |  2 ++
 src/test/coinstatsindex_tests.cpp | 19 ++++++++++++-------
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
index e9ee322793..2233066132 100644
--- a/src/index/coinstatsindex.cpp
+++ b/src/index/coinstatsindex.cpp
@@ -223,10 +223,9 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex)
     m_muhash.Finalize(out);
     value.second.muhash = out;
 
-    CDBBatch batch(*m_db);
-    batch.Write(DBHeightKey(pindex->nHeight), value);
-    batch.Write(DB_MUHASH, m_muhash);
-    return m_db->WriteBatch(batch);
+    // Intentionally do not update DB_MUHASH here so it stays in sync with
+    // DB_BEST_BLOCK, and the index is not corrupted if there is an unclean shutdown.
+    return m_db->Write(DBHeightKey(pindex->nHeight), value);
 }
 
 static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch,
@@ -381,6 +380,14 @@ bool CoinStatsIndex::Init()
     return true;
 }
 
+bool CoinStatsIndex::CommitInternal(CDBBatch& batch)
+{
+    // DB_MUHASH should always be committed in a batch together with DB_BEST_BLOCK
+    // to prevent an inconsistent state of the DB.
+    batch.Write(DB_MUHASH, m_muhash);
+    return BaseIndex::CommitInternal(batch);
+}
+
 // Reverse a single block as part of a reorg
 bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex)
 {
@@ -482,5 +489,5 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex
     Assert(m_total_unspendables_scripts == read_out.second.total_unspendables_scripts);
     Assert(m_total_unspendables_unclaimed_rewards == read_out.second.total_unspendables_unclaimed_rewards);
 
-    return m_db->Write(DB_MUHASH, m_muhash);
+    return true;
 }
diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h
index a575b37c7c..ed0c8673f8 100644
--- a/src/index/coinstatsindex.h
+++ b/src/index/coinstatsindex.h
@@ -39,6 +39,8 @@ private:
 protected:
     bool Init() override;
 
+    bool CommitInternal(CDBBatch& batch) override;
+
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 
     bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override;
diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp
index 106fcd2a33..a21c073a93 100644
--- a/src/test/coinstatsindex_tests.cpp
+++ b/src/test/coinstatsindex_tests.cpp
@@ -14,6 +14,17 @@
 
 BOOST_AUTO_TEST_SUITE(coinstatsindex_tests)
 
+static void IndexWaitSynced(BaseIndex& index)
+{
+    // Allow the CoinStatsIndex to catch up with the block index that is syncing
+    // in a background thread.
+    const auto timeout = GetTime<std::chrono::seconds>() + 120s;
+    while (!index.BlockUntilSyncedToCurrentChain()) {
+        BOOST_REQUIRE(timeout > GetTime<std::chrono::milliseconds>());
+        UninterruptibleSleep(100ms);
+    }
+}
+
 BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
 {
     CoinStatsIndex coin_stats_index{1 << 20, true};
@@ -34,13 +45,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
 
     BOOST_REQUIRE(coin_stats_index.Start(::ChainstateActive()));
 
-    // Allow the CoinStatsIndex to catch up with the block index that is syncing
-    // in a background thread.
-    const auto timeout = GetTime<std::chrono::seconds>() + 120s;
-    while (!coin_stats_index.BlockUntilSyncedToCurrentChain()) {
-        BOOST_REQUIRE(timeout > GetTime<std::chrono::milliseconds>());
-        UninterruptibleSleep(100ms);
-    }
+    IndexWaitSynced(coin_stats_index);
 
     // Check that CoinStatsIndex works for genesis block.
     const CBlockIndex* genesis_block_index;