From e1b7490fbc91c9a119d659461fa426810000f0ae Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 2 Jun 2026 16:10:40 +1000 Subject: [PATCH] net_processing: Replace CompareInvMempoolOrder Remove CompareInvMempoolOrder, replacing it with the new ExtractBestByMiningScoreWithTopology. The trickle send code is reworked accordingly. --- src/net_processing.cpp | 92 +++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 55 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b5b2375fe7f..b3e32499781 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -295,11 +295,11 @@ struct Peer { * us or we have announced to the peer. We use this to avoid announcing * the same (w)txid to a peer that already has the transaction. */ CRollingBloomFilter m_tx_inventory_known_filter GUARDED_BY(m_tx_inventory_mutex){50000, 0.000001}; - /** Set of wtxids we still have to announce. For non-wtxid-relay peers, + /** Vector of wtxids we still have to announce. For non-wtxid-relay peers, * we retrieve the txid from the corresponding mempool transaction when * constructing the `inv` message. 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::vector 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. */ @@ -2285,7 +2285,7 @@ void PeerManagerImpl::InitiateTxBroadcastToAll(const Txid& txid, const Wtxid& wt const uint256& hash{peer.m_wtxid_relay ? wtxid.ToUint256() : txid.ToUint256()}; if (!tx_relay->m_tx_inventory_known_filter.contains(hash)) { - tx_relay->m_tx_inventory_to_send.insert(wtxid); + tx_relay->m_tx_inventory_to_send.push_back(wtxid); } } } @@ -5627,22 +5627,6 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi } } -namespace { -class CompareInvMempoolOrder -{ - const CTxMemPool* m_mempool; -public: - explicit CompareInvMempoolOrder(CTxMemPool* mempool) : m_mempool{mempool} {} - - bool operator()(std::set::iterator a, std::set::iterator b) - { - /* As std::make_heap produces a max-heap, we want the entries with the - * higher mining score to sort later. */ - return m_mempool->CompareMiningScoreWithTopology(*b, *a); - } -}; -} // namespace - bool PeerManagerImpl::RejectIncomingTxs(const CNode& peer) const { // block-relay-only peers may never send txs to us @@ -6112,6 +6096,9 @@ bool PeerManagerImpl::SendMessages(CNode& node) tx_relay->m_send_mempool = false; const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()}; + // we'll send everything in the mempool momentarily, so this is redundant + tx_relay->m_tx_inventory_to_send.clear(); + LOCK(tx_relay->m_bloom_filter_mutex); for (const auto& txinfo : vtxinfo) { @@ -6120,7 +6107,6 @@ bool PeerManagerImpl::SendMessages(CNode& node) const auto inv = peer.m_wtxid_relay ? CInv{MSG_WTX, wtxid.ToUint256()} : CInv{MSG_TX, txid.ToUint256()}; - tx_relay->m_tx_inventory_to_send.erase(wtxid); // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { @@ -6140,47 +6126,47 @@ bool PeerManagerImpl::SendMessages(CNode& node) // Determine transactions to relay if (fSendTrickle) { - // Produce a vector with all candidates for sending - 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++) { - 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 even though all items are being processed/sent. - CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool); - std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); - LOCK(tx_relay->m_bloom_filter_mutex); - vInv.reserve(std::min(MAX_INV_SZ, tx_relay->m_tx_inventory_to_send.size())); - while (!vInvTx.empty()) { - // Fetch the top element from the heap - std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); - std::set::iterator it = vInvTx.back(); - vInvTx.pop_back(); - auto wtxid = *it; - // Remove it from the to-be-sent set - tx_relay->m_tx_inventory_to_send.erase(it); - // Not in the mempool anymore? don't bother sending it. - auto txinfo = m_mempool.info(wtxid); - if (!txinfo.tx) { - continue; + // (sorted from higher priority to lowest, skipping low fee) + const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()}; + + auto inv_tx = [&]() EXCLUSIVE_LOCKS_REQUIRED(tx_relay->m_tx_inventory_mutex) { + auto& invs = tx_relay->m_tx_inventory_to_send; + std::vector res; + + if (invs.size() == 0) return res; + + // if previous allocations were excessive, shrink to the current size + if (invs.capacity() > 2 * invs.size()) invs.shrink_to_fit(); + + LOCK(m_mempool.cs); + auto txiters = m_mempool.ExtractBestByMiningScoreWithTopology(invs, invs.size()); + res.reserve(txiters.size()); + for (auto txiter : txiters) { + if (txiter->GetFee() < filterrate.GetFee(txiter->GetTxSize())) { + continue; // higher feerate CPFP txs may follow, so just skip, don't stop + } + res.push_back(txiter->GetSharedTx()); } + // Ensure we'll respond to GETDATA requests for anything we're about to announce + tx_relay->m_last_inv_sequence = m_mempool.GetSequence(); + return res; + }(); + + LOCK(tx_relay->m_bloom_filter_mutex); + vInv.reserve(std::min(MAX_INV_SZ, vInv.size() + inv_tx.size())); + for (auto& tx : inv_tx) { // `TxRelay::m_tx_inventory_known_filter` contains either txids or wtxids // depending on whether our peer supports wtxid-relay. Therefore, first // construct the inv and then use its hash for the filter check. const auto inv = peer.m_wtxid_relay ? - CInv{MSG_WTX, wtxid.ToUint256()} : - CInv{MSG_TX, txinfo.tx->GetHash().ToUint256()}; + CInv{MSG_WTX, tx->GetWitnessHash().ToUint256()} : + CInv{MSG_TX, tx->GetHash().ToUint256()}; // Check if not in the filter already if (tx_relay->m_tx_inventory_known_filter.contains(inv.hash)) { continue; } - // Peer told you to not send transactions at that feerate? Don't bother sending it. - if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { - continue; - } - if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*tx)) continue; // Send vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { @@ -6189,10 +6175,6 @@ bool PeerManagerImpl::SendMessages(CNode& node) } tx_relay->m_tx_inventory_known_filter.insert(inv.hash); } - - // Ensure we'll respond to GETDATA requests for anything we've just announced - LOCK(m_mempool.cs); - tx_relay->m_last_inv_sequence = m_mempool.GetSequence(); } } if (!vInv.empty())