CVE-2023-33297: Improve performance of p2p inv to send queues by ajtowns · Pull Request #27610 · bitcoin/bitcoin

Conversation

We use CompareDepthAndScore to choose an order of txs to inv. Rather than sorting txs that have been evicted from the mempool at the end of the list, sort them at the beginning so they are removed from the queue immediately.

If transactions are being added to the mempool at a rate faster than 7tx/s (INVENTORY_BROADCAST_PER_SECOND) then peers’ inventory_to_send queue can become relatively large. If this happens, increase the number of txids we include in an INV message (normally capped at 35) by 5 for each 1000 txids in the queue.

This will tend to clear a temporary excess out reasonably quickly; an excess of 4000 invs to send will be cleared down to 1000 in about 30 minutes, while an excess of 20000 invs would be cleared down to 1000 in about 60 minutes.

@@ -5666,7 +5666,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

// especially since we have many peers and some will draw much shorter delays.

unsigned int nRelayedTransactions = 0;

LOCK(tx_relay->m_bloom_filter_mutex);

while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {

size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};

@@ -5666,7 +5666,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

// especially since we have many peers and some will draw much shorter delays.

unsigned int nRelayedTransactions = 0;

LOCK(tx_relay->m_bloom_filter_mutex);

while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {

size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};

broadcast_max = std::min<size_t>(1000, broadcast_max);

@@ -5666,7 +5666,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

// especially since we have many peers and some will draw much shorter delays.

unsigned int nRelayedTransactions = 0;

LOCK(tx_relay->m_bloom_filter_mutex);

while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {

size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};

@@ -5666,7 +5666,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

// especially since we have many peers and some will draw much shorter delays.

unsigned int nRelayedTransactions = 0;

LOCK(tx_relay->m_bloom_filter_mutex);

while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {

size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};

broadcast_max = std::min<size_t>(1000, broadcast_max);

@@ -5666,7 +5666,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)

// especially since we have many peers and some will draw much shorter delays.

unsigned int nRelayedTransactions = 0;

LOCK(tx_relay->m_bloom_filter_mutex);

while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {

size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};

@@ -755,11 +755,16 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei

bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid)

{

/* Return `true` if hasha should be considered sooner than hashb. Namely when:

* a is not in the mempool, but b is

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

We use CompareDepthAndScore to choose an order of txs to inv. Rather than sorting txs that have been evicted from the mempool at the end of the list, sort them at the beginning so they are removed from the queue immediately.

Github-Pull: bitcoin#27610 Rebased-From: 228e920

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

If transactions are being added to the mempool at a rate faster than 7tx/s (INVENTORY_BROADCAST_PER_SECOND) then peers’ inventory_to_send queue can become relatively large. If this happens, increase the number of txids we include in an INV message (normally capped at 35) by 5 for each 1000 txids in the queue.

This will tend to clear a temporary excess out reasonably quickly; an excess of 4000 invs to send will be cleared down to 1000 in about 30 minutes, while an excess of 20000 invs would be cleared down to 1000 in about 60 minutes.

Github-Pull: bitcoin#27610 Rebased-From: 5b34060

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

We use CompareDepthAndScore to choose an order of txs to inv. Rather than sorting txs that have been evicted from the mempool at the end of the list, sort them at the beginning so they are removed from the queue immediately.

Github-Pull: bitcoin#27610 Rebased-From: 228e920

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

If transactions are being added to the mempool at a rate faster than 7tx/s (INVENTORY_BROADCAST_PER_SECOND) then peers’ inventory_to_send queue can become relatively large. If this happens, increase the number of txids we include in an INV message (normally capped at 35) by 5 for each 1000 txids in the queue.

This will tend to clear a temporary excess out reasonably quickly; an excess of 4000 invs to send will be cleared down to 1000 in about 30 minutes, while an excess of 20000 invs would be cleared down to 1000 in about 60 minutes.

Github-Pull: bitcoin#27610 Rebased-From: 5b34060

indexed_transaction_set::const_iterator j = wtxid ? get_iter_from_wtxid(hashb) : mapTx.find(hashb);

if (j == mapTx.end()) return true;

if (j == mapTx.end()) return false;

indexed_transaction_set::const_iterator i = wtxid ? get_iter_from_wtxid(hasha) : mapTx.find(hasha);

achow101 added a commit that referenced this pull request

May 11, 2023

97f5e28 doc: update release notes for 24.1rc3 (fanquake) 7e9c7ae doc: update manual pages for v24.1rc3 (fanquake) abb9fa0 build: bump version to v24.1rc3 (fanquake) 128da6e net_processing: Boost inv trickle rate (Anthony Towns) a9a861a txmempool: have CompareDepthAndScore sort missing txs first (Anthony Towns) ec7cd33 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar)

Pull request description:

Backports for rc3. Currently: * #27608 * #27610

ACKs for top commit: josibake: ACK 97f5e28 dergoegge: ACK 97f5e28 achow101: ACK 97f5e28 glozow: ACK 97f5e28 brunoerg: ACK 97f5e28 hebasto: ACK 97f5e28, commits were backported locally, got zero diff.

Tree-SHA512: 09572285ed1e8169d7e77d12ec438586dab54c86064de85d0e743564e601686f884bf74f2bf8ed1be73bddcd7db6da4277c6dd6b9732e7eca383e108f8f37d58

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

We use CompareDepthAndScore to choose an order of txs to inv. Rather than sorting txs that have been evicted from the mempool at the end of the list, sort them at the beginning so they are removed from the queue immediately.

Github-Pull: bitcoin#27610 Rebased-From: 228e920

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request

May 11, 2023

If transactions are being added to the mempool at a rate faster than 7tx/s (INVENTORY_BROADCAST_PER_SECOND) then peers’ inventory_to_send queue can become relatively large. If this happens, increase the number of txids we include in an INV message (normally capped at 35) by 5 for each 1000 txids in the queue.

This will tend to clear a temporary excess out reasonably quickly; an excess of 4000 invs to send will be cleared down to 1000 in about 30 minutes, while an excess of 20000 invs would be cleared down to 1000 in about 60 minutes.

Github-Pull: bitcoin#27610 Rebased-From: 5b34060

fanquake added a commit that referenced this pull request

May 11, 2023

49a2d66 doc: update manual pages for v25.0rc2 (fanquake) 3ea4a11 build: bump version to v25.0rc2 (fanquake) 7ef71e3 net_processing: Boost inv trickle rate (Anthony Towns) 1adbcd3 txmempool: have CompareDepthAndScore sort missing txs first (Anthony Towns) 9a23079 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar)

Pull request description:

Backports for rc2. Currently: * #27608 * #27610

ACKs for top commit: achow101: ACK 49a2d66

Tree-SHA512: a1a7678e16136636ec8a232d12630529639bae3b577769b5a5fd204dda234a5e588f3d4dfebf4d7abe7111d13cc0714f9ccdea0a858fe821a7146e6a697308d3

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request

May 11, 2023

5b34060 net_processing: Boost inv trickle rate (Anthony Towns) 228e920 txmempool: have CompareDepthAndScore sort missing txs first (Anthony Towns)

Pull request description:

Couple of performance improvements when draining the inventory-to-send queue:

* drop txs that have already been evicted from the mempool (or included in a block) immediately, rather than at the end of processing * marginally increase outgoing trickle rate during spikes in tx volume

ACKs for top commit: willcl-ark: ACK 5b34060 instagibbs: ACK bitcoin@5b34060 darosior: utACK 5b34060 glozow: code review ACK 5b34060 dergoegge: utACK 5b34060

Tree-SHA512: 155cd3b5d150ba3417c1cd126f2be734497742e85358a19c9d365f4f97c555ff9e846405bbeada13c3575b3713c3a7eb2f780879a828cbbf032ad9a6e5416b30

@@ -755,11 +755,16 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei

bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid)

{

/* Return `true` if hasha should be considered sooner than hashb. Namely when:

fanquake added a commit that referenced this pull request

May 12, 2023

a26ff20 doc: add initial release notes for v23.2 (fanquake) 60edfd5 doc: update manual pages for v23.2rc1 (fanquake) b93814b doc: update version in bips.md to v23.2 (fanquake) 67bbe6d build: bump version to v23.2rc1 (fanquake) 06731d1 net_processing: Boost inv trickle rate (Anthony Towns) d0a2c87 txmempool: have CompareDepthAndScore sort missing txs first (Anthony Towns) ce8f812 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar)

Pull request description:

Final backports for `rc1`. Currently: * #27608 (not a clean cherry-pick) * #27610 (second commit is not clean)

ACKs for top commit: achow101: ACK a26ff20 dergoegge: ACK a26ff20 ajtowns: utACK a26ff20

Tree-SHA512: 59e43ec4d5004b3543d5c0366c9dc8c5f8a6a777b147628ebc0c03aeb0846312a7780376ebf40f389e3403e4501ba2b70bb97925479670bee13c89e5b6925137