From a0e35d33962acea678785346a84a048eb2b4977c Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 3 Nov 2026 04:47:00 +0000 Subject: [PATCH] fix(index): don't adopt an empty or rebuilt async index as migrated legacy data On a datadir upgraded from v23, the legacy "addressindex", "spentindex" and "timestampindex" flags in the block index database stay set after the first v24 start has moved the legacy entries into indexes/. If the index directories are later deleted to rebuild them, the next start finds nothing to migrate, but MigrateOldIndexData() still finalizes the empty database with the chainstate tip. The index then reports itself synced while every query returns nothing (zero balances, no txids, no block hashes), and the same would happen to a rebuild that was interrupted before its first commit. Only finalize a target that actually holds migrated entries, and clear each enabled index's legacy flag once its migration step has run, since the legacy entries no longer exist. A deleted or partially rebuilt index is now rebuilt from genesis. Older versions see the cleared flag and ask for the -reindex that the release notes already require after a downgrade, instead of serving the erased synchronous index. Co-Authored-By: Claude Opus 5.5 --- src/Makefile.test.include | 1 + src/test/index_migration_tests.cpp | 69 ++++++++++++++++++++++++++++++ src/txdb.cpp | 25 ++++++++--- test/util/data/non-backported.txt | 1 + 4 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 src/test/index_migration_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index a145cbdec341..f2c44568ef60 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -140,6 +140,7 @@ BITCOIN_TESTS =\ test/hash_tests.cpp \ test/httpserver_tests.cpp \ test/i2p_tests.cpp \ + test/index_migration_tests.cpp \ test/interfaces_tests.cpp \ test/key_io_tests.cpp \ test/key_tests.cpp \ diff --git a/src/test/index_migration_tests.cpp b/src/test/index_migration_tests.cpp new file mode 100644 index 000000000000..0a6c08d54b7f --- /dev/null +++ b/src/test/index_migration_tests.cpp @@ -0,0 +1,69 @@ +// Copyright (c) 2026 The Dash 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 +#include +#include + +#include + +namespace { +struct IndexMigrationSetup : public TestChainSetup { + IndexMigrationSetup() : + TestChainSetup{100, CBaseChainParams::REGTEST, {"-timestampindex=1"}} + { + } + + //! Give MigrateOldIndexData() the on-disk chainstate best block it reads on a real node. + void WriteChainstateBestBlock() + { + const uint256 tip{WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip()->GetBlockHash())}; + CCoinsViewDB coins_db{DBParams{.path = gArgs.GetDataDirNet() / "chainstate", .cache_bytes = 1 << 20}, + CoinsViewOptions{}}; + CCoinsMapMemoryResource resource; + CCoinsMap no_coins{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource}; + BOOST_REQUIRE(coins_db.BatchWrite(no_coins, tip)); + } + + //! Height the timestamp index resumes from, read before its sync thread can advance it. + int LoadedTimestampIndexHeight() + { + node::g_indexes_ready_to_sync = false; + TimestampIndex index{interfaces::MakeChain(m_node), /*n_cache_size=*/1 << 20}; + BOOST_REQUIRE(index.Start()); + const int height{index.GetSummary().best_block_height}; + index.Interrupt(); + index.Stop(); + node::g_indexes_ready_to_sync = true; + return height; + } +}; +} // namespace + +BOOST_AUTO_TEST_SUITE(index_migration_tests) + +// A datadir upgraded from v23 keeps its legacy "timestampindex" flag. If the index's own +// database is then missing (deleted to rebuild it), there is nothing left to migrate, and +// the empty database must not be stamped with the chain tip as if it were synced. +BOOST_FIXTURE_TEST_CASE(empty_index_is_not_adopted_as_migrated, IndexMigrationSetup) +{ + WriteChainstateBestBlock(); + CBlockTreeDB& block_tree_db{*WITH_LOCK(cs_main, return m_node.chainman->m_blockman.m_block_tree_db.get())}; + BOOST_REQUIRE(block_tree_db.WriteFlag("timestampindex", true)); + + BOOST_REQUIRE(WITH_LOCK(cs_main, return block_tree_db.MigrateOldIndexData())); + + BOOST_CHECK_EQUAL(LoadedTimestampIndexHeight(), 0); + // The legacy entries are gone, so older versions must not trust their synchronous index either. + bool legacy_current{true}; + BOOST_REQUIRE(block_tree_db.ReadFlag("timestampindex", legacy_current)); + BOOST_CHECK(!legacy_current); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txdb.cpp b/src/txdb.cpp index 29ed4a0633fa..6d5817d34c54 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -475,14 +475,17 @@ static bool FinalizeMigration(CDBWrapper& target_db, const uint256& best_block_h return true; } -// Returns true if the target index database has no best-block locator yet — -// i.e. the new async index has never been finalized on this datadir. Used to -// guard FinalizeMigration so we don't regress an already-advanced locator -// (e.g. left by ThreadSync after a previous successful migration). +// Returns true if the target index database holds migrated entries but no best-block +// locator yet, i.e. a migration copied data that was never finalized. Used to guard +// FinalizeMigration so we don't regress an already-advanced locator (e.g. left by +// ThreadSync after a previous successful migration), and so we never stamp the chain +// tip onto an empty target (e.g. index directories deleted after an earlier migration +// already moved the legacy entries), which would report an empty index as synced. static bool TargetNeedsLocator(CDBWrapper& target_db) { CBlockLocator existing; - return !target_db.Read(DB_BEST_BLOCK, existing) || existing.IsNull(); + if (target_db.Read(DB_BEST_BLOCK, existing) && !existing.IsNull()) return false; + return !target_db.IsEmpty(); } bool CBlockTreeDB::MigrateOldIndexData() @@ -519,7 +522,11 @@ bool CBlockTreeDB::MigrateOldIndexData() // the legacy entries don't cover the suffix of the chain that ran with the index // disabled, and copying them while stamping the chainstate tip as the locator would // mark an incomplete index as fully synced. In that case we discard the source entries - // instead and let the new async index resync from genesis. + // instead and let the new async index resync from genesis. Once an enabled index has been + // processed its legacy entries are gone, so its flag is cleared: a later start must never + // adopt whatever the new database then holds (for example a partial rebuild after the + // index directory was deleted) as migrated data, and older versions see that their + // synchronous index needs a reindex. bool legacy_flag = false; const bool addressindex_was_current = ReadFlag("addressindex", legacy_flag) && legacy_flag; const bool spentindex_was_current = ReadFlag("spentindex", legacy_flag) && legacy_flag; @@ -575,12 +582,14 @@ bool CBlockTreeDB::MigrateOldIndexData() return false; } // Finalize even when count==0 to recover from a previous run that crashed - // after writing target data but before writing the locator. + // after writing target data but before writing the locator. An empty target is + // left without a locator, so the index rebuilds from genesis. if (timestampindex_was_current && TargetNeedsLocator(timestamp_db)) { if (!FinalizeMigration(timestamp_db, best_block_hash, "timestamp index")) { return false; } } + WriteFlag("timestampindex", false); } // Migrate spent index (only if enabled) @@ -599,6 +608,7 @@ bool CBlockTreeDB::MigrateOldIndexData() return false; } } + WriteFlag("spentindex", false); } // Migrate address index (includes both address and unspent indexes) (only if enabled) @@ -627,6 +637,7 @@ bool CBlockTreeDB::MigrateOldIndexData() return false; } } + WriteFlag("addressindex", false); } if (total_count > 0) { diff --git a/test/util/data/non-backported.txt b/test/util/data/non-backported.txt index 19db26c1ce8f..1c868c6d1400 100644 --- a/test/util/data/non-backported.txt +++ b/test/util/data/non-backported.txt @@ -99,6 +99,7 @@ src/test/unordered_lru_cache_tests.cpp src/test/util/llmq_tests.h src/test/util/masternode.* src/test/governance*.cpp +src/test/index_migration_tests.cpp src/unordered_lru_cache.h src/util/edge.* src/util/helpers.h