p2p: Replace per-peer transaction rate-limiting with global rate limits

ajtowns commented at 3:09 am on February 20, 2026: contributor

Per-peer m_tx_inventory_to_send queues have CPU and memory costs that scale with both queue size and peer count. Under high transaction volume, this has previously caused severe issues (May 2023 disclosure) and still can cause measurable delays (Feb 2026 Runestone surge, with the msghand thread observed hitting 100% CPU and queue memory reaching ~95MB).

This PR replaces the per-peer rate limiting with a global queue using dual token buckets (limiting transaction by both count and serialized size). Transactions that arrive within the bucket capacity still relay nearly immediately, but excess transactions queue in a global backlog and drain as the token buckets refill.

Key parameters:

Count bucket: 14 tx/s, 420 capacity (30s buffer)
Size bucket: 20 kB/s (~12 MB/600s), 50 MB capacity
Outbound peers refill faster by a factor of 2.5

Per-peer queues are retained solely for privacy batching and are always fully emptied, removing the old INVENTORY_BROADCAST_MAX cap.

This reduces the memory and CPU burden during transaction spikes when the queuing logic is engaged from O(queue * peers) to O(queue), as the queued transactions no longer need to be retained per-peer or re-sorted per-peer.

Design discussion: https://gist.github.com/ajtowns/d61bea974a07190fa6c6c8eaef3638b9

DrahtBot added the label P2P on Feb 20, 2026

DrahtBot commented at 3:10 am on February 20, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	0xB10C

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34824 (net: refactor: replace Peer::TxRelay RecursiveMutex instances with Mutex by w0xlt)
#34435 (refactor: use _MiB/_GiB consistently for byte conversions by l0rinc)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Feb 20, 2026

ajtowns force-pushed on Feb 20, 2026

ajtowns commented at 12:43 pm on February 20, 2026: contributor

CI failure is presumably either #34631 or #34387

DrahtBot removed the label CI failed on Feb 24, 2026

in src/util/tokenbucket.h:57 in 869a1ae012 outdated

52+    }
53+
54+    /** Consume n tokens. Returns false if the balance dropped below m_max_debt. */
55+    bool decrement(double n = 1.0)
56+    {
57+        m_value -= n;

chriszeng1010 commented at 5:33 pm on March 2, 2026:

Decrement can still go below m_max_debt before checking is complete.

ajtowns commented at 12:57 pm on March 4, 2026:

decrement() can always go below m_max_debt, it only reports when it has done so – it leaves it up to the caller to not go further into debt.

DrahtBot added the label Needs rebase on Mar 11, 2026

ajtowns force-pushed on Mar 12, 2026

DrahtBot removed the label Needs rebase on Mar 12, 2026

0xB10C commented at 9:45 am on March 12, 2026: contributor

Concept ACK!

I’ve been running this for a few days now and written down a few observations on a small mass-broadcast event that happend a few hours ago: https://bnoc.xyz/t/increased-b-msghand-thread-utilization-due-to-runestone-transactions-on-2026-02-17/81/11

The node with this patch was significantly less affected than the others running a recent master.

I haven’t set up any monitoring for the newly added getnetworkinfo fields yet.

DrahtBot added the label CI failed on Mar 12, 2026

DrahtBot closed this on Mar 12, 2026

DrahtBot reopened this on Mar 12, 2026

ajtowns added this to the milestone 32.0 on Mar 12, 2026

DrahtBot removed the label CI failed on Mar 12, 2026

in src/txmempool.cpp:541 in 344de4b8dd outdated

537@@ -538,6 +538,55 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
538         for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout);
539         AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max());
540     }
541+

sipa commented at 6:22 pm on March 20, 2026:

In commit “txmempool: Add SortMiningScoreWithTopology”

This feels more like something for a fuzz or unit test. CTxMemPool::check is for internal consistency checks in the CTxMemPool representation, I feel.

ajtowns commented at 1:20 am on March 21, 2026:

…. I might have spent too much time vibecoding and caught hallucinations? I could have sworn I was replacing existing code here. EDIT: Dropped this code.

in src/txmempool.cpp:605 in 344de4b8dd outdated

601@@ -553,6 +602,27 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
602     assert(innerUsage == cachedInnerUsage);
603 }
604 
605+std::vector<CTxMemPool::txiter> CTxMemPool::SortMiningScoreWithTopology(std::span<const Wtxid> wtxids, size_t n) const

sipa commented at 6:46 pm on March 20, 2026:

It looks like both eventual production call sites of this function (BumpInvVecForProcessing and PeerManagerImpl::SendMessages) do a deduplication pass on the results.

Would it make sense to do this on the fly inside this function? It can’t use std::partial_sort anymore, but it can use std::make_heap and friends to implement partial sorting, with a dynamic end point until n distinct elements have been found? Something like

 0std::vector<CTxMemPool::txiter> CTxMemPool::SortMiningScoreWithTopology(std::span<const Wtxid> wtxids, size_t n) const
 1{
 2    auto cmp = [&](const auto& a, const auto& b) EXCLUSIVE_LOCKS_REQUIRED(cs) noexcept { return m_txgraph->CompareMainOrder(*a, *b) > 0; };
 3
 4    std::vector<txiter> res;
 5
 6    n = std::min(wtxids.size(), n);
 7    if (n > 0) {
 8        // Construct a heap with txiters for all wtxids that exist in the mempool.
 9        std::vector<txiter> heap;
10        heap.reserve(wtxids.size());
11        for (auto& wtxid : wtxids) {
12            if (auto i{GetIter(wtxid)}; i.has_value()) {
13                heap.push_back(i.value());
14            }
15        }
16        std::ranges::make_heap(heap, cmp);
17
18        // Pop transactions until n distinct ones in res have been found.
19        res.reserve(heap.size());
20        while (res.size() < n && !heap.empty()) {
21            std::ranges::pop_heap(heap, cmp);
22            if (res.empty() || heap.back() != res.back()) {
23                res.push_back(heap.back());
24            }
25            heap.pop_back();
26        }
27
28        // Copy the remainder over, without sorting or deduplication.
29        res.insert(res.end(), heap.begin(), heap.end());
30    }
31
32    return res;
33}

With even more low-level code the duplicate vector can be avoided, I think. Tests don’t pass with this, I haven’t investigated why.

ajtowns commented at 1:14 am on March 21, 2026:

Without having looked, is the comparison backwards?

My understanding is partial_sort has two benefits:

it only makes a heap out of the target size, so iterates through the source array once and then does log(k) work for each element, with better locality
when updating the k elements in the heap with a new element from the source, it does the sift-down algorithm which is more efficient than heap_push()/heap_pop(), but isn’t exposed via the STL so would mean writing your own heap implementation

Deduping the fairly small output list as you pass through it, when duplicates are rare anyway, seemed fine to me?

sipa commented at 2:20 pm on March 21, 2026:

Without having looked, is the comparison backwards?

I don’t think so. It’s a max heap, but I want to pop the “lowest” elements off first, so I needed to swap the comparator I think.

My understanding is partial_sort has two benefits:

Interesting. So it has complexity O(n log m) (with n = number of elements, m = sorted prefix size), while the approach I have in mind is O(n + (m log n)) (O(n) to construct the heap of all elements, and then m operations of each O(log n) to extract the best m elements. Complexity-wise, my approach seems better, since n > m, except it has worse memory locality. This makes me wonder if I’m missing something, since the cppreference.cpp documentation seems to imply std::partial_sort is intended for low m values.

Deduping the fairly small output list as you pass through it, when duplicates are rare anyway, seemed fine to me?

Yeah, it probably doesn’t matter that much. It just looked like the deduplication is something that CTxMemPool::SortMiningScoreWithTopology could do internally since both call sites need it anyway. And then it seemed possible to have the count be dynamic, but only when using a different approach than what std::partial_sort seems to enable.

ajtowns commented at 0:58 am on March 22, 2026:

Complexity-wise, my approach seems better, since n > m, except it has worse memory locality.

Yeah. I think the ratio between the number of swaps each approach performs in the worst case is log2(m) : 2 – if you give the input in exactly the wrong order, each element goes to the top of the heap with log(m) steps, whereas for the full heap, it adds up to 2. So for m~=100, that’s 3.3x more swaps, but the swaps are contained to a set of 100 elements, which might get you more than a 3.3x speedup per-swap due to locality? (In the average case, for most elements you’ll just compare to the top of heap element, find it’s worse and do 0 swaps, and overall it reduces from O(n log(m) + m log(m)) to O(n + m log(m)) which is better than the O(n + m log(n)) from the full heap.

Oh, hmm; in the per-peer thread we’re always taking everything, so I think there we should be explicitly using std::sort then and not any of this partial business anyway (pushed). That should ensure that the m sizes we’re using in practice are always about 70 (-txsendrate times inbound broadcast interval).

in src/txmempool.cpp:483 in ea647debfd

480@@ -481,10 +481,10 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
481         const CTransaction& tx = it->GetTx();
482 
483         // CompareMiningScoreWithTopology should agree with GetSortedScoreWithTopology()

sipa commented at 7:46 pm on March 20, 2026:

In commit “txmempool: Drop CompareMiningScoreWithTopology”

Comment is outdated now.

in src/util/tokenbucket.h:16 in 859e8eb020

11+
12+/** A token bucket rate limiter.
13+ *
14+ * Tokens are added at a steady rate (m_rate per second) up to a capacity
15+ * cap (m_cap). Tokens are removed by calling decrement(). The balance
16+ * may go negative down to m_max_debt; decrement() returns false when

sipa commented at 7:54 pm on March 20, 2026:

In commit “util/tokenbucket.h: Provide a generic TokenBucket class”

Is it useful to support debt? I believe it can be avoided by a transformation that raises both m_value and m_cap by -m_max_debt.

ajtowns commented at 4:04 am on March 21, 2026:

The distinction is in InvToSendBucket::avail() which says “you can start doing stuff as long as the size_bucket’s value is >=0”, which would have to get incremented as well to be equivalent.

The main effect is that when the size bucket is under pressure, you get a chance to relay at least 50kB each iteration, rather than the avail test passing as soon as you can relay 1B, then the loop ending immediately after you relay the first tx.

I think it can be simplified a bit by moving the max_debt value to just being a parameter of decrement() though. Will update.

ajtowns force-pushed on Mar 21, 2026

ajtowns force-pushed on Mar 22, 2026

net_processing: Pass full tx to InitiateTxBroadcastToAll()

All callers already have the full transaction available, so just pass that
through. This matches the signature for InitiateTxBroadcastPrivate()
and prepares for a later commit that needs the full transaction to
compute its serialized size for rate limiting.

da068ebd83

net_processing: Remove per-peer rate-limiting

Per-peer rate limiting introduces storage and compute costs proportional
to the number of peers. This has caused severe bugs in the past, and
continues to be a risk in the event of periods of extremely high rates
of transaction submission. Avoid these problems by always completely
emptying the m_tx_inventory_to_send queue when processing it.

Note that this increases the potential size of INV messages we send
for normal tx relay from ~1000 (limited by INVENTORY_BROADCAST_MAX)
to potentially 50000 (limited by MAX_INV_SZ).

d422448a4c

txmempool: Add SortMiningScoreWithTopology

Add a method for sorting a batch of transactions (specified as a vector
of wtxids) per mempool order, designed for transaction relay.

eea5b82494

net_processing: Change m_tx_inventory_to_send from set to vector

Change the per-peer tx relay queue from std::set to std::vector. This
reduces the memory usage and improves locality, at the cost of not
automatically deduping entries.

40b7d8d475

txmempool: Drop CompareMiningScoreWithTopology

Now unused; replaced by SortMiningScoreWithTopology.

c3b803eea2

util/tokenbucket.h: Provide a generic TokenBucket class

This is a simple token bucket parameterized on clock type, used in the
following commit.

35ddc0cc76

net_processing: add a global delay queue for sending txs

Without the per-peer rate limiting, nodes can act as an amplifier for
transaction spam -- receiving many transactions from one node, but
relaying each of them to over 100 other nodes. Limit the impact of this
by providing a global rate limit.

This is implemented using dual token buckets, one that consumes a
token for every transaction, and one that consumes a token for every
serialized byte. This rate limits both per-tx resource usage (eg INV
messages) and overall relay bandwidth.

Main bucket parameters:
 * Count: 14tx/s rate, 420tx (30s) capacity
 * Size: 12MB/600s rate (4-6 blocks per target block interval), 50MB capacity

The size bucket is expected to be large enough to almost never have an
impact in normal usage, even during transaction storms, and is primarily
intended to mitigate attack-like scenarios.

Outbound connections get a separate pair of buckets, with rates boosted
by a 2.5x multiplier.

This avoids the excessive memory and CPU usage due to the 100x multiplier
from the queues being per-peer.

Note that this also reduces the size of INV messages we send for general
tx relay back to a more reasonable level of under 600 txs in 99.999%
of cases.

5cb4a1485e

init: add -txsendrate configuration parameter

Adds a debug-only configuration option to set the target
transaction/second rate for relay to inbound connections. This is mostly
intended to be set to artificially low values to aid in testing behaviour
when a backlog occurs, but is also available in case the default 14tx/s
target is somehow too low in practice.

f47f7382a3

rpc: report -txsendrate and bucket info via getnetworkinfo

Add `tx_send_rate` and `inv_buckets` fields to getnetworkinfo. The latter
has `inbound` and `outbound` entries, reporting reports backlog count,
count tokens, and size tokens. Useful for monitoring relay behavior.

1b04771351

tests: basic functional test for tx rate limiting ba3a81d036

ajtowns force-pushed on Mar 22, 2026

DrahtBot added the label CI failed on Mar 22, 2026

DrahtBot removed the label CI failed on Mar 22, 2026

p2p: Replace per-peer transaction rate-limiting with global rate limits #34628

Reviews

Conflicts