Convert CompareInvMempoolOrder to GenTxidVariant

Now that we are storing `CTxMemPool::CompareDepthAndScore` parameters using
`std::variant` we have no portable zero-overhead way of accessing them,
so use `std::visit` and drop `bool wtxid` in-parameter.

Co-authored-by: stickies-v <stickies-v@protonmail.com>
This commit is contained in:
marcofleon
2025-03-31 17:42:20 +01:00
parent 243553d590
commit eee473d9f3
5 changed files with 30 additions and 33 deletions

View File

@@ -302,7 +302,7 @@ struct Peer {
* non-wtxid-relay peers, wtxid for wtxid-relay peers). We use the
* mempool to sort transactions in dependency order before relay, so
* this does not have to be sorted. */
std::set<uint256> m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex);
std::set<GenTxidVariant> m_tx_inventory_to_send GUARDED_BY(m_tx_inventory_mutex);
/** Whether the peer has requested us to send our complete mempool. Only
* permitted if the peer has NetPermissionFlags::Mempool or we advertise
* NODE_BLOOM. See BIP35. */
@@ -536,7 +536,7 @@ public:
std::vector<TxOrphanage::OrphanTxBase> GetOrphanTransactions() override EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayTransaction(const Txid& txid, const Wtxid& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void SetBestBlock(int height, std::chrono::seconds time) override
{
m_best_height = height;
@@ -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);
}
@@ -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 ? GenTxidVariant{wtxid} : GenTxidVariant{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,
@@ -5442,20 +5442,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<uint256>::iterator a, std::set<uint256>::iterator b)
bool operator()(std::set<GenTxidVariant>::iterator a, std::set<GenTxidVariant>::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
@@ -5773,7 +5768,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).ToVariant());
// Don't send transactions that peers will not put into their mempool
if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
@@ -5794,15 +5789,15 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Determine transactions to relay
if (fSendTrickle) {
// Produce a vector with all candidates for sending
std::vector<std::set<uint256>::iterator> vInvTx;
std::vector<std::set<GenTxidVariant>::iterator> vInvTx;
vInvTx.reserve(tx_relay->m_tx_inventory_to_send.size());
for (std::set<uint256>::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) {
for (std::set<GenTxidVariant>::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.
@@ -5813,14 +5808,15 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
while (!vInvTx.empty() && nRelayedTransactions < broadcast_max) {
// Fetch the top element from the heap
std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
std::set<uint256>::iterator it = vInvTx.back();
std::set<GenTxidVariant>::iterator it = vInvTx.back();
vInvTx.pop_back();
uint256 hash = *it;
CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash);
GenTxidVariant 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.
@@ -5840,7 +5836,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

View File

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

View File

@@ -42,7 +42,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
std::promise<void> promise;
Txid txid = tx->GetHash();
uint256 wtxid = tx->GetWitnessHash();
Wtxid wtxid = tx->GetWitnessHash();
bool callback_set = false;
{

View File

@@ -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 GenTxidVariant& hasha, const GenTxidVariant& hashb) const
{
/* Return `true` if hasha should be considered sooner than hashb. Namely when:
* a is not in the mempool, but b is
@@ -802,9 +802,9 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb
* both are in the mempool and a has a higher score than b
*/
LOCK(cs);
auto j = wtxid ? GetIter(Wtxid::FromUint256(hashb)) : GetIter(Txid::FromUint256(hashb));
auto j{std::visit([&](const auto& id) EXCLUSIVE_LOCKS_REQUIRED(cs) { return GetIter(id); }, hashb)};
if (!j.has_value()) return false;
auto i = wtxid ? GetIter(Wtxid::FromUint256(hasha)) : GetIter(Txid::FromUint256(hasha));
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();

View File

@@ -19,9 +19,10 @@
#include <primitives/transaction.h>
#include <sync.h>
#include <util/epochguard.h>
#include <util/feefrac.h>
#include <util/hasher.h>
#include <util/result.h>
#include <util/feefrac.h>
#include <util/transaction_identifier.h>
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/identity.hpp>
@@ -466,7 +467,7 @@ public:
void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false);
bool CompareDepthAndScore(const GenTxidVariant& hasha, const GenTxidVariant& hashb) const;
bool isSpent(const COutPoint& outpoint) const;
unsigned int GetTransactionsUpdated() const;
void AddTransactionsUpdated(unsigned int n);