merge bitcoin#22626: Remove txindex migration code

Author: kwvg

src/index/base.h

@@ -6,13 +6,12 @@
 #define BITCOIN_INDEX_BASE_H
 
 #include <dbwrapper.h>
-#include <primitives/block.h>
-#include <primitives/transaction.h>
 #include <threadinterrupt.h>
 #include <validationinterface.h>
 
 #include <atomic>
 
+class CBlock;
 class CBlockIndex;
 class CChainState;
 

src/index/txindex.cpp

@@ -2,18 +2,14 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <index/disktxpos.h>
 #include <index/txindex.h>
+
+#include <index/disktxpos.h>
 #include <node/blockstorage.h>
-#include <node/ui_interface.h>
-#include <shutdown.h>
 #include <util/system.h>
-#include <util/translation.h>
 #include <validation.h>
 
-constexpr uint8_t DB_BEST_BLOCK{'B'};
 constexpr uint8_t DB_TXINDEX{'t'};
-constexpr uint8_t DB_TXINDEX_BLOCK{'T'};
 
 std::unique_ptr<TxIndex> g_txindex;
 
@@ -30,10 +26,6 @@ class TxIndex::DB : public BaseIndex::DB
 
     /// Write a batch of transaction positions to the DB.
     bool WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_pos);
-
-    /// Migrate txindex data from the block tree DB, where it may be for older nodes that have not
-    /// been upgraded yet to the new database.
-    bool MigrateData(CBlockTreeDB& block_tree_db, const CBlockLocator& best_locator);
 };
 
 TxIndex::DB::DB(size_t n_cache_size, bool f_memory, bool f_wipe) :
@@ -54,163 +46,12 @@ bool TxIndex::DB::WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_
     return WriteBatch(batch);
 }
 
-/*
- * Safely persist a transfer of data from the old txindex database to the new one, and compact the
- * range of keys updated. This is used internally by MigrateData.
- */
-static void WriteTxIndexMigrationBatches(CDBWrapper& newdb, CDBWrapper& olddb,
-                                         CDBBatch& batch_newdb, CDBBatch& batch_olddb,
-                                         const std::pair<uint8_t, uint256>& begin_key,
-                                         const std::pair<uint8_t, uint256>& end_key)
-{
-    // Sync new DB changes to disk before deleting from old DB.
-    newdb.WriteBatch(batch_newdb, /*fSync=*/ true);
-    olddb.WriteBatch(batch_olddb);
-    olddb.CompactRange(begin_key, end_key);
-
-    batch_newdb.Clear();
-    batch_olddb.Clear();
-}
-
-bool TxIndex::DB::MigrateData(CBlockTreeDB& block_tree_db, const CBlockLocator& best_locator)
-{
-    // The prior implementation of txindex was always in sync with block index
-    // and presence was indicated with a boolean DB flag. If the flag is set,
-    // this means the txindex from a previous version is valid and in sync with
-    // the chain tip. The first step of the migration is to unset the flag and
-    // write the chain hash to a separate key, DB_TXINDEX_BLOCK. After that, the
-    // index entries are copied over in batches to the new database. Finally,
-    // DB_TXINDEX_BLOCK is erased from the old database and the block hash is
-    // written to the new database.
-    //
-    // Unsetting the boolean flag ensures that if the node is downgraded to a
-    // previous version, it will not see a corrupted, partially migrated index
-    // -- it will see that the txindex is disabled. When the node is upgraded
-    // again, the migration will pick up where it left off and sync to the block
-    // with hash DB_TXINDEX_BLOCK.
-    bool f_legacy_flag = false;
-    block_tree_db.ReadFlag("txindex", f_legacy_flag);
-    if (f_legacy_flag) {
-        if (!block_tree_db.Write(DB_TXINDEX_BLOCK, best_locator)) {
-            return error("%s: cannot write block indicator", __func__);
-        }
-        if (!block_tree_db.WriteFlag("txindex", false)) {
-            return error("%s: cannot write block index db flag", __func__);
-        }
-    }
-
-    CBlockLocator locator;
-    if (!block_tree_db.Read(DB_TXINDEX_BLOCK, locator)) {
-        return true;
-    }
-
-    int64_t count = 0;
-    LogPrintf("Upgrading txindex database... [0%%]\n");
-    uiInterface.ShowProgress(_("Upgrading txindex database").translated, 0, true);
-    int report_done = 0;
-    const size_t batch_size = 1 << 24; // 16 MiB
-
-    CDBBatch batch_newdb(*this);
-    CDBBatch batch_olddb(block_tree_db);
-
-    std::pair<uint8_t, uint256> key;
-    std::pair<uint8_t, uint256> begin_key{DB_TXINDEX, uint256()};
-    std::pair<uint8_t, uint256> prev_key = begin_key;
-
-    bool interrupted = false;
-    std::unique_ptr<CDBIterator> cursor(block_tree_db.NewIterator());
-    for (cursor->Seek(begin_key); cursor->Valid(); cursor->Next()) {
-        if (ShutdownRequested()) {
-            interrupted = true;
-            break;
-        }
-
-        if (!cursor->GetKey(key)) {
-            return error("%s: cannot get key from valid cursor", __func__);
-        }
-        if (key.first != DB_TXINDEX) {
-            break;
-        }
-
-        // Log progress every 10%.
-        if (++count % 256 == 0) {
-            // Since txids are uniformly random and traversed in increasing order, the high 16 bits
-            // of the hash can be used to estimate the current progress.
-            const uint256& txid = key.second;
-            uint32_t high_nibble =
-                (static_cast<uint32_t>(*(txid.begin() + 0)) << 8) +
-                (static_cast<uint32_t>(*(txid.begin() + 1)) << 0);
-            int percentage_done = (int)(high_nibble * 100.0 / 65536.0 + 0.5);
-
-            uiInterface.ShowProgress(_("Upgrading txindex database").translated, percentage_done, true);
-            if (report_done < percentage_done/10) {
-                LogPrintf("Upgrading txindex database... [%d%%]\n", percentage_done);
-                report_done = percentage_done/10;
-            }
-        }
-
-        CDiskTxPos value;
-        if (!cursor->GetValue(value)) {
-            return error("%s: cannot parse txindex record", __func__);
-        }
-        batch_newdb.Write(key, value);
-        batch_olddb.Erase(key);
-
-        if (batch_newdb.SizeEstimate() > batch_size || batch_olddb.SizeEstimate() > batch_size) {
-            // NOTE: it's OK to delete the key pointed at by the current DB cursor while iterating
-            // because LevelDB iterators are guaranteed to provide a consistent view of the
-            // underlying data, like a lightweight snapshot.
-            WriteTxIndexMigrationBatches(*this, block_tree_db,
-                                         batch_newdb, batch_olddb,
-                                         prev_key, key);
-            prev_key = key;
-        }
-    }
-
-    // If these final DB batches complete the migration, write the best block
-    // hash marker to the new database and delete from the old one. This signals
-    // that the former is fully caught up to that point in the blockchain and
-    // that all txindex entries have been removed from the latter.
-    if (!interrupted) {
-        batch_olddb.Erase(DB_TXINDEX_BLOCK);
-        batch_newdb.Write(DB_BEST_BLOCK, locator);
-    }
-
-    WriteTxIndexMigrationBatches(*this, block_tree_db,
-                                 batch_newdb, batch_olddb,
-                                 begin_key, key);
-
-    if (interrupted) {
-        LogPrintf("[CANCELLED].\n");
-        return false;
-    }
-
-    uiInterface.ShowProgress("", 100, false);
-
-    LogPrintf("[DONE].\n");
-    return true;
-}
-
 TxIndex::TxIndex(size_t n_cache_size, bool f_memory, bool f_wipe)
     : m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe))
 {}
 
 TxIndex::~TxIndex() {}
 
-bool TxIndex::Init()
-{
-    LOCK(cs_main);
-
-    // Attempt to migrate txindex from the old database to the new one. Even if
-    // chain_tip is null, the node could be reindexing and we still want to
-    // delete txindex records in the old database.
-    if (!m_db->MigrateData(*m_chainstate->m_blockman.m_block_tree_db, m_chainstate->m_chain.GetLocator())) {
-        return false;
-    }
-
-    return BaseIndex::Init();
-}
-
 bool TxIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex)
 {
     // Exclude genesis block transaction because outputs are not spendable.

src/index/txindex.h

@@ -5,9 +5,7 @@
 #ifndef BITCOIN_INDEX_TXINDEX_H
 #define BITCOIN_INDEX_TXINDEX_H
 
-#include <chain.h>
 #include <index/base.h>
-#include <txdb.h>
 
 /**
  * TxIndex is used to look up transactions included in the blockchain by hash.
@@ -23,9 +21,6 @@ class TxIndex final : public BaseIndex
     const std::unique_ptr<DB> m_db;
 
 protected:
-    /// Override base class init to migrate from old database.
-    bool Init() override;
-
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 
     BaseIndex::DB& GetDB() const override;

src/init.cpp

@@ -2199,6 +2199,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     // ********************************************************* Step 8: start indexers
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
+        LOCK(::cs_main);
+        if (const auto error{CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db))}) {
+            return InitError(*error);
+        }
+
         g_txindex = std::make_unique<TxIndex>(nTxIndexCache, false, fReindex);
         if (!g_txindex->Start(chainman.ActiveChainstate())) {
             return false;

src/txdb.cpp

@@ -5,6 +5,7 @@
 
 #include <txdb.h>
 
+#include <chain.h>
 #include <node/ui_interface.h>
 #include <pow.h>
 #include <random.h>
@@ -31,6 +32,28 @@ static constexpr uint8_t DB_FLAG{'F'};
 static constexpr uint8_t DB_REINDEX_FLAG{'R'};
 static constexpr uint8_t DB_LAST_BLOCK{'l'};
 
+// Keys used in previous version that might still be found in the DB:
+static constexpr uint8_t DB_TXINDEX_BLOCK{'T'};
+//               uint8_t DB_TXINDEX{'t'}
+
+std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db)
+{
+    CBlockLocator ignored{};
+    if (block_tree_db.Read(DB_TXINDEX_BLOCK, ignored)) {
+        return _("The -txindex upgrade started by a previous version can not be completed. Restart with the previous version or run a full -reindex.");
+    }
+    bool txindex_legacy_flag{false};
+    block_tree_db.ReadFlag("txindex", txindex_legacy_flag);
+    if (txindex_legacy_flag) {
+        // Disable legacy txindex and warn once about occupied disk space
+        if (!block_tree_db.WriteFlag("txindex", false)) {
+            return Untranslated("Failed to write block index db flag 'txindex'='0'");
+        }
+        return _("The block index db contains a legacy 'txindex'. To clear the occupied disk space, run a full -reindex, otherwise ignore this error. This error message will not be displayed again.");
+    }
+    return std::nullopt;
+}
+
 namespace {
 
 struct CoinEntry {

src/txdb.h

@@ -8,19 +8,22 @@
 
 #include <coins.h>
 #include <dbwrapper.h>
-#include <chain.h>
-#include <primitives/block.h>
 #include <spentindex.h>
 #include <timestampindex.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
 
+class CBlockFileInfo;
 class CBlockIndex;
-class CCoinsViewDBCursor;
 class uint256;
+namespace Consensus {
+struct Params;
+};
+struct bilingual_str;
 
 //! -dbcache default (MiB)
 static const int64_t nDefaultDbCache = 300;
@@ -108,4 +111,6 @@ class CBlockTreeDB : public CDBWrapper
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 };
 
+std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);
+
 #endif // BITCOIN_TXDB_H

src/validation.cpp

@@ -155,7 +155,7 @@ bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, i
 
     // CheckFinalTx() uses active_chain_tip.Height()+1 to evaluate
     // nLockTime because when IsFinalTx() is called within
-    // CBlock::AcceptBlock(), the height of the block *being*
+    // AcceptBlock(), the height of the block *being*
     // evaluated is what is used. Thus if we want to know if a
     // transaction can be part of the *next* block, we need to call
     // IsFinalTx() with one more than active_chain_tip.Height().

src/validation.h

@@ -12,17 +12,19 @@
 #endif
 
 #include <amount.h>
+#include <arith_uint256.h>
 #include <attributes.h>
 #include <chain.h>
 #include <fs.h>
 #include <node/blockstorage.h>
 #include <policy/feerate.h>
 #include <policy/packages.h>
 #include <script/script_error.h>
+#include <serialize.h>
 #include <sync.h>
 #include <txdb.h>
 #include <txmempool.h> // For CTxMemPool::cs
-#include <serialize.h>
+#include <uint256.h>
 #include <util/check.h>
 #include <util/hasher.h>
 #include <util/translation.h>
@@ -38,17 +40,10 @@
 #include <utility>
 #include <vector>
 
-namespace llmq {
-class CChainLocksHandler;
-class CInstantSendManager;
-} // namespace llmq
-
-class CEvoDB;
-
 class CChainState;
-class CBlockIndex;
 class CBlockTreeDB;
 class CChainParams;
+class CEvoDB;
 class CMNHFManager;
 class CTxMemPool;
 class TxValidationState;
@@ -61,6 +56,11 @@ struct DisconnectedBlockTransactions;
 struct LockPoints;
 struct AssumeutxoData;
 
+namespace llmq {
+class CChainLocksHandler;
+class CInstantSendManager;
+} // namespace llmq
+
 /** Default for -minrelaytxfee, minimum relay fee for transactions */
 static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000;
 /** Default for -limitancestorcount, max number of in-mempool ancestors */