Merge acbd6faa78 into c5e44a0435

32d55e28af test: Use the correct node for doubled keypath test (Ava Chow)

Pull request description:

  #29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node.

ACKs for top commit:
  maflcko:
    lgtm ACK 32d55e28af
  rkrux:
    ACK 32d55e28af
  BrandonOdiwuor:
    Code Review ACK 32d55e28af

Tree-SHA512: 1e0231985beb382b16e1d608c874750423d0502388db0c8ad450b22d17f9d96f5e16a6b44948ebda5efc750f62b60d0de8dd20131f449427426a36caf374af92

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;
 }
 
 

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.h

@@ -224,7 +224,7 @@ private:
     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 <util/transaction_identifier.h>
 #include <validation.h>
 
 constexpr uint8_t DB_TXINDEX{'t'};
@@ -24,26 +25,26 @@ public:
 
     /// 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);
+        batch.Write(std::make_pair(DB_TXINDEX, tuple.first.ToUint256()), tuple.second);
     }
     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 @@ public:
     /// @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

@@ -207,10 +207,10 @@ public:
     virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0;
 
     //! Check if transaction is in mempool.
-    virtual bool isInMempool(const uint256& txid) = 0;
+    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.
@@ -221,7 +221,7 @@ public:
         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

@@ -159,16 +159,16 @@ public:
         WalletOrderForm order_form) = 0;
 
     //! Return whether transaction can be abandoned.
-    virtual bool transactionCanBeAbandoned(const uint256& txid) = 0;
+    virtual bool transactionCanBeAbandoned(const Txid& txid) = 0;
 
     //! Abandon transaction.
-    virtual bool abandonTransaction(const uint256& txid) = 0;
+    virtual bool abandonTransaction(const Txid& txid) = 0;
 
     //! Return whether transaction can be bumped.
-    virtual bool transactionCanBeBumped(const uint256& txid) = 0;
+    virtual bool transactionCanBeBumped(const Txid& txid) = 0;
 
     //! Create bump transaction.
-    virtual bool createBumpTransaction(const uint256& txid,
+    virtual bool createBumpTransaction(const Txid& txid,
         const wallet::CCoinControl& coin_control,
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
@@ -179,28 +179,28 @@ public:
     virtual bool signBumpTransaction(CMutableTransaction& mtx) = 0;
 
     //! Commit bump transaction.
-    virtual bool commitBumpTransaction(const uint256& txid,
+    virtual bool commitBumpTransaction(const Txid& txid,
         CMutableTransaction&& mtx,
         std::vector<bilingual_str>& errors,
-        uint256& bumped_txid) = 0;
+        Txid& bumped_txid) = 0;
 
     //! Get a transaction.
-    virtual CTransactionRef getTx(const uint256& txid) = 0;
+    virtual CTransactionRef getTx(const Txid& txid) = 0;
 
     //! Get transaction information.
-    virtual WalletTx getWalletTx(const uint256& txid) = 0;
+    virtual WalletTx getWalletTx(const Txid& txid) = 0;
 
     //! Get list of all wallet transactions.
     virtual std::set<WalletTx> getWalletTxs() = 0;
 
     //! Try to get updated status for a particular transaction, if possible without blocking.
-    virtual bool tryGetTxStatus(const uint256& txid,
+    virtual bool tryGetTxStatus(const Txid& txid,
         WalletTxStatus& tx_status,
         int& num_blocks,
         int64_t& block_time) = 0;
 
     //! Get transaction details.
-    virtual WalletTx getWalletTxDetails(const uint256& txid,
+    virtual WalletTx getWalletTxDetails(const Txid& txid,
         WalletTxStatus& tx_status,
         WalletOrderForm& order_form,
         bool& in_mempool,
@@ -306,7 +306,7 @@ public:
     virtual std::unique_ptr<Handler> handleAddressBookChanged(AddressBookChangedFn fn) = 0;
 
     //! Register handler for transaction changed messages.
-    using TransactionChangedFn = std::function<void(const uint256& txid, ChangeType status)>;
+    using TransactionChangedFn = std::function<void(const Txid& txid, ChangeType status)>;
     virtual std::unique_ptr<Handler> handleTransactionChanged(TransactionChangedFn fn) = 0;
 
     //! Register handler for watchonly changed messages.

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 @@ private:
     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();

src/merkleblock.cpp

@@ -44,11 +44,11 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std:
             vMatch.push_back(true);
         } else if (filter && filter->IsRelevantAndUpdate(*block.vtx[i])) {
             vMatch.push_back(true);
-            vMatchedTxn.emplace_back(i, hash);
+            vMatchedTxn.emplace_back(i, hash.ToUint256());
         } else {
             vMatch.push_back(false);
         }
-        vHashes.push_back(hash);
+        vHashes.push_back(hash.ToUint256());
     }
 
     txn = CPartialMerkleTree(vHashes, vMatch);

src/net_processing.cpp

@@ -302,7 +302,7 @@ struct Peer {
          *  non-wtxid-relay peers, wtxid for wtxid-relay peers). We use the
          *  mempool to sort transactions in dependency order before relay, so
          *  this does not have to be sorted. */
-        std::set<uint256> m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex);
+        std::set<GenTxid> m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex);
         /** Whether the peer has requested us to send our complete mempool. Only
          *  permitted if the peer has NetPermissionFlags::Mempool or we advertise
          *  NODE_BLOOM. See BIP35. */
@@ -536,7 +536,7 @@ public:
     std::vector<TxOrphanage::OrphanTxBase> GetOrphanTransactions() override EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
     PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
-    void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
+    void RelayTransaction(const Txid& txid, const Wtxid& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void SetBestBlock(int height, std::chrono::seconds time) override
     {
         m_best_height = height;
@@ -856,7 +856,7 @@ private:
     std::shared_ptr<const CBlock> m_most_recent_block GUARDED_BY(m_most_recent_block_mutex);
     std::shared_ptr<const CBlockHeaderAndShortTxIDs> m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex);
     uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex);
-    std::unique_ptr<const std::map<uint256, CTransactionRef>> m_most_recent_block_txs GUARDED_BY(m_most_recent_block_mutex);
+    std::unique_ptr<const std::map<GenTxid, CTransactionRef>> m_most_recent_block_txs GUARDED_BY(m_most_recent_block_mutex);
 
     // Data about the low-work headers synchronization, aggregated from all peers' HeadersSyncStates.
     /** Mutex guarding the other m_headers_presync_* variables. */
@@ -1575,7 +1575,7 @@ void PeerManagerImpl::InitializeNode(const CNode& node, ServiceFlags our_service
 
 void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
 {
-    std::set<uint256> unbroadcast_txids = m_mempool.GetUnbroadcastTxs();
+    std::set<Txid> unbroadcast_txids = m_mempool.GetUnbroadcastTxs();
 
     for (const auto& txid : unbroadcast_txids) {
         CTransactionRef tx = m_mempool.get(txid);
@@ -2025,7 +2025,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha
         std::async(std::launch::deferred, [&] { return NetMsg::Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })};
 
     {
-        auto most_recent_block_txs = std::make_unique<std::map<uint256, CTransactionRef>>();
+        auto most_recent_block_txs = std::make_unique<std::map<GenTxid, CTransactionRef>>();
         for (const auto& tx : pblock->vtx) {
             most_recent_block_txs->emplace(tx->GetHash(), tx);
             most_recent_block_txs->emplace(tx->GetWitnessHash(), tx);
@@ -2148,7 +2148,7 @@ void PeerManagerImpl::SendPings()
     for(auto& it : m_peer_map) it.second->m_ping_queued = true;
 }
 
-void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid)
+void PeerManagerImpl::RelayTransaction(const Txid& txid, const Wtxid& wtxid)
 {
     LOCK(m_peer_mutex);
     for(auto& it : m_peer_map) {
@@ -2164,11 +2164,17 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid
         // in the announcement.
         if (tx_relay->m_next_inv_send_time == 0s) continue;
 
-        const uint256& hash{peer.m_wtxid_relay ? wtxid : txid};
-        if (!tx_relay->m_tx_inventory_known_filter.contains(hash)) {
-            tx_relay->m_tx_inventory_to_send.insert(hash);
+        GenTxid gtxid;
+        if (peer.m_wtxid_relay) {
+            gtxid = wtxid;
+        } else {
+            gtxid = txid;
         }
-    };
+
+        if (!tx_relay->m_tx_inventory_known_filter.contains(gtxid.ToUint256())) {
+            tx_relay->m_tx_inventory_to_send.insert(gtxid);
+        }
+    }
 }
 
 void PeerManagerImpl::RelayAddress(NodeId originator,
@@ -2401,7 +2407,7 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay,
     {
         LOCK(m_most_recent_block_mutex);
         if (m_most_recent_block_txs != nullptr) {
-            auto it = m_most_recent_block_txs->find(gtxid.GetHash());
+            auto it = m_most_recent_block_txs->find(gtxid);
             if (it != m_most_recent_block_txs->end()) return it->second;
         }
     }
@@ -3016,7 +3022,7 @@ std::optional<node::PackageToValidate> PeerManagerImpl::ProcessInvalidTx(NodeId
         AddToCompactExtraTransactions(ptx);
     }
     for (const Txid& parent_txid : unique_parents) {
-        if (peer) AddKnownTx(*peer, parent_txid);
+        if (peer) AddKnownTx(*peer, parent_txid.ToUint256());
     }
 
     MaybePunishNodeForTx(nodeid, state);
@@ -4252,12 +4258,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 
         CTransactionRef ptx;
         vRecv >> TX_WITH_WITNESS(ptx);
-        const CTransaction& tx = *ptx;
 
-        const uint256& txid = ptx->GetHash();
-        const uint256& wtxid = ptx->GetWitnessHash();
+        const Txid& txid = ptx->GetHash();
+        const Wtxid& wtxid = ptx->GetWitnessHash();
 
-        const uint256& hash = peer->m_wtxid_relay ? wtxid : txid;
+        const uint256& hash = peer->m_wtxid_relay ? wtxid.ToUint256() : txid.ToUint256();
         AddKnownTx(*peer, hash);
 
         LOCK2(cs_main, m_tx_download_mutex);
@@ -4268,13 +4273,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 // Always relay transactions received from peers with forcerelay
                 // permission, even if they were already in the mempool, allowing
                 // the node to function as a gateway for nodes hidden behind it.
-                if (!m_mempool.exists(GenTxid::Txid(tx.GetHash()))) {
+                if (!m_mempool.exists(txid)) {
                     LogPrintf("Not relaying non-mempool transaction %s (wtxid=%s) from forcerelay peer=%d\n",
-                              tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), pfrom.GetId());
+                              txid.ToString(), wtxid.ToString(), pfrom.GetId());
                 } else {
                     LogPrintf("Force relaying tx %s (wtxid=%s) from peer=%d\n",
-                              tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), pfrom.GetId());
-                    RelayTransaction(tx.GetHash(), tx.GetWitnessHash());
+                              txid.ToString(), wtxid.ToString(), pfrom.GetId());
+                    RelayTransaction(txid, wtxid);
                 }
             }
 
@@ -4901,11 +4906,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
     if (msg_type == NetMsgType::NOTFOUND) {
         std::vector<CInv> vInv;
         vRecv >> vInv;
-        std::vector<uint256> tx_invs;
+        std::vector<GenTxid> tx_invs;
         if (vInv.size() <= node::MAX_PEER_TX_ANNOUNCEMENTS + MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
             for (CInv &inv : vInv) {
                 if (inv.IsGenTxMsg()) {
-                    tx_invs.emplace_back(inv.hash);
+                    tx_invs.emplace_back(ToGenTxid(inv));
                 }
             }
         }
@@ -5406,19 +5411,17 @@ namespace {
 class CompareInvMempoolOrder
 {
     CTxMemPool* mp;
-    bool m_wtxid_relay;
 public:
-    explicit CompareInvMempoolOrder(CTxMemPool *_mempool, bool use_wtxid)
+    explicit CompareInvMempoolOrder(CTxMemPool *_mempool)
     {
         mp = _mempool;
-        m_wtxid_relay = use_wtxid;
     }
 
-    bool operator()(std::set<uint256>::iterator a, std::set<uint256>::iterator b)
+    bool operator()(std::set<GenTxid>::iterator a, std::set<GenTxid>::iterator b)
     {
         /* As std::make_heap produces a max-heap, we want the entries with the
          * fewest ancestors/highest fee to sort later. */
-        return mp->CompareDepthAndScore(*b, *a, m_wtxid_relay);
+        return mp->CompareDepthAndScore(*b, *a);
     }
 };
 } // namespace
@@ -5736,7 +5739,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                                 txinfo.tx->GetWitnessHash().ToUint256() :
                                 txinfo.tx->GetHash().ToUint256(),
                         };
-                        tx_relay->m_tx_inventory_to_send.erase(inv.hash);
+                        tx_relay->m_tx_inventory_to_send.erase(ToGenTxid(inv));
 
                         // Don't send transactions that peers will not put into their mempool
                         if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
@@ -5757,15 +5760,15 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                 // Determine transactions to relay
                 if (fSendTrickle) {
                     // Produce a vector with all candidates for sending
-                    std::vector<std::set<uint256>::iterator> vInvTx;
+                    std::vector<std::set<GenTxid>::iterator> vInvTx;
                     vInvTx.reserve(tx_relay->m_tx_inventory_to_send.size());
-                    for (std::set<uint256>::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) {
+                    for (std::set<GenTxid>::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) {
                         vInvTx.push_back(it);
                     }
                     const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()};
                     // Topologically and fee-rate sort the inventory we send for privacy and priority reasons.
                     // A heap is used so that not all items need sorting if only a few are being sent.
-                    CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, peer->m_wtxid_relay);
+                    CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool);
                     std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
                     // No reason to drain out at many times the network's capacity,
                     // especially since we have many peers and some will draw much shorter delays.
@@ -5776,14 +5779,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                     while (!vInvTx.empty() && nRelayedTransactions < broadcast_max) {
                         // Fetch the top element from the heap
                         std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
-                        std::set<uint256>::iterator it = vInvTx.back();
+                        std::set<GenTxid>::iterator it = vInvTx.back();
                         vInvTx.pop_back();
-                        uint256 hash = *it;
-                        CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash);
+                        GenTxid hash = *it;
+                        CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash.ToUint256());
                         // Remove it from the to-be-sent set
                         tx_relay->m_tx_inventory_to_send.erase(it);
                         // Check if not in the filter already
-                        if (tx_relay->m_tx_inventory_known_filter.contains(hash)) {
+                        if (tx_relay->m_tx_inventory_known_filter.contains(hash.ToUint256())) {
                             continue;
                         }
                         // Not in the mempool anymore? don't bother sending it.
@@ -5803,7 +5806,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                             MakeAndPushMessage(*pto, NetMsgType::INV, vInv);
                             vInv.clear();
                         }
-                        tx_relay->m_tx_inventory_known_filter.insert(hash);
+                        tx_relay->m_tx_inventory_known_filter.insert(hash.ToUint256());
                     }
 
                     // Ensure we'll respond to GETDATA requests for anything we've just announced
@@ -5926,7 +5929,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
         {
             LOCK(m_tx_download_mutex);
             for (const GenTxid& gtxid : m_txdownloadman.GetRequestsToSend(pto->GetId(), current_time)) {
-                vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash());
+                vGetData.emplace_back(std::holds_alternative<Wtxid>(gtxid) ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.ToUint256());
                 if (vGetData.size() >= MAX_GETDATA_SZ) {
                     MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData);
                     vGetData.clear();

src/net_processing.h

@@ -119,7 +119,7 @@ public:
     virtual PeerManagerInfo GetInfo() const = 0;
 
     /** Relay transaction to all peers. */
-    virtual void RelayTransaction(const uint256& txid, const uint256& wtxid) = 0;
+    virtual void RelayTransaction(const Txid& txid, const Wtxid& wtxid) = 0;
 
     /** Send ping message to all peers */
     virtual void SendPings() = 0;

src/node/interfaces.cpp

@@ -669,17 +669,17 @@ public:
         LOCK(m_node.mempool->cs);
         return IsRBFOptIn(tx, *m_node.mempool);
     }
-    bool isInMempool(const uint256& txid) override
+    bool isInMempool(const Txid& txid) override
     {
         if (!m_node.mempool) return false;
         LOCK(m_node.mempool->cs);
-        return m_node.mempool->exists(GenTxid::Txid(txid));
+        return m_node.mempool->exists(txid);
     }
-    bool hasDescendantsInMempool(const uint256& txid) override
+    bool hasDescendantsInMempool(const Txid& txid) override
     {
         if (!m_node.mempool) return false;
         LOCK(m_node.mempool->cs);
-        const auto entry{m_node.mempool->GetEntry(Txid::FromUint256(txid))};
+        const auto entry{m_node.mempool->GetEntry(txid)};
         if (entry == nullptr) return false;
         return entry->GetCountWithDescendants() > 1;
     }
@@ -694,7 +694,7 @@ public:
         // that Chain clients do not need to know about.
         return TransactionError::OK == err;
     }
-    void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize, CAmount* ancestorfees) override
+    void getTransactionAncestry(const Txid& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize, CAmount* ancestorfees) override
     {
         ancestors = descendants = 0;
         if (!m_node.mempool) return;

src/node/mempool_persist.cpp

@@ -106,7 +106,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active
                     // wallet(s) having loaded it while we were processing
                     // mempool transactions; consider these as valid, instead of
                     // failed, but mark them as 'already there'
-                    if (pool.exists(GenTxid::Txid(tx->GetHash()))) {
+                    if (pool.exists(tx->GetHash())) {
                         ++already_there;
                     } else {
                         ++failed;
@@ -118,7 +118,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active
             if (active_chainstate.m_chainman.m_interrupt)
                 return false;
         }
-        std::map<uint256, CAmount> mapDeltas;
+        std::map<Txid, CAmount> mapDeltas;
         file >> mapDeltas;
 
         if (opts.apply_fee_delta_priority) {
@@ -127,7 +127,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active
             }
         }
 
-        std::set<uint256> unbroadcast_txids;
+        std::set<Txid> unbroadcast_txids;
         file >> unbroadcast_txids;
         if (opts.apply_unbroadcast_set) {
             unbroadcast = unbroadcast_txids.size();
@@ -150,9 +150,9 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock
 {
     auto start = SteadyClock::now();
 
-    std::map<uint256, CAmount> mapDeltas;
+    std::map<Txid, CAmount> mapDeltas;
     std::vector<TxMempoolInfo> vinfo;
-    std::set<uint256> unbroadcast_txids;
+    std::set<Txid> unbroadcast_txids;
 
     static Mutex dump_mutex;
     LOCK(dump_mutex);

src/node/mini_miner.cpp

@@ -27,7 +27,7 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
     // Anything that's spent by the mempool is to-be-replaced
     // Anything otherwise unavailable just has a bump fee of 0
     for (const auto& outpoint : outpoints) {
-        if (!mempool.exists(GenTxid::Txid(outpoint.hash))) {
+        if (!mempool.exists(outpoint.hash)) {
             // This UTXO is either confirmed or not yet submitted to mempool.
             // If it's confirmed, no bump fee is required.
             // If it's not yet submitted, we have no information, so return 0.
@@ -61,7 +61,7 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
     if (m_requested_outpoints_by_txid.empty()) return;
 
     // Calculate the cluster and construct the entry map.
-    std::vector<uint256> txids_needed;
+    std::vector<Txid> txids_needed;
     txids_needed.reserve(m_requested_outpoints_by_txid.size());
     for (const auto& [txid, _]: m_requested_outpoints_by_txid) {
         txids_needed.push_back(txid);
@@ -286,7 +286,7 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
         }
         // Track the order in which transactions were selected.
         for (const auto& ancestor : ancestors) {
-            m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
+            m_inclusion_order.emplace(ancestor->first, sequence_num);
         }
         DeleteAncestorPackage(ancestors);
         SanityCheck();
@@ -409,7 +409,7 @@ std::optional<CAmount> MiniMiner::CalculateTotalBumpFees(const CFeeRate& target_
         ancestors.insert(iter);
     }
 
-    std::set<uint256> has_been_processed;
+    std::set<Txid> has_been_processed;
     while (!to_process.empty()) {
         auto iter = to_process.begin();
         const CTransaction& tx = (*iter)->second.GetTx();

src/node/mini_miner.h

@@ -83,12 +83,12 @@ class MiniMiner
 
     // Set once per lifetime, fill in during initialization.
     // txids of to-be-replaced transactions
-    std::set<uint256> m_to_be_replaced;
+    std::set<Txid> m_to_be_replaced;
 
     // If multiple argument outpoints correspond to the same transaction, cache them together in
     // a single entry indexed by txid. Then we can just work with txids since all outpoints from
     // the same tx will have the same bumpfee. Excludes non-mempool transactions.
-    std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
+    std::map<Txid, std::vector<COutPoint>> m_requested_outpoints_by_txid;
 
     // Txid to a number representing the order in which this transaction was included (smaller
     // number = included earlier).  Transactions included in an ancestor set together have the same
@@ -98,21 +98,21 @@ class MiniMiner
     std::map<COutPoint, CAmount> m_bump_fees;
 
     // The constructed block template
-    std::set<uint256> m_in_block;
+    std::set<Txid> m_in_block;
 
     // Information on the current status of the block
     CAmount m_total_fees{0};
     int32_t m_total_vsize{0};
 
     /** Main data structure holding the entries, can be indexed by txid */
-    std::map<uint256, MiniMinerMempoolEntry> m_entries_by_txid;
+    std::map<Txid, MiniMinerMempoolEntry> m_entries_by_txid;
     using MockEntryMap = decltype(m_entries_by_txid);
 
     /** Vector of entries, can be sorted by ancestor feerate. */
     std::vector<MockEntryMap::iterator> m_entries;
 
     /** Map of txid to its descendants. Should be inclusive. */
-    std::map<uint256, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
+    std::map<Txid, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
 
     /** Consider this ancestor package "mined" so remove all these entries from our data structures. */
     void DeleteAncestorPackage(const std::set<MockEntryMap::iterator, IteratorComparator>& ancestors);
@@ -129,7 +129,7 @@ public:
     void BuildMockTemplate(std::optional<CFeeRate> target_feerate);
 
     /** Returns set of txids in the block template if one has been constructed. */
-    std::set<uint256> GetMockTemplateTxids() const { return m_in_block; }
+    std::set<Txid> GetMockTemplateTxids() const { return m_in_block; }
 
     /** Constructor that takes a list of outpoints that may or may not belong to transactions in the
      * mempool. Copies out information about the relevant transactions in the mempool into

src/node/transaction.cpp

@@ -42,7 +42,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
 
     std::promise<void> promise;
     Txid txid = tx->GetHash();
-    uint256 wtxid = tx->GetWitnessHash();
+    Wtxid wtxid = tx->GetWitnessHash();
     bool callback_set = false;
 
     {
@@ -123,7 +123,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
     return TransactionError::OK;
 }
 
-CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, uint256& hashBlock, const BlockManager& blockman)
+CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const Txid& hash, uint256& hashBlock, const BlockManager& blockman)
 {
     if (mempool && !block_index) {
         CTransactionRef ptx = mempool->get(hash);

src/node/transaction.h

@@ -63,7 +63,7 @@ static const CAmount DEFAULT_MAX_BURN_AMOUNT{0};
  * @param[out] hashBlock       The block hash, if the tx was found via -txindex or block_index
  * @returns                    The tx if found, otherwise nullptr
  */
-CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, uint256& hashBlock, const BlockManager& blockman);
+CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const Txid& hash, uint256& hashBlock, const BlockManager& blockman);
 } // namespace node
 
 #endif // BITCOIN_NODE_TRANSACTION_H

src/node/txdownloadman.h

@@ -8,6 +8,7 @@
 #include <net.h>
 #include <policy/packages.h>
 #include <txorphanage.h>
+#include <util/transaction_identifier.h>
 
 #include <cstdint>
 #include <memory>
@@ -143,7 +144,7 @@ public:
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);
 
     /** Should be called when a notfound for a tx has been received. */
-    void ReceivedNotFound(NodeId nodeid, const std::vector<uint256>& txhashes);
+    void ReceivedNotFound(NodeId nodeid, const std::vector<GenTxid>& txhashes);
 
     /** Respond to successful transaction submission to mempool */
     void MempoolAcceptedTx(const CTransactionRef& tx);

src/node/txdownloadman_impl.cpp

@@ -47,7 +47,7 @@ std::vector<GenTxid> TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::ch
 {
     return m_impl->GetRequestsToSend(nodeid, current_time);
 }
-void TxDownloadManager::ReceivedNotFound(NodeId nodeid, const std::vector<uint256>& txhashes)
+void TxDownloadManager::ReceivedNotFound(NodeId nodeid, const std::vector<GenTxid>& txhashes)
 {
     m_impl->ReceivedNotFound(nodeid, txhashes);
 }
@@ -124,12 +124,9 @@ void TxDownloadManagerImpl::BlockDisconnected()
 
 bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable)
 {
-    const uint256& hash = gtxid.GetHash();
-
-    if (gtxid.IsWtxid()) {
+    bool in_orphanage = std::visit(util::Overloaded{
         // Normal query by wtxid.
-        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
-    } else {
+        [this](const Wtxid& wtxid) { return m_orphanage.HaveTx(wtxid); },
         // Never query by txid: it is possible that the transaction in the orphanage has the same
         // txid but a different witness, which would give us a false positive result. If we decided
         // not to request the transaction based on this result, an attacker could prevent us from
@@ -141,8 +138,12 @@ bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_rec
         // While we won't query by txid, we can try to "guess" what the wtxid is based on the txid.
         // A non-segwit transaction's txid == wtxid. Query this txid "casted" to a wtxid. This will
         // help us find non-segwit transactions, saving bandwidth, and should have no false positives.
-        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
-    }
+        [this](const Txid& txid) { return m_orphanage.HaveTx(Wtxid::FromUint256(txid.ToUint256())); }
+    }, gtxid);
+
+    if (in_orphanage) return true;
+
+    const uint256& hash = gtxid.ToUint256();
 
     if (include_reconsiderable && RecentRejectsReconsiderableFilter().contains(hash)) return true;
 
@@ -178,12 +179,11 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
     // - is wtxid matching something in orphanage
     // - exists in orphanage
     // - peer can be an orphan resolution candidate
-    if (gtxid.IsWtxid()) {
-        const auto wtxid{Wtxid::FromUint256(gtxid.GetHash())};
-        if (auto orphan_tx{m_orphanage.GetTx(wtxid)}) {
+    if (const auto* wtxid = std::get_if<Wtxid>(&gtxid)) {
+        if (auto orphan_tx{m_orphanage.GetTx(*wtxid)}) {
             auto unique_parents{GetUniqueParents(*orphan_tx)};
             std::erase_if(unique_parents, [&](const auto& txid){
-                return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+                return AlreadyHaveTx(txid, /*include_reconsiderable=*/false);
             });
 
             // The missing parents may have all been rejected or accepted since the orphan was added to the orphanage.
@@ -192,7 +192,7 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
                 return true;
             }
 
-            if (MaybeAddOrphanResolutionCandidate(unique_parents, wtxid, peer, now)) {
+            if (MaybeAddOrphanResolutionCandidate(unique_parents, *wtxid, peer, now)) {
                 m_orphanage.AddAnnouncer(orphan_tx->GetWitnessHash(), peer);
             }
 
@@ -220,7 +220,7 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
     //     MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have NetPermissionFlags::Relay).
     auto delay{0us};
     if (!info.m_preferred) delay += NONPREF_PEER_TX_DELAY;
-    if (!gtxid.IsWtxid() && m_num_wtxid_peers > 0) delay += TXID_RELAY_DELAY;
+    if (!std::holds_alternative<Wtxid>(gtxid) && m_num_wtxid_peers > 0) delay += TXID_RELAY_DELAY;
     const bool overloaded = !info.m_relay_permissions && m_txrequest.CountInFlight(peer) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
     if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
 
@@ -261,7 +261,7 @@ bool TxDownloadManagerImpl::MaybeAddOrphanResolutionCandidate(const std::vector<
     // Treat finding orphan resolution candidate as equivalent to the peer announcing all missing parents.
     // In the future, orphan resolution may include more explicit steps
     for (const auto& parent_txid : unique_parents) {
-        m_txrequest.ReceivedInv(nodeid, GenTxid::Txid(parent_txid), info.m_preferred, now + delay);
+        m_txrequest.ReceivedInv(nodeid, parent_txid, info.m_preferred, now + delay);
     }
     LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", nodeid, wtxid.ToString());
     return true;
@@ -273,25 +273,25 @@ std::vector<GenTxid> TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std
     std::vector<std::pair<NodeId, GenTxid>> expired;
     auto requestable = m_txrequest.GetRequestable(nodeid, current_time, &expired);
     for (const auto& entry : expired) {
-        LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", entry.second.IsWtxid() ? "wtx" : "tx",
-            entry.second.GetHash().ToString(), entry.first);
+        LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", std::holds_alternative<Wtxid>(entry.second) ? "wtx" : "tx",
+            entry.second.ToUint256().ToString(), entry.first);
     }
     for (const GenTxid& gtxid : requestable) {
         if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) {
-            LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx",
-                gtxid.GetHash().ToString(), nodeid);
+            LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", std::holds_alternative<Wtxid>(gtxid) ? "wtx" : "tx",
+                gtxid.ToUint256().ToString(), nodeid);
             requests.emplace_back(gtxid);
-            m_txrequest.RequestedTx(nodeid, gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL);
+            m_txrequest.RequestedTx(nodeid, gtxid, current_time + GETDATA_TX_INTERVAL);
         } else {
             // We have already seen this transaction, no need to download. This is just a belt-and-suspenders, as
             // this should already be called whenever a transaction becomes AlreadyHaveTx().
-            m_txrequest.ForgetTxHash(gtxid.GetHash());
+            m_txrequest.ForgetTxHash(gtxid);
         }
     }
     return requests;
 }
 
-void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector<uint256>& txhashes)
+void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector<GenTxid>& txhashes)
 {
     for (const auto& txhash : txhashes) {
         // If we receive a NOTFOUND message for a tx we requested, mark the announcement for it as
@@ -378,19 +378,19 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
             // We can tolerate having up to 1 parent in m_lazy_recent_rejects_reconsiderable since we
             // submit 1p1c packages. However, fail immediately if any are in m_lazy_recent_rejects.
             std::optional<uint256> rejected_parent_reconsiderable;
-            for (const uint256& parent_txid : unique_parents) {
-                if (RecentRejectsFilter().contains(parent_txid)) {
+            for (const Txid& parent_txid : unique_parents) {
+                if (RecentRejectsFilter().contains(parent_txid.ToUint256())) {
                     fRejectedParents = true;
                     break;
-                } else if (RecentRejectsReconsiderableFilter().contains(parent_txid) &&
-                           !m_opts.m_mempool.exists(GenTxid::Txid(parent_txid))) {
+                } else if (RecentRejectsReconsiderableFilter().contains(parent_txid.ToUint256()) &&
+                           !m_opts.m_mempool.exists(parent_txid)) {
                     // More than 1 parent in m_lazy_recent_rejects_reconsiderable: 1p1c will not be
                     // sufficient to accept this package, so just give up here.
                     if (rejected_parent_reconsiderable.has_value()) {
                         fRejectedParents = true;
                         break;
                     }
-                    rejected_parent_reconsiderable = parent_txid;
+                    rejected_parent_reconsiderable = parent_txid.ToUint256();
                 }
             }
             if (!fRejectedParents) {
@@ -398,7 +398,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been
                 // previously rejected for being too low feerate. This orphan might CPFP it.
                 std::erase_if(unique_parents, [&](const auto& txid){
-                    return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+                    return AlreadyHaveTx(txid, /*include_reconsiderable=*/false);
                 });
                 const auto now{GetTime<std::chrono::microseconds>()};
                 const auto& wtxid = ptx->GetWitnessHash();
@@ -412,8 +412,8 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 //
                 // Search by txid and, if the tx has a witness, wtxid
                 std::vector<NodeId> orphan_resolution_candidates{nodeid};
-                m_txrequest.GetCandidatePeers(ptx->GetHash().ToUint256(), orphan_resolution_candidates);
-                if (ptx->HasWitness()) m_txrequest.GetCandidatePeers(ptx->GetWitnessHash().ToUint256(), orphan_resolution_candidates);
+                m_txrequest.GetCandidatePeers(ptx->GetHash(), orphan_resolution_candidates);
+                if (ptx->HasWitness()) m_txrequest.GetCandidatePeers(ptx->GetWitnessHash(), orphan_resolution_candidates);
 
                 for (const auto& nodeid : orphan_resolution_candidates) {
                     if (MaybeAddOrphanResolutionCandidate(unique_parents, ptx->GetWitnessHash(), nodeid, now)) {
@@ -515,8 +515,8 @@ void TxDownloadManagerImpl::MempoolRejectedPackage(const Package& package)
 
 std::pair<bool, std::optional<PackageToValidate>> TxDownloadManagerImpl::ReceivedTx(NodeId nodeid, const CTransactionRef& ptx)
 {
-    const uint256& txid = ptx->GetHash();
-    const uint256& wtxid = ptx->GetWitnessHash();
+    const Txid& txid = ptx->GetHash();
+    const Wtxid& wtxid = ptx->GetWitnessHash();
 
     // Mark that we have received a response
     m_txrequest.ReceivedResponse(nodeid, txid);
@@ -535,7 +535,7 @@ std::pair<bool, std::optional<PackageToValidate>> TxDownloadManagerImpl::Receive
     // already; and an adversary can already relay us old transactions
     // (older than our recency filter) if trying to DoS us, without any need
     // for witness malleation.
-    if (AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/false)) {
+    if (AlreadyHaveTx(wtxid, /*include_reconsiderable=*/false)) {
         // If a tx is detected by m_lazy_recent_rejects it is ignored. Because we haven't
         // submitted the tx to our mempool, we won't have computed a DoS
         // score for it or determined exactly why we consider it invalid.
@@ -552,7 +552,7 @@ std::pair<bool, std::optional<PackageToValidate>> TxDownloadManagerImpl::Receive
         // peer simply for relaying a tx that our m_lazy_recent_rejects has caught,
         // regardless of false positives.
         return {false, std::nullopt};
-    } else if (RecentRejectsReconsiderableFilter().contains(wtxid)) {
+    } else if (RecentRejectsReconsiderableFilter().contains(wtxid.ToUint256())) {
         // When a transaction is already in m_lazy_recent_rejects_reconsiderable, we shouldn't submit
         // it by itself again. However, look for a matching child in the orphanage, as it is
         // possible that they succeed as a package.

src/node/txdownloadman_impl.h

@@ -169,7 +169,7 @@ public:
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);
 
     /** Marks a tx as ReceivedResponse in txrequest. */
-    void ReceivedNotFound(NodeId nodeid, const std::vector<uint256>& txhashes);
+    void ReceivedNotFound(NodeId nodeid, const std::vector<GenTxid>& txhashes);
 
     /** Look for a child of this transaction in the orphanage to form a 1-parent-1-child package,
      * skipping any combinations that have already been tried. Return the resulting package along with

src/policy/fees.cpp

@@ -518,16 +518,16 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe
     }
 }
 
-bool CBlockPolicyEstimator::removeTx(uint256 hash)
+bool CBlockPolicyEstimator::removeTx(Txid hash)
 {
     LOCK(m_cs_fee_estimator);
     return _removeTx(hash, /*inBlock=*/false);
 }
 
-bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock)
+bool CBlockPolicyEstimator::_removeTx(const Txid& hash, bool inBlock)
 {
     AssertLockHeld(m_cs_fee_estimator);
-    std::map<uint256, TxStatsInfo>::iterator pos = mapMemPoolTxs.find(hash);
+    std::map<Txid, TxStatsInfo>::iterator pos = mapMemPoolTxs.find(hash);
     if (pos != mapMemPoolTxs.end()) {
         feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock);
         shortStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock);

src/policy/fees.h

@@ -212,7 +212,7 @@ public:
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Remove a transaction from the mempool tracking stats for non BLOCK removal reasons*/
-    bool removeTx(uint256 hash)
+    bool removeTx(Txid hash)
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** DEPRECATED. Return a feerate estimate */
@@ -287,7 +287,7 @@ private:
     };
 
     // map of txids to information about that transaction
-    std::map<uint256, TxStatsInfo> mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator);
+    std::map<Txid, TxStatsInfo> mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator);
 
     /** Classes to track historical data on transaction confirmations */
     std::unique_ptr<TxConfirmStats> feeStats PT_GUARDED_BY(m_cs_fee_estimator);
@@ -315,7 +315,7 @@ private:
     unsigned int MaxUsableEstimate() const EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
 
     /** A non-thread-safe helper for the removeTx function */
-    bool _removeTx(const uint256& hash, bool inBlock)
+    bool _removeTx(const Txid& hash, bool inBlock)
         EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator);
 };
 

src/policy/packages.cpp

@@ -16,7 +16,7 @@
 
 /** IsTopoSortedPackage where a set of txids has been pre-populated. The set is assumed to be correct and
  * is mutated within this function (even if return value is false). */
-bool IsTopoSortedPackage(const Package& txns, std::unordered_set<uint256, SaltedTxidHasher>& later_txids)
+bool IsTopoSortedPackage(const Package& txns, std::unordered_set<Txid, SaltedTxidHasher>& later_txids)
 {
     // Avoid misusing this function: later_txids should contain the txids of txns.
     Assume(txns.size() == later_txids.size());
@@ -42,7 +42,7 @@ bool IsTopoSortedPackage(const Package& txns, std::unordered_set<uint256, Salted
 
 bool IsTopoSortedPackage(const Package& txns)
 {
-    std::unordered_set<uint256, SaltedTxidHasher> later_txids;
+    std::unordered_set<Txid, SaltedTxidHasher> later_txids;
     std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()),
                    [](const auto& tx) { return tx->GetHash(); });
 
@@ -91,7 +91,7 @@ bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, boo
         return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-large");
     }
 
-    std::unordered_set<uint256, SaltedTxidHasher> later_txids;
+    std::unordered_set<Txid, SaltedTxidHasher> later_txids;
     std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()),
                    [](const auto& tx) { return tx->GetHash(); });
 
@@ -123,7 +123,7 @@ bool IsChildWithParents(const Package& package)
 
     // The package is expected to be sorted, so the last transaction is the child.
     const auto& child = package.back();
-    std::unordered_set<uint256, SaltedTxidHasher> input_txids;
+    std::unordered_set<Txid, SaltedTxidHasher> input_txids;
     std::transform(child->vin.cbegin(), child->vin.cend(),
                    std::inserter(input_txids, input_txids.end()),
                    [](const auto& input) { return input.prevout.hash; });
@@ -136,7 +136,7 @@ bool IsChildWithParents(const Package& package)
 bool IsChildWithParentsTree(const Package& package)
 {
     if (!IsChildWithParents(package)) return false;
-    std::unordered_set<uint256, SaltedTxidHasher> parent_txids;
+    std::unordered_set<Txid, SaltedTxidHasher> parent_txids;
     std::transform(package.cbegin(), package.cend() - 1, std::inserter(parent_txids, parent_txids.end()),
                    [](const auto& ptx) { return ptx->GetHash(); });
     // Each parent must not have an input who is one of the other parents.

src/policy/rbf.cpp

@@ -32,7 +32,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
 
     // If this transaction is not in our mempool, then we can't be sure
     // we will know about all its inputs.
-    if (!pool.exists(GenTxid::Txid(tx.GetHash()))) {
+    if (!pool.exists(tx.GetHash())) {
         return RBFTransactionState::UNKNOWN;
     }
 
@@ -62,7 +62,7 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
                                                   CTxMemPool::setEntries& all_conflicts)
 {
     AssertLockHeld(pool.cs);
-    const uint256 txid = tx.GetHash();
+    const Txid txid = tx.GetHash();
     uint64_t nConflictingCount = 0;
     for (const auto& mi : iters_conflicting) {
         nConflictingCount += mi->GetCountWithDescendants();
@@ -89,7 +89,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
                                                const CTxMemPool::setEntries& iters_conflicting)
 {
     AssertLockHeld(pool.cs);
-    std::set<uint256> parents_of_conflicts;
+    std::set<Txid> parents_of_conflicts;
     for (const auto& mi : iters_conflicting) {
         for (const CTxIn& txin : mi->GetTx().vin) {
             parents_of_conflicts.insert(txin.prevout.hash);
@@ -107,7 +107,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
         if (!parents_of_conflicts.count(tx.vin[j].prevout.hash)) {
             // Rather than check the UTXO set - potentially expensive - it's cheaper to just check
             // if the new input refers to a tx that's in the mempool.
-            if (pool.exists(GenTxid::Txid(tx.vin[j].prevout.hash))) {
+            if (pool.exists(tx.vin[j].prevout.hash)) {
                 return strprintf("replacement %s adds unconfirmed input, idx %d",
                                  tx.GetHash().ToString(), j);
             }
@@ -118,7 +118,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
 
 std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
                                                    const std::set<Txid>& direct_conflicts,
-                                                   const uint256& txid)
+                                                   const Txid& txid)
 {
     for (CTxMemPool::txiter ancestorIt : ancestors) {
         const Txid& hashAncestor = ancestorIt->GetTx().GetHash();
@@ -133,7 +133,7 @@ std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries&
 
 std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting,
                                                  CFeeRate replacement_feerate,
-                                                 const uint256& txid)
+                                                 const Txid& txid)
 {
     for (const auto& mi : iters_conflicting) {
         // Don't allow the replacement to reduce the feerate of the mempool.
@@ -161,7 +161,7 @@ std::optional<std::string> PaysForRBF(CAmount original_fees,
                                       CAmount replacement_fees,
                                       size_t replacement_vsize,
                                       CFeeRate relay_fee,
-                                      const uint256& txid)
+                                      const Txid& txid)
 {
     // Rule #3: The replacement fees must be greater than or equal to fees of the
     // transactions it replaces, otherwise the bandwidth used by those conflicting transactions

src/policy/rbf.h

@@ -90,7 +90,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTx
  */
 std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
                                                    const std::set<Txid>& direct_conflicts,
-                                                   const uint256& txid);
+                                                   const Txid& txid);
 
 /** Check that the feerate of the replacement transaction(s) is higher than the feerate of each
  * of the transactions in iters_conflicting.
@@ -98,7 +98,7 @@ std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries&
  * @returns error message if fees insufficient, otherwise std::nullopt.
  */
 std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting,
-                                                 CFeeRate replacement_feerate, const uint256& txid);
+                                                 CFeeRate replacement_feerate, const Txid& txid);
 
 /** The replacement transaction must pay more fees than the original transactions. The additional
  * fees must pay for the replacement's bandwidth at or above the incremental relay feerate.
@@ -113,7 +113,7 @@ std::optional<std::string> PaysForRBF(CAmount original_fees,
                                       CAmount replacement_fees,
                                       size_t replacement_vsize,
                                       CFeeRate relay_fee,
-                                      const uint256& txid);
+                                      const Txid& txid);
 
 /**
  * The replacement transaction must improve the feerate diagram of the mempool.

src/primitives/transaction.h

@@ -423,20 +423,4 @@ struct CMutableTransaction
 typedef std::shared_ptr<const CTransaction> CTransactionRef;
 template <typename Tx> static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared<const CTransaction>(std::forward<Tx>(txIn)); }
 
-/** A generic txid reference (txid or wtxid). */
-class GenTxid
-{
-    bool m_is_wtxid;
-    uint256 m_hash;
-    GenTxid(bool is_wtxid, const uint256& hash) : m_is_wtxid(is_wtxid), m_hash(hash) {}
-
-public:
-    static GenTxid Txid(const uint256& hash) { return GenTxid{false, hash}; }
-    static GenTxid Wtxid(const uint256& hash) { return GenTxid{true, hash}; }
-    bool IsWtxid() const { return m_is_wtxid; }
-    const uint256& GetHash() const LIFETIMEBOUND { return m_hash; }
-    friend bool operator==(const GenTxid& a, const GenTxid& b) { return a.m_is_wtxid == b.m_is_wtxid && a.m_hash == b.m_hash; }
-    friend bool operator<(const GenTxid& a, const GenTxid& b) { return std::tie(a.m_is_wtxid, a.m_hash) < std::tie(b.m_is_wtxid, b.m_hash); }
-};
-
 #endif // BITCOIN_PRIMITIVES_TRANSACTION_H

src/protocol.cpp

@@ -121,5 +121,9 @@ std::vector<std::string> serviceFlagsToStr(uint64_t flags)
 GenTxid ToGenTxid(const CInv& inv)
 {
     assert(inv.IsGenTxMsg());
-    return inv.IsMsgWtx() ? GenTxid::Wtxid(inv.hash) : GenTxid::Txid(inv.hash);
+    if (inv.IsMsgWtx()) {
+        return Wtxid::FromUint256(inv.hash);
+    } else {
+        return Txid::FromUint256(inv.hash);
+    }
 }

src/qt/sendcoinsdialog.h

@@ -61,7 +61,7 @@ public Q_SLOTS:
     void setBalance(const interfaces::WalletBalances& balances);
 
 Q_SIGNALS:
-    void coinsSent(const uint256& txid);
+    void coinsSent(const Txid& txid);
 
 private:
     Ui::SendCoinsDialog *ui;

src/qt/test/wallettests.cpp

@@ -75,7 +75,7 @@ void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_ty
 }
 
 //! Send coins to address and return txid.
-uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
+Txid SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
                   QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
 {
     QVBoxLayout* entries = sendCoinsDialog.findChild<QVBoxLayout*>("entries");
@@ -86,8 +86,8 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe
         ->findChild<QFrame*>("frameFeeSelection")
         ->findChild<QCheckBox*>("optInRBF")
         ->setCheckState(rbf ? Qt::Checked : Qt::Unchecked);
-    uint256 txid;
-    boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const uint256& hash, ChangeType status) {
+    Txid txid;
+    boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const Txid& hash, ChangeType status) {
         if (status == CT_NEW) txid = hash;
     }));
     ConfirmSend(/*text=*/nullptr, confirm_type);
@@ -97,7 +97,7 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe
 }
 
 //! Find index of txid in transaction list.
-QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
+QModelIndex FindTx(const QAbstractItemModel& model, const Txid& txid)
 {
     QString hash = QString::fromStdString(txid.ToString());
     int rows = model.rowCount({});
@@ -111,7 +111,7 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
 }
 
 //! Invoke bumpfee on txid and check results.
-void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, std::string expectError, bool cancel)
+void BumpFee(TransactionView& view, const Txid& txid, bool expectDisabled, std::string expectError, bool cancel)
 {
     QTableView* table = view.findChild<QTableView*>("transactionView");
     QModelIndex index = FindTx(*table->selectionModel()->model(), txid);
@@ -285,8 +285,8 @@ void TestGUI(interfaces::Node& node, const std::shared_ptr<CWallet>& wallet)
     // Send two transactions, and verify they are added to transaction list.
     TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
     QCOMPARE(transactionTableModel->rowCount({}), 105);
-    uint256 txid1 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 5 * COIN, /*rbf=*/false);
-    uint256 txid2 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 10 * COIN, /*rbf=*/true);
+    Txid txid1 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 5 * COIN, /*rbf=*/false);
+    Txid txid2 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 10 * COIN, /*rbf=*/true);
     // Transaction table model updates on a QueuedConnection, so process events to ensure it's updated.
     qApp->processEvents();
     QCOMPARE(transactionTableModel->rowCount({}), 107);

src/qt/transactionrecord.cpp

@@ -37,7 +37,7 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const interface
     CAmount nCredit = wtx.credit;
     CAmount nDebit = wtx.debit;
     CAmount nNet = nCredit - nDebit;
-    uint256 hash = wtx.tx->GetHash();
+    Txid hash = wtx.tx->GetHash();
     std::map<std::string, std::string> mapValue = wtx.value_map;
 
     bool involvesWatchAddress = false;

src/qt/transactionrecord.h

@@ -7,6 +7,7 @@
 
 #include <consensus/amount.h>
 #include <uint256.h>
+#include <util/transaction_identifier.h>
 
 #include <QList>
 #include <QString>
@@ -79,13 +80,13 @@ public:
     {
     }
 
-    TransactionRecord(uint256 _hash, qint64 _time):
+    TransactionRecord(Txid _hash, qint64 _time):
             hash(_hash), time(_time), type(Other), debit(0),
             credit(0), idx(0)
     {
     }
 
-    TransactionRecord(uint256 _hash, qint64 _time,
+    TransactionRecord(Txid _hash, qint64 _time,
                 Type _type, const std::string &_address,
                 const CAmount& _debit, const CAmount& _credit):
             hash(_hash), time(_time), type(_type), address(_address), debit(_debit), credit(_credit),
@@ -100,7 +101,7 @@ public:
 
     /** @name Immutable transaction attributes
       @{*/
-    uint256 hash;
+    Txid hash;
     qint64 time;
     Type type;
     std::string address;

src/qt/transactiontablemodel.cpp

@@ -49,11 +49,11 @@ struct TxLessThan
     {
         return a.hash < b.hash;
     }
-    bool operator()(const TransactionRecord &a, const uint256 &b) const
+    bool operator()(const TransactionRecord &a, const Txid &b) const
     {
         return a.hash < b;
     }
-    bool operator()(const uint256 &a, const TransactionRecord &b) const
+    bool operator()(const Txid &a, const TransactionRecord &b) const
     {
         return a < b.hash;
     }
@@ -64,7 +64,7 @@ struct TransactionNotification
 {
 public:
     TransactionNotification() = default;
-    TransactionNotification(uint256 _hash, ChangeType _status, bool _showTransaction):
+    TransactionNotification(Txid _hash, ChangeType _status, bool _showTransaction):
         hash(_hash), status(_status), showTransaction(_showTransaction) {}
 
     void invoke(QObject *ttm)
@@ -78,7 +78,7 @@ public:
         assert(invoked);
     }
 private:
-    uint256 hash;
+    Txid hash;
     ChangeType status;
     bool showTransaction;
 };
@@ -103,7 +103,7 @@ public:
     bool m_loading = false;
     std::vector< TransactionNotification > vQueueNotifications;
 
-    void NotifyTransactionChanged(const uint256 &hash, ChangeType status);
+    void NotifyTransactionChanged(const Txid& hash, ChangeType status);
     void DispatchNotifications();
 
     /* Query entire wallet anew from core.
@@ -127,7 +127,7 @@ public:
 
        Call with transaction that was added, removed or changed.
      */
-    void updateWallet(interfaces::Wallet& wallet, const uint256 &hash, int status, bool showTransaction)
+    void updateWallet(interfaces::Wallet& wallet, const Txid &hash, int status, bool showTransaction)
     {
         qDebug() << "TransactionTablePriv::updateWallet: " + QString::fromStdString(hash.ToString()) + " " + QString::number(status);
 
@@ -699,7 +699,7 @@ void TransactionTableModel::updateDisplayUnit()
     Q_EMIT dataChanged(index(0, Amount), index(priv->size()-1, Amount));
 }
 
-void TransactionTablePriv::NotifyTransactionChanged(const uint256 &hash, ChangeType status)
+void TransactionTablePriv::NotifyTransactionChanged(const Txid& hash, ChangeType status)
 {
     // Find transaction in wallet
     // Determine whether to show transaction or not (determine this here so that no relocking is needed in GUI thread)

src/qt/transactionview.cpp

@@ -193,7 +193,7 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
     // Double-clicking on a transaction on the transaction history page shows details
     connect(this, &TransactionView::doubleClicked, this, &TransactionView::showDetails);
     // Highlight transaction after fee bump
-    connect(this, &TransactionView::bumpedFee, [this](const uint256& txid) {
+    connect(this, &TransactionView::bumpedFee, [this](const Txid& txid) {
       focusTransaction(txid);
     });
 }
@@ -436,7 +436,7 @@ void TransactionView::bumpFee([[maybe_unused]] bool checked)
     Txid hash = Txid::FromHex(hashQStr.toStdString()).value();
 
     // Bump tx fee over the walletModel
-    uint256 newHash;
+    Txid newHash;
     if (model->bumpFee(hash, newHash)) {
         // Update the table
         transactionView->selectionModel()->clearSelection();
@@ -604,7 +604,7 @@ void TransactionView::focusTransaction(const QModelIndex &idx)
     transactionView->setFocus();
 }
 
-void TransactionView::focusTransaction(const uint256& txid)
+void TransactionView::focusTransaction(const Txid& txid)
 {
     if (!transactionProxyModel)
         return;

src/qt/transactionview.h

@@ -115,7 +115,7 @@ Q_SIGNALS:
     /**  Fired when a message should be reported to the user */
     void message(const QString &title, const QString &message, unsigned int style);
 
-    void bumpedFee(const uint256& txid);
+    void bumpedFee(const Txid& txid);
 
 public Q_SLOTS:
     void chooseDate(int idx);
@@ -126,7 +126,7 @@ public Q_SLOTS:
     void exportClicked();
     void closeOpenedDialogs();
     void focusTransaction(const QModelIndex&);
-    void focusTransaction(const uint256& txid);
+    void focusTransaction(const Txid& txid);
 };
 
 #endif // BITCOIN_QT_TRANSACTIONVIEW_H

src/qt/walletmodel.cpp

@@ -388,7 +388,7 @@ static void NotifyAddressBookChanged(WalletModel *walletmodel,
     assert(invoked);
 }
 
-static void NotifyTransactionChanged(WalletModel *walletmodel, const uint256 &hash, ChangeType status)
+static void NotifyTransactionChanged(WalletModel *walletmodel, const Txid& hash, ChangeType status)
 {
     Q_UNUSED(hash);
     Q_UNUSED(status);
@@ -479,7 +479,7 @@ WalletModel::UnlockContext::~UnlockContext()
     }
 }
 
-bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
+bool WalletModel::bumpFee(Txid hash, Txid& new_hash)
 {
     CCoinControl coin_control;
     coin_control.m_signal_bip125_rbf = true;

src/qt/walletmodel.h

@@ -129,7 +129,7 @@ public:
 
     UnlockContext requestUnlock();
 
-    bool bumpFee(uint256 hash, uint256& new_hash);
+    bool bumpFee(Txid hash, Txid& new_hash);
     void displayAddress(std::string sAddress) const;
 
     static bool isWalletEnabled();

src/qt/walletview.cpp

@@ -82,7 +82,7 @@ WalletView::WalletView(WalletModel* wallet_model, const PlatformStyle* _platform
 
     connect(sendCoinsPage, &SendCoinsDialog::coinsSent, this, &WalletView::coinsSent);
     // Highlight transaction after send
-    connect(sendCoinsPage, &SendCoinsDialog::coinsSent, transactionView, qOverload<const uint256&>(&TransactionView::focusTransaction));
+    connect(sendCoinsPage, &SendCoinsDialog::coinsSent, transactionView, qOverload<const Txid&>(&TransactionView::focusTransaction));
 
     // Clicking on "Export" allows to export the transaction list
     connect(exportButton, &QPushButton::clicked, transactionView, &TransactionView::exportClicked);

src/rest.cpp

@@ -709,7 +709,7 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string
     std::string hashStr;
     const RESTResponseFormat rf = ParseDataFormat(hashStr, strURIPart);
 
-    auto hash{uint256::FromHex(hashStr)};
+    auto hash{Txid::FromHex(hashStr)};
     if (!hash) {
         return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);
     }

src/rpc/mempool.cpp

@@ -309,7 +309,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
     std::set<std::string> setDepends;
     for (const CTxIn& txin : tx.vin)
     {
-        if (pool.exists(GenTxid::Txid(txin.prevout.hash)))
+        if (pool.exists(txin.prevout.hash))
             setDepends.insert(txin.prevout.hash.ToString());
     }
 
@@ -457,12 +457,12 @@ static RPCHelpMan getmempoolancestors()
     if (!request.params[1].isNull())
         fVerbose = request.params[1].get_bool();
 
-    uint256 hash = ParseHashV(request.params[0], "parameter 1");
+    Txid hash = Txid::FromUint256(ParseHashV(request.params[0], "parameter 1"));
 
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
 
-    const auto entry{mempool.GetEntry(Txid::FromUint256(hash))};
+    const auto entry{mempool.GetEntry(hash)};
     if (entry == nullptr) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
     }
@@ -479,7 +479,7 @@ static RPCHelpMan getmempoolancestors()
         UniValue o(UniValue::VOBJ);
         for (CTxMemPool::txiter ancestorIt : ancestors) {
             const CTxMemPoolEntry &e = *ancestorIt;
-            const uint256& _hash = e.GetTx().GetHash();
+            const Txid& _hash = e.GetTx().GetHash();
             UniValue info(UniValue::VOBJ);
             entryToJSON(mempool, info, e);
             o.pushKV(_hash.ToString(), std::move(info));
@@ -518,7 +518,7 @@ static RPCHelpMan getmempooldescendants()
     if (!request.params[1].isNull())
         fVerbose = request.params[1].get_bool();
 
-    uint256 hash = ParseHashV(request.params[0], "parameter 1");
+    Txid hash = Txid::FromUint256(ParseHashV(request.params[0], "parameter 1"));
 
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
@@ -544,7 +544,7 @@ static RPCHelpMan getmempooldescendants()
         UniValue o(UniValue::VOBJ);
         for (CTxMemPool::txiter descendantIt : setDescendants) {
             const CTxMemPoolEntry &e = *descendantIt;
-            const uint256& _hash = e.GetTx().GetHash();
+            const Txid& _hash = e.GetTx().GetHash();
             UniValue info(UniValue::VOBJ);
             entryToJSON(mempool, info, e);
             o.pushKV(_hash.ToString(), std::move(info));
@@ -570,12 +570,12 @@ static RPCHelpMan getmempoolentry()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    uint256 hash = ParseHashV(request.params[0], "parameter 1");
+    Txid hash = Txid::FromUint256(ParseHashV(request.params[0], "parameter 1"));
 
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
 
-    const auto entry{mempool.GetEntry(Txid::FromUint256(hash))};
+    const auto entry{mempool.GetEntry(hash)};
     if (entry == nullptr) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
     }
@@ -1030,7 +1030,7 @@ static RPCHelpMan submitpackage()
                     // Belt-and-suspenders check; everything should be successful here
                     CHECK_NONFATAL(package_result.m_tx_results.size() == txns.size());
                     for (const auto& tx : txns) {
-                        CHECK_NONFATAL(mempool.exists(GenTxid::Txid(tx->GetHash())));
+                        CHECK_NONFATAL(mempool.exists(tx->GetHash()));
                     }
                     break;
                 }
@@ -1054,7 +1054,7 @@ static RPCHelpMan submitpackage()
             size_t num_broadcast{0};
             for (const auto& tx : txns) {
                 // We don't want to re-submit the txn for validation in BroadcastTransaction
-                if (!mempool.exists(GenTxid::Txid(tx->GetHash()))) {
+                if (!mempool.exists(tx->GetHash())) {
                     continue;
                 }
 
@@ -1072,7 +1072,7 @@ static RPCHelpMan submitpackage()
             UniValue rpc_result{UniValue::VOBJ};
             rpc_result.pushKV("package_msg", package_msg);
             UniValue tx_result_map{UniValue::VOBJ};
-            std::set<uint256> replaced_txids;
+            std::set<Txid> replaced_txids;
             for (const auto& tx : txns) {
                 UniValue result_inner{UniValue::VOBJ};
                 result_inner.pushKV("txid", tx->GetHash().GetHex());
@@ -1116,7 +1116,7 @@ static RPCHelpMan submitpackage()
             }
             rpc_result.pushKV("tx-results", std::move(tx_result_map));
             UniValue replaced_list(UniValue::VARR);
-            for (const uint256& hash : replaced_txids) replaced_list.push_back(hash.ToString());
+            for (const Txid& hash : replaced_txids) replaced_list.push_back(hash.ToString());
             rpc_result.pushKV("replaced-transactions", std::move(replaced_list));
             return rpc_result;
         },

src/rpc/mining.cpp

@@ -352,7 +352,7 @@ static RPCHelpMan generateblock()
         const auto& str{raw_txs_or_txids[i].get_str()};
 
         CMutableTransaction mtx;
-        if (auto hash{uint256::FromHex(str)}) {
+        if (auto hash{Txid::FromHex(str)}) {
             const auto tx{mempool.get(*hash)};
             if (!tx) {
                 throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Transaction %s not in mempool.", str));
@@ -516,7 +516,7 @@ static RPCHelpMan prioritisetransaction()
 {
     LOCK(cs_main);
 
-    uint256 hash(ParseHashV(request.params[0], "txid"));
+    Txid hash = Txid::FromUint256((ParseHashV(request.params[0], "txid")));
     const auto dummy{self.MaybeArg<double>("dummy")};
     CAmount nAmount = request.params[2].getInt<int64_t>();
 
@@ -890,14 +890,14 @@ static RPCHelpMan getblocktemplate()
     UniValue aCaps(UniValue::VARR); aCaps.push_back("proposal");
 
     UniValue transactions(UniValue::VARR);
-    std::map<uint256, int64_t> setTxIndex;
+    std::map<Txid, int64_t> setTxIndex;
     std::vector<CAmount> tx_fees{block_template->getTxFees()};
     std::vector<CAmount> tx_sigops{block_template->getTxSigops()};
 
     int i = 0;
     for (const auto& it : block.vtx) {
         const CTransaction& tx = *it;
-        uint256 txHash = tx.GetHash();
+        Txid txHash = tx.GetHash();
         setTxIndex[txHash] = i++;
 
         if (tx.IsCoinBase())

src/rpc/rawtransaction.cpp

@@ -319,10 +319,10 @@ static RPCHelpMan getrawtransaction()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     ChainstateManager& chainman = EnsureChainman(node);
 
-    uint256 hash = ParseHashV(request.params[0], "parameter 1");
+    Txid hash = Txid::FromUint256(ParseHashV(request.params[0], "parameter 1"));
     const CBlockIndex* blockindex = nullptr;
 
-    if (hash == chainman.GetParams().GenesisBlock().hashMerkleRoot) {
+    if (hash.ToUint256() == chainman.GetParams().GenesisBlock().hashMerkleRoot) {
         // Special exception for the genesis block coinbase transaction
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The genesis block coinbase is not considered an ordinary transaction and cannot be retrieved");
     }

src/signet.cpp

@@ -59,9 +59,9 @@ static uint256 ComputeModifiedMerkleRoot(const CMutableTransaction& cb, const CB
 {
     std::vector<uint256> leaves;
     leaves.resize(block.vtx.size());
-    leaves[0] = cb.GetHash();
+    leaves[0] = cb.GetHash().ToUint256();
     for (size_t s = 1; s < block.vtx.size(); ++s) {
-        leaves[s] = block.vtx[s]->GetHash();
+        leaves[s] = block.vtx[s]->GetHash().ToUint256();
     }
     return ComputeMerkleRoot(std::move(leaves));
 }

src/test/blockencodings_tests.cpp

@@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
     AddToMempool(pool, entry.FromTx(block.vtx[2]));
     BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0);
 
-    uint256 txhash;
+    Txid txhash;
 
     // Test with pre-forwarding tx 1, but not coinbase
     {
@@ -225,7 +225,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
     AddToMempool(pool, entry.FromTx(block.vtx[1]));
     BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0);
 
-    uint256 txhash;
+    Txid txhash;
 
     // Test with pre-forwarding coinbase + tx 2 with tx 1 in mempool
     {

src/test/disconnected_transactions.cpp

@@ -22,7 +22,7 @@ BOOST_AUTO_TEST_CASE(disconnectpool_memory_limits)
     // is within an expected range.
 
     // Overhead for the hashmap depends on number of buckets
-    std::unordered_map<uint256, CTransaction*, SaltedTxidHasher> temp_map;
+    std::unordered_map<uint256, CTransaction*, SaltedUint256Hasher> temp_map;
     temp_map.reserve(1);
     const size_t MAP_1{memusage::DynamicUsage(temp_map)};
     temp_map.reserve(100);

src/test/fuzz/mini_miner.cpp

@@ -173,7 +173,7 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
         if (!pool.GetConflictTx(coin)) outpoints.push_back(coin);
     }
     for (const auto& tx : transactions) {
-        assert(pool.exists(GenTxid::Txid(tx->GetHash())));
+        assert(pool.exists(tx->GetHash()));
         for (uint32_t n{0}; n < tx->vout.size(); ++n) {
             COutPoint coin{tx->GetHash(), n};
             if (!pool.GetConflictTx(coin)) outpoints.push_back(coin);

src/test/fuzz/p2p_headers_presync.cpp

@@ -139,7 +139,7 @@ CBlock ConsumeBlock(FuzzedDataProvider& fuzzed_data_provider, const uint256& pre
     tx.vout[0].nValue = 0;
     tx.vin[0].scriptSig.resize(2);
     block.vtx.push_back(MakeTransactionRef(tx));
-    block.hashMerkleRoot = block.vtx[0]->GetHash();
+    block.hashMerkleRoot = block.vtx[0]->GetHash().ToUint256();
     return block;
 }
 

src/test/fuzz/package_eval.cpp

@@ -310,10 +310,10 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool)
             const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
             // We only prioritise out of mempool transactions since PrioritiseTransaction doesn't
             // filter for ephemeral dust
-            if (tx_pool.exists(GenTxid::Txid(txid))) {
-                const auto tx_info{tx_pool.info(GenTxid::Txid(txid))};
+            if (tx_pool.exists(txid)) {
+                const auto tx_info{tx_pool.info(txid)};
                 if (GetDust(*tx_info.tx, tx_pool.m_opts.dust_relay_feerate).empty()) {
-                    tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
+                    tx_pool.PrioritiseTransaction(txid, delta);
                 }
             }
         }
@@ -476,7 +476,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
                                    txs.back()->GetHash() :
                                    PickValue(fuzzed_data_provider, mempool_outpoints).hash;
             const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
-            tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
+            tx_pool.PrioritiseTransaction(txid, delta);
         }
 
         // Remember all added transactions

src/test/fuzz/partially_downloaded_block.cpp

@@ -79,7 +79,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb)
             available.insert(i);
         }
 
-        if (add_to_mempool && !pool.exists(GenTxid::Txid(tx->GetHash()))) {
+        if (add_to_mempool && !pool.exists(tx->GetHash())) {
             LOCK2(cs_main, pool.cs);
             AddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx));
             available.insert(i);

src/test/fuzz/policy_estimator.cpp

@@ -85,7 +85,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
                 block_policy_estimator.processBlock(txs, current_height);
             },
             [&] {
-                (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider));
+                (void)block_policy_estimator.removeTx(Txid::FromUint256(ConsumeUInt256(fuzzed_data_provider)));
             },
             [&] {
                 block_policy_estimator.FlushUnconfirmed();

src/test/fuzz/rbf.cpp

@@ -160,7 +160,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
         }
 
         if (fuzzed_data_provider.ConsumeBool()) {
-            pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000));
+            pool.PrioritiseTransaction(mempool_txs.back().GetHash(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000));
         }
     }
 

src/test/fuzz/tx_pool.cpp

@@ -287,7 +287,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
                                    tx->GetHash() :
                                    PickValue(fuzzed_data_provider, outpoints_rbf).hash;
             const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
-            tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
+            tx_pool.PrioritiseTransaction(txid, delta);
         }
 
         // Remember all removed and added transactions
@@ -315,8 +315,8 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
         node.validation_signals->SyncWithValidationInterfaceQueue();
         node.validation_signals->UnregisterSharedValidationInterface(txr);
 
-        bool txid_in_mempool = tx_pool.exists(GenTxid::Txid(tx->GetHash()));
-        bool wtxid_in_mempool = tx_pool.exists(GenTxid::Wtxid(tx->GetWitnessHash()));
+        bool txid_in_mempool = tx_pool.exists(tx->GetHash());
+        bool wtxid_in_mempool = tx_pool.exists(tx->GetWitnessHash());
         CheckATMPInvariants(res, txid_in_mempool, wtxid_in_mempool);
 
         Assert(accepted != added.empty());
@@ -408,7 +408,7 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
                                    mut_tx.GetHash() :
                                    PickValue(fuzzed_data_provider, txids);
             const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
-            tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
+            tx_pool.PrioritiseTransaction(txid, delta);
         }
 
         const auto tx = MakeTransactionRef(mut_tx);

src/test/fuzz/txdownloadman.cpp

@@ -227,9 +227,12 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
                 Assert(first_time_failure || !todo.m_should_add_extra_compact_tx);
             },
             [&] {
-                GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
-                                GenTxid::Txid(rand_tx->GetHash()) :
-                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                GenTxid gtxid;
+                if (fuzzed_data_provider.ConsumeBool()) {
+                    rand_tx->GetHash();
+                } else {
+                    rand_tx->GetWitnessHash();
+                }
                 txdownloadman.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
@@ -373,9 +376,12 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                 if (!reject_contains_wtxid) Assert(todo.m_unique_parents.size() <= rand_tx->vin.size());
             },
             [&] {
-                GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
-                                GenTxid::Txid(rand_tx->GetHash()) :
-                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                GenTxid gtxid;
+                if (fuzzed_data_provider.ConsumeBool()) {
+                    rand_tx->GetHash();
+                } else {
+                    rand_tx->GetWitnessHash();
+                }
                 txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
@@ -395,7 +401,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                 // The only combination that doesn't make sense is validate both tx and package.
                 Assert(!(should_validate && maybe_package.has_value()));
                 if (should_validate) {
-                    Assert(!txdownload_impl.AlreadyHaveTx(GenTxid::Wtxid(rand_tx->GetWitnessHash()), /*include_reconsiderable=*/true));
+                    Assert(!txdownload_impl.AlreadyHaveTx(rand_tx->GetWitnessHash(), /*include_reconsiderable=*/true));
                 }
                 if (maybe_package.has_value()) {
                     CheckPackageToValidate(*maybe_package, rand_peer);
@@ -424,7 +430,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                     // However, if there was a non-null tx in the workset, HaveMoreWork should have
                     // returned true.
                     Assert(expect_work);
-                    Assert(txdownload_impl.AlreadyHaveTx(GenTxid::Wtxid(ptx->GetWitnessHash()), /*include_reconsiderable=*/false));
+                    Assert(txdownload_impl.AlreadyHaveTx(ptx->GetWitnessHash(), /*include_reconsiderable=*/false));
                     // Presumably we have validated this tx. Use "missing inputs" to keep it in the
                     // orphanage longer. Later iterations might call MempoolAcceptedTx or
                     // MempoolRejectedTx with a different error.

src/test/fuzz/txrequest.cpp

@@ -20,7 +20,7 @@ constexpr int MAX_TXHASHES = 16;
 constexpr int MAX_PEERS = 16;
 
 //! Randomly generated GenTxids used in this test (length is MAX_TXHASHES).
-uint256 TXHASHES[MAX_TXHASHES];
+GenTxid TXHASHES[MAX_TXHASHES];
 
 //! Precomputed random durations (positive and negative, each ~exponentially distributed).
 std::chrono::microseconds DELAYS[256];
@@ -30,7 +30,13 @@ struct Initializer
     Initializer()
     {
         for (uint8_t txhash = 0; txhash < MAX_TXHASHES; txhash += 1) {
-            CSHA256().Write(&txhash, 1).Finalize(TXHASHES[txhash].begin());
+            uint256 hash;
+            CSHA256().Write(&txhash, 1).Finalize(hash.begin());
+            if (txhash % 2 == 0) {
+                TXHASHES[txhash] = Txid::FromUint256(hash);
+            } else {
+                TXHASHES[txhash] = Wtxid::FromUint256(hash);
+            }
         }
         int i = 0;
         // DELAYS[N] for N=0..15 is just N microseconds.
@@ -94,7 +100,6 @@ class Tester
         uint64_t m_sequence;
         State m_state{State::NOTHING};
         bool m_preferred;
-        bool m_is_wtxid;
         uint64_t m_priority; //!< Precomputed priority.
     };
 
@@ -186,7 +191,7 @@ public:
         m_tracker.ForgetTxHash(TXHASHES[txhash]);
     }
 
-    void ReceivedInv(int peer, int txhash, bool is_wtxid, bool preferred, std::chrono::microseconds reqtime)
+    void ReceivedInv(int peer, int txhash, bool preferred, std::chrono::microseconds reqtime)
     {
         // Apply to naive structure: if no announcement for txidnum/peer combination
         // already, create a new CANDIDATE; otherwise do nothing.
@@ -195,7 +200,6 @@ public:
             ann.m_preferred = preferred;
             ann.m_state = State::CANDIDATE;
             ann.m_time = reqtime;
-            ann.m_is_wtxid = is_wtxid;
             ann.m_sequence = m_current_sequence++;
             ann.m_priority = m_tracker.ComputePriority(TXHASHES[txhash], peer, ann.m_preferred);
 
@@ -204,7 +208,7 @@ public:
         }
 
         // Call TxRequestTracker's implementation.
-        m_tracker.ReceivedInv(peer, is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash]), preferred, reqtime);
+        m_tracker.ReceivedInv(peer, TXHASHES[txhash], preferred, reqtime);
     }
 
     void RequestedTx(int peer, int txhash, std::chrono::microseconds exptime)
@@ -252,7 +256,7 @@ public:
             for (int peer2 = 0; peer2 < MAX_PEERS; ++peer2) {
                 Announcement& ann2 = m_announcements[txhash][peer2];
                 if (ann2.m_state == State::REQUESTED && ann2.m_time <= m_now) {
-                    expected_expired.emplace_back(peer2, ann2.m_is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash]));
+                    expected_expired.emplace_back(peer2, TXHASHES[txhash]);
                     ann2.m_state = State::COMPLETED;
                     break;
                 }
@@ -262,7 +266,7 @@ public:
             // CANDIDATEs for which this announcement has the highest priority get returned.
             const Announcement& ann = m_announcements[txhash][peer];
             if (ann.m_state == State::CANDIDATE && GetSelected(txhash) == peer) {
-                result.emplace_back(ann.m_sequence, txhash, ann.m_is_wtxid);
+                result.emplace_back(ann.m_sequence, txhash, std::holds_alternative<Wtxid>(TXHASHES[txhash]));
             }
         }
         // Sort the results by sequence number.
@@ -278,8 +282,8 @@ public:
         m_tracker.PostGetRequestableSanityCheck(m_now);
         assert(result.size() == actual.size());
         for (size_t pos = 0; pos < actual.size(); ++pos) {
-            assert(TXHASHES[std::get<1>(result[pos])] == actual[pos].GetHash());
-            assert(std::get<2>(result[pos]) == actual[pos].IsWtxid());
+            assert(TXHASHES[std::get<1>(result[pos])] == actual[pos]);
+            assert(std::get<2>(result[pos]) == std::holds_alternative<Wtxid>(actual[pos]));
         }
     }
 
@@ -357,16 +361,14 @@ FUZZ_TARGET(txrequest)
         case 6: // Same, but non-preferred.
             peer = it == buffer.end() ? 0 : *(it++) % MAX_PEERS;
             txidnum = it == buffer.end() ? 0 : *(it++);
-            tester.ReceivedInv(peer, txidnum % MAX_TXHASHES, (txidnum / MAX_TXHASHES) & 1, cmd & 1,
-                std::chrono::microseconds::min());
+            tester.ReceivedInv(peer, txidnum % MAX_TXHASHES, cmd & 1, std::chrono::microseconds::min());
             break;
         case 7: // Received delayed preferred inv
         case 8: // Same, but non-preferred.
             peer = it == buffer.end() ? 0 : *(it++) % MAX_PEERS;
             txidnum = it == buffer.end() ? 0 : *(it++);
             delaynum = it == buffer.end() ? 0 : *(it++);
-            tester.ReceivedInv(peer, txidnum % MAX_TXHASHES, (txidnum / MAX_TXHASHES) & 1, cmd & 1,
-                tester.Now() + DELAYS[delaynum]);
+            tester.ReceivedInv(peer, txidnum % MAX_TXHASHES, cmd & 1, tester.Now() + DELAYS[delaynum]);
             break;
         case 9: // Requested tx from peer
             peer = it == buffer.end() ? 0 : *(it++) % MAX_PEERS;

src/test/mempool_tests.cpp

@@ -454,12 +454,12 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     AddToMempool(pool, entry.Fee(5000LL).FromTx(tx2));
 
     pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash())));
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash())));
+    BOOST_CHECK(pool.exists(tx1.GetHash()));
+    BOOST_CHECK(pool.exists(tx2.GetHash()));
 
     pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // should remove the lower-feerate transaction
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash())));
+    BOOST_CHECK(pool.exists(tx1.GetHash()));
+    BOOST_CHECK(!pool.exists(tx2.GetHash()));
 
     AddToMempool(pool, entry.FromTx(tx2));
     CMutableTransaction tx3 = CMutableTransaction();
@@ -472,14 +472,14 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     AddToMempool(pool, entry.Fee(20000LL).FromTx(tx3));
 
     pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP)
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash())));
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash())));
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx3.GetHash())));
+    BOOST_CHECK(!pool.exists(tx1.GetHash()));
+    BOOST_CHECK(pool.exists(tx2.GetHash()));
+    BOOST_CHECK(pool.exists(tx3.GetHash()));
 
     pool.TrimToSize(GetVirtualTransactionSize(CTransaction(tx1))); // mempool is limited to tx1's size in memory usage, so nothing fits
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx3.GetHash())));
+    BOOST_CHECK(!pool.exists(tx1.GetHash()));
+    BOOST_CHECK(!pool.exists(tx2.GetHash()));
+    BOOST_CHECK(!pool.exists(tx3.GetHash()));
 
     CFeeRate maxFeeRateRemoved(25000, GetVirtualTransactionSize(CTransaction(tx3)) + GetVirtualTransactionSize(CTransaction(tx2)));
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
@@ -539,19 +539,19 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
 
     // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that
     pool.TrimToSize(pool.DynamicMemoryUsage() - 1);
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash())));
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash())));
+    BOOST_CHECK(pool.exists(tx4.GetHash()));
+    BOOST_CHECK(pool.exists(tx6.GetHash()));
+    BOOST_CHECK(!pool.exists(tx7.GetHash()));
 
-    if (!pool.exists(GenTxid::Txid(tx5.GetHash())))
+    if (!pool.exists(tx5.GetHash()))
         AddToMempool(pool, entry.Fee(1000LL).FromTx(tx5));
     AddToMempool(pool, entry.Fee(9000LL).FromTx(tx7));
 
     pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx5.GetHash())));
-    BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash())));
-    BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash())));
+    BOOST_CHECK(pool.exists(tx4.GetHash()));
+    BOOST_CHECK(!pool.exists(tx5.GetHash()));
+    BOOST_CHECK(pool.exists(tx6.GetHash()));
+    BOOST_CHECK(!pool.exists(tx7.GetHash()));
 
     AddToMempool(pool, entry.Fee(1000LL).FromTx(tx5));
     AddToMempool(pool, entry.Fee(9000LL).FromTx(tx7));

src/test/merkle_tests.cpp

@@ -29,7 +29,7 @@ static uint256 BlockBuildMerkleTree(const CBlock& block, bool* fMutated, std::ve
     vMerkleTree.clear();
     vMerkleTree.reserve(block.vtx.size() * 2 + 16); // Safe upper bound for the number of total nodes.
     for (std::vector<CTransactionRef>::const_iterator it(block.vtx.begin()); it != block.vtx.end(); ++it)
-        vMerkleTree.push_back((*it)->GetHash());
+        vMerkleTree.push_back((*it)->GetHash().ToUint256());
     int j = 0;
     bool mutated = false;
     for (int nSize = block.vtx.size(); nSize > 1; nSize = (nSize + 1) / 2)
@@ -138,7 +138,7 @@ BOOST_AUTO_TEST_CASE(merkle_test)
                     std::vector<uint256> newBranch = TransactionMerklePath(block, mtx);
                     std::vector<uint256> oldBranch = BlockGetMerkleBranch(block, merkleTree, mtx);
                     BOOST_CHECK(oldBranch == newBranch);
-                    BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx]->GetHash(), newBranch, mtx) == oldRoot);
+                    BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx]->GetHash().ToUint256(), newBranch, mtx) == oldRoot);
                 }
             }
         }
@@ -166,7 +166,7 @@ BOOST_AUTO_TEST_CASE(merkle_test_oneTx_block)
     mtx.nLockTime = 0;
     block.vtx[0] = MakeTransactionRef(std::move(mtx));
     uint256 root = BlockMerkleRoot(block, &mutated);
-    BOOST_CHECK_EQUAL(root, block.vtx[0]->GetHash());
+    BOOST_CHECK_EQUAL(root, block.vtx[0]->GetHash().ToUint256());
     BOOST_CHECK_EQUAL(mutated, false);
 }
 
@@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness)
     std::vector<uint256> hashes;
     hashes.resize(block.vtx.size());
     hashes[0].SetNull();
-    hashes[1] = block.vtx[1]->GetHash();
+    hashes[1] = block.vtx[1]->GetHash().ToUint256();
 
     uint256 merkleRootofHashes = ComputeMerkleRoot(hashes);
 

src/test/miner_tests.cpp

@@ -591,7 +591,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
     tx.vin[0].scriptSig = CScript() << OP_1;
     tx.vout.resize(1);
     tx.vout[0].nValue = 5000000000LL; // 0 fee
-    uint256 hashFreePrioritisedTx = tx.GetHash();
+    Txid hashFreePrioritisedTx = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(0).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
     tx_mempool.PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN);
 

src/test/miniminer_tests.cpp

@@ -103,7 +103,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup)
     mini_miner_no_target.BuildMockTemplate(std::nullopt);
     const auto template_txids{mini_miner_no_target.GetMockTemplateTxids()};
     BOOST_CHECK_EQUAL(template_txids.size(), 1);
-    BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash().ToUint256()) > 0);
+    BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash()) > 0);
 }
 
 BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
@@ -177,7 +177,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
     struct TxDimensions {
         int32_t vsize; CAmount mod_fee; CFeeRate feerate;
     };
-    std::map<uint256, TxDimensions> tx_dims;
+    std::map<Txid, TxDimensions> tx_dims;
     for (const auto& tx : all_transactions) {
         const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
         tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(),
@@ -590,14 +590,6 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
     CTxMemPool& pool = *Assert(m_node.mempool);
     LOCK2(cs_main, pool.cs);
 
-    // TODO this can be removed once the mempool interface uses Txid, Wtxid
-    auto convert_to_uint256_vec = [](const std::vector<Txid>& vec) -> std::vector<uint256> {
-        std::vector<uint256> out;
-        std::transform(vec.begin(), vec.end(), std::back_inserter(out),
-                       [](const Txid& txid) { return txid.ToUint256(); });
-        return out;
-    };
-
     // Add chain of size 500
     TestMemPoolEntryHelper entry;
     std::vector<Txid> chain_txids;
@@ -611,7 +603,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
     const auto cluster_500tx = pool.GatherClusters({lasttx->GetHash()});
     CTxMemPool::setEntries cluster_500tx_set{cluster_500tx.begin(), cluster_500tx.end()};
     BOOST_CHECK_EQUAL(cluster_500tx.size(), cluster_500tx_set.size());
-    const auto vec_iters_500 = pool.GetIterVec(convert_to_uint256_vec(chain_txids));
+    const auto vec_iters_500 = pool.GetIterVec(chain_txids);
     for (const auto& iter : vec_iters_500) BOOST_CHECK(cluster_500tx_set.count(iter));
 
     // GatherClusters stops at 500 transactions.
@@ -637,7 +629,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
         AddToMempool(pool, entry.Fee(CENT).FromTx(txc));
         zigzag_txids.push_back(txc->GetHash());
     }
-    const auto vec_iters_zigzag = pool.GetIterVec(convert_to_uint256_vec(zigzag_txids));
+    const auto vec_iters_zigzag = pool.GetIterVec(zigzag_txids);
     // It doesn't matter which tx we calculate cluster for, everybody is in it.
     const std::vector<size_t> indices{0, 22, 72, zigzag_txids.size() - 1};
     for (const auto index : indices) {

src/test/pmt_tests.cpp

@@ -50,7 +50,7 @@ BOOST_AUTO_TEST_CASE(pmt_test1)
         uint256 merkleRoot1 = BlockMerkleRoot(block);
         std::vector<uint256> vTxid(nTx, uint256());
         for (unsigned int j=0; j<nTx; j++)
-            vTxid[j] = block.vtx[j]->GetHash();
+            vTxid[j] = block.vtx[j]->GetHash().ToUint256();
         int nHeight = 1, nTx_ = nTx;
         while (nTx_ > 1) {
             nTx_ = (nTx_+1)/2;

src/test/policyestimator_tests.cpp

@@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     // added to the mempool by their associate fee
     // txHashes[j] is populated with transactions either of
     // fee = basefee * (j+1)
-    std::vector<uint256> txHashes[10];
+    std::vector<Txid> txHashes[10];
 
     // Create a transaction template
     CScript garbage;
@@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                                                                                       /*has_no_mempool_parents=*/true)};
                     m_node.validation_signals->TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
                 }
-                uint256 hash = tx.GetHash();
+                Txid hash = tx.GetHash();
                 txHashes[j].push_back(hash);
             }
         }
@@ -178,7 +178,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                                                                                       /*has_no_mempool_parents=*/true)};
                     m_node.validation_signals->TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
                 }
-                uint256 hash = tx.GetHash();
+                Txid hash = tx.GetHash();
                 txHashes[j].push_back(hash);
             }
         }
@@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                                                                                       /*has_no_mempool_parents=*/true)};
                     m_node.validation_signals->TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
                 }
-                uint256 hash = tx.GetHash();
+                Txid hash = tx.GetHash();
                 CTransactionRef ptx = mpool.get(hash);
                 if (ptx)
                     block.push_back(ptx);

src/test/rbf_tests.cpp

@@ -196,7 +196,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
                                        entry5_low, entry6_low_prioritised, entry7_high, entry8_high};
     CTxMemPool::setEntries empty_set;
 
-    const auto unused_txid{GetRandHash()};
+    const auto unused_txid = Txid::FromUint256(GetRandHash());
 
     // Tests for PaysMoreThanConflicts
     // These tests use feerate, not absolute fee.
@@ -293,7 +293,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
     const auto spends_unconfirmed = make_tx({tx1}, {36 * CENT});
     for (const auto& input : spends_unconfirmed->vin) {
         // Spends unconfirmed inputs.
-        BOOST_CHECK(pool.exists(GenTxid::Txid(input.prevout.hash)));
+        BOOST_CHECK(pool.exists(input.prevout.hash));
     }
     BOOST_CHECK(HasNoNewUnconfirmed(/*tx=*/ *spends_unconfirmed.get(),
                                     /*pool=*/ pool,

src/test/txdownload_tests.cpp

@@ -126,10 +126,10 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
         for (const auto segwit_child : {true, false}) {
             const auto ptx_parent = CreatePlaceholderTx(segwit_parent);
             const auto ptx_child = CreatePlaceholderTx(segwit_child);
-            const auto& parent_txid = ptx_parent->GetHash().ToUint256();
-            const auto& parent_wtxid = ptx_parent->GetWitnessHash().ToUint256();
-            const auto& child_txid = ptx_child->GetHash().ToUint256();
-            const auto& child_wtxid = ptx_child->GetWitnessHash().ToUint256();
+            const auto& parent_txid = ptx_parent->GetHash();
+            const auto& parent_wtxid = ptx_parent->GetWitnessHash();
+            const auto& child_txid = ptx_child->GetHash();
+            const auto& child_wtxid = ptx_child->GetWitnessHash();
 
             for (const auto& [result, expected_behavior] : expected_behaviors) {
                 node::TxDownloadManagerImpl txdownload_impl{DEFAULT_OPTS};
@@ -141,13 +141,13 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
                 // No distinction between txid and wtxid caching for nonsegwit transactions, so only test these specific
                 // behaviors for segwit transactions.
                 Behaviors actual_behavior{
-                    /*txid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_txid),
-                    /*wtxid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_wtxid),
-                    /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid),
-                    /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid),
+                    /*txid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_txid.ToUint256()),
+                    /*wtxid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_wtxid.ToUint256()),
+                    /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid.ToUint256()),
+                    /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid.ToUint256()),
                     /*keep=*/keep,
-                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now),
-                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), now),
+                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, parent_txid, now),
+                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, parent_wtxid, now),
                 };
                 BOOST_TEST_MESSAGE("Testing behavior for " << result << (segwit_parent ? " segwit " : " nonsegwit"));
                 actual_behavior.CheckEqual(expected_behavior, /*segwit=*/segwit_parent);
@@ -158,8 +158,8 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
 
                 // If parent (by txid) was rejected, child is too.
                 const bool parent_txid_rejected{segwit_parent ? expected_behavior.m_txid_in_rejects : expected_behavior.m_wtxid_in_rejects};
-                BOOST_CHECK_EQUAL(parent_txid_rejected, txdownload_impl.RecentRejectsFilter().contains(child_txid));
-                BOOST_CHECK_EQUAL(parent_txid_rejected, txdownload_impl.RecentRejectsFilter().contains(child_wtxid));
+                BOOST_CHECK_EQUAL(parent_txid_rejected, txdownload_impl.RecentRejectsFilter().contains(child_txid.ToUint256()));
+                BOOST_CHECK_EQUAL(parent_txid_rejected, txdownload_impl.RecentRejectsFilter().contains(child_wtxid.ToUint256()));
 
                 // Unless rejected, the child should be in orphanage.
                 BOOST_CHECK_EQUAL(!parent_txid_rejected, txdownload_impl.m_orphanage.HaveTx(ptx_child->GetWitnessHash()));

src/test/txpackage_tests.cpp

@@ -1090,7 +1090,7 @@ BOOST_AUTO_TEST_CASE(package_rbf_tests)
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
 
         // child1 has been replaced
-        BOOST_CHECK(!m_node.mempool->exists(GenTxid::Txid(tx_child_1->GetHash())));
+        BOOST_CHECK(!m_node.mempool->exists(tx_child_1->GetHash()));
     }
 
     // Test package rbf.

src/test/txrequest_tests.cpp

@@ -56,7 +56,7 @@ struct Runner
     std::set<NodeId> peerset;
 
     /** Which txhashes have been assigned already (to prevent reuse). */
-    std::set<uint256> txhashset;
+    std::set<GenTxid> txhashset;
 
     /** Which (peer, gtxid) combinations are known to be expired. These need to be accumulated here instead of
      *  checked directly in the GetRequestable return value to avoid introducing a dependency between the various
@@ -100,7 +100,7 @@ public:
     }
 
     /** Schedule a ForgetTxHash call at the Scheduler's current time. */
-    void ForgetTxHash(const uint256& txhash)
+    void ForgetTxHash(const GenTxid& txhash)
     {
         auto& runner = m_runner;
         runner.actions.emplace_back(m_now, [=,&runner]() {
@@ -130,7 +130,7 @@ public:
     }
 
     /** Schedule a RequestedTx call at the Scheduler's current time. */
-    void RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds exptime)
+    void RequestedTx(NodeId peer, const GenTxid& txhash, std::chrono::microseconds exptime)
     {
         auto& runner = m_runner;
         runner.actions.emplace_back(m_now, [=,&runner]() {
@@ -140,7 +140,7 @@ public:
     }
 
     /** Schedule a ReceivedResponse call at the Scheduler's current time. */
-    void ReceivedResponse(NodeId peer, const uint256& txhash)
+    void ReceivedResponse(NodeId peer, const GenTxid& txhash)
     {
         auto& runner = m_runner;
         runner.actions.emplace_back(m_now, [=,&runner]() {
@@ -208,12 +208,17 @@ public:
      * where priority is the predicted internal TxRequestTracker's priority, assuming all announcements
      * are within the same preferredness class.
      */
-    uint256 NewTxHash(const std::vector<std::vector<NodeId>>& orders = {})
+    GenTxid NewGTxid(const std::vector<std::vector<NodeId>>& orders = {})
     {
-        uint256 ret;
+        GenTxid ret;
         bool ok;
         do {
-            ret = m_rng.rand256();
+            uint256 hash = m_rng.rand256();
+            if (m_rng.randbool()) {
+                ret = Txid::FromUint256(hash);
+            } else {
+                ret = Wtxid::FromUint256(hash);
+            }
             ok = true;
             for (const auto& order : orders) {
                 for (size_t pos = 1; pos < order.size(); ++pos) {
@@ -233,12 +238,6 @@ public:
         return ret;
     }
 
-    /** Generate a random GenTxid; the txhash follows NewTxHash; the is_wtxid flag is random. */
-    GenTxid NewGTxid(const std::vector<std::vector<NodeId>>& orders = {})
-    {
-        return m_rng.randbool() ? GenTxid::Wtxid(NewTxHash(orders)) : GenTxid::Txid(NewTxHash(orders));
-    }
-
     /** Generate a new random NodeId to use as peer. The same NodeId is never returned twice
      *  (across all Scenarios combined). */
     NodeId NewPeer()
@@ -285,7 +284,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config)
         scenario.AdvanceTime(RandomTime8s());
         auto expiry = RandomTime8s();
         scenario.Check(peer, {gtxid}, 1, 0, 0, "s5");
-        scenario.RequestedTx(peer, gtxid.GetHash(), scenario.Now() + expiry);
+        scenario.RequestedTx(peer, gtxid, scenario.Now() + expiry);
         scenario.Check(peer, {}, 0, 1, 0, "s6");
 
         if ((config >> 3) == 1) { // The request will time out
@@ -299,7 +298,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config)
             scenario.AdvanceTime(std::chrono::microseconds{m_rng.randrange(expiry.count())});
             scenario.Check(peer, {}, 0, 1, 0, "s9");
             if ((config >> 3) == 3) { // A response will arrive for the transaction
-                scenario.ReceivedResponse(peer, gtxid.GetHash());
+                scenario.ReceivedResponse(peer, gtxid);
                 scenario.Check(peer, {}, 0, 0, 0, "s10");
                 return;
             }
@@ -309,7 +308,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config)
     if (config & 4) { // The peer will go offline
         scenario.DisconnectedPeer(peer);
     } else { // The transaction is no longer needed
-        scenario.ForgetTxHash(gtxid.GetHash());
+        scenario.ForgetTxHash(gtxid);
     }
     scenario.Check(peer, {}, 0, 0, 0, "s11");
 }
@@ -355,7 +354,7 @@ void TxRequestTest::BuildPriorityTest(Scenario& scenario, int config)
 
     // We possibly request from the selected peer.
     if (config & 8) {
-        scenario.RequestedTx(priopeer, gtxid.GetHash(), MAX_TIME);
+        scenario.RequestedTx(priopeer, gtxid, MAX_TIME);
         scenario.Check(priopeer, {}, 0, 1, 0, "p7");
         scenario.Check(otherpeer, {}, 1, 0, 0, "p8");
         if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
@@ -365,7 +364,7 @@ void TxRequestTest::BuildPriorityTest(Scenario& scenario, int config)
     if (config & 16) {
         scenario.DisconnectedPeer(priopeer);
     } else {
-        scenario.ReceivedResponse(priopeer, gtxid.GetHash());
+        scenario.ReceivedResponse(priopeer, gtxid);
     }
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
     scenario.Check(priopeer, {}, 0, 0, !(config & 16), "p8");
@@ -449,7 +448,7 @@ void TxRequestTest::BuildBigPriorityTest(Scenario& scenario, int peers)
             scenario.DisconnectedPeer(peer);
             scenario.Check(peer, {}, 0, 0, 0, "b4");
         } else {
-            scenario.ReceivedResponse(peer, gtxid.GetHash());
+            scenario.ReceivedResponse(peer, gtxid);
             scenario.Check(peer, {}, 0, 0, request_order.size() > 0, "b5");
         }
         if (request_order.size()) {
@@ -509,9 +508,9 @@ void TxRequestTest::BuildWtxidTest(Scenario& scenario, int config)
 
     auto peerT = scenario.NewPeer();
     auto peerW = scenario.NewPeer();
-    auto txhash = scenario.NewTxHash();
-    auto txid{GenTxid::Txid(txhash)};
-    auto wtxid{GenTxid::Wtxid(txhash)};
+    auto gtxid = scenario.NewGTxid();
+    auto txid = Txid::FromUint256(gtxid.ToUint256());
+    auto wtxid = Wtxid::FromUint256(gtxid.ToUint256());
 
     auto reqtimeT = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s();
     auto reqtimeW = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s();
@@ -542,11 +541,11 @@ void TxRequestTest::BuildWtxidTest(Scenario& scenario, int config)
     // Let the preferred announcement be requested. It's not going to be delivered.
     auto expiry = RandomTime8s();
     if (config & 2) {
-        scenario.RequestedTx(peerT, txid.GetHash(), scenario.Now() + expiry);
+        scenario.RequestedTx(peerT, txid, scenario.Now() + expiry);
         scenario.Check(peerT, {}, 0, 1, 0, "w5");
         scenario.Check(peerW, {}, 1, 0, 0, "w6");
     } else {
-        scenario.RequestedTx(peerW, wtxid.GetHash(), scenario.Now() + expiry);
+        scenario.RequestedTx(peerW, wtxid, scenario.Now() + expiry);
         scenario.Check(peerT, {}, 1, 0, 0, "w7");
         scenario.Check(peerW, {}, 0, 1, 0, "w8");
     }
@@ -567,7 +566,7 @@ void TxRequestTest::BuildWtxidTest(Scenario& scenario, int config)
     // If a good transaction with either that hash as wtxid or txid arrives, both
     // announcements are gone.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
-    scenario.ForgetTxHash(txhash);
+    scenario.ForgetTxHash(gtxid);
     scenario.Check(peerT, {}, 0, 0, 0, "w13");
     scenario.Check(peerW, {}, 0, 0, 0, "w14");
 }
@@ -599,7 +598,7 @@ void TxRequestTest::BuildTimeBackwardsTest(Scenario& scenario)
     // Request from peer1.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
     auto expiry = scenario.Now() + RandomTime8s();
-    scenario.RequestedTx(peer1, gtxid.GetHash(), expiry);
+    scenario.RequestedTx(peer1, gtxid, expiry);
     scenario.Check(peer1, {}, 0, 1, 0, "r7");
     scenario.Check(peer2, {}, 1, 0, 0, "r8");
 
@@ -638,20 +637,20 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario)
 
     // We request gtxid2 from *peer1* - no effect.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
-    scenario.RequestedTx(peer1, gtxid2.GetHash(), MAX_TIME);
+    scenario.RequestedTx(peer1, gtxid2, MAX_TIME);
     scenario.Check(peer1, {gtxid1}, 1, 0, 0, "q4");
     scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q5");
 
     // Now request gtxid1 from peer1 - marks it as REQUESTED.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
     auto expiryA = scenario.Now() + RandomTime8s();
-    scenario.RequestedTx(peer1, gtxid1.GetHash(), expiryA);
+    scenario.RequestedTx(peer1, gtxid1, expiryA);
     scenario.Check(peer1, {}, 0, 1, 0, "q6");
     scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q7");
 
     // Request it a second time - nothing happens, as it's already REQUESTED.
     auto expiryB = expiryA + RandomTime8s();
-    scenario.RequestedTx(peer1, gtxid1.GetHash(), expiryB);
+    scenario.RequestedTx(peer1, gtxid1, expiryB);
     scenario.Check(peer1, {}, 0, 1, 0, "q8");
     scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q9");
 
@@ -668,7 +667,7 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario)
 
     // Requesting it yet again from peer1 doesn't do anything, as it's already COMPLETED.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
-    scenario.RequestedTx(peer1, gtxid1.GetHash(), MAX_TIME);
+    scenario.RequestedTx(peer1, gtxid1, MAX_TIME);
     scenario.Check(peer1, {}, 0, 0, 1, "q14");
     scenario.Check(peer2, {gtxid2, gtxid1}, 2, 0, 0, "q15");
 
@@ -680,13 +679,13 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario)
 
     // And request it from peer1 (weird as peer2 has the preference).
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
-    scenario.RequestedTx(peer1, gtxid2.GetHash(), MAX_TIME);
+    scenario.RequestedTx(peer1, gtxid2, MAX_TIME);
     scenario.Check(peer1, {}, 0, 1, 1, "q18");
     scenario.Check(peer2, {gtxid1}, 2, 0, 0, "q19");
 
     // If peer2 now (normally) requests gtxid2, the existing request by peer1 becomes COMPLETED.
     if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s());
-    scenario.RequestedTx(peer2, gtxid2.GetHash(), MAX_TIME);
+    scenario.RequestedTx(peer2, gtxid2, MAX_TIME);
     scenario.Check(peer1, {}, 0, 0, 2, "q20");
     scenario.Check(peer2, {gtxid1}, 1, 1, 0, "q21");
 

src/test/txvalidation_tests.cpp

@@ -304,7 +304,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
 
         Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
         BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
-        CTxMemPool::setEntries entries_mempool_v3{pool.GetIter(mempool_tx_v3->GetHash().ToUint256()).value()};
+        CTxMemPool::setEntries entries_mempool_v3{pool.GetIter(mempool_tx_v3->GetHash()).value()};
         BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), {tx_v2_from_v3}, entries_mempool_v3), expected_error_str);
 
         // mempool_tx_v3  mempool_tx_v2
@@ -339,7 +339,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
 
         Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
         BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
-        CTxMemPool::setEntries entries_mempool_v2{pool.GetIter(mempool_tx_v2->GetHash().ToUint256()).value()};
+        CTxMemPool::setEntries entries_mempool_v2{pool.GetIter(mempool_tx_v2->GetHash()).value()};
         BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), {tx_v3_from_v2}, entries_mempool_v2), expected_error_str);
 
         // mempool_tx_v3  mempool_tx_v2
@@ -536,7 +536,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
         // Configuration where parent already has 2 other children in mempool (no sibling eviction allowed). This may happen as the result of a reorg.
         AddToMempool(pool, entry.FromTx(tx_v3_child2));
         auto tx_v3_child3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 24}}, /*version=*/3);
-        auto entry_mempool_parent = pool.GetIter(mempool_tx_v3->GetHash().ToUint256()).value();
+        auto entry_mempool_parent = pool.GetIter(mempool_tx_v3->GetHash()).value();
         BOOST_CHECK_EQUAL(entry_mempool_parent->GetCountWithDescendants(), 3);
         auto ancestors_2siblings{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child3), m_limits)};
 

src/test/util/setup_common.cpp

@@ -629,3 +629,11 @@ std::ostream& operator<<(std::ostream& os, const uint256& num)
 {
     return os << num.ToString();
 }
+
+std::ostream& operator<<(std::ostream& os, const Txid& txid) {
+    return os << txid.ToString();
+}
+
+std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid) {
+    return os << wtxid.ToString();
+}

src/test/util/setup_common.h

@@ -291,6 +291,8 @@ inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
 std::ostream& operator<<(std::ostream& os, const arith_uint256& num);
 std::ostream& operator<<(std::ostream& os, const uint160& num);
 std::ostream& operator<<(std::ostream& os, const uint256& num);
+std::ostream& operator<<(std::ostream& os, const Txid& txid);
+std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid);
 // @}
 
 /**

src/test/util/txmempool.cpp

@@ -122,17 +122,17 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
         if (mempool) {
             // The tx by txid should be in the mempool iff the result was not INVALID.
             const bool txid_in_mempool{atmp_result.m_result_type != MempoolAcceptResult::ResultType::INVALID};
-            if (mempool->exists(GenTxid::Txid(tx->GetHash())) != txid_in_mempool) {
+            if (mempool->exists(tx->GetHash()) != txid_in_mempool) {
                 return strprintf("tx %s should %sbe in mempool", wtxid.ToString(), txid_in_mempool ? "" : "not ");
             }
             // Additionally, if the result was DIFFERENT_WITNESS, we shouldn't be able to find the tx in mempool by wtxid.
             if (tx->HasWitness() && atmp_result.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS) {
-                if (mempool->exists(GenTxid::Wtxid(wtxid))) {
+                if (mempool->exists(wtxid)) {
                     return strprintf("wtxid %s should not be in mempool", wtxid.ToString());
                 }
             }
             for (const auto& tx_ref : atmp_result.m_replaced_transactions) {
-                if (mempool->exists(GenTxid::Txid(tx_ref->GetHash()))) {
+                if (mempool->exists(tx_ref->GetHash())) {
                     return strprintf("tx %s should not be in mempool as it was replaced", tx_ref->GetWitnessHash().ToString());
                 }
             }

src/test/validation_tests.cpp

@@ -214,9 +214,9 @@ BOOST_AUTO_TEST_CASE(block_malleation)
         // Block with a single coinbase tx is mutated if the merkle root is not
         // equal to the coinbase tx's hash.
         block.vtx.push_back(create_coinbase_tx());
-        BOOST_CHECK(block.vtx[0]->GetHash() != block.hashMerkleRoot);
+        BOOST_CHECK(block.vtx[0]->GetHash().ToUint256() != block.hashMerkleRoot);
         BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
-        block.hashMerkleRoot = block.vtx[0]->GetHash();
+        block.hashMerkleRoot = block.vtx[0]->GetHash().ToUint256();
         BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
 
         // Block with two transactions is mutated if the merkle root does not
@@ -248,7 +248,7 @@ BOOST_AUTO_TEST_CASE(block_malleation)
             mtx.vout.resize(1);
             mtx.vout[0].scriptPubKey.resize(4);
             block.vtx.push_back(MakeTransactionRef(mtx));
-            block.hashMerkleRoot = block.vtx.back()->GetHash();
+            block.hashMerkleRoot = block.vtx.back()->GetHash().ToUint256();
             assert(block.vtx.back()->IsCoinBase());
             assert(GetSerializeSize(TX_NO_WITNESS(block.vtx.back())) == 64);
         }
@@ -285,7 +285,7 @@ BOOST_AUTO_TEST_CASE(block_malleation)
             HashWriter hasher;
             hasher.write(tx1.GetHash());
             hasher.write(tx2.GetHash());
-            assert(hasher.GetHash() == tx3.GetHash());
+            assert(hasher.GetHash() == tx3.GetHash().ToUint256());
             // Verify that tx3 is 64 bytes in size (without witness).
             assert(GetSerializeSize(TX_NO_WITNESS(tx3)) == 64);
         }

src/txmempool.cpp

@@ -55,7 +55,7 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
 }
 
 void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
-                                      const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove)
+                                      const std::set<Txid>& setExclude, std::set<Txid>& descendants_to_remove)
 {
     CTxMemPoolEntry::Children stageEntries, descendants;
     stageEntries = updateIt->GetMemPoolChildrenConst();
@@ -105,7 +105,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan
     mapTx.modify(updateIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); });
 }
 
-void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate)
+void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate)
 {
     AssertLockHeld(cs);
     // For each entry in vHashesToUpdate, store the set of in-mempool, but not
@@ -115,29 +115,29 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes
 
     // Use a set for lookups into vHashesToUpdate (these entries are already
     // accounted for in the state of their ancestors)
-    std::set<uint256> setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end());
+    std::set<Txid> setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end());
 
-    std::set<uint256> descendants_to_remove;
+    std::set<Txid> descendants_to_remove;
 
     // Iterate in reverse, so that whenever we are looking at a transaction
     // we are sure that all in-mempool descendants have already been processed.
     // This maximizes the benefit of the descendant cache and guarantees that
     // CTxMemPoolEntry::m_children will be updated, an assumption made in
     // UpdateForDescendants.
-    for (const uint256& hash : vHashesToUpdate | std::views::reverse) {
+    for (const Txid& hash : vHashesToUpdate | std::views::reverse) {
         // calculate children from mapNextTx
         txiter it = mapTx.find(hash);
         if (it == mapTx.end()) {
             continue;
         }
-        auto iter = mapNextTx.lower_bound(COutPoint(Txid::FromUint256(hash), 0));
+        auto iter = mapNextTx.lower_bound(COutPoint(hash, 0));
         // First calculate the children, and update CTxMemPoolEntry::m_children to
         // include them, and update their CTxMemPoolEntry::m_parents to include this tx.
         // we cache the in-mempool children to avoid duplicate updates
         {
             WITH_FRESH_EPOCH(m_epoch);
             for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) {
-                const uint256 &childHash = iter->second->GetHash();
+                const Txid &childHash = iter->second->GetHash();
                 txiter childIter = mapTx.find(childHash);
                 assert(childIter != mapTx.end());
                 // We can skip updating entries we've encountered before or that
@@ -782,7 +782,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
         AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max());
     }
     for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) {
-        uint256 hash = it->second->GetHash();
+        Txid hash = it->second->GetHash();
         indexed_transaction_set::const_iterator it2 = mapTx.find(hash);
         const CTransaction& tx = it2->GetTx();
         assert(it2 != mapTx.end());
@@ -794,7 +794,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
     assert(innerUsage == cachedInnerUsage);
 }
 
-bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid)
+bool CTxMemPool::CompareDepthAndScore(const GenTxid& hasha, const GenTxid& hashb)
 {
     /* Return `true` if hasha should be considered sooner than hashb. Namely when:
      *   a is not in the mempool, but b is
@@ -802,10 +802,18 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb
      *   both are in the mempool and a has a higher score than b
      */
     LOCK(cs);
-    indexed_transaction_set::const_iterator j = wtxid ? get_iter_from_wtxid(hashb) : mapTx.find(hashb);
+    indexed_transaction_set::const_iterator j = std::visit(util::Overloaded{
+        [this](const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return get_iter_from_wtxid(wtxid); },
+        [this](const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return mapTx.find(txid); }
+    }, hashb);
     if (j == mapTx.end()) return false;
-    indexed_transaction_set::const_iterator i = wtxid ? get_iter_from_wtxid(hasha) : mapTx.find(hasha);
+
+    indexed_transaction_set::const_iterator i = std::visit(util::Overloaded{
+        [this](const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return get_iter_from_wtxid(wtxid); },
+        [this](const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return mapTx.find(txid); }
+    }, hasha);
     if (i == mapTx.end()) return true;
+
     uint64_t counta = i->GetCountWithAncestors();
     uint64_t countb = j->GetCountWithAncestors();
     if (counta == countb) {
@@ -881,7 +889,7 @@ const CTxMemPoolEntry* CTxMemPool::GetEntry(const Txid& txid) const
     return i == mapTx.end() ? nullptr : &(*i);
 }
 
-CTransactionRef CTxMemPool::get(const uint256& hash) const
+CTransactionRef CTxMemPool::get(const Txid& hash) const
 {
     LOCK(cs);
     indexed_transaction_set::const_iterator i = mapTx.find(hash);
@@ -893,7 +901,11 @@ CTransactionRef CTxMemPool::get(const uint256& hash) const
 TxMempoolInfo CTxMemPool::info(const GenTxid& gtxid) const
 {
     LOCK(cs);
-    indexed_transaction_set::const_iterator i = (gtxid.IsWtxid() ? get_iter_from_wtxid(gtxid.GetHash()) : mapTx.find(gtxid.GetHash()));
+    indexed_transaction_set::const_iterator i = std::visit(util::Overloaded{
+        [this](const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return get_iter_from_wtxid(wtxid); },
+        [this](const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return mapTx.find(txid); }
+    }, gtxid);
+
     if (i == mapTx.end())
         return TxMempoolInfo();
     return GetInfo(i);
@@ -902,7 +914,11 @@ TxMempoolInfo CTxMemPool::info(const GenTxid& gtxid) const
 TxMempoolInfo CTxMemPool::info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const
 {
     LOCK(cs);
-    indexed_transaction_set::const_iterator i = (gtxid.IsWtxid() ? get_iter_from_wtxid(gtxid.GetHash()) : mapTx.find(gtxid.GetHash()));
+    indexed_transaction_set::const_iterator i = std::visit(util::Overloaded{
+        [this](const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return get_iter_from_wtxid(wtxid); },
+        [this](const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return mapTx.find(txid); }
+    }, gtxid);
+
     if (i != mapTx.end() && i->GetSequence() < last_sequence) {
         return GetInfo(i);
     } else {
@@ -910,7 +926,7 @@ TxMempoolInfo CTxMemPool::info_for_relay(const GenTxid& gtxid, uint64_t last_seq
     }
 }
 
-void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta)
+void CTxMemPool::PrioritiseTransaction(const Txid& hash, const CAmount& nFeeDelta)
 {
     {
         LOCK(cs);
@@ -946,17 +962,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
     }
 }
 
-void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const
+void CTxMemPool::ApplyDelta(const Txid& hash, CAmount &nFeeDelta) const
 {
     AssertLockHeld(cs);
-    std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(hash);
+    std::map<Txid, CAmount>::const_iterator pos = mapDeltas.find(hash);
     if (pos == mapDeltas.end())
         return;
     const CAmount &delta = pos->second;
     nFeeDelta += delta;
 }
 
-void CTxMemPool::ClearPrioritisation(const uint256& hash)
+void CTxMemPool::ClearPrioritisation(const Txid& hash)
 {
     AssertLockHeld(cs);
     mapDeltas.erase(hash);
@@ -984,7 +1000,7 @@ const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const
     return it == mapNextTx.end() ? nullptr : it->second;
 }
 
-std::optional<CTxMemPool::txiter> CTxMemPool::GetIter(const uint256& txid) const
+std::optional<CTxMemPool::txiter> CTxMemPool::GetIter(const Txid& txid) const
 {
     auto it = mapTx.find(txid);
     if (it != mapTx.end()) return it;
@@ -1001,7 +1017,7 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<Txid>& hashes) cons
     return ret;
 }
 
-std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<uint256>& txids) const
+std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<Txid>& txids) const
 {
     AssertLockHeld(cs);
     std::vector<txiter> ret;
@@ -1017,7 +1033,7 @@ std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<uint256
 bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const
 {
     for (unsigned int i = 0; i < tx.vin.size(); i++)
-        if (exists(GenTxid::Txid(tx.vin[i].prevout.hash)))
+        if (exists(tx.vin[i].prevout.hash))
             return false;
     return true;
 }
@@ -1066,7 +1082,7 @@ size_t CTxMemPool::DynamicMemoryUsage() const {
     return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + cachedInnerUsage;
 }
 
-void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) {
+void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) {
     LOCK(cs);
 
     if (m_unbroadcast_txids.erase(txid))
@@ -1187,7 +1203,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
         if (pvNoSpendsRemaining) {
             for (const CTransaction& tx : txn) {
                 for (const CTxIn& txin : tx.vin) {
-                    if (exists(GenTxid::Txid(txin.prevout.hash))) continue;
+                    if (exists(txin.prevout.hash)) continue;
                     pvNoSpendsRemaining->push_back(txin.prevout);
                 }
             }
@@ -1221,7 +1237,7 @@ uint64_t CTxMemPool::CalculateDescendantMaximum(txiter entry) const {
     return maximum;
 }
 
-void CTxMemPool::GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* const ancestorsize, CAmount* const ancestorfees) const {
+void CTxMemPool::GetTransactionAncestry(const Txid& txid, size_t& ancestors, size_t& descendants, size_t* const ancestorsize, CAmount* const ancestorfees) const {
     LOCK(cs);
     auto it = mapTx.find(txid);
     ancestors = descendants = 0;
@@ -1245,7 +1261,7 @@ void CTxMemPool::SetLoadTried(bool load_tried)
     m_load_tried = load_tried;
 }
 
-std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<uint256>& txids) const
+std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<Txid>& txids) const
 {
     AssertLockHeld(cs);
     std::vector<txiter> clustered_txs{GetIterVec(txids)};

src/txmempool.h

@@ -20,6 +20,7 @@
 #include <sync.h>
 #include <util/epochguard.h>
 #include <util/hasher.h>
+#include <util/overloaded.h>
 #include <util/result.h>
 #include <util/feefrac.h>
 
@@ -38,6 +39,7 @@
 #include <string>
 #include <string_view>
 #include <utility>
+#include <variant>
 #include <vector>
 
 class CChain;
@@ -56,7 +58,7 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) EXCLUSIVE
 // extracts a transaction hash from CTxMemPoolEntry or CTransactionRef
 struct mempoolentry_txid
 {
-    typedef uint256 result_type;
+    typedef Txid result_type;
     result_type operator() (const CTxMemPoolEntry &entry) const
     {
         return entry.GetTx().GetHash();
@@ -71,7 +73,7 @@ struct mempoolentry_txid
 // extracts a transaction witness-hash from CTxMemPoolEntry or CTransactionRef
 struct mempoolentry_wtxid
 {
-    typedef uint256 result_type;
+    typedef Wtxid result_type;
     result_type operator() (const CTxMemPoolEntry &entry) const
     {
         return entry.GetTx().GetWitnessHash();
@@ -410,7 +412,7 @@ private:
     /**
      * Track locally submitted transactions to periodically retry initial broadcast.
      */
-    std::set<uint256> m_unbroadcast_txids GUARDED_BY(cs);
+    std::set<Txid> m_unbroadcast_txids GUARDED_BY(cs);
 
 
     /**
@@ -432,7 +434,7 @@ private:
 
 public:
     indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs);
-    std::map<uint256, CAmount> mapDeltas GUARDED_BY(cs);
+    std::map<Txid, CAmount> mapDeltas GUARDED_BY(cs);
 
     using Options = kernel::MemPoolOptions;
 
@@ -466,7 +468,7 @@ public:
     void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
     void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false);
+    bool CompareDepthAndScore(const GenTxid& hasha, const GenTxid& hashb);
     bool isSpent(const COutPoint& outpoint) const;
     unsigned int GetTransactionsUpdated() const;
     void AddTransactionsUpdated(unsigned int n);
@@ -477,9 +479,9 @@ public:
     bool HasNoInputsOf(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Affect CreateNewBlock prioritisation of transactions */
-    void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta);
-    void ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const EXCLUSIVE_LOCKS_REQUIRED(cs);
-    void ClearPrioritisation(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    void PrioritiseTransaction(const Txid& hash, const CAmount& nFeeDelta);
+    void ApplyDelta(const Txid& hash, CAmount &nFeeDelta) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    void ClearPrioritisation(const Txid& hash) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     struct delta_info {
         /** Whether this transaction is in the mempool. */
@@ -489,7 +491,7 @@ public:
         /** The modified fee (base fee + delta) of this entry. Only present if in_mempool=true. */
         std::optional<CAmount> modified_fee;
         /** The prioritised transaction's txid. */
-        const uint256 txid;
+        const Txid txid;
     };
     /** Return a vector of all entries in mapDeltas with their corresponding delta_info. */
     std::vector<delta_info> GetPrioritisedTransactions() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -498,7 +500,7 @@ public:
     const CTransaction* GetConflictTx(const COutPoint& prevout) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Returns an iterator to the given hash, if found */
-    std::optional<txiter> GetIter(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::optional<txiter> GetIter(const Txid& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Translate a set of hashes into a set of pool iterators to avoid repeated lookups.
      * Does not require that all of the hashes correspond to actual transactions in the mempool,
@@ -508,7 +510,7 @@ public:
     /** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups.
      * The nth element in txids becomes the nth element in the returned vector. If any of the txids
      * don't actually exist in the mempool, returns an empty vector. */
-    std::vector<txiter> GetIterVec(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::vector<txiter> GetIterVec(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** UpdateTransactionsFromBlock is called when adding transactions from a
      * disconnected block back to the mempool, new mempool entries may have
@@ -523,7 +525,7 @@ public:
      * @param[in] vHashesToUpdate          The set of txids from the
      *     disconnected block that have been accepted back into the mempool.
      */
-    void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
+    void UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
 
     /**
      * Try to calculate all in-mempool ancestors of entry.
@@ -565,7 +567,7 @@ public:
      * All txids must correspond to transaction entries in the mempool, otherwise this returns an
      * empty vector. This call will also exit early and return an empty vector if it collects 500 or
      * more transactions as a DoS protection. */
-    std::vector<txiter> GatherClusters(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::vector<txiter> GatherClusters(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and
      * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and
@@ -612,7 +614,7 @@ public:
      * When ancestors is non-zero (ie, the transaction itself is in the mempool),
      * ancestorsize and ancestorfees will also be set to the appropriate values.
      */
-    void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const;
+    void GetTransactionAncestry(const Txid& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const;
 
     /**
      * @returns true if an initial attempt to load the persisted mempool was made, regardless of
@@ -647,16 +649,16 @@ public:
     bool exists(const GenTxid& gtxid) const
     {
         LOCK(cs);
-        if (gtxid.IsWtxid()) {
-            return (mapTx.get<index_by_wtxid>().count(gtxid.GetHash()) != 0);
-        }
-        return (mapTx.count(gtxid.GetHash()) != 0);
+        return std::visit(util::Overloaded{
+            [this](const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return (mapTx.get<index_by_wtxid>().count(wtxid) != 0); },
+            [this](const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs) { return (mapTx.count(txid) != 0); }
+        }, gtxid);
     }
 
     const CTxMemPoolEntry* GetEntry(const Txid& txid) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    CTransactionRef get(const uint256& hash) const;
-    txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
+    CTransactionRef get(const Txid& hash) const;
+    txiter get_iter_from_wtxid(const Wtxid& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
     {
         AssertLockHeld(cs);
         return mapTx.project<0>(mapTx.get<index_by_wtxid>().find(wtxid));
@@ -672,26 +674,26 @@ public:
     size_t DynamicMemoryUsage() const;
 
     /** Adds a transaction to the unbroadcast set */
-    void AddUnbroadcastTx(const uint256& txid)
+    void AddUnbroadcastTx(const Txid& txid)
     {
         LOCK(cs);
         // Sanity check the transaction is in the mempool & insert into
         // unbroadcast set.
-        if (exists(GenTxid::Txid(txid))) m_unbroadcast_txids.insert(txid);
+        if (exists(txid)) m_unbroadcast_txids.insert(txid);
     };
 
     /** Removes a transaction from the unbroadcast set */
-    void RemoveUnbroadcastTx(const uint256& txid, const bool unchecked = false);
+    void RemoveUnbroadcastTx(const Txid& txid, const bool unchecked = false);
 
     /** Returns transactions in unbroadcast set */
-    std::set<uint256> GetUnbroadcastTxs() const
+    std::set<Txid> GetUnbroadcastTxs() const
     {
         LOCK(cs);
         return m_unbroadcast_txids;
     }
 
     /** Returns whether a txid is in the unbroadcast set */
-    bool IsUnbroadcastTx(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
+    bool IsUnbroadcastTx(const Txid& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
     {
         AssertLockHeld(cs);
         return m_unbroadcast_txids.count(txid) != 0;
@@ -750,7 +752,7 @@ private:
      *     removeRecursive them.
      */
     void UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
-                              const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs);
+                              const std::set<Txid>& setExclude, std::set<Txid>& descendants_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs);
     /** Update ancestors of hash to add/remove it as a descendant transaction. */
     void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs);
     /** Set ancestor state for an entry */

src/txrequest.cpp

@@ -60,7 +60,7 @@ using SequenceNumber = uint64_t;
 /** An announcement. This is the data we track for each txid or wtxid that is announced to us by each peer. */
 struct Announcement {
     /** Txid or wtxid that was announced. */
-    const uint256 m_txhash;
+    const GenTxid m_txhash;
     /** For CANDIDATE_{DELAYED,BEST,READY} the reqtime; for REQUESTED the expiry. */
     std::chrono::microseconds m_time;
     /** What peer the request was from. */
@@ -70,7 +70,7 @@ struct Announcement {
     /** Whether the request is preferred. */
     const bool m_preferred : 1;
     /** Whether this is a wtxid request. */
-    const bool m_is_wtxid : 1;
+    //const bool m_is_wtxid : 1;
 
     /** What state this announcement is in. */
     State m_state : 3 {State::CANDIDATE_DELAYED};
@@ -98,8 +98,7 @@ struct Announcement {
     /** Construct a new announcement from scratch, initially in CANDIDATE_DELAYED state. */
     Announcement(const GenTxid& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime,
                  SequenceNumber sequence)
-        : m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred),
-          m_is_wtxid{gtxid.IsWtxid()} {}
+        : m_txhash(gtxid), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred) {}
 };
 
 //! Type alias for priorities.
@@ -116,9 +115,9 @@ public:
         m_k0{deterministic ? 0 : FastRandomContext().rand64()},
         m_k1{deterministic ? 0 : FastRandomContext().rand64()} {}
 
-    Priority operator()(const uint256& txhash, NodeId peer, bool preferred) const
+    Priority operator()(const GenTxid& txhash, NodeId peer, bool preferred) const
     {
-        uint64_t low_bits = CSipHasher(m_k0, m_k1).Write(txhash).Write(peer).Finalize() >> 1;
+        uint64_t low_bits = CSipHasher(m_k0, m_k1).Write(txhash.ToUint256()).Write(peer).Finalize() >> 1;
         return low_bits | uint64_t{preferred} << 63;
     }
 
@@ -142,7 +141,7 @@ public:
 //   (peer, true, txhash).
 // * Finding all CANDIDATE_BEST announcements for a given peer in GetRequestable.
 struct ByPeer {};
-using ByPeerView = std::tuple<NodeId, bool, const uint256&>;
+using ByPeerView = std::tuple<NodeId, bool, const GenTxid&>;
 struct ByPeerViewExtractor
 {
     using result_type = ByPeerView;
@@ -163,7 +162,7 @@ struct ByPeerViewExtractor
 // * Determining when no more non-COMPLETED announcements for a given txhash exist, so the COMPLETED ones can be
 //   deleted.
 struct ByTxHash {};
-using ByTxHashView = std::tuple<const uint256&, State, Priority>;
+using ByTxHashView = std::tuple<const GenTxid&, State, Priority>;
 class ByTxHashViewExtractor {
     const PriorityComputer& m_computer;
 public:
@@ -276,9 +275,9 @@ std::unordered_map<NodeId, PeerInfo> RecomputePeerInfo(const Index& index)
 }
 
 /** Compute the TxHashInfo map. Only used for sanity checking. */
-std::map<uint256, TxHashInfo> ComputeTxHashInfo(const Index& index, const PriorityComputer& computer)
+std::map<GenTxid, TxHashInfo> ComputeTxHashInfo(const Index& index, const PriorityComputer& computer)
 {
-    std::map<uint256, TxHashInfo> ret;
+    std::map<GenTxid, TxHashInfo> ret;
     for (const Announcement& ann : index) {
         TxHashInfo& info = ret[ann.m_txhash];
         // Classify how many announcements of each state we have for this txhash.
@@ -299,11 +298,6 @@ std::map<uint256, TxHashInfo> ComputeTxHashInfo(const Index& index, const Priori
     return ret;
 }
 
-GenTxid ToGenTxid(const Announcement& ann)
-{
-    return ann.m_is_wtxid ? GenTxid::Wtxid(ann.m_txhash) : GenTxid::Txid(ann.m_txhash);
-}
-
 }  // namespace
 
 /** Actual implementation for TxRequestTracker's data structure. */
@@ -472,7 +466,7 @@ private:
 
         if (IsOnlyNonCompleted(it)) {
             // This is the last non-COMPLETED announcement for this txhash. Delete all.
-            uint256 txhash = it->m_txhash;
+            GenTxid txhash = it->m_txhash;
             do {
                 it = Erase<ByTxHash>(it);
             } while (it != m_index.get<ByTxHash>().end() && it->m_txhash == txhash);
@@ -501,7 +495,7 @@ private:
             if (it->GetState() == State::CANDIDATE_DELAYED && it->m_time <= now) {
                 PromoteCandidateReady(m_index.project<ByTxHash>(it));
             } else if (it->GetState() == State::REQUESTED && it->m_time <= now) {
-                if (expired) expired->emplace_back(it->m_peer, ToGenTxid(*it));
+                if (expired) expired->emplace_back(it->m_peer, it->m_txhash);
                 MakeCompleted(m_index.project<ByTxHash>(it));
             } else {
                 break;
@@ -538,7 +532,8 @@ public:
     void DisconnectedPeer(NodeId peer)
     {
         auto& index = m_index.get<ByPeer>();
-        auto it = index.lower_bound(ByPeerView{peer, false, uint256::ZERO});
+        GenTxid lowerzero = Txid::FromUint256(uint256::ZERO);
+        auto it = index.lower_bound(ByPeerView{peer, false, lowerzero});
         while (it != index.end() && it->m_peer == peer) {
             // Check what to continue with after this iteration. 'it' will be deleted in what follows, so we need to
             // decide what to continue with afterwards. There are a number of cases to consider:
@@ -566,7 +561,7 @@ public:
         }
     }
 
-    void ForgetTxHash(const uint256& txhash)
+    void ForgetTxHash(const GenTxid& txhash)
     {
         auto it = m_index.get<ByTxHash>().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0});
         while (it != m_index.get<ByTxHash>().end() && it->m_txhash == txhash) {
@@ -574,7 +569,7 @@ public:
         }
     }
 
-    void GetCandidatePeers(const uint256& txhash, std::vector<NodeId>& result_peers) const
+    void GetCandidatePeers(const GenTxid& txhash, std::vector<NodeId>& result_peers) const
     {
         auto it = m_index.get<ByTxHash>().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0});
         while (it != m_index.get<ByTxHash>().end() && it->m_txhash == txhash && it->GetState() != State::COMPLETED) {
@@ -589,7 +584,7 @@ public:
         // Bail out if we already have a CANDIDATE_BEST announcement for this (txhash, peer) combination. The case
         // where there is a non-CANDIDATE_BEST announcement already will be caught by the uniqueness property of the
         // ByPeer index when we try to emplace the new object below.
-        if (m_index.get<ByPeer>().count(ByPeerView{peer, true, gtxid.GetHash()})) return;
+        if (m_index.get<ByPeer>().count(ByPeerView{peer, true, gtxid})) return;
 
         // Try creating the announcement with CANDIDATE_DELAYED state (which will fail due to the uniqueness
         // of the ByPeer index if a non-CANDIDATE_BEST announcement already exists with the same txhash and peer).
@@ -611,7 +606,8 @@ public:
 
         // Find all CANDIDATE_BEST announcements for this peer.
         std::vector<const Announcement*> selected;
-        auto it_peer = m_index.get<ByPeer>().lower_bound(ByPeerView{peer, true, uint256::ZERO});
+        GenTxid lowerzero = Txid::FromUint256(uint256::ZERO);
+        auto it_peer = m_index.get<ByPeer>().lower_bound(ByPeerView{peer, true, lowerzero});
         while (it_peer != m_index.get<ByPeer>().end() && it_peer->m_peer == peer &&
             it_peer->GetState() == State::CANDIDATE_BEST) {
             selected.emplace_back(&*it_peer);
@@ -627,12 +623,12 @@ public:
         std::vector<GenTxid> ret;
         ret.reserve(selected.size());
         std::transform(selected.begin(), selected.end(), std::back_inserter(ret), [](const Announcement* ann) {
-            return ToGenTxid(*ann);
+            return ann->m_txhash;
         });
         return ret;
     }
 
-    void RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds expiry)
+    void RequestedTx(NodeId peer, const GenTxid& txhash, std::chrono::microseconds expiry)
     {
         auto it = m_index.get<ByPeer>().find(ByPeerView{peer, true, txhash});
         if (it == m_index.get<ByPeer>().end()) {
@@ -677,7 +673,7 @@ public:
         });
     }
 
-    void ReceivedResponse(NodeId peer, const uint256& txhash)
+    void ReceivedResponse(NodeId peer, const GenTxid& txhash)
     {
         // We need to search the ByPeer index for both (peer, false, txhash) and (peer, true, txhash).
         auto it = m_index.get<ByPeer>().find(ByPeerView{peer, false, txhash});
@@ -711,7 +707,7 @@ public:
     //! Count how many announcements are being tracked in total across all peers and transactions.
     size_t Size() const { return m_index.size(); }
 
-    uint64_t ComputePriority(const uint256& txhash, NodeId peer, bool preferred) const
+    uint64_t ComputePriority(const GenTxid& txhash, NodeId peer, bool preferred) const
     {
         // Return Priority as a uint64_t as Priority is internal.
         return uint64_t{m_computer(txhash, peer, preferred)};
@@ -724,13 +720,13 @@ TxRequestTracker::TxRequestTracker(bool deterministic) :
 
 TxRequestTracker::~TxRequestTracker() = default;
 
-void TxRequestTracker::ForgetTxHash(const uint256& txhash) { m_impl->ForgetTxHash(txhash); }
+void TxRequestTracker::ForgetTxHash(const GenTxid& txhash) { m_impl->ForgetTxHash(txhash); }
 void TxRequestTracker::DisconnectedPeer(NodeId peer) { m_impl->DisconnectedPeer(peer); }
 size_t TxRequestTracker::CountInFlight(NodeId peer) const { return m_impl->CountInFlight(peer); }
 size_t TxRequestTracker::CountCandidates(NodeId peer) const { return m_impl->CountCandidates(peer); }
 size_t TxRequestTracker::Count(NodeId peer) const { return m_impl->Count(peer); }
 size_t TxRequestTracker::Size() const { return m_impl->Size(); }
-void TxRequestTracker::GetCandidatePeers(const uint256& txhash, std::vector<NodeId>& result_peers) const { return m_impl->GetCandidatePeers(txhash, result_peers); }
+void TxRequestTracker::GetCandidatePeers(const GenTxid& txhash, std::vector<NodeId>& result_peers) const { return m_impl->GetCandidatePeers(txhash, result_peers); }
 void TxRequestTracker::SanityCheck() const { m_impl->SanityCheck(); }
 
 void TxRequestTracker::PostGetRequestableSanityCheck(std::chrono::microseconds now) const
@@ -744,12 +740,12 @@ void TxRequestTracker::ReceivedInv(NodeId peer, const GenTxid& gtxid, bool prefe
     m_impl->ReceivedInv(peer, gtxid, preferred, reqtime);
 }
 
-void TxRequestTracker::RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds expiry)
+void TxRequestTracker::RequestedTx(NodeId peer, const GenTxid& txhash, std::chrono::microseconds expiry)
 {
     m_impl->RequestedTx(peer, txhash, expiry);
 }
 
-void TxRequestTracker::ReceivedResponse(NodeId peer, const uint256& txhash)
+void TxRequestTracker::ReceivedResponse(NodeId peer, const GenTxid& txhash)
 {
     m_impl->ReceivedResponse(peer, txhash);
 }
@@ -760,7 +756,7 @@ std::vector<GenTxid> TxRequestTracker::GetRequestable(NodeId peer, std::chrono::
     return m_impl->GetRequestable(peer, now, expired);
 }
 
-uint64_t TxRequestTracker::ComputePriority(const uint256& txhash, NodeId peer, bool preferred) const
+uint64_t TxRequestTracker::ComputePriority(const GenTxid& txhash, NodeId peer, bool preferred) const
 {
     return m_impl->ComputePriority(txhash, peer, preferred);
 }

src/txrequest.h

@@ -146,7 +146,7 @@ public:
      * This should be called when a transaction is no longer needed. The caller should ensure that new announcements
      * for the same txhash will not trigger new ReceivedInv calls, at least in the short term after this call.
      */
-    void ForgetTxHash(const uint256& txhash);
+    void ForgetTxHash(const GenTxid& txhash);
 
     /** Find the txids to request now from peer.
      *
@@ -175,7 +175,7 @@ public:
      *    was made (GetRequestable will never advise doing so). In this case it is converted to COMPLETED, as we're
      *    no longer waiting for a response to it.
      */
-    void RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds expiry);
+    void RequestedTx(NodeId peer, const GenTxid& txhash, std::chrono::microseconds expiry);
 
     /** Converts a CANDIDATE or REQUESTED announcement to a COMPLETED one. If no such announcement exists for the
      *  provided peer and txhash, nothing happens.
@@ -183,7 +183,7 @@ public:
      * It should be called whenever a transaction or NOTFOUND was received from a peer. When the transaction is
      * not needed entirely anymore, ForgetTxhash should be called instead of, or in addition to, this call.
      */
-    void ReceivedResponse(NodeId peer, const uint256& txhash);
+    void ReceivedResponse(NodeId peer, const GenTxid& txhash);
 
     // The operations below inspect the data structure.
 
@@ -201,10 +201,10 @@ public:
 
     /** For some txhash (txid or wtxid), finds all peers with non-COMPLETED announcements and appends them to
      * result_peers. Does not try to ensure that result_peers contains no duplicates. */
-    void GetCandidatePeers(const uint256& txhash, std::vector<NodeId>& result_peers) const;
+    void GetCandidatePeers(const GenTxid& txhash, std::vector<NodeId>& result_peers) const;
 
     /** Access to the internal priority computation (testing only) */
-    uint64_t ComputePriority(const uint256& txhash, NodeId peer, bool preferred) const;
+    uint64_t ComputePriority(const GenTxid& txhash, NodeId peer, bool preferred) const;
 
     /** Run internal consistency check (testing only). */
     void SanityCheck() const;

src/util/hasher.cpp

@@ -7,6 +7,10 @@
 #include <span.h>
 #include <util/hasher.h>
 
+SaltedUint256Hasher::SaltedUint256Hasher() :
+    k0{FastRandomContext().rand64()},
+    k1{FastRandomContext().rand64()} {}
+
 SaltedTxidHasher::SaltedTxidHasher() :
     k0{FastRandomContext().rand64()},
     k1{FastRandomContext().rand64()} {}

src/util/hasher.h

@@ -11,9 +11,24 @@
 #include <span.h>
 #include <uint256.h>
 
+#include <concepts>
 #include <cstdint>
 #include <cstring>
 
+class SaltedUint256Hasher
+{
+private:
+    /** Salt */
+    const uint64_t k0, k1;
+
+public:
+    SaltedUint256Hasher();
+
+    size_t operator()(const uint256& hash) const {
+        return SipHashUint256(k0, k1, hash);
+    }
+};
+
 class SaltedTxidHasher
 {
 private:
@@ -23,8 +38,10 @@ private:
 public:
     SaltedTxidHasher();
 
-    size_t operator()(const uint256& txid) const {
-        return SipHashUint256(k0, k1, txid);
+    template <typename T>
+        requires std::same_as<T, Txid> || std::same_as<T, Wtxid>
+    size_t operator()(const T& txid) const {
+        return SipHashUint256(k0, k1, txid.ToUint256());
     }
 };
 
@@ -47,7 +64,7 @@ public:
      * @see https://gcc.gnu.org/onlinedocs/gcc-13.2.0/libstdc++/manual/manual/unordered_associative.html
      */
     size_t operator()(const COutPoint& id) const noexcept {
-        return SipHashUint256Extra(k0, k1, id.hash, id.n);
+        return SipHashUint256Extra(k0, k1, id.hash.ToUint256(), id.n);
     }
 };
 

src/util/transaction_identifier.h

@@ -5,6 +5,8 @@
 #include <uint256.h>
 #include <util/types.h>
 
+#include <variant>
+
 /** transaction_identifier represents the two canonical transaction identifier
  * types (txid, wtxid).*/
 template <bool has_witness>
@@ -15,9 +17,6 @@ class transaction_identifier
     // Note: Use FromUint256 externally instead.
     transaction_identifier(const uint256& wrapped) : m_wrapped{wrapped} {}
 
-    // TODO: Comparisons with uint256 should be disallowed once we have
-    // converted most of the code to using the new txid types.
-    constexpr int Compare(const uint256& other) const { return m_wrapped.Compare(other); }
     constexpr int Compare(const transaction_identifier<has_witness>& other) const { return m_wrapped.Compare(other.m_wrapped); }
     template <typename Other>
     constexpr int Compare(const Other& other) const
@@ -56,15 +55,6 @@ public:
     constexpr const std::byte* end() const { return reinterpret_cast<const std::byte*>(m_wrapped.end()); }
     template <typename Stream> void Serialize(Stream& s) const { m_wrapped.Serialize(s); }
     template <typename Stream> void Unserialize(Stream& s) { m_wrapped.Unserialize(s); }
-
-    /** Conversion function to `uint256`.
-     *
-     * Note: new code should use `ToUint256`.
-     *
-     * TODO: This should be removed once the majority of the code has switched
-     * to using the Txid and Wtxid types. Until then it makes for a smoother
-     * transition to allow this conversion. */
-    operator const uint256&() const LIFETIMEBOUND { return m_wrapped; }
 };
 
 /** Txid commits to all transaction fields except the witness. */
@@ -72,4 +62,25 @@ using Txid = transaction_identifier<false>;
 /** Wtxid commits to all transaction fields including the witness. */
 using Wtxid = transaction_identifier<true>;
 
+/** A generic txid reference (txid or wtxid). */
+class GenTxid : public std::variant<Txid, Wtxid>
+{
+public:
+    using variant::variant;
+
+    const uint256& ToUint256() const LIFETIMEBOUND {
+        return std::visit([](const auto& id) -> const uint256& { return id.ToUint256(); }, *this);
+    }
+
+    friend bool operator==(const GenTxid& a, const GenTxid& b) {
+        //return a.index() == b.index() && a.ToUint256() == b.ToUint256();
+        return a.ToUint256() == b.ToUint256();
+    }
+
+    friend bool operator<(const GenTxid& a, const GenTxid& b) {
+        //return std::tuple(a.index(), a.ToUint256()) < std::tuple(b.index(), b.ToUint256());
+        return a.ToUint256() < b.ToUint256();
+    }
+};
+
 #endif // BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H

src/validation.cpp

@@ -301,7 +301,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
 
     AssertLockHeld(cs_main);
     AssertLockHeld(m_mempool->cs);
-    std::vector<uint256> vHashUpdate;
+    std::vector<Txid> vHashUpdate;
     {
         // disconnectpool is ordered so that the front is the most recently-confirmed
         // transaction (the last tx of the block at the tip) in the disconnected chain.
@@ -319,7 +319,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
                 // If the transaction doesn't make it in to the mempool, remove any
                 // transactions that depend on it (which would now be orphans).
                 m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG);
-            } else if (m_mempool->exists(GenTxid::Txid((*it)->GetHash()))) {
+            } else if (m_mempool->exists((*it)->GetHash())) {
                 vHashUpdate.push_back((*it)->GetHash());
             }
             ++it;
@@ -370,7 +370,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
         // If the transaction spends any coinbase outputs, it must be mature.
         if (it->GetSpendsCoinbase()) {
             for (const CTxIn& txin : tx.vin) {
-                if (m_mempool->exists(GenTxid::Txid(txin.prevout.hash))) continue;
+                if (m_mempool->exists(txin.prevout.hash)) continue;
                 const Coin& coin{CoinsTip().AccessCoin(txin.prevout)};
                 assert(!coin.IsSpent());
                 const auto mempool_spend_height{m_chain.Tip()->nHeight + 1};
@@ -805,10 +805,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final");
     }
 
-    if (m_pool.exists(GenTxid::Wtxid(tx.GetWitnessHash()))) {
+    if (m_pool.exists(tx.GetWitnessHash())) {
         // Exact transaction already exists in the mempool.
         return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-in-mempool");
-    } else if (m_pool.exists(GenTxid::Txid(tx.GetHash()))) {
+    } else if (m_pool.exists(tx.GetHash())) {
         // Transaction with the same non-witness data but different witness (same txid, different
         // wtxid) already exists in the mempool.
         return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-same-nonwitness-data-in-mempool");
@@ -1069,7 +1069,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
     AssertLockHeld(m_pool.cs);
 
     const CTransaction& tx = *ws.m_ptx;
-    const uint256& hash = ws.m_hash;
+    const Txid& hash = ws.m_hash;
     TxValidationState& state = ws.m_state;
 
     CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize);
@@ -1134,7 +1134,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
 
     // CheckPackageLimits expects the package transactions to not already be in the mempool.
     assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx)
-                       { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));}));
+                       { return !m_pool.exists(tx->GetHash());}));
 
     assert(txns.size() == workspaces.size());
 
@@ -1257,7 +1257,7 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws)
     AssertLockHeld(cs_main);
     AssertLockHeld(m_pool.cs);
     const CTransaction& tx = *ws.m_ptx;
-    const uint256& hash = ws.m_hash;
+    const Txid& hash = ws.m_hash;
     TxValidationState& state = ws.m_state;
 
     // Check again against the current block tip's script verification
@@ -1304,7 +1304,7 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args)
         const bool replaced_with_tx{m_subpackage.m_changeset->GetTxCount() == 1};
         if (replaced_with_tx) {
             const CTransaction& tx = m_subpackage.m_changeset->GetAddedTxn(0);
-            tx_or_package_hash = tx.GetHash();
+            tx_or_package_hash = tx.GetHash().ToUint256();
             log_string += strprintf("New tx %s (wtxid=%s, fees=%s, vsize=%s)",
                                     tx.GetHash().ToString(),
                                     tx.GetWitnessHash().ToString(),
@@ -1345,7 +1345,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
     // Sanity check: none of the transactions should be in the mempool, and none of the transactions
     // should have a same-txid-different-witness equivalent in the mempool.
     assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws){
-        return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); }));
+        return !m_pool.exists(ws.m_ptx->GetHash()); }));
 
     bool all_submitted = true;
     FinalizeSubpackage(args);
@@ -1470,7 +1470,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
     // Limit the mempool, if appropriate.
     if (!args.m_package_submission && !args.m_bypass_limits) {
         LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
-        if (!m_pool.exists(GenTxid::Txid(ws.m_hash))) {
+        if (!m_pool.exists(ws.m_hash)) {
             // The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package.
             ws.m_state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full");
             return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), {ws.m_ptx->GetWitnessHash()});
@@ -1716,7 +1716,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
         // The package must be 1 child with all of its unconfirmed parents. The package is expected to
         // be sorted, so the last transaction is the child.
         const auto& child = package.back();
-        std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
+        std::unordered_set<Txid, SaltedTxidHasher> unconfirmed_parent_txids;
         std::transform(package.cbegin(), package.cend() - 1,
                        std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
                        [](const auto& tx) { return tx->GetHash(); });
@@ -1765,7 +1765,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
         // There are 3 possibilities: already in mempool, same-txid-diff-wtxid already in mempool,
         // or not in mempool. An already confirmed tx is treated as one not in mempool, because all
         // we know is that the inputs aren't available.
-        if (m_pool.exists(GenTxid::Wtxid(wtxid))) {
+        if (m_pool.exists(wtxid)) {
             // Exact transaction already exists in the mempool.
             // Node operators are free to set their mempool policies however they please, nodes may receive
             // transactions in different orders, and malicious counterparties may try to take advantage of
@@ -1777,7 +1777,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy.
             const auto& entry{*Assert(m_pool.GetEntry(txid))};
             results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(entry.GetTxSize(), entry.GetFee()));
-        } else if (m_pool.exists(GenTxid::Txid(txid))) {
+        } else if (m_pool.exists(txid)) {
             // Transaction with the same non-witness data but different witness (same txid,
             // different wtxid) already exists in the mempool.
             //
@@ -1796,7 +1796,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             if (single_res.m_result_type == MempoolAcceptResult::ResultType::VALID) {
                 // The transaction succeeded on its own and is now in the mempool. Don't include it
                 // in package validation, because its fees should only be "used" once.
-                assert(m_pool.exists(GenTxid::Wtxid(wtxid)));
+                assert(m_pool.exists(wtxid));
                 results_final.emplace(wtxid, single_res);
             } else if (package.size() == 1 || // If there is only one transaction, no need to retry it "as a package"
                        (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE &&
@@ -1841,7 +1841,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             // If it was submitted, check to see if the tx is still in the mempool. It could have
             // been evicted due to LimitMempoolSize() above.
             const auto& txresult = multi_submission_result.m_tx_results.at(wtxid);
-            if (txresult.m_result_type == MempoolAcceptResult::ResultType::VALID && !m_pool.exists(GenTxid::Wtxid(wtxid))) {
+            if (txresult.m_result_type == MempoolAcceptResult::ResultType::VALID && !m_pool.exists(wtxid)) {
                 package_state_final.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
                 TxValidationState mempool_full_state;
                 mempool_full_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full");
@@ -1855,7 +1855,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             Assume(it->second.m_result_type != MempoolAcceptResult::ResultType::INVALID);
             Assume(individual_results_nonfinal.count(wtxid) == 0);
             // Query by txid to include the same-txid-different-witness ones.
-            if (!m_pool.exists(GenTxid::Txid(tx->GetHash()))) {
+            if (!m_pool.exists(tx->GetHash())) {
                 package_state_final.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
                 TxValidationState mempool_full_state;
                 mempool_full_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full");

src/wallet/feebumper.cpp

@@ -146,7 +146,7 @@ static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, con
 
 namespace feebumper {
 
-bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
+bool TransactionCanBeBumped(const CWallet& wallet, const Txid& txid)
 {
     LOCK(wallet.cs_wallet);
     const CWalletTx* wtx = wallet.GetWalletTx(txid);
@@ -157,7 +157,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
     return res == feebumper::Result::OK;
 }
 
-Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
+Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
                                  CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> original_change_index)
 {
     // For now, cannot specify both new outputs to use and an output index to send change
@@ -348,7 +348,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
     }
 }
 
-Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, uint256& bumped_txid)
+Result CommitTransaction(CWallet& wallet, const Txid& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid)
 {
     LOCK(wallet.cs_wallet);
     if (!errors.empty()) {

src/wallet/feebumper.h

@@ -31,7 +31,7 @@ enum class Result
 };
 
 //! Return whether transaction can be bumped.
-bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
+bool TransactionCanBeBumped(const CWallet& wallet, const Txid& txid);
 
 /** Create bumpfee transaction based on feerate estimates.
  *
@@ -47,7 +47,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
  * @param[in] original_change_index The position of the change output to deduct the fee from in the transaction being bumped
  */
 Result CreateRateBumpTransaction(CWallet& wallet,
-    const uint256& txid,
+    const Txid& txid,
     const CCoinControl& coin_control,
     std::vector<bilingual_str>& errors,
     CAmount& old_fee,
@@ -67,10 +67,10 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx);
 //! but sets errors if the tx could not be added to the mempool (will try later)
 //! or if the old transaction could not be marked as replaced.
 Result CommitTransaction(CWallet& wallet,
-    const uint256& txid,
+    const Txid& txid,
     CMutableTransaction&& mtx,
     std::vector<bilingual_str>& errors,
-    uint256& bumped_txid);
+    Txid& bumped_txid);
 
 struct SignatureWeights
 {

src/wallet/interfaces.cpp

@@ -299,17 +299,17 @@ public:
         LOCK(m_wallet->cs_wallet);
         m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));
     }
-    bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
-    bool abandonTransaction(const uint256& txid) override
+    bool transactionCanBeAbandoned(const Txid& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
+    bool abandonTransaction(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         return m_wallet->AbandonTransaction(txid);
     }
-    bool transactionCanBeBumped(const uint256& txid) override
+    bool transactionCanBeBumped(const Txid& txid) override
     {
         return feebumper::TransactionCanBeBumped(*m_wallet.get(), txid);
     }
-    bool createBumpTransaction(const uint256& txid,
+    bool createBumpTransaction(const Txid& txid,
         const CCoinControl& coin_control,
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
@@ -320,15 +320,15 @@ public:
         return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs) == feebumper::Result::OK;
     }
     bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
-    bool commitBumpTransaction(const uint256& txid,
+    bool commitBumpTransaction(const Txid& txid,
         CMutableTransaction&& mtx,
         std::vector<bilingual_str>& errors,
-        uint256& bumped_txid) override
+        Txid& bumped_txid) override
     {
         return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==
                feebumper::Result::OK;
     }
-    CTransactionRef getTx(const uint256& txid) override
+    CTransactionRef getTx(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         auto mi = m_wallet->mapWallet.find(txid);
@@ -337,7 +337,7 @@ public:
         }
         return {};
     }
-    WalletTx getWalletTx(const uint256& txid) override
+    WalletTx getWalletTx(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         auto mi = m_wallet->mapWallet.find(txid);
@@ -355,7 +355,7 @@ public:
         }
         return result;
     }
-    bool tryGetTxStatus(const uint256& txid,
+    bool tryGetTxStatus(const Txid& txid,
         interfaces::WalletTxStatus& tx_status,
         int& num_blocks,
         int64_t& block_time) override
@@ -374,7 +374,7 @@ public:
         tx_status = MakeWalletTxStatus(*m_wallet, mi->second);
         return true;
     }
-    WalletTx getWalletTxDetails(const uint256& txid,
+    WalletTx getWalletTxDetails(const Txid& txid,
         WalletTxStatus& tx_status,
         WalletOrderForm& order_form,
         bool& in_mempool,
@@ -548,7 +548,7 @@ public:
     std::unique_ptr<Handler> handleTransactionChanged(TransactionChangedFn fn) override
     {
         return MakeSignalHandler(m_wallet->NotifyTransactionChanged.connect(
-            [fn](const uint256& txid, ChangeType status) { fn(txid, status); }));
+            [fn](const Txid& txid, ChangeType status) { fn(txid, status); }));
     }
     std::unique_ptr<Handler> handleWatchOnlyChanged(WatchOnlyChangedFn fn) override
     {

src/wallet/receive.cpp

@@ -254,7 +254,7 @@ bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminef
 }
 
 // NOLINTNEXTLINE(misc-no-recursion)
-bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents)
+bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<Txid>& trusted_parents)
 {
     AssertLockHeld(wallet.cs_wallet);
     if (wtx.isConfirmed()) return true;
@@ -285,7 +285,7 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uin
 
 bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx)
 {
-    std::set<uint256> trusted_parents;
+    std::set<Txid> trusted_parents;
     LOCK(wallet.cs_wallet);
     return CachedTxIsTrusted(wallet, wtx, trusted_parents);
 }
@@ -296,7 +296,7 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
     isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED;
     {
         LOCK(wallet.cs_wallet);
-        std::set<uint256> trusted_parents;
+        std::set<Txid> trusted_parents;
         for (const auto& entry : wallet.mapWallet)
         {
             const CWalletTx& wtx = entry.second;
@@ -325,7 +325,7 @@ std::map<CTxDestination, CAmount> GetAddressBalances(const CWallet& wallet)
 
     {
         LOCK(wallet.cs_wallet);
-        std::set<uint256> trusted_parents;
+        std::set<Txid> trusted_parents;
         for (const auto& walletEntry : wallet.mapWallet)
         {
             const CWalletTx& wtx = walletEntry.second;
@@ -348,7 +348,7 @@ std::map<CTxDestination, CAmount> GetAddressBalances(const CWallet& wallet)
                 if(!ExtractDestination(output.scriptPubKey, addr))
                     continue;
 
-                CAmount n = wallet.IsSpent(COutPoint(Txid::FromUint256(walletEntry.first), i)) ? 0 : output.nValue;
+                CAmount n = wallet.IsSpent(COutPoint(walletEntry.first, i)) ? 0 : output.nValue;
                 balances[addr] += n;
             }
         }

src/wallet/receive.h

@@ -45,7 +45,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
                         CAmount& nFee, const isminefilter& filter,
                         bool include_change);
 bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
-bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<Txid>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx);
 
 struct Balance {

src/wallet/rpc/backup.cpp

@@ -337,7 +337,7 @@ RPCHelpMan importprunedfunds()
     if (!DecodeHexTx(tx, request.params[0].get_str())) {
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
     }
-    uint256 hashTx = tx.GetHash();
+    Txid hashTx = tx.GetHash();
 
     DataStream ssMB{ParseHexV(request.params[1], "proof")};
     CMerkleBlock merkleBlock;
@@ -357,7 +357,7 @@ RPCHelpMan importprunedfunds()
     }
 
     std::vector<uint256>::const_iterator it;
-    if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx)) == vMatch.end()) {
+    if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx.ToUint256())) == vMatch.end()) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction given doesn't exist in proof");
     }
 
@@ -394,8 +394,8 @@ RPCHelpMan removeprunedfunds()
 
     LOCK(pwallet->cs_wallet);
 
-    uint256 hash(ParseHashV(request.params[0], "txid"));
-    std::vector<uint256> vHash;
+    Txid hash = Txid::FromUint256((ParseHashV(request.params[0], "txid")));
+    std::vector<Txid> vHash;
     vHash.push_back(hash);
     if (auto res = pwallet->RemoveTxs(vHash); !res) {
         throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(res).original);

src/wallet/rpc/coins.cpp

@@ -56,7 +56,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
 
     // Tally
     CAmount amount = 0;
-    for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) {
+    for (const std::pair<const Txid, CWalletTx>& wtx_pair : wallet.mapWallet) {
         const CWalletTx& wtx = wtx_pair.second;
         int depth{wallet.GetTxDepthInMainChain(wtx)};
         if (depth < min_depth

src/wallet/rpc/spend.cpp

@@ -1068,7 +1068,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
         throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead.");
     }
 
-    uint256 hash(ParseHashV(request.params[0], "txid"));
+    Txid hash = Txid::FromUint256((ParseHashV(request.params[0], "txid")));
 
     CCoinControl coin_control;
     coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
@@ -1166,7 +1166,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
             throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");
         }
 
-        uint256 txid;
+        Txid txid;
         if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
             throw JSONRPCError(RPC_WALLET_ERROR, errors[0].original);
         }

src/wallet/rpc/transactions.cpp

@@ -7,6 +7,7 @@
 #include <policy/rbf.h>
 #include <rpc/util.h>
 #include <rpc/blockchain.h>
+#include <util/transaction_identifier.h>
 #include <util/vector.h>
 #include <wallet/receive.h>
 #include <wallet/rpc/util.h>
@@ -34,11 +35,11 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue
     } else {
         entry.pushKV("trusted", CachedTxIsTrusted(wallet, wtx));
     }
-    uint256 hash = wtx.GetHash();
+    Txid hash = wtx.GetHash();
     entry.pushKV("txid", hash.GetHex());
     entry.pushKV("wtxid", wtx.GetWitnessHash().GetHex());
     UniValue conflicts(UniValue::VARR);
-    for (const uint256& conflict : wallet.GetTxConflicts(wtx))
+    for (const Txid& conflict : wallet.GetTxConflicts(wtx))
         conflicts.push_back(conflict.GetHex());
     entry.pushKV("walletconflicts", std::move(conflicts));
     UniValue mempool_conflicts(UniValue::VARR);
@@ -67,7 +68,7 @@ struct tallyitem
 {
     CAmount nAmount{0};
     int nConf{std::numeric_limits<int>::max()};
-    std::vector<uint256> txids;
+    std::vector<Txid> txids;
     bool fIsWatchonly{false};
     tallyitem() = default;
 };
@@ -100,7 +101,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
 
     // Tally
     std::map<CTxDestination, tallyitem> mapTally;
-    for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
+    for (const std::pair<const Txid, CWalletTx>& pairWtx : wallet.mapWallet) {
         const CWalletTx& wtx = pairWtx.second;
 
         int nDepth = wallet.GetTxDepthInMainChain(wtx);
@@ -169,7 +170,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
             obj.pushKV("label", label);
             UniValue transactions(UniValue::VARR);
             if (it != mapTally.end()) {
-                for (const uint256& _item : (*it).second.txids) {
+                for (const Txid& _item : (*it).second.txids) {
                     transactions.push_back(_item.GetHex());
                 }
             }
@@ -650,7 +651,7 @@ RPCHelpMan listsinceblock()
 
     UniValue transactions(UniValue::VARR);
 
-    for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
+    for (const std::pair<const Txid, CWalletTx>& pairWtx : wallet.mapWallet) {
         const CWalletTx& tx = pairWtx.second;
 
         if (depth == -1 || abs(wallet.GetTxDepthInMainChain(tx)) < depth) {
@@ -760,7 +761,7 @@ RPCHelpMan gettransaction()
 
     LOCK(pwallet->cs_wallet);
 
-    uint256 hash(ParseHashV(request.params[0], "txid"));
+    Txid hash = Txid::FromUint256((ParseHashV(request.params[0], "txid")));
 
     isminefilter filter = ISMINE_SPENDABLE;
 
@@ -833,7 +834,7 @@ RPCHelpMan abandontransaction()
 
     LOCK(pwallet->cs_wallet);
 
-    uint256 hash(ParseHashV(request.params[0], "txid"));
+    Txid hash = Txid::FromUint256((ParseHashV(request.params[0], "txid")));
 
     if (!pwallet->mapWallet.count(hash)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");

src/wallet/spend.cpp

@@ -328,10 +328,10 @@ CoinsResult AvailableCoins(const CWallet& wallet,
     const bool can_grind_r = wallet.CanGrindR();
     std::vector<COutPoint> outpoints;
 
-    std::set<uint256> trusted_parents;
+    std::set<Txid> trusted_parents;
     for (const auto& entry : wallet.mapWallet)
     {
-        const uint256& txid = entry.first;
+        const Txid& txid = entry.first;
         const CWalletTx& wtx = entry.second;
 
         if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase)
@@ -391,7 +391,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
 
         for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {
             const CTxOut& output = wtx.tx->vout[i];
-            const COutPoint outpoint(Txid::FromUint256(txid), i);
+            const COutPoint outpoint(txid, i);
 
             if (output.nValue < params.min_amount || output.nValue > params.max_amount)
                 continue;

src/wallet/test/coinselector_tests.cpp

@@ -80,7 +80,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun
     if (spendable) {
         tx.vout[nInput].scriptPubKey = GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, "")));
     }
-    uint256 txid = tx.GetHash();
+    Txid txid = tx.GetHash();
 
     LOCK(wallet.cs_wallet);
     auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{}));

src/wallet/test/group_outputs_tests.cpp

@@ -40,7 +40,7 @@ static void addCoin(CoinsResult& coins,
     tx.vout[0].nValue = nValue;
     tx.vout[0].scriptPubKey = GetScriptForDestination(dest);
 
-    const auto txid{tx.GetHash().ToUint256()};
+    const auto txid{tx.GetHash()};
     LOCK(wallet.cs_wallet);
     auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{}));
     assert(ret.second);

src/wallet/test/wallet_tests.cpp

@@ -726,7 +726,7 @@ BOOST_FIXTURE_TEST_CASE(RemoveTxs, TestChain100Setup)
         BOOST_CHECK(wallet->HasWalletSpend(prev_tx));
         BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 1u);
 
-        std::vector<uint256> vHashIn{ block_hash };
+        std::vector<Txid> vHashIn{ block_hash };
         BOOST_CHECK(wallet->RemoveTxs(vHashIn));
 
         BOOST_CHECK(!wallet->HasWalletSpend(prev_tx));
@@ -776,7 +776,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
     {
         // Verify balance update for the new tx and the old one
         LOCK(wallet.cs_wallet);
-        const CWalletTx* new_wtx = wallet.GetWalletTx(good_tx_id.ToUint256());
+        const CWalletTx* new_wtx = wallet.GetWalletTx(good_tx_id);
         BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *new_wtx), 1 * COIN);
 
         // Now the old wtx

src/wallet/wallet.cpp

@@ -539,7 +539,7 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b
  * @{
  */
 
-const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const
+const CWalletTx* CWallet::GetWalletTx(const Txid& hash) const
 {
     AssertLockHeld(cs_wallet);
     const auto it = mapWallet.find(hash);
@@ -676,9 +676,9 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in)
     }
 }
 
-std::set<uint256> CWallet::GetConflicts(const uint256& txid) const
+std::set<Txid> CWallet::GetConflicts(const Txid& txid) const
 {
-    std::set<uint256> result;
+    std::set<Txid> result;
     AssertLockHeld(cs_wallet);
 
     const auto it = mapWallet.find(txid);
@@ -744,7 +744,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
     // Now copy data from copyFrom to rest:
     for (TxSpends::iterator it = range.first; it != range.second; ++it)
     {
-        const uint256& hash = it->second;
+        const Txid& hash = it->second;
         CWalletTx* copyTo = &mapWallet.at(hash);
         if (copyFrom == copyTo) continue;
         assert(copyFrom && "Oldest wallet transaction in range assumed to have been found.");
@@ -770,7 +770,7 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const
     range = mapTxSpends.equal_range(outpoint);
 
     for (TxSpends::const_iterator it = range.first; it != range.second; ++it) {
-        const uint256& wtxid = it->second;
+        const Txid& wtxid = it->second;
         const auto mit = mapWallet.find(wtxid);
         if (mit != mapWallet.end()) {
             const auto& wtx = mit->second;
@@ -781,7 +781,7 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const
     return false;
 }
 
-void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch)
+void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& wtxid, WalletBatch* batch)
 {
     mapTxSpends.insert(std::make_pair(outpoint, wtxid));
 
@@ -983,12 +983,12 @@ void CWallet::MarkDirty()
 {
     {
         LOCK(cs_wallet);
-        for (std::pair<const uint256, CWalletTx>& item : mapWallet)
+        for (std::pair<const Txid, CWalletTx>& item : mapWallet)
             item.second.MarkDirty();
     }
 }
 
-bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash)
+bool CWallet::MarkReplaced(const Txid& originalHash, const Txid& newHash)
 {
     LOCK(cs_wallet);
 
@@ -1020,7 +1020,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash)
     return success;
 }
 
-void CWallet::SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations)
+void CWallet::SetSpentKeyState(WalletBatch& batch, const Txid& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations)
 {
     AssertLockHeld(cs_wallet);
     const CWalletTx* srctx = GetWalletTx(hash);
@@ -1075,7 +1075,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
 
     WalletBatch batch(GetDatabase(), fFlushOnClose);
 
-    uint256 hash = tx->GetHash();
+    Txid hash = tx->GetHash();
 
     if (IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) {
         // Mark used destinations
@@ -1197,7 +1197,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
     return &wtx;
 }
 
-bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx)
+bool CWallet::LoadToWallet(const Txid& hash, const UpdateWalletTxFn& fill_wtx)
 {
     const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(nullptr, TxStateInactive{}));
     CWalletTx& wtx = ins.first->second;
@@ -1295,7 +1295,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
     return false;
 }
 
-bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const
+bool CWallet::TransactionCanBeAbandoned(const Txid& hashTx) const
 {
     LOCK(cs_wallet);
     const CWalletTx* wtx = GetWalletTx(hashTx);
@@ -1312,7 +1312,7 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx)
     }
 }
 
-bool CWallet::AbandonTransaction(const uint256& hashTx)
+bool CWallet::AbandonTransaction(const Txid& hashTx)
 {
     LOCK(cs_wallet);
     auto it = mapWallet.find(hashTx);
@@ -1350,7 +1350,7 @@ bool CWallet::AbandonTransaction(CWalletTx& tx)
     return true;
 }
 
-void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx)
+void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const Txid& hashTx)
 {
     LOCK(cs_wallet);
 
@@ -1380,20 +1380,20 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
 
 }
 
-void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) {
+void CWallet::RecursiveUpdateTxState(const Txid& tx_hash, const TryUpdatingStateFn& try_updating_state) {
     // Do not flush the wallet here for performance reasons
     WalletBatch batch(GetDatabase(), false);
     RecursiveUpdateTxState(&batch, tx_hash, try_updating_state);
 }
 
-void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) {
-    std::set<uint256> todo;
-    std::set<uint256> done;
+void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const Txid& tx_hash, const TryUpdatingStateFn& try_updating_state) {
+    std::set<Txid> todo;
+    std::set<Txid> done;
 
     todo.insert(tx_hash);
 
     while (!todo.empty()) {
-        uint256 now = *todo.begin();
+        Txid now = *todo.begin();
         todo.erase(now);
         done.insert(now);
         auto it = mapWallet.find(now);
@@ -1406,7 +1406,7 @@ void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash,
             if (batch) batch->WriteTx(wtx);
             // Iterate over all its outputs, and update those tx states as well (if applicable)
             for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) {
-                std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(Txid::FromUint256(now), i));
+                std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i));
                 for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) {
                     if (!done.count(iter->second)) {
                         todo.insert(iter->second);
@@ -1450,7 +1450,7 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx) {
     for (const CTxIn& tx_in : tx->vin) {
         // For each wallet transaction spending this prevout..
         for (auto range = mapTxSpends.equal_range(tx_in.prevout); range.first != range.second; range.first++) {
-            const uint256& spent_id = range.first->second;
+            const Txid& spent_id = range.first->second;
             // Skip the recently added tx
             if (spent_id == txid) continue;
             RecursiveUpdateTxState(/*batch=*/nullptr, spent_id, [&txid](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
@@ -1503,7 +1503,7 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe
         // and recursively mark them as no longer conflicting with
         // txid
         for (auto range = mapTxSpends.equal_range(tx_in.prevout); range.first != range.second; range.first++) {
-            const uint256& spent_id = range.first->second;
+            const Txid& spent_id = range.first->second;
 
             RecursiveUpdateTxState(/*batch=*/nullptr, spent_id, [&txid](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
                 return wtx.mempool_conflicts.erase(txid) ? TxUpdate::CHANGED : TxUpdate::UNCHANGED;
@@ -2072,12 +2072,12 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string
     return ret;
 }
 
-std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const
+std::set<Txid> CWallet::GetTxConflicts(const CWalletTx& wtx) const
 {
     AssertLockHeld(cs_wallet);
 
-    const uint256 myHash{wtx.GetHash()};
-    std::set<uint256> result{GetConflicts(myHash)};
+    const Txid myHash{wtx.GetHash()};
+    std::set<Txid> result{GetConflicts(myHash)};
     result.erase(myHash);
     return result;
 }
@@ -2228,7 +2228,7 @@ std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo
 
         // If we have no utxo, grab it from the wallet.
         if (!input.non_witness_utxo) {
-            const uint256& txhash = txin.prevout.hash;
+            const Txid& txhash = txin.prevout.hash;
             const auto it = mapWallet.find(txhash);
             if (it != mapWallet.end()) {
                 const CWalletTx& wtx = it->second;
@@ -2405,7 +2405,7 @@ DBErrors CWallet::LoadWallet()
     return nLoadWalletRet;
 }
 
-util::Result<void> CWallet::RemoveTxs(std::vector<uint256>& txs_to_remove)
+util::Result<void> CWallet::RemoveTxs(std::vector<Txid>& txs_to_remove)
 {
     AssertLockHeld(cs_wallet);
     bilingual_str str_err;  // future: make RunWithinTxn return a util::Result
@@ -2419,16 +2419,16 @@ util::Result<void> CWallet::RemoveTxs(std::vector<uint256>& txs_to_remove)
     return {}; // all good
 }
 
-util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<uint256>& txs_to_remove)
+util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs_to_remove)
 {
     AssertLockHeld(cs_wallet);
     if (!batch.HasActiveTxn()) return util::Error{strprintf(_("The transactions removal process can only be executed within a db txn"))};
 
     // Check for transaction existence and remove entries from disk
-    using TxIterator = std::unordered_map<uint256, CWalletTx, SaltedTxidHasher>::const_iterator;
+    using TxIterator = std::unordered_map<Txid, CWalletTx, SaltedTxidHasher>::const_iterator;
     std::vector<TxIterator> erased_txs;
     bilingual_str str_err;
-    for (const uint256& hash : txs_to_remove) {
+    for (const Txid& hash : txs_to_remove) {
         auto it_wtx = mapWallet.find(hash);
         if (it_wtx == mapWallet.end()) {
             return util::Error{strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex())};
@@ -2443,7 +2443,7 @@ util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<uint256>&
     batch.RegisterTxnListener({.on_commit=[&, erased_txs]() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
         // Update the in-memory state and notify upper layers about the removals
         for (const auto& it : erased_txs) {
-            const uint256 hash{it->first};
+            const Txid hash{it->first};
             wtxOrdered.erase(it->second.m_it_wtxOrdered);
             for (const auto& txin : it->second.tx->vin)
                 mapTxSpends.erase(txin.prevout);
@@ -4164,7 +4164,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
 
     // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet.
     // We need to go through these in the tx insertion order so that lookups to spends works.
-    std::vector<uint256> txids_to_delete;
+    std::vector<Txid> txids_to_delete;
     std::unique_ptr<WalletBatch> watchonly_batch;
     if (data.watchonly_wallet) {
         watchonly_batch = std::make_unique<WalletBatch>(data.watchonly_wallet->GetDatabase());
@@ -4195,7 +4195,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
             LOCK(data.watchonly_wallet->cs_wallet);
             if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) {
                 // Add to watchonly wallet
-                const uint256& hash = wtx->GetHash();
+                const Txid& hash = wtx->GetHash();
                 const CWalletTx& to_copy_wtx = *wtx;
                 if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) {
                     if (!new_tx) return false;

src/wallet/wallet.h

@@ -331,9 +331,9 @@ private:
      * detect and report conflicts (double-spends or
      * mutated transactions where the mutant gets mined).
      */
-    typedef std::unordered_multimap<COutPoint, uint256, SaltedOutpointHasher> TxSpends;
+    typedef std::unordered_multimap<COutPoint, Txid, SaltedOutpointHasher> TxSpends;
     TxSpends mapTxSpends GUARDED_BY(cs_wallet);
-    void AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void AddToSpends(const COutPoint& outpoint, const Txid& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /**
@@ -355,15 +355,15 @@ private:
     bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const SyncTxState& state, bool fUpdate, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /** Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */
-    void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx);
+    void MarkConflicted(const uint256& hashBlock, int conflicting_height, const Txid& hashTx);
 
     enum class TxUpdate { UNCHANGED, CHANGED, NOTIFY_CHANGED };
 
     using TryUpdatingStateFn = std::function<TxUpdate(CWalletTx& wtx)>;
 
     /** Mark a transaction (and its in-wallet descendants) as a particular tx state. */
-    void RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    void RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void RecursiveUpdateTxState(const Txid& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void RecursiveUpdateTxState(WalletBatch* batch, const Txid& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /** Mark a transaction's inputs dirty, thus forcing the outputs to be recomputed */
     void MarkInputsDirty(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
@@ -481,7 +481,7 @@ public:
 
     /** Map from txid to CWalletTx for all transactions this wallet is
      * interested in, including received and sent transactions. */
-    std::unordered_map<uint256, CWalletTx, SaltedTxidHasher> mapWallet GUARDED_BY(cs_wallet);
+    std::unordered_map<Txid, CWalletTx, SaltedTxidHasher> mapWallet GUARDED_BY(cs_wallet);
 
     typedef std::multimap<int64_t, CWalletTx*> TxItems;
     TxItems wtxOrdered;
@@ -503,9 +503,9 @@ public:
     /** Interface for accessing chain state. */
     interfaces::Chain& chain() const { assert(m_chain); return *m_chain; }
 
-    const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    const CWalletTx* GetWalletTx(const Txid& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
-    std::set<uint256> GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    std::set<Txid> GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /**
      * Return depth of transaction in blockchain:
@@ -537,7 +537,7 @@ public:
 
     // Whether this or any known scriptPubKey with the same single key has been spent.
     bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void SetSpentKeyState(WalletBatch& batch, const Txid& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /** Display address on an external signer. */
     util::Result<void> DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
@@ -608,7 +608,7 @@ public:
      * @return the recently added wtx pointer or nullptr if there was a db write error.
      */
     CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
-    bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    bool LoadToWallet(const Txid& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void transactionAddedToMempool(const CTransactionRef& tx) override;
     void blockConnected(ChainstateRole role, const interfaces::BlockInfo& block) override;
     void blockDisconnected(const interfaces::BlockInfo& block) override;
@@ -793,8 +793,8 @@ public:
     DBErrors LoadWallet();
 
     /** Erases the provided transactions from the wallet. */
-    util::Result<void> RemoveTxs(std::vector<uint256>& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    util::Result<void> RemoveTxs(WalletBatch& batch, std::vector<uint256>& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    util::Result<void> RemoveTxs(std::vector<Txid>& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    util::Result<void> RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& purpose);
 
@@ -817,7 +817,7 @@ public:
     int GetVersion() const { LOCK(cs_wallet); return nWalletVersion; }
 
     //! Get wallet transactions that conflict with given transaction (spend same outputs)
-    std::set<uint256> GetConflicts(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    std::set<Txid> GetConflicts(const Txid& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     //! Check if a given transaction has any of its outputs spent by another transaction in the wallet
     bool HasWalletSpend(const CTransactionRef& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
@@ -844,7 +844,7 @@ public:
      * Wallet transaction added, removed or updated.
      * @note called with lock cs_wallet held.
      */
-    boost::signals2::signal<void(const uint256& hashTx, ChangeType status)> NotifyTransactionChanged;
+    boost::signals2::signal<void(const Txid& hashTx, ChangeType status)> NotifyTransactionChanged;
 
     /** Show progress e.g. for rescan */
     boost::signals2::signal<void (const std::string &title, int nProgress)> ShowProgress;
@@ -867,14 +867,14 @@ public:
     void SetBroadcastTransactions(bool broadcast) { fBroadcastTransactions = broadcast; }
 
     /** Return whether transaction can be abandoned */
-    bool TransactionCanBeAbandoned(const uint256& hashTx) const;
+    bool TransactionCanBeAbandoned(const Txid& hashTx) const;
 
     /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */
-    bool AbandonTransaction(const uint256& hashTx);
+    bool AbandonTransaction(const Txid& hashTx);
     bool AbandonTransaction(CWalletTx& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /** Mark a transaction as replaced by another transaction. */
-    bool MarkReplaced(const uint256& originalHash, const uint256& newHash);
+    bool MarkReplaced(const Txid& originalHash, const Txid& newHash);
 
     /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */
     static std::shared_ptr<CWallet> Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings);

src/wallet/walletdb.cpp

@@ -16,6 +16,7 @@
 #include <util/bip32.h>
 #include <util/check.h>
 #include <util/fs.h>
+#include <util/transaction_identifier.h>
 #include <util/time.h>
 #include <util/translation.h>
 #ifdef USE_BDB
@@ -96,9 +97,9 @@ bool WalletBatch::WriteTx(const CWalletTx& wtx)
     return WriteIC(std::make_pair(DBKeys::TX, wtx.GetHash()), wtx);
 }
 
-bool WalletBatch::EraseTx(uint256 hash)
+bool WalletBatch::EraseTx(Txid hash)
 {
-    return EraseIC(std::make_pair(DBKeys::TX, hash));
+    return EraseIC(std::make_pair(DBKeys::TX, hash.ToUint256()));
 }
 
 bool WalletBatch::WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite)
@@ -1048,7 +1049,7 @@ static DBErrors LoadAddressBookRecords(CWallet* pwallet, DatabaseBatch& batch) E
     return result;
 }
 
-static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vector<uint256>& upgraded_txs, bool& any_unordered) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
+static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vector<Txid>& upgraded_txs, bool& any_unordered) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
 {
     AssertLockHeld(pwallet->cs_wallet);
     DBErrors result = DBErrors::LOAD_OK;
@@ -1058,7 +1059,7 @@ static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vecto
     LoadResult tx_res = LoadRecords(pwallet, batch, DBKeys::TX,
         [&any_unordered, &upgraded_txs] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
         DBErrors result = DBErrors::LOAD_OK;
-        uint256 hash;
+        Txid hash;
         key >> hash;
         // LoadToWallet call below creates a new CWalletTx that fill_wtx
         // callback fills with transaction metadata.
@@ -1192,7 +1193,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
 {
     DBErrors result = DBErrors::LOAD_OK;
     bool any_unordered = false;
-    std::vector<uint256> upgraded_txs;
+    std::vector<Txid> upgraded_txs;
 
     LOCK(pwallet->cs_wallet);
 
@@ -1247,7 +1248,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     if (result != DBErrors::LOAD_OK)
         return result;
 
-    for (const uint256& hash : upgraded_txs)
+    for (const Txid& hash : upgraded_txs)
         WriteTx(pwallet->mapWallet.at(hash));
 
     if (!has_last_client || last_client != CLIENT_VERSION) // Update

src/wallet/walletdb.h

@@ -238,7 +238,7 @@ public:
     bool ErasePurpose(const std::string& strAddress);
 
     bool WriteTx(const CWalletTx& wtx);
-    bool EraseTx(uint256 hash);
+    bool EraseTx(Txid hash);
 
     bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite);
     bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata &keyMeta);

src/zmq/zmqpublishnotifier.cpp

@@ -230,7 +230,7 @@ bool CZMQPublishHashBlockNotifier::NotifyBlock(const CBlockIndex *pindex)
 
 bool CZMQPublishHashTransactionNotifier::NotifyTransaction(const CTransaction &transaction)
 {
-    uint256 hash = transaction.GetHash();
+    uint256 hash = transaction.GetHash().ToUint256();
     LogDebug(BCLog::ZMQ, "Publish hashtx %s to %s\n", hash.GetHex(), this->address);
     uint8_t data[32];
     for (unsigned int i = 0; i < 32; i++) {
@@ -254,7 +254,7 @@ bool CZMQPublishRawBlockNotifier::NotifyBlock(const CBlockIndex *pindex)
 
 bool CZMQPublishRawTransactionNotifier::NotifyTransaction(const CTransaction &transaction)
 {
-    uint256 hash = transaction.GetHash();
+    uint256 hash = transaction.GetHash().ToUint256();
     LogDebug(BCLog::ZMQ, "Publish rawtx %s to %s\n", hash.GetHex(), this->address);
     DataStream ss;
     ss << TX_WITH_WITNESS(transaction);
@@ -290,14 +290,14 @@ bool CZMQPublishSequenceNotifier::NotifyBlockDisconnect(const CBlockIndex *pinde
 
 bool CZMQPublishSequenceNotifier::NotifyTransactionAcceptance(const CTransaction &transaction, uint64_t mempool_sequence)
 {
-    uint256 hash = transaction.GetHash();
+    uint256 hash = transaction.GetHash().ToUint256();
     LogDebug(BCLog::ZMQ, "Publish hashtx mempool acceptance %s to %s\n", hash.GetHex(), this->address);
     return SendSequenceMsg(*this, hash, /* Mempool (A)cceptance */ 'A', mempool_sequence);
 }
 
 bool CZMQPublishSequenceNotifier::NotifyTransactionRemoval(const CTransaction &transaction, uint64_t mempool_sequence)
 {
-    uint256 hash = transaction.GetHash();
+    uint256 hash = transaction.GetHash().ToUint256();
     LogDebug(BCLog::ZMQ, "Publish hashtx mempool removal %s to %s\n", hash.GetHex(), this->address);
     return SendSequenceMsg(*this, hash, /* Mempool (R)emoval */ 'R', mempool_sequence);
 }

test/functional/wallet_backwards_compatibility.py

@@ -87,7 +87,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         # 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
         # Make sure that this is being automatically cleaned up by migration
         node_master = self.nodes[1]
-        node_v22 = self.nodes[self.num_nodes - 5]
+        node_v22 = self.nodes[self.num_nodes - 3]
         wallet_name = "bad_deriv_path"
         node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
         bad_deriv_wallet = node_v22.get_wallet_rpc(wallet_name)