net processing: Check if we are in ibd before processing block for txdownloadman
What the developer wrote
Authored by sedited
net processing: Check if we are in ibd before processing block for txdownloadman
This avoids wasting work on calculating bloom filters that aren't
consumed during ibd and continuously re-calculated as now blocks get
validated.
Also update the functional test to document that transactions would now
be requested again once out of IBD.
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
What changed, and why it matters
This commit is a performance optimization, not a security fix. It stops Bitcoin nodes from building unnecessary 'bloom filters' (a data structure used to track recently confirmed transactions) while they are still downloading the historical blockchain (IBD). Previously, the node wasted CPU recalculating these filters for old blocks even though it wasn't accepting new transactions from peers yet. The functional test was updated to reflect that transactions confirmed during IBD will now be requested again once the node is fully synced, because those filters were not built during IBD.
No security action required. Treat as a routine performance and behavior-correctness change. Reviewers may want to confirm that skipping BlockConnected during IBD does not cause any edge cases in transaction reconciliation once IBD completes, but the change is straightforward and well-documented.
Security signals we found
Behavior change in transaction download/re-request logic after IBD
Performance optimization reducing redundant bloom filter computation
Functional test inversion: previously-expected filter membership is now negated
Evidence from the diff
In PeerManagerImpl::BlockConnected, the code previously called m_txdownloadman.BlockConnected() only when the chainstate role was not historical. This commit adds an additional guard: it also skips the call during initial block download (IBD). During IBD, the mempool does not accept incoming transactions from peers, so the recently-confirmed bloom filter maintained by TxDownloadManager is unused and its repeated recomputation per block is wasted work. The functional test p2p_ibd_txrelay.py was updated to assert the opposite behavior: a transaction confirmed during IBD is NOT in the recently-confirmed filter after IBD, so the node will request it again if announced.
Changed components
src/net_processing.cpptest/functional/p2p_ibd_txrelay.pyTxDownloadManager block-connected processingIBD transaction relay logicInspect captured patch +7 / −9
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 16b4735e..0ec1103e 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2054,12 +2054,12 @@ void PeerManagerImpl::BlockConnected(
}
// The following task can be skipped since we don't maintain a mempool for
- // the historical chainstate.
- if (role.historical) {
- return;
+ // the historical chainstate, or during ibd since we don't receive incoming
+ // transactions from peers into the mempool.
+ if (!role.historical && !m_chainman.IsInitialBlockDownload()) {
+ LOCK(m_tx_download_mutex);
+ m_txdownloadman.BlockConnected(pblock);
}
- LOCK(m_tx_download_mutex);
- m_txdownloadman.BlockConnected(pblock);
}
void PeerManagerImpl::BlockDisconnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex* pindex)
diff --git a/test/functional/p2p_ibd_txrelay.py b/test/functional/p2p_ibd_txrelay.py
index a622554a..fe70d046 100755
--- a/test/functional/p2p_ibd_txrelay.py
+++ b/test/functional/p2p_ibd_txrelay.py
@@ -89,13 +89,11 @@ class P2PIBDTxRelayTest(BitcoinTestFramework):
assert not node.getblockchaininfo()['initialblockdownload']
self.wait_until(lambda: all(peer['minfeefilter'] == NORMAL_FEE_FILTER for peer in node.getpeerinfo()))
- self.log.info("Check that txs confirmed during IBD are in the recently-confirmed filter once out of ibd")
+ self.log.info("Check that txs confirmed during IBD are not in the recently-confirmed filter once out of ibd")
peer_inver = self.nodes[0].add_p2p_connection(P2PDataStore())
peer_inver.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=ibd_wtxid)]))
self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
- peer_inver.sync_with_ping()
- with p2p_lock:
- assert ibd_wtxid not in peer_inver.getdata_requests
+ peer_inver.wait_for_getdata([ibd_wtxid])
self.nodes[0].disconnect_p2ps()
self.log.info("Check that nodes process the same transaction, even when unsolicited, when no longer in IBD")
Why this scored 21/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.