diff --git a/src/Makefile.test.include b/src/Makefile.test.include index f2c44568ef60..efc9a88fcfc3 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -82,6 +82,7 @@ BITCOIN_TESTS =\ test/base32_tests.cpp \ test/base58_tests.cpp \ test/base64_tests.cpp \ + test/baseindex_tests.cpp \ test/bech32_tests.cpp \ test/bip32_tests.cpp \ test/bip324_tests.cpp \ diff --git a/src/index/addressindex.cpp b/src/index/addressindex.cpp index 342de4e359d6..8ce3783ea36f 100644 --- a/src/index/addressindex.cpp +++ b/src/index/addressindex.cpp @@ -141,11 +141,9 @@ bool AddressIndex::DB::UpdateAddressUnspentIndex(const std::vector& address_entries, +void AddressIndex::DB::RewindBatch(CDBBatch& batch, const std::vector& address_entries, const std::vector& unspent_entries) { - CDBBatch batch(*this); - for (const auto& [key, _] : address_entries) { batch.Erase(std::make_pair(DB_ADDRESSINDEX, key)); } @@ -157,8 +155,6 @@ bool AddressIndex::DB::RewindBatch(const std::vector& addres batch.Write(std::make_pair(DB_ADDRESSUNSPENTINDEX, key), value); } } - - return CDBWrapper::WriteBatch(batch); } AddressIndex::AddressIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory, bool f_wipe) : @@ -285,17 +281,17 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i // Rewind the unspent index by processing blocks in reverse // We need to undo all operations from current_tip back to (but not including) new_tip - for (const CBlockIndex* pindex = current_tip_index; pindex != new_tip_index; pindex = pindex->pprev) { + return RewindBlockByBlock(current_tip_index, new_tip_index, [&](const CBlockIndex* pindex, CDBBatch& batch) { CBlock block; if (!node::ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { - LogError("%s: Failed to read block %s from disk during rewind\n", __func__, + LogError("%s: Failed to read block %s from disk during rewind\n", "CustomRewind", pindex->GetBlockHash().ToString()); return false; } CBlockUndo blockundo; if (pindex->nHeight > 0 && !node::UndoReadFromDisk(blockundo, pindex)) { - LogError("%s: Failed to read undo data for block %s during rewind\n", __func__, + LogError("%s: Failed to read undo data for block %s during rewind\n", "CustomRewind", pindex->GetBlockHash().ToString()); return false; } @@ -308,7 +304,7 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i // reverse order ensures spends are undone before outputs, preventing phantom UTXOs. // blockundo.vtxundo[i] corresponds to block.vtx[i+1] (coinbase skipped) if (blockundo.vtxundo.size() != block.vtx.size() - 1) { - LogError("%s: Undo data size mismatch for block %s (expected %zu, got %zu)\n", __func__, + LogError("%s: Undo data size mismatch for block %s (expected %zu, got %zu)\n", "CustomRewind", pindex->GetBlockHash().ToString(), block.vtx.size() - 1, blockundo.vtxundo.size()); return false; } @@ -340,7 +336,7 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i // Undo inputs (restore to unspent index, remove spending from history) if (tx->vin.size() != txundo.vprevout.size()) { - LogError("%s: Undo data mismatch for tx %s\n", __func__, txhash.ToString()); + LogError("%s: Undo data mismatch for tx %s\n", "CustomRewind", txhash.ToString()); return false; } @@ -391,14 +387,9 @@ bool AddressIndex::CustomRewind(const interfaces::BlockKey& current_tip, const i } } - // Apply both rewind updates in a single batch to avoid leaving the index half-rewound. - if (!m_db->RewindBatch(addressIndex, addressUnspentIndex)) { - LogError("%s: Failed to apply address index rewind batch\n", __func__); - return false; - } - } - - return true; + m_db->RewindBatch(batch, addressIndex, addressUnspentIndex); + return true; + }); } BaseIndex::DB& AddressIndex::GetDB() const { return *m_db; } diff --git a/src/index/addressindex.h b/src/index/addressindex.h index 06497faba7cf..265e5243dbdd 100644 --- a/src/index/addressindex.h +++ b/src/index/addressindex.h @@ -54,13 +54,14 @@ class AddressIndex final : public BaseIndex /// Update address unspent index (handles both adds and deletes) bool UpdateAddressUnspentIndex(const std::vector& entries); - /// Atomically erase address history entries and update unspent outputs during rewind. - bool RewindBatch(const std::vector& address_entries, + /// Add the erasures of address history entries and the unspent output updates of a rewind to batch. + void RewindBatch(CDBBatch& batch, const std::vector& address_entries, const std::vector& unspent_entries); }; /// Override to return false - we need undo data bool AllowPrune() const override { return false; } + bool WritesEagerly() const override { return true; } /// Write block data to the index databases bool CustomAppend(const interfaces::BlockInfo& block) override; diff --git a/src/index/base.cpp b/src/index/base.cpp index 42f6f12df59c..bbdc430b0218 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -27,6 +27,7 @@ using node::ReadBlockFromDisk; using node::g_indexes_ready_to_sync; constexpr uint8_t DB_BEST_BLOCK{'B'}; +constexpr uint8_t DB_UNCOMMITTED_TIP{'U'}; constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; @@ -55,13 +56,16 @@ BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool .options = [] { DBOptions options; node::ReadDatabaseArgs(gArgs, options); return options; }()}} {} -bool BaseIndex::DB::ReadBestBlock(CBlockLocator& locator) const +CBlockLocator BaseIndex::DB::ReadBestBlock() const { + CBlockLocator locator; + bool success = Read(DB_BEST_BLOCK, locator); if (!success) { locator.SetNull(); } - return success; + + return locator; } void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator) @@ -69,6 +73,23 @@ void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator batch.Write(DB_BEST_BLOCK, locator); } +std::optional BaseIndex::DB::ReadUncommittedTip() const +{ + uint256 block_hash; + if (!Read(DB_UNCOMMITTED_TIP, block_hash)) return std::nullopt; + return block_hash; +} + +void BaseIndex::DB::WriteUncommittedTip(CDBBatch& batch, const uint256& block_hash) +{ + batch.Write(DB_UNCOMMITTED_TIP, block_hash); +} + +void BaseIndex::DB::EraseUncommittedTip(CDBBatch& batch) +{ + batch.Erase(DB_UNCOMMITTED_TIP); +} + BaseIndex::BaseIndex(std::unique_ptr chain, std::string name) : m_chain{std::move(chain)}, m_name{std::move(name)} {} @@ -80,24 +101,34 @@ BaseIndex::~BaseIndex() bool BaseIndex::Init() { - CBlockLocator locator; - if (!GetDB().ReadBestBlock(locator)) { - locator.SetNull(); - } + const auto locator{GetDB().ReadBestBlock()}; LOCK(cs_main); CChain& active_chain = m_chainstate->m_chain; - if (locator.IsNull()) { - SetBestBlockIndex(nullptr); - } else { + const CBlockIndex* locator_index{nullptr}; + if (!locator.IsNull()) { // Setting the best block to the locator's top block. If it is not part of the // best chain, we will rewind to the fork point during index sync - const CBlockIndex* locator_index{m_chainstate->m_blockman.LookupBlockIndex(locator.vHave.at(0))}; + locator_index = m_chainstate->m_blockman.LookupBlockIndex(locator.vHave.at(0)); if (!locator_index) { return InitError(strprintf(Untranslated("%s: best block of the index not found. Please rebuild the index."), GetName())); } - SetBestBlockIndex(locator_index); } + const CBlockIndex* resume_block{ResumeBlock(m_chainstate->m_blockman, locator_index)}; + const auto uncommitted_tip{WritesEagerly() ? GetDB().ReadUncommittedTip() : std::nullopt}; + if (uncommitted_tip && resume_block == locator_index) { + // The entries of the blocks up to it are on disk, but without the blocks the index can't tell whether they + // are still on the active chain, nor rewind them. + if (!m_chainstate->m_blockman.LookupBlockIndex(*uncommitted_tip)) { + return InitError(strprintf(Untranslated("%s: the block the index was last synced to is not found. Please " + "rebuild the index."), + GetName())); + } + CDBBatch batch(GetDB()); + GetDB().EraseUncommittedTip(batch); + GetDB().WriteBatch(batch); + } + SetBestBlockIndex(resume_block); // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain). const CBlockIndex* start_block = m_best_block_index.load(); @@ -157,11 +188,17 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& return chain.Genesis(); } - const CBlockIndex* pindex = chain.Next(pindex_prev); - if (pindex) { + if (const auto* pindex{chain.Next(pindex_prev)}) { return pindex; } + // If there is no next block, we might be synced + if (pindex_prev == chain.Tip()) { + return nullptr; + } + + // Since block is not in the chain, return the next block in the chain AFTER the last common ancestor. + // Caller will be responsible for rewinding back to the common ancestor. return chain.Next(chain.FindFork(pindex_prev)); } @@ -177,8 +214,8 @@ void BaseIndex::ThreadSync() if (!m_synced) { auto& consensus_params = Params().GetConsensus(); - std::chrono::steady_clock::time_point last_log_time{0s}; - std::chrono::steady_clock::time_point last_locator_write_time{0s}; + auto last_log_time{NodeClock::now()}; + auto last_locator_write_time{last_log_time}; while (true) { if (m_interrupt) { SetBestBlockIndex(pindex); @@ -222,14 +259,14 @@ void BaseIndex::ThreadSync() return; } - auto current_time{std::chrono::steady_clock::now()}; - if (last_log_time + SYNC_LOG_INTERVAL < current_time) { + auto current_time{NodeClock::now()}; + if (current_time - last_log_time >= SYNC_LOG_INTERVAL) { LogPrintf("Syncing %s with block chain from height %d\n", GetName(), pindex->nHeight); last_log_time = current_time; } - if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) { + if (current_time - last_locator_write_time >= SYNC_LOCATOR_WRITE_INTERVAL) { SetBestBlockIndex(pindex); last_locator_write_time = current_time; // No need to handle errors in Commit. See rationale above. @@ -245,47 +282,98 @@ void BaseIndex::ThreadSync() } } -bool BaseIndex::Commit() +void BaseIndex::Commit() { // Don't commit anything if we haven't indexed any block yet // (this could happen if init is interrupted). bool ok = m_best_block_index != nullptr; if (ok) { + // Don't commit if the index best block is not an ancestor of the chainstate's last flushed + // block. Otherwise, after an unclean shutdown, the index could be + // persisted ahead of a chainstate it can no longer roll back to, which + // would corrupt indexes with state (e.g. coinstatsindex). + const CBlockIndex* index_tip = m_best_block_index.load(); + const CBlockIndex* last_flushed = WITH_LOCK(::cs_main, return m_chainstate->GetLastFlushedBlock()); + if (!last_flushed || last_flushed->GetAncestor(index_tip->nHeight) != index_tip) { + LogDebug(BCLog::COINDB, "Skipping commit, index is ahead of flushed chainstate (index height %d, last flush at height %d)\n", + index_tip->nHeight, last_flushed ? last_flushed->nHeight : -1); + if (WritesEagerly()) { + CDBBatch batch(GetDB()); + GetDB().WriteUncommittedTip(batch, index_tip->GetBlockHash()); + GetDB().WriteBatch(batch); + } + return; + } CDBBatch batch(GetDB()); ok = CustomCommit(batch); if (ok) { GetDB().WriteBestBlock(batch, GetLocator(*m_chain, m_best_block_index.load()->GetBlockHash())); + if (WritesEagerly()) GetDB().EraseUncommittedTip(batch); ok = GetDB().WriteBatch(batch); } } if (!ok) { LogError("%s: Failed to commit latest %s state\n", __func__, GetName()); - return false; + } +} + +void BaseIndex::WriteRewindLocator(CDBBatch& batch, const CBlockIndex* new_tip) const +{ + const auto above_new_tip{[&](const uint256& block_hash) { + const CBlockIndex* block{WITH_LOCK(::cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block_hash))}; + return block && block->nHeight > new_tip->nHeight && block->GetAncestor(new_tip->nHeight) == new_tip; + }}; + // The committed tip was an ancestor of a flushed block when it was written, so it and new_tip below it are in + // the block index after a restart. A committed tip at or below new_tip needs no change. + if (const auto locator{GetDB().ReadBestBlock()}; !locator.IsNull() && above_new_tip(locator.vHave.front())) { + GetDB().WriteBestBlock(batch, GetLocator(*m_chain, new_tip->GetBlockHash())); + } + // Init() resumes from the uncommitted tip only while the block index has it, and then new_tip below it too. + if (const auto uncommitted_tip{GetDB().ReadUncommittedTip()}; uncommitted_tip && above_new_tip(*uncommitted_tip)) { + GetDB().WriteUncommittedTip(batch, new_tip->GetBlockHash()); + } +} + +const CBlockIndex* BaseIndex::ResumeBlock(const node::BlockManager& blockman, const CBlockIndex* locator_block) const +{ + AssertLockHeld(::cs_main); + // The entries up to the uncommitted tip are on disk, so resume from it while the block index has it above the + // locator. Commit() and rewinds keep the locator below it; one that isn't was left behind by an older binary. + const auto uncommitted_tip_hash{WritesEagerly() ? GetDB().ReadUncommittedTip() : std::nullopt}; + const CBlockIndex* uncommitted_tip{uncommitted_tip_hash ? blockman.LookupBlockIndex(*uncommitted_tip_hash) : nullptr}; + if (uncommitted_tip && (!locator_block || uncommitted_tip->GetAncestor(locator_block->nHeight) == locator_block)) { + return uncommitted_tip; + } + return locator_block; +} + +bool BaseIndex::RewindBlockByBlock(const CBlockIndex* current_tip, const CBlockIndex* new_tip, + const std::function& rewind_block) +{ + for (const CBlockIndex* pindex = current_tip; pindex != new_tip; pindex = pindex->pprev) { + CDBBatch batch(GetDB()); + if (!rewind_block(pindex, batch)) return false; + WriteRewindLocator(batch, pindex->pprev); + if (!GetDB().WriteBatch(batch)) return false; } return true; } bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) { - assert(current_tip == m_best_block_index); assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); if (!CustomRewind({current_tip->GetBlockHash(), current_tip->nHeight}, {new_tip->GetBlockHash(), new_tip->nHeight})) { return false; } - // In the case of a reorg, ensure persisted block locator is not stale. + // Don't commit here - the committed index state must never be ahead of the + // flushed chainstate, otherwise unclean restarts would lead to index corruption. // Pruning has a minimum of 288 blocks-to-keep and getting the index // 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 SetBestBlockIndex(new_tip); - if (!Commit()) { - // If commit fails, revert the best block index to avoid corruption. - SetBestBlockIndex(current_tip); - return false; - } - return true; } @@ -463,9 +551,9 @@ void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) { assert(!m_chainstate->m_blockman.IsPruneMode() || AllowPrune()); - if (AllowPrune() && block) { + if (AllowPrune()) { PruneLockInfo prune_lock; - prune_lock.height_first = block->nHeight; + prune_lock.height_first = block ? block->nHeight : 0; 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 db67924d9e75..d59e535f653e 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -12,11 +12,16 @@ #include #include +#include +#include #include class CBlock; class CBlockIndex; class Chainstate; +namespace node { +class BlockManager; +} // namespace node namespace interfaces { class Chain; } // namespace interfaces @@ -49,10 +54,16 @@ class BaseIndex : public CValidationInterface bool f_memory = false, bool f_wipe = false, bool f_obfuscate = false); /// Read block locator of the chain that the index is in sync with. - bool ReadBestBlock(CBlockLocator& locator) const; + /// Note, the returned locator will be empty if no record exists. + CBlockLocator ReadBestBlock() const; /// Write block locator of the chain that the index is in sync with. void WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator); + + /// The index tip that Commit() last skipped, if no commit has succeeded since. See WritesEagerly(). + std::optional ReadUncommittedTip() const; + void WriteUncommittedTip(CDBBatch& batch, const uint256& block_hash); + void EraseUncommittedTip(CDBBatch& batch); }; private: @@ -82,20 +93,23 @@ class BaseIndex : public CValidationInterface void ThreadSync(); /// Write the current index state (eg. chain block locator and subclass-specific items) to disk. - /// - /// Recommendations for error handling: - /// If called on a successor of the previous committed best block in the index, the index can - /// continue processing without risk of corruption, though the index state will need to catch up - /// from further behind on reboot. If the new state is not a successor of the previous state (due - /// to a chain reorganization), the index must halt until Commit succeeds or else it could end up - /// getting corrupted. - bool Commit(); + /// Will skip the commit if no block has been indexed yet or if the index's best block is + /// ahead of the chainstate's last flushed block. This avoids persisting state an unclean shutdown + /// could not roll back from. A later call commits when the chainstate has flushed far enough. + /// Indexes that write eagerly record a skipped tip instead, see WritesEagerly(). + void Commit(); /// Loop over disconnected blocks and call CustomRewind. bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip); virtual bool AllowPrune() const = 0; + /// Whether CustomAppend() persists a block's entries right away. Commit() can't write the locator for a tip + /// above the last flush, so for these indexes it records that tip separately, and Init() resumes from it, or + /// fails if the block index lost it. Otherwise a restart would resume from the older locator and never rewind the + /// entries of blocks above it that are no longer on the active chain. + virtual bool WritesEagerly() const { return false; } + protected: std::unique_ptr m_chain; Chainstate* m_chainstate{nullptr}; @@ -121,6 +135,22 @@ class BaseIndex : public CValidationInterface /// be an ancestor of the current best block. [[nodiscard]] virtual bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) { return true; } + /// For indexes whose CustomRewind() erases entries: if the committed locator is above new_tip on the same + /// branch, move it to new_tip. Commit() may not write new_tip yet (it can be ahead of the last flush), but + /// leaving the old locator above erased entries would make an unclean restart resume past them. + void WriteRewindLocator(CDBBatch& batch, const CBlockIndex* new_tip) const; + + /// The block Init() resumes from, given the block of the committed locator: the uncommitted tip if the block + /// index has it above that block (see WritesEagerly()), otherwise the locator block itself. + const CBlockIndex* ResumeBlock(const node::BlockManager& blockman, const CBlockIndex* locator_block) const + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + + /// For indexes that erase per block in CustomRewind(): walk from current_tip down to new_tip and write each + /// block's erasures, filled in by rewind_block, in one batch with WriteRewindLocator() for its parent. A rewind + /// interrupted between blocks then never leaves the committed locator above erased entries. + [[nodiscard]] bool RewindBlockByBlock(const CBlockIndex* current_tip, const CBlockIndex* new_tip, + const std::function& rewind_block); + virtual DB& GetDB() const = 0; /// Get the name of the index for display in logs. diff --git a/src/index/govcollateralindex.cpp b/src/index/govcollateralindex.cpp index cab00c694e1c..64913943b3c9 100644 --- a/src/index/govcollateralindex.cpp +++ b/src/index/govcollateralindex.cpp @@ -117,7 +117,7 @@ bool GovCollateralIndex::CustomRewind(const interfaces::BlockKey& current_tip, c m_db->EraseCollateral(batch, txid); } } - m_db->WriteBestBlock(batch, GetLocator(new_tip_index)); + WriteRewindLocator(batch, new_tip_index); return m_db->WriteBatch(batch); } @@ -126,10 +126,18 @@ BaseIndex::DB& GovCollateralIndex::GetDB() const { return *m_db; } bool GovCollateralIndex::CanSync(Chainstate& chainstate) const { AssertLockHeld(::cs_main); - CBlockLocator locator; + const auto locator{m_db->ReadBestBlock()}; const CBlockIndex* best_block{nullptr}; - if (m_db->ReadBestBlock(locator) && !locator.IsNull()) { + const CBlockIndex* locator_block{nullptr}; + if (!locator.IsNull()) { best_block = chainstate.FindForkInGlobalIndex(locator); + locator_block = chainstate.m_blockman.LookupBlockIndex(locator.vHave.front()); + } + // Startup resumes from the uncommitted tip instead when there is one, and only needs the blocks above it. It + // fails without the locator's block, so the uncommitted tip can't stand in for that. + if (locator.IsNull() || locator_block) { + const CBlockIndex* resume_block{ResumeBlock(chainstate.m_blockman, locator_block)}; + if (resume_block != locator_block) best_block = chainstate.m_chain.FindFork(resume_block); } for (const CBlockIndex* block{chainstate.m_chain.Tip()}; block != best_block; block = block->pprev) { if (!(block->nStatus & BLOCK_HAVE_DATA)) return false; diff --git a/src/index/govcollateralindex.h b/src/index/govcollateralindex.h index d7bfddb83159..b39b37b56406 100644 --- a/src/index/govcollateralindex.h +++ b/src/index/govcollateralindex.h @@ -31,6 +31,8 @@ class GovCollateralIndex final : public BaseIndex, public governance::Collateral bool AllowPrune() const override { return true; } protected: + bool WritesEagerly() const override { return true; } + bool CustomInit(const std::optional& block) override; bool CustomAppend(const interfaces::BlockInfo& block) override; diff --git a/src/index/spentindex.cpp b/src/index/spentindex.cpp index 64c373a151ff..06af32143709 100644 --- a/src/index/spentindex.cpp +++ b/src/index/spentindex.cpp @@ -43,13 +43,11 @@ bool SpentIndex::DB::ReadSpentIndex(const CSpentIndexKey& key, CSpentIndexValue& return Read(std::make_pair(DB_SPENTINDEX, key), value); } -bool SpentIndex::DB::EraseSpentIndex(const std::vector& keys) +void SpentIndex::DB::EraseSpentIndex(CDBBatch& batch, const std::vector& keys) { - CDBBatch batch(*this); for (const auto& key : keys) { batch.Erase(std::make_pair(DB_SPENTINDEX, key)); } - return CDBWrapper::WriteBatch(batch); } SpentIndex::SpentIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory, bool f_wipe) : @@ -127,14 +125,14 @@ bool SpentIndex::CustomRewind(const interfaces::BlockKey& current_tip, const int assert(current_tip_index->GetAncestor(new_tip_index->nHeight) == new_tip_index); // Erase spent index entries for blocks being rewound - for (const CBlockIndex* pindex = current_tip_index; pindex != new_tip_index; pindex = pindex->pprev) { + return RewindBlockByBlock(current_tip_index, new_tip_index, [&](const CBlockIndex* pindex, CDBBatch& batch) { // Skip genesis block - if (pindex->nHeight == 0) continue; + if (pindex->nHeight == 0) return true; // Read block to get transactions CBlock block; if (!node::ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { - LogError("%s: Failed to read block %s from disk during rewind\n", __func__, + LogError("%s: Failed to read block %s from disk during rewind\n", "CustomRewind", pindex->GetBlockHash().ToString()); return false; } @@ -152,13 +150,9 @@ bool SpentIndex::CustomRewind(const interfaces::BlockKey& current_tip, const int } } - if (!keys_to_erase.empty() && !m_db->EraseSpentIndex(keys_to_erase)) { - LogError("%s: Failed to erase spent index during rewind\n", __func__); - return false; - } - } - - return true; + m_db->EraseSpentIndex(batch, keys_to_erase); + return true; + }); } BaseIndex::DB& SpentIndex::GetDB() const { return *m_db; } diff --git a/src/index/spentindex.h b/src/index/spentindex.h index 8a47e5babc8e..b7ba2122c274 100644 --- a/src/index/spentindex.h +++ b/src/index/spentindex.h @@ -42,7 +42,7 @@ class SpentIndex final : public BaseIndex bool ReadSpentIndex(const CSpentIndexKey& key, CSpentIndexValue& value); /// Erase spent index entries - bool EraseSpentIndex(const std::vector& keys); + void EraseSpentIndex(CDBBatch& batch, const std::vector& keys); }; bool CustomAppend(const interfaces::BlockInfo& block) override; @@ -53,6 +53,7 @@ class SpentIndex final : public BaseIndex /// SpentIndex cannot work with pruned nodes as it requires UTXO data bool AllowPrune() const override { return false; } + bool WritesEagerly() const override { return true; } public: /// Constructs a new SpentIndex. diff --git a/src/index/timestampindex.cpp b/src/index/timestampindex.cpp index 6f1d9af18952..dca38910eb02 100644 --- a/src/index/timestampindex.cpp +++ b/src/index/timestampindex.cpp @@ -41,9 +41,9 @@ void TimestampIndex::DB::ReadRange(uint32_t high, uint32_t low, std::vector chain, size_t n_cache_size, bool f_memory, bool f_wipe) : @@ -72,20 +72,17 @@ bool TimestampIndex::CustomRewind(const interfaces::BlockKey& current_tip, const const CBlockIndex* new_tip_index = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(new_tip.hash)); assert(current_tip_index->GetAncestor(new_tip_index->nHeight) == new_tip_index); - // Erase timestamp index entries for blocks being rewound + // Erase timestamp index entries for blocks being rewound, in one batch with the locator + CDBBatch batch(GetDB()); for (const CBlockIndex* pindex = current_tip_index; pindex != new_tip_index; pindex = pindex->pprev) { // Skip genesis block if (pindex->nHeight == 0) continue; - CTimestampIndexKey key(pindex->nTime, pindex->GetBlockHash()); - if (!m_db->EraseTimestampIndex(key)) { - LogError("%s: Failed to erase timestamp index for block %s during rewind\n", __func__, - pindex->GetBlockHash().ToString()); - return false; - } + m_db->EraseTimestampIndex(batch, CTimestampIndexKey(pindex->nTime, pindex->GetBlockHash())); } - return true; + WriteRewindLocator(batch, new_tip_index); + return GetDB().WriteBatch(batch); } BaseIndex::DB& TimestampIndex::GetDB() const { return *m_db; } diff --git a/src/index/timestampindex.h b/src/index/timestampindex.h index d24e58ebcd0b..0317c89db51c 100644 --- a/src/index/timestampindex.h +++ b/src/index/timestampindex.h @@ -37,7 +37,7 @@ class TimestampIndex final : public BaseIndex void ReadRange(uint32_t high, uint32_t low, std::vector& hashes); /// Erase timestamp index entry - bool EraseTimestampIndex(const CTimestampIndexKey& key); + void EraseTimestampIndex(CDBBatch& batch, const CTimestampIndexKey& key); }; bool CustomAppend(const interfaces::BlockInfo& block) override; @@ -49,6 +49,7 @@ class TimestampIndex final : public BaseIndex /// TimestampIndex works with pruned nodes since it only stores block metadata bool AllowPrune() const override { return true; } + bool WritesEagerly() const override { return true; } public: /// Constructs a new TimestampIndex. diff --git a/src/test/baseindex_tests.cpp b/src/test/baseindex_tests.cpp new file mode 100644 index 000000000000..bf3117554bf4 --- /dev/null +++ b/src/test/baseindex_tests.cpp @@ -0,0 +1,328 @@ +// Copyright (c) 2020-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include