Merge bitcoin#34054: net processing: Add ibd check before processing block for txdownloadman

e5f0613 net processing: Check if we are in ibd before processing block for txdownloadman (sedited)
ce8b692 Add functional test exercising tx downloadman recently confirmed filter (Lőrinc)

Pull request description:

  Calculating the rolling bloom filters for the txorphanage takes some CPU time from the scheduler thread. This can be observed for example in [this flamegraph](https://bitcoin-dev-tools.github.io/benchcoin/results/pr-172/20066462508/mainnet-default-instrumented-base-flamegraph.svg?x=920203898521&y=780), where handling the filter takes about 2.6% of total time (and most of the scheduler thread's time).

  During ibd the entries in the tx download bloom filter are just continuously rolled over and aren't consumed, since no mempool entries are created by incoming transactions from peers during ibd. The mempool does accept transactions via RPC, or the wallet at the time, however these don't interact with the orphanage and the txdownloadman, because adding anything to those is guarded by IsInitialBlockDownload() checks as well.

  We're usually latching ibd to false a few blocks before catching up to the tip, so this should also not significantly degrade the performance of the filter once fully caught up.

ACKs for top commit:
  l0rinc:
    ACK e5f0613
  instagibbs:
    ACK e5f0613
  fjahr:
    Code review ACK e5f0613

Tree-SHA512: d667e677f5723c438cdf5b34f0f9c1ade7cc1b2e98530c23f14384514daa38217c4e7c3b756194b6831b590a487449c4514b52bf0fb461ae8083061722824270

Author: fanquake

src/net_processing.cpp

@@ -2084,12 +2084,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)

test/functional/p2p_ibd_txrelay.py

@@ -11,6 +11,7 @@
 from decimal import Decimal
 import time
 
+from test_framework.blocktools import create_block, create_coinbase
 from test_framework.messages import (
         CInv,
         COIN,
@@ -48,6 +49,12 @@ def run_test(self):
             self.wait_until(lambda: all(peer['minfeefilter'] == MAX_FEE_FILTER for peer in node.getpeerinfo()))
 
         self.nodes[0].setmocktime(int(time.time()))
+        self.log.info("Mine one old block so we stay in IBD, then remember its coinbase wtxid")
+        block = create_block(int(self.nodes[0].getbestblockhash(), 16), create_coinbase(1), int(time.time()) - 2 * 24 * 60 * 60)
+        block.solve()
+        self.nodes[0].submitblock(block.serialize().hex())
+        assert self.nodes[0].getblockchaininfo()['initialblockdownload']
+        ibd_wtxid = int(self.nodes[0].getblock(f"{block.hash_int:064x}", 2)["tx"][0]["hash"], 16)
 
         self.log.info("Check that nodes don't send getdatas for transactions while still in IBD")
         peer_inver = self.nodes[0].add_p2p_connection(P2PDataStore())
@@ -82,6 +89,13 @@ def run_test(self):
             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 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.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")
         peer_txer = self.nodes[0].add_p2p_connection(P2PInterface())
         with self.nodes[0].assert_debug_log(expected_msgs=["was not accepted"]):