Convert txdownloadman_impl to GenTxidVariant

Convert all of `txdownloadman_impl` to the new variant except for
`GetRequestsToSend`, which will be easier to switch at the same
time as `txrequest`.

src/net_processing.cpp

@@ -3949,7 +3949,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 AddKnownTx(*peer, inv.hash);
 
                 if (!m_chainman.IsInitialBlockDownload()) {
-                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time)};
+                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid.ToVariant(), current_time)};
                     LogDebug(BCLog::NET, "got inv: %s  %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
                 }
             } else {
@@ -4877,11 +4877,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<GenTxidVariant> 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(ToGenTxidVariant(inv));
                 }
             }
         }

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>
@@ -15,7 +16,7 @@
 class CBlock;
 class CRollingBloomFilter;
 class CTxMemPool;
-class GenTxid;
+class GenTxidVariant;
 class TxRequestTracker;
 namespace node {
 class TxDownloadManagerImpl;
@@ -137,13 +138,13 @@ public:
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
      * Returns true if this was a dropped inv (p2p_inv=true and we already have the tx), false otherwise. */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
+    bool AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     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<GenTxidVariant>& txhashes);
 
     /** Respond to successful transaction submission to mempool */
     void MempoolAcceptedTx(const CTransactionRef& tx);

src/node/txdownloadman_impl.cpp

@@ -39,7 +39,7 @@ void TxDownloadManager::DisconnectedPeer(NodeId nodeid)
 {
     m_impl->DisconnectedPeer(nodeid);
 }
-bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
+bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now)
 {
     return m_impl->AddTxAnnouncement(peer, gtxid, now);
 }
@@ -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<GenTxidVariant>& txhashes)
 {
     m_impl->ReceivedNotFound(nodeid, txhashes);
 }
@@ -122,14 +122,11 @@ void TxDownloadManagerImpl::BlockDisconnected()
     RecentConfirmedTransactionsFilter().reset();
 }
 
-bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable)
+bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxidVariant& gtxid, bool include_reconsiderable)
 {
-    const uint256& hash = gtxid.GetHash();
+    bool in_orphanage = std::visit(util::Overloaded{
-
-    if (gtxid.IsWtxid()) {
         // Normal query by wtxid.
-        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
+        [this](const Wtxid& wtxid) { return m_orphanage.HaveTx(wtxid); },
-    } else {
         // 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,14 +138,18 @@ 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;
 
     if (RecentConfirmedTransactionsFilter().contains(hash)) return true;
 
-    return RecentRejectsFilter().contains(hash) || m_opts.m_mempool.exists(gtxid.ToVariant());
+    return RecentRejectsFilter().contains(hash) || m_opts.m_mempool.exists(gtxid);
 }
 
 void TxDownloadManagerImpl::ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info)
@@ -172,18 +173,17 @@ void TxDownloadManagerImpl::DisconnectedPeer(NodeId nodeid)
 
 }
 
-bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
+bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now)
 {
     // If this is an orphan we are trying to resolve, consider this peer as a orphan resolution candidate instead.
     // - is wtxid matching something in orphanage
     // - exists in orphanage
     // - peer can be an orphan resolution candidate
-    if (gtxid.IsWtxid()) {
+    if (const auto* wtxid = std::get_if<Wtxid>(&gtxid)) {
-        const auto wtxid{Wtxid::FromUint256(gtxid.GetHash())};
+        if (auto orphan_tx{m_orphanage.GetTx(*wtxid)}) {
-        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,11 +220,11 @@ 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;
 
-    m_txrequest.ReceivedInv(peer, gtxid, info.m_preferred, now + delay);
+    m_txrequest.ReceivedInv(peer, GenTxid::FromVariant(gtxid), info.m_preferred, now + delay);
 
     return false;
 }
@@ -277,7 +277,7 @@ std::vector<GenTxid> TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std
             entry.second.GetHash().ToString(), entry.first);
     }
     for (const GenTxid& gtxid : requestable) {
-        if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) {
+        if (!AlreadyHaveTx(gtxid.ToVariant(), /*include_reconsiderable=*/false)) {
             LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx",
                 gtxid.GetHash().ToString(), nodeid);
             requests.emplace_back(gtxid);
@@ -291,12 +291,12 @@ std::vector<GenTxid> TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std
     return requests;
 }
 
-void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector<uint256>& txhashes)
+void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector<GenTxidVariant>& txhashes)
 {
     for (const auto& txhash : txhashes) {
         // If we receive a NOTFOUND message for a tx we requested, mark the announcement for it as
         // completed in TxRequestTracker.
-        m_txrequest.ReceivedResponse(nodeid, txhash);
+        m_txrequest.ReceivedResponse(nodeid, txhash.ToUint256());
     }
 }
 
@@ -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) {
+            for (const Txid& parent_txid : unique_parents) {
-                if (RecentRejectsFilter().contains(parent_txid)) {
+                if (RecentRejectsFilter().contains(parent_txid.ToUint256())) {
                     fRejectedParents = true;
                     break;
-                } else if (RecentRejectsReconsiderableFilter().contains(parent_txid) &&
+                } else if (RecentRejectsReconsiderableFilter().contains(parent_txid.ToUint256()) &&
-                           !m_opts.m_mempool.exists(Txid::FromUint256(parent_txid))) {
+                           !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);
+                m_txrequest.GetCandidatePeers(ptx->GetHash(), orphan_resolution_candidates);
-                if (ptx->HasWitness()) m_txrequest.GetCandidatePeers(ptx->GetWitnessHash().ToUint256(), 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 Txid& txid = ptx->GetHash();
-    const uint256& wtxid = ptx->GetWitnessHash();
+    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

@@ -155,7 +155,7 @@ public:
      *  - m_recent_rejects_reconsiderable (if include_reconsiderable = true)
      *  - m_recent_confirmed_transactions
      *  */
-    bool AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable);
+    bool AlreadyHaveTx(const GenTxidVariant& gtxid, bool include_reconsiderable);
 
     void ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info);
     void DisconnectedPeer(NodeId nodeid);
@@ -163,13 +163,13 @@ public:
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
      */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
+    bool AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     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<GenTxidVariant>& 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/protocol.cpp

@@ -118,8 +118,20 @@ std::vector<std::string> serviceFlagsToStr(uint64_t flags)
     return str_flags;
 }
 
+// TODO: Remove this function and rename the one below to ToGenTxid.
+// This is only here for intermediate commits.
 GenTxid ToGenTxid(const CInv& inv)
 {
     assert(inv.IsGenTxMsg());
     return inv.IsMsgWtx() ? GenTxid::Wtxid(inv.hash) : GenTxid::Txid(inv.hash);
 }
+
+GenTxidVariant ToGenTxidVariant(const CInv& inv)
+{
+    assert(inv.IsGenTxMsg());
+    if (inv.IsMsgWtx()) {
+        return Wtxid::FromUint256(inv.hash);
+    } else {
+        return Txid::FromUint256(inv.hash);
+    }
+}

src/protocol.h

@@ -527,5 +527,7 @@ public:
 
 /** Convert a TX/WITNESS_TX/WTX CInv to a GenTxid. */
 GenTxid ToGenTxid(const CInv& inv);
+// Remove this once all GenTxids are switched to the variant.
+GenTxidVariant ToGenTxidVariant(const CInv& inv);
 
 #endif // BITCOIN_PROTOCOL_H

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() ?
+                GenTxidVariant gtxid;
-                                GenTxid::Txid(rand_tx->GetHash()) :
+                if (fuzzed_data_provider.ConsumeBool()) {
-                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                    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() ?
+                GenTxidVariant gtxid;
-                                GenTxid::Txid(rand_tx->GetHash()) :
+                if (fuzzed_data_provider.ConsumeBool()) {
-                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                    rand_tx->GetHash();
+                } else {
+                    rand_tx->GetWitnessHash();
+                }
                 txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
@@ -383,7 +389,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                 // TxDownloadManager should not be telling us to request things we already have.
                 // Exclude m_lazy_recent_rejects_reconsiderable because it may request low-feerate parent of orphan.
                 for (const auto& gtxid : getdata_requests) {
-                    Assert(!txdownload_impl.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false));
+                    Assert(!txdownload_impl.AlreadyHaveTx(gtxid.ToVariant(), /*include_reconsiderable=*/false));
                 }
             },
             [&] {
@@ -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/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_txid = ptx_parent->GetHash();
-            const auto& parent_wtxid = ptx_parent->GetWitnessHash().ToUint256();
+            const auto& parent_wtxid = ptx_parent->GetWitnessHash();
-            const auto& child_txid = ptx_child->GetHash().ToUint256();
+            const auto& child_txid = ptx_child->GetHash();
-            const auto& child_wtxid = ptx_child->GetWitnessHash().ToUint256();
+            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),
+                    /*txid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_txid.ToUint256()),
-                    /*wtxid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_wtxid),
+                    /*wtxid_rejects=*/txdownload_impl.RecentRejectsFilter().contains(parent_wtxid.ToUint256()),
-                    /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid),
+                    /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid.ToUint256()),
-                    /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid),
+                    /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid.ToUint256()),
                     /*keep=*/keep,
-                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now),
+                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, parent_txid, now),
-                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), 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_txid.ToUint256()));
-                BOOST_CHECK_EQUAL(parent_txid_rejected, txdownload_impl.RecentRejectsFilter().contains(child_wtxid));
+                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()));