Merge bitcoin/bitcoin#22626: Remove txindex migration code

fa20f81 Remove txindex migration code (MarcoFalke)
fae8786 doc: Fix validation typo (MarcoFalke)
fab8900 Add missing includes and forward declarations, remove unused ones (MarcoFalke)

Pull request description:

  No supported version of Bitcoin Core used the legacy txindex, so all relevant nodes can be assumed to have upgraded. Thus, there is no need to keep this code any longer.

  As a temporary courtesy, provide a one-time warning on how to free the disk space used by the legacy txindex.

  Fixes #22615

ACKs for top commit:
  laanwj:
    Code review ACK fa20f81
  hebasto:
    ACK fa20f81, tested on Linux Mint 20.2 (x86_64).
  Zero-1729:
    crACK fa20f81
  theStack:
    Approach ACK fa20f81

Tree-SHA512: 68aa32d064d1e3932e6e382816a4b5de417bd7e82861fea1ee50660e8c397f4efeb88ae4ed54a8ad1952c3563eb0b8449d7ccf883c353cc4d4dc7e15c53d78e8

Author: laanwj

src/index/base.h

@@ -6,11 +6,10 @@
 #define BITCOIN_INDEX_BASE_H
 
 #include <dbwrapper.h>
-#include <primitives/block.h>
-#include <primitives/transaction.h>
 #include <threadinterrupt.h>
 #include <validationinterface.h>
 
+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

@@ -1567,6 +1567,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     // ********************************************************* Step 8: start indexers
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
+        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>
@@ -27,6 +28,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,17 +8,20 @@
 
 #include <coins.h>
 #include <dbwrapper.h>
-#include <chain.h>
-#include <primitives/block.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 = 450;
@@ -86,4 +89,6 @@ class CBlockTreeDB : public CDBWrapper
     bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex);
 };
 
+std::optional<bilingual_str> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);
+
 #endif // BITCOIN_TXDB_H

src/validation.cpp

@@ -192,7 +192,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

@@ -11,7 +11,9 @@
 #endif
 
 #include <amount.h>
+#include <arith_uint256.h>
 #include <attributes.h>
+#include <chain.h>
 #include <coins.h>
 #include <consensus/validation.h>
 #include <crypto/common.h> // for ReadLE64
@@ -21,10 +23,11 @@
 #include <policy/packages.h>
 #include <protocol.h> // For CMessageHeader::MessageStartChars
 #include <script/script_error.h>
+#include <serialize.h>
 #include <sync.h>
-#include <txmempool.h> // For CTxMemPool::cs
 #include <txdb.h>
-#include <serialize.h>
+#include <txmempool.h> // For CTxMemPool::cs
+#include <uint256.h>
 #include <util/check.h>
 #include <util/hasher.h>
 #include <util/translation.h>
@@ -42,7 +45,6 @@
 
 class CChainState;
 class BlockValidationState;
-class CBlockIndex;
 class CBlockTreeDB;
 class CBlockUndo;
 class CChainParams;