Merge bitcoin/bitcoin#22626: Remove txindex migration code

fa20f815a9cb438c5ab61e97a453612ddd8b21b5 Remove txindex migration code (MarcoFalke)
fae878603345854527c211ebb7d1967f12c8bb9d doc: Fix validation typo (MarcoFalke)
fab89006d656261770503e54fdd01ac9167bdd49 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 fa20f815a9cb438c5ab61e97a453612ddd8b21b5
  hebasto:
    ACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5, tested on Linux Mint 20.2 (x86_64).
  Zero-1729:
    crACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5
  theStack:
    Approach ACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5

Tree-SHA512: 68aa32d064d1e3932e6e382816a4b5de417bd7e82861fea1ee50660e8c397f4efeb88ae4ed54a8ad1952c3563eb0b8449d7ccf883c353cc4d4dc7e15c53d78e8

8 files changed, 44 insertions, 175 deletions

diff --git a/src/index/base.h b/src/index/base.h
index df4bdff1ea..1390e3e570 100644
--- a/src/index/base.h
+++ b/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;
 
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
index cde9821f3d..209785d487 100644
--- a/src/index/txindex.cpp
+++ b/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 @@ public:
 
     /// 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.
diff --git a/src/index/txindex.h b/src/index/txindex.h
index 8202c3c951..59375bc204 100644
--- a/src/index/txindex.h
+++ b/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 @@ private:
     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;
diff --git a/src/init.cpp b/src/init.cpp
index 636b089cda..ff36ec805c 100644
--- a/src/init.cpp
+++ b/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;
diff --git a/src/txdb.cpp b/src/txdb.cpp
index 4b76bee5ab..cfa864668a 100644
--- a/src/txdb.cpp
+++ b/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 {
diff --git a/src/txdb.h b/src/txdb.h
index 845d80788f..1bdce71126 100644
--- a/src/txdb.h
+++ b/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 @@ public:
     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
diff --git a/src/validation.cpp b/src/validation.cpp
index 8696f1af85..cc87f98913 100644
--- a/src/validation.cpp
+++ b/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().
diff --git a/src/validation.h b/src/validation.h
index d4fcac1d48..078b988052 100644
--- a/src/validation.h
+++ b/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;