index: Commit DB_MUHASH and DB_BEST_BLOCK to disk together

If these are written to disk at different times,
unclean shutdowns can lead to index corruption.

Author: mzumsande

src/index/coinstatsindex.cpp

@@ -228,10 +228,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,
@@ -388,6 +387,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)
 {
@@ -489,5 +496,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;
 }

src/index/coinstatsindex.h

@@ -39,6 +39,8 @@ class CoinStatsIndex final : public BaseIndex
 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;