This is part of the libbitcoinkernel project: #24303, https://github.com/bitcoin/bitcoin/projects/18
This PR reduces the number of call sites where we explicitly construct CTxMemPool. This is done in preparation for later PRs which decouple the mempool module from ArgsManager, eventually all of libbitcoinkernel will be decoupled from ArgsManager.
The changes in this PR:
- Allows us to have less code churn as we modify
CTxMemPool’s constructor in later PRs - In many cases, we can make use of existing
CTxMemPoolinstances, getting rid of extraneous constructions - In other cases, we construct a
ChainTestingSetupand use theCTxMemPoolthere, so that we can rely on the logic insetup_commonto set things up correctly
Notes for Reviewers
A note on using existing mempools
When evaluating whether or not it’s appropriate to use an existing mempool in a *TestingSetup struct, the key is to make sure that the mempool has the same lifetime as the *TestingSetup struct.
Example 1: In src/fuzz/tx_pool.cpp, the TestingSetup is initialized in initialize_tx_pool and lives as a static global, while the CTxMemPool is in the tx_pool_standard fuzz target, meaning that each time the tx_pool_standard fuzz target gets run, a new CTxMemPool is created. If we were to use the static global TestingSetup’s CTxMemPool we might run into problems since its CTxMemPool will carry state between subsequent runs. This is why we don’t modify src/fuzz/tx_pool.cpp in this PR.
Example 2: In src/bench/mempool_eviction.cpp, we see that the TestingSetup is in the same scope as the constructed CTxMemPool, so it is safe to use its CTxMemPool.
A note on checking CTxMemPool ctor call sites
After the “tree-wide: clang-format CTxMemPool references” commit, you can find all CTxMemPool ctor call sites with the following command:
0git grep -E -e 'make_unique<CTxMemPool>' \
1 -e '\bCTxMemPool\s+[^({;]+[({]' \
2 -e '\bCTxMemPool\s+[^;]+;' \
3 -e '\bnew\s+CTxMemPool\b'
At the end of the PR, you will find that there are still quite a few call sites that we can seemingly get rid of:
0$ git grep -E -e 'make_unique<CTxMemPool>' -e '\bCTxMemPool\s+[^({;]+[({]' -e '\bCTxMemPool\s+[^;]+;' -e '\bnew\s+CTxMemPool\b'
1# rearranged for easier explication
2src/init.cpp: node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), mempool_check_ratio);
3src/test/util/setup_common.cpp: m_node.mempool = std::make_unique<CTxMemPool>(m_node.fee_estimator.get(), 1);
4src/rpc/mining.cpp: CTxMemPool empty_mempool;
5src/test/util/setup_common.cpp: CTxMemPool empty_pool;
6src/bench/mempool_stress.cpp: CTxMemPool pool;
7src/bench/mempool_stress.cpp: CTxMemPool pool;
8src/test/fuzz/rbf.cpp: CTxMemPool pool;
9src/test/fuzz/tx_pool.cpp: CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1};
10src/test/fuzz/tx_pool.cpp: CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1};
11src/test/fuzz/validation_load_mempool.cpp: CTxMemPool pool{};
12src/txmempool.h: /** Create a new CTxMemPool.
Let’s break them down one by one:
0src/init.cpp: node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), mempool_check_ratio);
1src/test/util/setup_common.cpp: m_node.mempool = std::make_unique<CTxMemPool>(m_node.fee_estimator.get(), 1);
Necessary
0src/rpc/mining.cpp: CTxMemPool empty_mempool;
1src/test/util/setup_common.cpp: CTxMemPool empty_pool;
These are fixed in #25223 where we stop requiring the BlockAssembler to have a CTxMemPool if it’s not going to consult it anyway (as is the case in these two call sites)
0src/bench/mempool_stress.cpp: CTxMemPool pool;
1src/bench/mempool_stress.cpp: CTxMemPool pool;
Fixed in #24927.
0src/test/fuzz/rbf.cpp: CTxMemPool pool;
1src/test/fuzz/tx_pool.cpp: CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1};
2src/test/fuzz/tx_pool.cpp: CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1};
3src/test/fuzz/validation_load_mempool.cpp: CTxMemPool pool{};
These are all cases where we don’t want the CTxMemPool state to persist between runs, see the previous section “A note on using existing mempools”
0src/txmempool.h: /** Create a new CTxMemPool.
It’s a comment (someone link me to a grep that understands syntax plz thx)
