Merge bitcoin/bitcoin#33116: refactor: Convert uint256 to Txid

de0675f refactor: Move `transaction_identifier.h` to primitives (marcofleon)
6f068f6 Remove implicit uint256 conversion and comparison (marcofleon)
9c24cda refactor: Convert remaining instances from uint256 to Txid (marcofleon)
d2ecd68 policy, refactor: Convert uint256 to Txid (marcofleon)
f6c0d1d mempool, refactor: Convert uint256 to Txid (marcofleon)
aeb0f78 refactor: Convert `mini_miner` from uint256 to Txid (marcofleon)
326f244 refactor: Convert RPCs and `merkleblock` from uint256 to Txid (marcofleon)
49b3d3a Clean up `FindTxForGetData` (marcofleon)

Pull request description:

  This is the final leg of the [type safety refactor](bitcoin/bitcoin#32189).

  All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.

ACKs for top commit:
  stickies-v:
    re-ACK de0675f, no changes since a20724d926d5844168c6a13fa8293df8c8927efe except address review nits.
  janb84:
    re ACK de0675f
  dergoegge:
    re-ACK de0675f
  theStack:
    Code-review ACK de0675f

Tree-SHA512: 2413160fca7ab146a8d79d18ce3afcf7384cacc73c513d41928904aa453b4dd7a350064cee71e9c5d015da5904c7c81ac17603e50a47441ebc5b0c653235dd08

Author: glozow

src/blockencodings.cpp

@@ -42,7 +42,7 @@ void CBlockHeaderAndShortTxIDs::FillShortTxIDSelector() const {
 
 uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const Wtxid& wtxid) const {
     static_assert(SHORTTXIDS_LENGTH == 6, "shorttxids calculation assumes 6-byte shorttxids");
-    return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0xffffffffffffL;
+    return SipHashUint256(shorttxidk0, shorttxidk1, wtxid.ToUint256()) & 0xffffffffffffL;
 }
 
 ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector<CTransactionRef>& extra_txn) {

src/consensus/merkle.cpp

@@ -68,7 +68,7 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated)
     std::vector<uint256> leaves;
     leaves.resize(block.vtx.size());
     for (size_t s = 0; s < block.vtx.size(); s++) {
-        leaves[s] = block.vtx[s]->GetHash();
+        leaves[s] = block.vtx[s]->GetHash().ToUint256();
     }
     return ComputeMerkleRoot(std::move(leaves), mutated);
 }
@@ -79,7 +79,7 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
     leaves.resize(block.vtx.size());
     leaves[0].SetNull(); // The witness hash of the coinbase is 0.
     for (size_t s = 1; s < block.vtx.size(); s++) {
-        leaves[s] = block.vtx[s]->GetWitnessHash();
+        leaves[s] = block.vtx[s]->GetWitnessHash().ToUint256();
     }
     return ComputeMerkleRoot(std::move(leaves), mutated);
 }
@@ -185,7 +185,7 @@ std::vector<uint256> TransactionMerklePath(const CBlock& block, uint32_t positio
     std::vector<uint256> leaves;
     leaves.resize(block.vtx.size());
     for (size_t s = 0; s < block.vtx.size(); s++) {
-        leaves[s] = block.vtx[s]->GetHash();
+        leaves[s] = block.vtx[s]->GetHash().ToUint256();
     }
     return ComputeMerklePath(leaves, position);
 }

src/headerssync.cpp

@@ -48,7 +48,7 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus
 
 /** Free any memory in use, and mark this object as no longer usable. This is
  * required to guarantee that we won't reuse this object with the same
- * SaltedTxidHasher for another sync. */
+ * SaltedUint256Hasher for another sync. */
 void HeadersSyncState::Finalize()
 {
     Assume(m_download_state != State::FINAL);

src/headerssync.h

@@ -224,7 +224,7 @@ class HeadersSyncState {
     arith_uint256 m_current_chain_work;
 
     /** m_hasher is a salted hasher for making our 1-bit commitments to headers we've seen. */
-    const SaltedTxidHasher m_hasher;
+    const SaltedUint256Hasher m_hasher;
 
     /** A queue of commitment bits, created during the 1st phase, and verified during the 2nd. */
     bitdeque<> m_header_commitments;

src/index/txindex.cpp

@@ -9,6 +9,7 @@
 #include <index/disktxpos.h>
 #include <logging.h>
 #include <node/blockstorage.h>
+#include <primitives/transaction_identifier.h>
 #include <validation.h>
 
 constexpr uint8_t DB_TXINDEX{'t'};
@@ -24,26 +25,26 @@ class TxIndex::DB : public BaseIndex::DB
 
     /// Read the disk location of the transaction data with the given hash. Returns false if the
     /// transaction hash is not indexed.
-    bool ReadTxPos(const uint256& txid, CDiskTxPos& pos) const;
+    bool ReadTxPos(const Txid& txid, CDiskTxPos& pos) const;
 
     /// Write a batch of transaction positions to the DB.
-    [[nodiscard]] bool WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_pos);
+    [[nodiscard]] bool WriteTxs(const std::vector<std::pair<Txid, CDiskTxPos>>& v_pos);
 };
 
 TxIndex::DB::DB(size_t n_cache_size, bool f_memory, bool f_wipe) :
     BaseIndex::DB(gArgs.GetDataDirNet() / "indexes" / "txindex", n_cache_size, f_memory, f_wipe)
 {}
 
-bool TxIndex::DB::ReadTxPos(const uint256 &txid, CDiskTxPos& pos) const
+bool TxIndex::DB::ReadTxPos(const Txid& txid, CDiskTxPos& pos) const
 {
-    return Read(std::make_pair(DB_TXINDEX, txid), pos);
+    return Read(std::make_pair(DB_TXINDEX, txid.ToUint256()), pos);
 }
 
-bool TxIndex::DB::WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_pos)
+bool TxIndex::DB::WriteTxs(const std::vector<std::pair<Txid, CDiskTxPos>>& v_pos)
 {
     CDBBatch batch(*this);
-    for (const auto& tuple : v_pos) {
-        batch.Write(std::make_pair(DB_TXINDEX, tuple.first), tuple.second);
+    for (const auto& [txid, pos] : v_pos) {
+        batch.Write(std::make_pair(DB_TXINDEX, txid.ToUint256()), pos);
     }
     return WriteBatch(batch);
 }
@@ -61,7 +62,7 @@ bool TxIndex::CustomAppend(const interfaces::BlockInfo& block)
 
     assert(block.data);
     CDiskTxPos pos({block.file_number, block.data_pos}, GetSizeOfCompactSize(block.data->vtx.size()));
-    std::vector<std::pair<uint256, CDiskTxPos>> vPos;
+    std::vector<std::pair<Txid, CDiskTxPos>> vPos;
     vPos.reserve(block.data->vtx.size());
     for (const auto& tx : block.data->vtx) {
         vPos.emplace_back(tx->GetHash(), pos);
@@ -72,7 +73,7 @@ bool TxIndex::CustomAppend(const interfaces::BlockInfo& block)
 
 BaseIndex::DB& TxIndex::GetDB() const { return *m_db; }
 
-bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRef& tx) const
+bool TxIndex::FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const
 {
     CDiskTxPos postx;
     if (!m_db->ReadTxPos(tx_hash, postx)) {

src/index/txindex.h

@@ -42,7 +42,7 @@ class TxIndex final : public BaseIndex
     /// @param[out]  block_hash  The hash of the block the transaction is found in.
     /// @param[out]  tx  The transaction itself.
     /// @return  true if transaction is found, false otherwise
-    bool FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRef& tx) const;
+    bool FindTx(const Txid& tx_hash, uint256& block_hash, CTransactionRef& tx) const;
 };
 
 /// The global transaction index, used in GetTransaction. May be null.

src/interfaces/chain.h

@@ -211,7 +211,7 @@ class Chain
     virtual bool isInMempool(const Txid& txid) = 0;
 
     //! Check if transaction has descendants in mempool.
-    virtual bool hasDescendantsInMempool(const uint256& txid) = 0;
+    virtual bool hasDescendantsInMempool(const Txid& txid) = 0;
 
     //! Transaction is added to memory pool, if the transaction fee is below the
     //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true.
@@ -222,7 +222,7 @@ class Chain
         std::string& err_string) = 0;
 
     //! Calculate mempool ancestor and descendant counts for the given transaction.
-    virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) = 0;
+    virtual void getTransactionAncestry(const Txid& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) = 0;
 
     //! For each outpoint, calculate the fee-bumping cost to spend this outpoint at the specified
     //  feerate, including bumping its ancestors. For example, if the target feerate is 10sat/vbyte

src/interfaces/wallet.h

@@ -9,12 +9,12 @@
 #include <common/signmessage.h>
 #include <consensus/amount.h>
 #include <interfaces/chain.h>
+#include <primitives/transaction_identifier.h>
 #include <pubkey.h>
 #include <script/script.h>
 #include <support/allocators/secure.h>
 #include <util/fs.h>
 #include <util/result.h>
-#include <util/transaction_identifier.h>
 #include <util/ui_change_type.h>
 
 #include <cstdint>

src/kernel/coinstats.cpp

@@ -98,7 +98,7 @@ static void ApplyHash(T& hash_obj, const Txid& hash, const std::map<uint32_t, Co
     }
 }
 
-static void ApplyStats(CCoinsStats& stats, const uint256& hash, const std::map<uint32_t, Coin>& outputs)
+static void ApplyStats(CCoinsStats& stats, const std::map<uint32_t, Coin>& outputs)
 {
     assert(!outputs.empty());
     stats.nTransactions++;
@@ -126,7 +126,7 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
         Coin coin;
         if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {
             if (!outputs.empty() && key.hash != prevkey) {
-                ApplyStats(stats, prevkey, outputs);
+                ApplyStats(stats, outputs);
                 ApplyHash(hash_obj, prevkey, outputs);
                 outputs.clear();
             }
@@ -140,7 +140,7 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
         pcursor->Next();
     }
     if (!outputs.empty()) {
-        ApplyStats(stats, prevkey, outputs);
+        ApplyStats(stats, outputs);
         ApplyHash(hash_obj, prevkey, outputs);
     }
 

src/kernel/disconnected_transactions.h

@@ -41,7 +41,7 @@ class DisconnectedBlockTransactions {
     const size_t m_max_mem_usage;
     std::list<CTransactionRef> queuedTx;
     using TxList = decltype(queuedTx);
-    std::unordered_map<uint256, TxList::iterator, SaltedTxidHasher> iters_by_txid;
+    std::unordered_map<Txid, TxList::iterator, SaltedTxidHasher> iters_by_txid;
 
     /** Trim the earliest-added entries until we are within memory bounds. */
     std::vector<CTransactionRef> LimitMemoryUsage();