diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -40,6 +40,8 @@ protected: bool Init() override; + bool CommitInternal(CDBBatch &batch) override; + bool WriteBlock(const CBlock &block, const CBlockIndex *pindex) override; bool Rewind(const CBlockIndex *current_tip, diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -248,10 +248,10 @@ 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, @@ -421,6 +421,13 @@ 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) { @@ -540,5 +547,5 @@ 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/test/CMakeLists.txt b/src/test/CMakeLists.txt --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -53,6 +53,7 @@ util/setup_common.cpp util/str.cpp util/transaction_utils.cpp + util/validation.cpp util/wallet.cpp ) diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -2,8 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include #include +#include #include #include @@ -16,6 +19,16 @@ 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() + 120s; + while (!index.BlockUntilSyncedToCurrentChain()) { + BOOST_REQUIRE(timeout > GetTime()); + UninterruptibleSleep(100ms); + } +} + BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) { CoinStatsIndex coin_stats_index{1 << 20, true}; @@ -35,13 +48,7 @@ BOOST_REQUIRE(coin_stats_index.Start(m_node.chainman->ActiveChainstate())); - // Allow the CoinStatsIndex to catch up with the block index that is syncing - // in a background thread. - const auto timeout = GetTime() + 120s; - while (!coin_stats_index.BlockUntilSyncedToCurrentChain()) { - BOOST_REQUIRE(timeout > GetTime()); - UninterruptibleSleep(100ms); - } + IndexWaitSynced(coin_stats_index); // Check that CoinStatsIndex works for genesis block. const CBlockIndex *genesis_block_index; @@ -78,4 +85,59 @@ // Rest of shutdown sequence and destructors happen in ~TestingSetup() } +// 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) { + Chainstate &chainstate = Assert(m_node.chainman)->ActiveChainstate(); + const Config &config = GetConfig(); + { + CoinStatsIndex index{1 << 20}; + BOOST_REQUIRE(index.Start(chainstate)); + IndexWaitSynced(index); + std::shared_ptr 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(block); + + LOCK(cs_main); + BlockValidationState state; + BlockValidationOptions options{config}; + BOOST_CHECK(CheckBlock( + block, state, config.GetChainParams().GetConsensus(), options)); + BOOST_CHECK(chainstate.AcceptBlock(config, new_block, state, true, + nullptr, nullptr)); + + // Get the block index (not returned by AcceptBlock since D2127) + auto it{m_node.chainman->m_blockman.m_block_index.find( + new_block->GetHash())}; + if (it != m_node.chainman->m_blockman.m_block_index.end()) { + new_block_index = &(it->second); + } + + CCoinsViewCache view(&chainstate.CoinsTip()); + BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, + view, options)); + } + // 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.h b/src/test/util/validation.h new file mode 100644 --- /dev/null +++ b/src/test/util/validation.h @@ -0,0 +1,19 @@ +// 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. + +#ifndef BITCOIN_TEST_UTIL_VALIDATION_H +#define BITCOIN_TEST_UTIL_VALIDATION_H + +#include + +class CValidationInterface; + +class ValidationInterfaceTest { +public: + static void BlockConnected(CValidationInterface &obj, + const std::shared_ptr &block, + const CBlockIndex *pindex); +}; + +#endif // BITCOIN_TEST_UTIL_VALIDATION_H diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp new file mode 100644 --- /dev/null +++ b/src/test/util/validation.cpp @@ -0,0 +1,14 @@ +// 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. + +#include + +#include +#include + +void ValidationInterfaceTest::BlockConnected( + CValidationInterface &obj, const std::shared_ptr &block, + const CBlockIndex *pindex) { + obj.BlockConnected(block, pindex); +} diff --git a/src/validationinterface.h b/src/validationinterface.h --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -191,6 +191,7 @@ virtual void BlockFinalized(const CBlockIndex *pindex){}; friend class CMainSignals; + friend class ValidationInterfaceTest; }; class MainSignalsImpl;