diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 239106afedc..56716ec673f 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -208,7 +208,7 @@ 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; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 92a2a17956f..5e0651aa3a3 100644 --- a/src/net_processing.cpp +++ b/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 m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex); + std::set 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 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 m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr 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> m_most_recent_block_txs GUARDED_BY(m_most_recent_block_mutex); + std::unique_ptr> 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. */ @@ -947,7 +947,7 @@ private: std::atomic m_last_tip_update{0s}; /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ - CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid) + CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) @@ -1583,7 +1583,7 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) CTransactionRef tx = m_mempool.get(txid); if (tx != nullptr) { - RelayTransaction(txid, tx->GetWitnessHash()); + RelayTransaction(Txid::FromUint256(txid), tx->GetWitnessHash()); } else { m_mempool.RemoveUnbroadcastTx(txid, true); } @@ -2027,7 +2027,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>(); + auto most_recent_block_txs = std::make_unique>(); for (const auto& tx : pblock->vtx) { most_recent_block_txs->emplace(tx->GetHash(), tx); most_recent_block_txs->emplace(tx->GetWitnessHash(), tx); @@ -2150,7 +2150,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) { @@ -2166,11 +2166,11 @@ 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); + const auto gtxid{peer.m_wtxid_relay ? GenTxid{wtxid} : GenTxid{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, @@ -2391,10 +2391,15 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } } -CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid) +CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const CInv& inv) { + auto gtxid{ToGenTxid(inv)}; // If a tx was in the mempool prior to the last INV for this peer, permit the request. - auto txinfo = m_mempool.info_for_relay(gtxid, tx_relay.m_last_inv_sequence); + auto txinfo{std::visit( + [&](const auto& id) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) { + return m_mempool.info_for_relay(id, tx_relay.m_last_inv_sequence); + }, + gtxid)}; if (txinfo.tx) { return std::move(txinfo.tx); } @@ -2403,7 +2408,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; } } @@ -2437,8 +2442,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic continue; } - CTransactionRef tx = FindTxForGetData(*tx_relay, ToGenTxid(inv)); - if (tx) { + if (auto tx{FindTxForGetData(*tx_relay, inv)}) { // WTX and WITNESS_TX imply we serialize with witness const auto maybe_with_witness = (inv.IsMsgTx() ? TX_NO_WITNESS : TX_WITH_WITNESS); MakeAndPushMessage(pfrom, NetMsgType::TX, maybe_with_witness(*tx)); @@ -4294,7 +4298,7 @@ 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(tx.GetHash())) { LogPrintf("Not relaying non-mempool transaction %s (wtxid=%s) from forcerelay peer=%d\n", tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), pfrom.GetId()); } else { @@ -4928,11 +4932,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::NOTFOUND) { std::vector vInv; vRecv >> vInv; - std::vector tx_invs; + std::vector 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)); } } } @@ -5432,20 +5436,15 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi namespace { class CompareInvMempoolOrder { - CTxMemPool* mp; - bool m_wtxid_relay; + const CTxMemPool* m_mempool; public: - explicit CompareInvMempoolOrder(CTxMemPool *_mempool, bool use_wtxid) - { - mp = _mempool; - m_wtxid_relay = use_wtxid; - } + explicit CompareInvMempoolOrder(CTxMemPool* mempool) : m_mempool{mempool} {} - bool operator()(std::set::iterator a, std::set::iterator b) + bool operator()(std::set::iterator a, std::set::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 m_mempool->CompareDepthAndScore(*b, *a); } }; } // namespace @@ -5763,7 +5762,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)) { @@ -5784,15 +5783,15 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Determine transactions to relay if (fSendTrickle) { // Produce a vector with all candidates for sending - std::vector::iterator> vInvTx; + std::vector::iterator> vInvTx; vInvTx.reserve(tx_relay->m_tx_inventory_to_send.size()); - for (std::set::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) { + for (std::set::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. @@ -5803,18 +5802,19 @@ 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::iterator it = vInvTx.back(); + std::set::iterator it = vInvTx.back(); vInvTx.pop_back(); - uint256 hash = *it; - CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); + GenTxid hash = *it; + Assume(peer->m_wtxid_relay == hash.IsWtxid()); + 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. - auto txinfo = m_mempool.info(ToGenTxid(inv)); + auto txinfo{std::visit([&](const auto& id) { return m_mempool.info(id); }, hash)}; if (!txinfo.tx) { continue; } @@ -5830,7 +5830,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 @@ -5953,7 +5953,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(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.ToUint256()); if (vGetData.size() >= MAX_GETDATA_SZ) { MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); vGetData.clear(); diff --git a/src/net_processing.h b/src/net_processing.h index 1156ec08f73..147ab681bed 100644 --- a/src/net_processing.h +++ b/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; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 0485ff48bd1..62172930dca 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -667,11 +667,11 @@ 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 { diff --git a/src/node/mempool_persist.cpp b/src/node/mempool_persist.cpp index d4d9263973e..ff47172c274 100644 --- a/src/node/mempool_persist.cpp +++ b/src/node/mempool_persist.cpp @@ -107,7 +107,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; diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index d7d15554b32..dec1060ab7f 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -27,7 +27,7 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector& 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. diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 666597391e3..9162557bca1 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -42,7 +42,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t std::promise promise; Txid txid = tx->GetHash(); - uint256 wtxid = tx->GetWitnessHash(); + Wtxid wtxid = tx->GetWitnessHash(); bool callback_set = false; { diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h index 1ae833a6be6..9eb246740b9 100644 --- a/src/node/txdownloadman.h +++ b/src/node/txdownloadman.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -143,7 +144,7 @@ public: std::vector 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& txhashes); + void ReceivedNotFound(NodeId nodeid, const std::vector& gtxids); /** Respond to successful transaction submission to mempool */ void MempoolAcceptedTx(const CTransactionRef& tx); diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index f319414042f..5bb435f5321 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -47,9 +47,9 @@ std::vector TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::ch { return m_impl->GetRequestsToSend(nodeid, current_time); } -void TxDownloadManager::ReceivedNotFound(NodeId nodeid, const std::vector& txhashes) +void TxDownloadManager::ReceivedNotFound(NodeId nodeid, const std::vector& gtxids) { - m_impl->ReceivedNotFound(nodeid, txhashes); + m_impl->ReceivedNotFound(nodeid, gtxids); } void TxDownloadManager::MempoolAcceptedTx(const CTransactionRef& tx) { @@ -124,31 +124,26 @@ void TxDownloadManagerImpl::BlockDisconnected() bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable) { - const uint256& hash = gtxid.GetHash(); + const uint256& hash = gtxid.ToUint256(); - if (gtxid.IsWtxid()) { - // Normal query by wtxid. - if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true; - } 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 - // downloading a transaction by intentionally creating a malleated version of it. While - // only one (or none!) of these transactions can ultimately be confirmed, we have no way of - // discerning which one that is, so the orphanage can store multiple transactions with the - // same txid. - // - // 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; - } + // 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 + // downloading a transaction by intentionally creating a malleated version of it. While + // only one (or none!) of these transactions can ultimately be confirmed, we have no way of + // discerning which one that is, so the orphanage can store multiple transactions with the + // same txid. + // + // 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 txhash "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; if (include_reconsiderable && RecentRejectsReconsiderableFilter().contains(hash)) return true; if (RecentConfirmedTransactionsFilter().contains(hash)) return true; - return RecentRejectsFilter().contains(hash) || m_opts.m_mempool.exists(gtxid); + return RecentRejectsFilter().contains(hash) || std::visit([&](const auto& id) { return m_opts.m_mempool.exists(id); }, gtxid); } void TxDownloadManagerImpl::ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info) @@ -178,12 +173,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(>xid)) { + 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); + std::erase_if(unique_parents, [&](const auto& txid) { + 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 +186,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); } @@ -261,7 +255,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; @@ -272,31 +266,31 @@ std::vector TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std std::vector requests; std::vector> 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); + for (const auto& [expired_nodeid, gtxid] : expired) { + LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", + gtxid.ToUint256().ToString(), expired_nodeid); } 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); + gtxid.ToUint256().ToString(), nodeid); requests.emplace_back(gtxid); - m_txrequest.RequestedTx(nodeid, gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL); + m_txrequest.RequestedTx(nodeid, gtxid.ToUint256(), 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.ToUint256()); } } return requests; } -void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector& txhashes) +void TxDownloadManagerImpl::ReceivedNotFound(NodeId nodeid, const std::vector& gtxids) { - for (const auto& txhash : txhashes) { + for (const auto& gtxid : gtxids) { // 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, gtxid.ToUint256()); } } @@ -377,13 +371,13 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction // Distinguish between parents in m_lazy_recent_rejects and m_lazy_recent_rejects_reconsiderable. // 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 rejected_parent_reconsiderable; - for (const uint256& parent_txid : unique_parents) { - if (RecentRejectsFilter().contains(parent_txid)) { + std::optional rejected_parent_reconsiderable; + 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()) { @@ -397,8 +391,8 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction // Filter parents that we already have. // 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); + std::erase_if(unique_parents, [&](const auto& txid) { + return AlreadyHaveTx(txid, /*include_reconsiderable=*/false); }); const auto now{GetTime()}; const auto& wtxid = ptx->GetWitnessHash(); @@ -412,8 +406,8 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction // // Search by txid and, if the tx has a witness, wtxid std::vector 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 +509,8 @@ void TxDownloadManagerImpl::MempoolRejectedPackage(const Package& package) std::pair> 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 +529,7 @@ std::pair> 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 +546,7 @@ std::pair> 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. diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h index f2dee0adaf8..3e0213352ca 100644 --- a/src/node/txdownloadman_impl.h +++ b/src/node/txdownloadman_impl.h @@ -169,7 +169,7 @@ public: std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); /** Marks a tx as ReceivedResponse in txrequest. */ - void ReceivedNotFound(NodeId nodeid, const std::vector& txhashes); + void ReceivedNotFound(NodeId nodeid, const std::vector& gtxids); /** 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 diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index bc76361fba3..dc74f43b354 100644 --- a/src/policy/rbf.cpp +++ b/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; } @@ -107,7 +107,7 @@ std::optional 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); } diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index ad817270949..c5d2759117a 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -423,20 +423,4 @@ struct CMutableTransaction typedef std::shared_ptr CTransactionRef; template static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared(std::forward(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 diff --git a/src/protocol.cpp b/src/protocol.cpp index bdf0a669863..5217c45daaf 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -121,5 +121,5 @@ std::vector serviceFlagsToStr(uint64_t flags) GenTxid ToGenTxid(const CInv& inv) { assert(inv.IsGenTxMsg()); - return inv.IsMsgWtx() ? GenTxid::Wtxid(inv.hash) : GenTxid::Txid(inv.hash); + return inv.IsMsgWtx() ? GenTxid{Wtxid::FromUint256(inv.hash)} : GenTxid{Txid::FromUint256(inv.hash)}; } diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index e9bae93642c..7b5597916c7 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -311,7 +311,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool std::set 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()); } @@ -523,12 +523,12 @@ static RPCHelpMan getmempooldescendants() if (!request.params[1].isNull()) fVerbose = request.params[1].get_bool(); - uint256 hash = ParseHashV(request.params[0], "parameter 1"); + Txid txid{Txid::FromUint256(ParseHashV(request.params[0], "parameter 1"))}; const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - const auto it{mempool.GetIter(hash)}; + const auto it{mempool.GetIter(txid)}; if (!it) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } @@ -1038,7 +1038,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; } @@ -1062,7 +1062,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; } diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index baa28affcec..9c3ec7debf5 100644 --- a/src/test/fuzz/mini_miner.cpp +++ b/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); diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index f263ffb6031..aa73fcdd9b5 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -311,8 +311,8 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) const auto delta = fuzzed_data_provider.ConsumeIntegralInRange(-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); } diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 116b446e535..e0c997494ce 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/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); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 025e6d51ef6..ea65700c522 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -316,8 +316,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()); diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index 06385e7bddb..b65ba0eb383 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -227,9 +227,9 @@ 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()); + auto gtxid = fuzzed_data_provider.ConsumeBool() ? + GenTxid{rand_tx->GetHash()} : + GenTxid{rand_tx->GetWitnessHash()}; txdownloadman.AddTxAnnouncement(rand_peer, gtxid, time); }, [&] { @@ -260,8 +260,7 @@ FUZZ_TARGET(txdownloadman, .init = initialize) // returned true. Assert(expect_work); } - } - ); + }); // Jump forwards or backwards auto time_skip = fuzzed_data_provider.PickValueInArray(TIME_SKIPS); if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1; @@ -373,9 +372,9 @@ 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()); + auto gtxid = fuzzed_data_provider.ConsumeBool() ? + GenTxid{rand_tx->GetHash()} : + GenTxid{rand_tx->GetWitnessHash()}; txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time); }, [&] { @@ -395,7 +394,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 +423,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. @@ -432,8 +431,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) state_missing_inputs.Invalid(TxValidationResult::TX_MISSING_INPUTS, ""); txdownload_impl.MempoolRejectedTx(ptx, state_missing_inputs, rand_peer, fuzzed_data_provider.ConsumeBool()); } - } - ); + }); auto time_skip = fuzzed_data_provider.PickValueInArray(TIME_SKIPS); if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1; diff --git a/src/test/fuzz/txrequest.cpp b/src/test/fuzz/txrequest.cpp index 931ddf03288..c1585d7c00e 100644 --- a/src/test/fuzz/txrequest.cpp +++ b/src/test/fuzz/txrequest.cpp @@ -19,7 +19,7 @@ namespace { constexpr int MAX_TXHASHES = 16; constexpr int MAX_PEERS = 16; -//! Randomly generated GenTxids used in this test (length is MAX_TXHASHES). +//! Randomly generated txhashes used in this test (length is MAX_TXHASHES). uint256 TXHASHES[MAX_TXHASHES]; //! Precomputed random durations (positive and negative, each ~exponentially distributed). @@ -204,7 +204,8 @@ public: } // Call TxRequestTracker's implementation. - m_tracker.ReceivedInv(peer, is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash]), preferred, reqtime); + auto gtxid = is_wtxid ? GenTxid{Wtxid::FromUint256(TXHASHES[txhash])} : GenTxid{Txid::FromUint256(TXHASHES[txhash])}; + m_tracker.ReceivedInv(peer, gtxid, preferred, reqtime); } void RequestedTx(int peer, int txhash, std::chrono::microseconds exptime) @@ -252,7 +253,8 @@ 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])); + auto gtxid = ann2.m_is_wtxid ? GenTxid{Wtxid::FromUint256(TXHASHES[txhash])} : GenTxid{Txid::FromUint256(TXHASHES[txhash])}; + expected_expired.emplace_back(peer2, gtxid); ann2.m_state = State::COMPLETED; break; } @@ -278,7 +280,7 @@ 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(TXHASHES[std::get<1>(result[pos])] == actual[pos].ToUint256()); assert(std::get<2>(result[pos]) == actual[pos].IsWtxid()); } } diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index f0094dce59f..5992d2a41eb 100644 --- a/src/test/mempool_tests.cpp +++ b/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)); diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index 41bfe28c37d..0052276eac4 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -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, diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp index 463eb210139..f9d50108c24 100644 --- a/src/test/txdownload_tests.cpp +++ b/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())); diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 3c36b782604..9b6d22dde32 100644 --- a/src/test/txpackage_tests.cpp +++ b/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. diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp index d73692d8f98..ba82a986a1b 100644 --- a/src/test/txrequest_tests.cpp +++ b/src/test/txrequest_tests.cpp @@ -103,7 +103,7 @@ public: void ForgetTxHash(const uint256& txhash) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ForgetTxHash(txhash); runner.txrequest.SanityCheck(); }); @@ -113,7 +113,7 @@ public: void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool pref, std::chrono::microseconds reqtime) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ReceivedInv(peer, gtxid, pref, reqtime); runner.txrequest.SanityCheck(); }); @@ -123,7 +123,7 @@ public: void DisconnectedPeer(NodeId peer) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.DisconnectedPeer(peer); runner.txrequest.SanityCheck(); }); @@ -133,7 +133,7 @@ public: void RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds exptime) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.RequestedTx(peer, txhash, exptime); runner.txrequest.SanityCheck(); }); @@ -143,7 +143,7 @@ public: void ReceivedResponse(NodeId peer, const uint256& txhash) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ReceivedResponse(peer, txhash); runner.txrequest.SanityCheck(); }); @@ -161,17 +161,19 @@ public: * backwards (but note that the ordering of this event only follows the scenario's m_now. */ void Check(NodeId peer, const std::vector& expected, size_t candidates, size_t inflight, - size_t completed, const std::string& checkname, - std::chrono::microseconds offset = std::chrono::microseconds{0}) + size_t completed, const std::string& checkname, + std::chrono::microseconds offset = std::chrono::microseconds{0}) { const auto comment = m_testname + " " + checkname; auto& runner = m_runner; const auto now = m_now; assert(offset.count() <= 0); - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { std::vector> expired_now; auto ret = runner.txrequest.GetRequestable(peer, now + offset, &expired_now); - for (const auto& entry : expired_now) runner.expired.insert(entry); + for (const auto& entry : expired_now) { + runner.expired.insert(entry); + } runner.txrequest.SanityCheck(); runner.txrequest.PostGetRequestableSanityCheck(now + offset); size_t total = candidates + inflight + completed; @@ -193,7 +195,7 @@ public: { const auto& testname = m_testname; auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { auto it = runner.expired.find(std::pair{peer, gtxid}); BOOST_CHECK_MESSAGE(it != runner.expired.end(), "[" + testname + "] missing expiration"); if (it != runner.expired.end()) runner.expired.erase(it); @@ -233,10 +235,11 @@ public: return ret; } - /** Generate a random GenTxid; the txhash follows NewTxHash; the is_wtxid flag is random. */ + /** Generate a random GenTxid; the txhash follows NewTxHash; the transaction identifier is random. */ GenTxid NewGTxid(const std::vector>& orders = {}) { - return m_rng.randbool() ? GenTxid::Wtxid(NewTxHash(orders)) : GenTxid::Txid(NewTxHash(orders)); + const uint256 txhash{NewTxHash(orders)}; + return m_rng.randbool() ? GenTxid{Wtxid::FromUint256(txhash)} : GenTxid{Txid::FromUint256(txhash)}; } /** Generate a new random NodeId to use as peer. The same NodeId is never returned twice @@ -285,7 +288,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.ToUint256(), scenario.Now() + expiry); scenario.Check(peer, {}, 0, 1, 0, "s6"); if ((config >> 3) == 1) { // The request will time out @@ -299,7 +302,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.ToUint256()); scenario.Check(peer, {}, 0, 0, 0, "s10"); return; } @@ -309,7 +312,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.ToUint256()); } scenario.Check(peer, {}, 0, 0, 0, "s11"); } @@ -355,7 +358,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.ToUint256(), 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 +368,7 @@ void TxRequestTest::BuildPriorityTest(Scenario& scenario, int config) if (config & 16) { scenario.DisconnectedPeer(priopeer); } else { - scenario.ReceivedResponse(priopeer, gtxid.GetHash()); + scenario.ReceivedResponse(priopeer, gtxid.ToUint256()); } if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); scenario.Check(priopeer, {}, 0, 0, !(config & 16), "p8"); @@ -449,7 +452,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.ToUint256()); scenario.Check(peer, {}, 0, 0, request_order.size() > 0, "b5"); } if (request_order.size()) { @@ -510,8 +513,8 @@ 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 txid{Txid::FromUint256(txhash)}; + auto wtxid{Wtxid::FromUint256(txhash)}; auto reqtimeT = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s(); auto reqtimeW = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s(); @@ -542,11 +545,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.ToUint256(), 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.ToUint256(), scenario.Now() + expiry); scenario.Check(peerT, {}, 1, 0, 0, "w7"); scenario.Check(peerW, {}, 0, 1, 0, "w8"); } @@ -599,7 +602,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.ToUint256(), expiry); scenario.Check(peer1, {}, 0, 1, 0, "r7"); scenario.Check(peer2, {}, 1, 0, 0, "r8"); @@ -638,20 +641,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.ToUint256(), 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.ToUint256(), 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.ToUint256(), expiryB); scenario.Check(peer1, {}, 0, 1, 0, "q8"); scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q9"); @@ -668,7 +671,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.ToUint256(), MAX_TIME); scenario.Check(peer1, {}, 0, 0, 1, "q14"); scenario.Check(peer2, {gtxid2, gtxid1}, 2, 0, 0, "q15"); @@ -680,13 +683,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.ToUint256(), 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.ToUint256(), MAX_TIME); scenario.Check(peer1, {}, 0, 0, 2, "q20"); scenario.Check(peer2, {gtxid1}, 1, 1, 0, "q21"); diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 83d6c246849..d7f78443a88 100644 --- a/src/test/txvalidation_tests.cpp +++ b/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)}; diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index f1ca33bec78..5febb6791c6 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -122,17 +122,17 @@ std::optional 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()); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 10ff535828a..d65005fc645 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -154,7 +154,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashes for (const auto& txid : descendants_to_remove) { // This txid may have been removed already in a prior call to removeRecursive. // Therefore we ensure it is not yet removed already. - if (const std::optional txiter = GetIter(txid)) { + if (const std::optional txiter = GetIter(Txid::FromUint256(txid))) { removeRecursive((*txiter)->GetTx(), MemPoolRemovalReason::SIZELIMIT); } } @@ -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) const { /* Return `true` if hasha should be considered sooner than hashb. Namely when: * a is not in the mempool, but b is @@ -802,14 +802,14 @@ 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); - if (j == mapTx.end()) return false; - indexed_transaction_set::const_iterator i = wtxid ? get_iter_from_wtxid(hasha) : mapTx.find(hasha); - if (i == mapTx.end()) return true; - uint64_t counta = i->GetCountWithAncestors(); - uint64_t countb = j->GetCountWithAncestors(); + auto j{std::visit([&](const auto& id) EXCLUSIVE_LOCKS_REQUIRED(cs) { return GetIter(id); }, hashb)}; + if (!j.has_value()) return false; + auto i{std::visit([&](const auto& id) EXCLUSIVE_LOCKS_REQUIRED(cs) { return GetIter(id); }, hasha)}; + if (!i.has_value()) return true; + uint64_t counta = i.value()->GetCountWithAncestors(); + uint64_t countb = j.value()->GetCountWithAncestors(); if (counta == countb) { - return CompareTxMemPoolEntryByScore()(*i, *j); + return CompareTxMemPoolEntryByScore()(*i.value(), *j.value()); } return counta < countb; } @@ -844,10 +844,6 @@ std::vector CTxMemPool::Get return iters; } -static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) { - return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; -} - std::vector CTxMemPool::entryAll() const { AssertLockHeld(cs); @@ -890,26 +886,6 @@ CTransactionRef CTxMemPool::get(const uint256& hash) const return i->GetSharedTx(); } -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())); - if (i == mapTx.end()) - return TxMempoolInfo(); - return GetInfo(i); -} - -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())); - if (i != mapTx.end() && i->GetSequence() < last_sequence) { - return GetInfo(i); - } else { - return TxMempoolInfo(); - } -} - void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { { @@ -984,13 +960,20 @@ const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const return it == mapNextTx.end() ? nullptr : it->second; } -std::optional CTxMemPool::GetIter(const uint256& txid) const +std::optional CTxMemPool::GetIter(const Txid& txid) const { - auto it = mapTx.find(txid); + auto it = mapTx.find(txid.ToUint256()); if (it != mapTx.end()) return it; return std::nullopt; } +std::optional CTxMemPool::GetIter(const Wtxid& wtxid) const +{ + AssertLockHeld(cs); + auto it{mapTx.project<0>(mapTx.get().find(wtxid))}; + return it != mapTx.end() ? std::make_optional(it) : std::nullopt; +} + CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set& hashes) const { CTxMemPool::setEntries ret; @@ -1007,7 +990,7 @@ std::vector CTxMemPool::GetIterVec(const std::vector ret; ret.reserve(txids.size()); for (const auto& txid : txids) { - const auto it{GetIter(txid)}; + const auto it{GetIter(Txid::FromUint256(txid))}; if (!it) return {}; ret.push_back(*it); } @@ -1017,7 +1000,7 @@ std::vector CTxMemPool::GetIterVec(const std::vector* 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); } } diff --git a/src/txmempool.h b/src/txmempool.h index 83926927cd6..687c474f8e0 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -19,9 +19,10 @@ #include #include #include +#include #include #include -#include +#include #include #include @@ -406,6 +407,11 @@ private: const Limits& limits ) const EXCLUSIVE_LOCKS_REQUIRED(cs); + static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) + { + return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; + } + public: indirectmap mapNextTx GUARDED_BY(cs); std::map mapDeltas GUARDED_BY(cs); @@ -442,7 +448,7 @@ public: void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& 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) const; bool isSpent(const COutPoint& outpoint) const; unsigned int GetTransactionsUpdated() const; void AddTransactionsUpdated(unsigned int n); @@ -474,7 +480,8 @@ public: const CTransaction* GetConflictTx(const COutPoint& prevout) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Returns an iterator to the given hash, if found */ - std::optional GetIter(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::optional GetIter(const Txid& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::optional GetIter(const Wtxid& wtxid) 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, @@ -620,27 +627,38 @@ public: return m_total_fee; } - bool exists(const GenTxid& gtxid) const + bool exists(const Txid& txid) const { LOCK(cs); - if (gtxid.IsWtxid()) { - return (mapTx.get().count(gtxid.GetHash()) != 0); - } - return (mapTx.count(gtxid.GetHash()) != 0); + return (mapTx.count(txid) != 0); + } + + bool exists(const Wtxid& wtxid) const + { + LOCK(cs); + return (mapTx.get().count(wtxid) != 0); } 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) + + template + TxMempoolInfo info(const T& id) const { - AssertLockHeld(cs); - return mapTx.project<0>(mapTx.get().find(wtxid)); + LOCK(cs); + auto i{GetIter(id)}; + return i.has_value() ? GetInfo(*i) : TxMempoolInfo{}; } - TxMempoolInfo info(const GenTxid& gtxid) const; /** Returns info for a transaction if its entry_sequence < last_sequence */ - TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const; + template + TxMempoolInfo info_for_relay(const T& id, uint64_t last_sequence) const + { + LOCK(cs); + auto i{GetIter(id)}; + return (i.has_value() && i.value()->GetSequence() < last_sequence) ? GetInfo(*i) : TxMempoolInfo{}; + } std::vector entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector infoAll() const; @@ -653,7 +671,7 @@ public: 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::FromUint256(txid))) m_unbroadcast_txids.insert(txid); }; /** Removes a transaction from the unbroadcast set */ diff --git a/src/txrequest.cpp b/src/txrequest.cpp index 03b4cd6b161..37496614129 100644 --- a/src/txrequest.cpp +++ b/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_gtxid; /** For CANDIDATE_{DELAYED,BEST,READY} the reqtime; for REQUESTED the expiry. */ std::chrono::microseconds m_time; /** What peer the request was from. */ @@ -69,9 +69,6 @@ struct Announcement { const SequenceNumber m_sequence : 59; /** Whether the request is preferred. */ const bool m_preferred : 1; - /** Whether this is a wtxid request. */ - const bool m_is_wtxid : 1; - /** What state this announcement is in. */ State m_state : 3 {State::CANDIDATE_DELAYED}; State GetState() const { return m_state; } @@ -98,8 +95,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_gtxid(gtxid), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred) {} }; //! Type alias for priorities. @@ -124,7 +120,7 @@ public: Priority operator()(const Announcement& ann) const { - return operator()(ann.m_txhash, ann.m_peer, ann.m_preferred); + return operator()(ann.m_gtxid.ToUint256(), ann.m_peer, ann.m_preferred); } }; @@ -148,7 +144,7 @@ struct ByPeerViewExtractor using result_type = ByPeerView; result_type operator()(const Announcement& ann) const { - return ByPeerView{ann.m_peer, ann.GetState() == State::CANDIDATE_BEST, ann.m_txhash}; + return ByPeerView{ann.m_peer, ann.GetState() == State::CANDIDATE_BEST, ann.m_gtxid.ToUint256()}; } }; @@ -172,7 +168,7 @@ public: result_type operator()(const Announcement& ann) const { const Priority prio = (ann.GetState() == State::CANDIDATE_READY) ? m_computer(ann) : 0; - return ByTxHashView{ann.m_txhash, ann.GetState(), prio}; + return ByTxHashView{ann.m_gtxid.ToUint256(), ann.GetState(), prio}; } }; @@ -280,7 +276,7 @@ std::map ComputeTxHashInfo(const Index& index, const Priori { std::map ret; for (const Announcement& ann : index) { - TxHashInfo& info = ret[ann.m_txhash]; + TxHashInfo& info = ret[ann.m_gtxid.ToUint256()]; // Classify how many announcements of each state we have for this txhash. info.m_candidate_delayed += (ann.GetState() == State::CANDIDATE_DELAYED); info.m_candidate_ready += (ann.GetState() == State::CANDIDATE_READY); @@ -299,11 +295,6 @@ std::map 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. */ @@ -409,7 +400,7 @@ private: // priority) comes last. Thus, if an existing _BEST exists for the same txhash that this announcement may // be preferred over, it must immediately follow the newly created _READY. auto it_next = std::next(it); - if (it_next == m_index.get().end() || it_next->m_txhash != it->m_txhash || + if (it_next == m_index.get().end() || it_next->m_gtxid.ToUint256() != it->m_gtxid.ToUint256() || it_next->GetState() == State::COMPLETED) { // This is the new best CANDIDATE_READY, and there is no IsSelected() announcement for this txhash // already. @@ -435,7 +426,7 @@ private: auto it_prev = std::prev(it); // The next best CANDIDATE_READY, if any, immediately precedes the REQUESTED or CANDIDATE_BEST // announcement in the ByTxHash index. - if (it_prev->m_txhash == it->m_txhash && it_prev->GetState() == State::CANDIDATE_READY) { + if (it_prev->m_gtxid.ToUint256() == it->m_gtxid.ToUint256() && it_prev->GetState() == State::CANDIDATE_READY) { // If one such CANDIDATE_READY exists (for this txhash), convert it to CANDIDATE_BEST. Modify(it_prev, [](Announcement& ann){ ann.SetState(State::CANDIDATE_BEST); }); } @@ -451,10 +442,10 @@ private: // This announcement has a predecessor that belongs to the same txhash. Due to ordering, and the // fact that 'it' is not COMPLETED, its predecessor cannot be COMPLETED here. - if (it != m_index.get().begin() && std::prev(it)->m_txhash == it->m_txhash) return false; + if (it != m_index.get().begin() && std::prev(it)->m_gtxid.ToUint256() == it->m_gtxid.ToUint256()) return false; // This announcement has a successor that belongs to the same txhash, and is not COMPLETED. - if (std::next(it) != m_index.get().end() && std::next(it)->m_txhash == it->m_txhash && + if (std::next(it) != m_index.get().end() && std::next(it)->m_gtxid.ToUint256() == it->m_gtxid.ToUint256() && std::next(it)->GetState() != State::COMPLETED) return false; return true; @@ -472,10 +463,10 @@ private: if (IsOnlyNonCompleted(it)) { // This is the last non-COMPLETED announcement for this txhash. Delete all. - uint256 txhash = it->m_txhash; + uint256 txhash = it->m_gtxid.ToUint256(); do { it = Erase(it); - } while (it != m_index.get().end() && it->m_txhash == txhash); + } while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash); return false; } @@ -501,7 +492,7 @@ private: if (it->GetState() == State::CANDIDATE_DELAYED && it->m_time <= now) { PromoteCandidateReady(m_index.project(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_gtxid); MakeCompleted(m_index.project(it)); } else { break; @@ -569,7 +560,7 @@ public: void ForgetTxHash(const uint256& txhash) { auto it = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0}); - while (it != m_index.get().end() && it->m_txhash == txhash) { + while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash) { it = Erase(it); } } @@ -577,19 +568,19 @@ public: void GetCandidatePeers(const uint256& txhash, std::vector& result_peers) const { auto it = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0}); - while (it != m_index.get().end() && it->m_txhash == txhash && it->GetState() != State::COMPLETED) { + while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash && it->GetState() != State::COMPLETED) { result_peers.push_back(it->m_peer); ++it; } } void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime) + std::chrono::microseconds reqtime) { // 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().count(ByPeerView{peer, true, gtxid.GetHash()})) return; + if (m_index.get().count(ByPeerView{peer, true, gtxid.ToUint256()})) 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). @@ -604,7 +595,7 @@ public: //! Find the GenTxids to request now from peer. std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired) + std::vector>* expired) { // Move time. SetTimePoint(now, expired); @@ -627,7 +618,7 @@ public: std::vector ret; ret.reserve(selected.size()); std::transform(selected.begin(), selected.end(), std::back_inserter(ret), [](const Announcement* ann) { - return ToGenTxid(*ann); + return ann->m_gtxid; }); return ret; } @@ -654,7 +645,7 @@ public: // found announcement had a different state than CANDIDATE_BEST. If it did, invariants guarantee that no // other CANDIDATE_BEST or REQUESTED can exist. auto it_old = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_BEST, 0}); - if (it_old != m_index.get().end() && it_old->m_txhash == txhash) { + if (it_old != m_index.get().end() && it_old->m_gtxid.ToUint256() == txhash) { if (it_old->GetState() == State::CANDIDATE_BEST) { // The data structure's invariants require that there can be at most one CANDIDATE_BEST or one // REQUESTED announcement per txhash (but not both simultaneously), so we have to convert any @@ -739,7 +730,7 @@ void TxRequestTracker::PostGetRequestableSanityCheck(std::chrono::microseconds n } void TxRequestTracker::ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime) + std::chrono::microseconds reqtime) { m_impl->ReceivedInv(peer, gtxid, preferred, reqtime); } @@ -755,7 +746,7 @@ void TxRequestTracker::ReceivedResponse(NodeId peer, const uint256& txhash) } std::vector TxRequestTracker::GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired) + std::vector>* expired) { return m_impl->GetRequestable(peer, now, expired); } diff --git a/src/txrequest.h b/src/txrequest.h index 98ad43b79aa..084c5ffffea 100644 --- a/src/txrequest.h +++ b/src/txrequest.h @@ -133,7 +133,7 @@ public: * from the specified gtxid. */ void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime); + std::chrono::microseconds reqtime); /** Deletes all announcements for a given peer. * @@ -158,14 +158,13 @@ public: * exists, and for which the specified peer is the best choice among all (reqtime <= now) CANDIDATE * announcements with the same txhash (subject to preferredness rules, and tiebreaking using a deterministic * salted hash of peer and txhash). - * - The selected announcements are converted to GenTxids using their is_wtxid flag, and returned in - * announcement order (even if multiple were added at the same time, or when the clock went backwards while - * they were being added). This is done to minimize disruption from dependent transactions being requested - * out of order: if multiple dependent transactions are announced simultaneously by one peer, and end up - * being requested from them, the requests will happen in announcement order. + * - The selected announcements are returned in announcement order (even if multiple were added at the same + * time, or when the clock went backwards while they were being added). This is done to minimize disruption + * from dependent transactions being requested out of order: if multiple dependent transactions are announced + * simultaneously by one peer, and end up being requested from them, the requests will happen in announcement order. */ std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired = nullptr); + std::vector>* expired = nullptr); /** Marks a transaction as requested, with a specified expiry. * diff --git a/src/util/transaction_identifier.h b/src/util/transaction_identifier.h index c86c8930158..02e8ec077db 100644 --- a/src/util/transaction_identifier.h +++ b/src/util/transaction_identifier.h @@ -9,6 +9,11 @@ #include #include +#include +#include +#include +#include + /** transaction_identifier represents the two canonical transaction identifier * types (txid, wtxid).*/ template @@ -76,4 +81,25 @@ using Txid = transaction_identifier; /** Wtxid commits to all transaction fields including the witness. */ using Wtxid = transaction_identifier; +template +concept TxidOrWtxid = std::is_same_v || std::is_same_v; + +class GenTxid : public std::variant +{ +public: + using variant::variant; + + bool IsWtxid() const { return std::holds_alternative(*this); } + + const uint256& ToUint256() const LIFETIMEBOUND + { + return std::visit([](const auto& id) -> const uint256& { return id.ToUint256(); }, *this); + } + + friend auto operator<=>(const GenTxid& a, const GenTxid& b) + { + return std::tuple(a.IsWtxid(), a.ToUint256()) <=> std::tuple(b.IsWtxid(), b.ToUint256()); + } +}; + #endif // BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H diff --git a/src/validation.cpp b/src/validation.cpp index 3cfcd2728cd..7992b857129 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -321,7 +321,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; @@ -372,7 +372,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}; @@ -807,10 +807,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"); @@ -1135,8 +1135,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn AssertLockHeld(m_pool.cs); // 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()));})); + assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) { return !m_pool.exists(tx->GetHash()); })); assert(txns.size() == workspaces.size()); @@ -1346,8 +1345,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& AssertLockHeld(m_pool.cs); // 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())); })); + assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws) { return !m_pool.exists(ws.m_ptx->GetHash()); })); bool all_submitted = true; FinalizeSubpackage(args); @@ -1472,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()}); @@ -1767,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 @@ -1779,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. // @@ -1798,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 && @@ -1843,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"); @@ -1857,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");