Skip to content

Commit faa92a2

Browse files
author
MarcoFalke
committed
rpc: Remove mempool global from miner
1 parent 6666ef1 commit faa92a2

File tree

13 files changed

+92
-53
lines changed

13 files changed

+92
-53
lines changed

‎src/bench/bench.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#include <numeric>
1616
#include <regex>
1717

18+
const RegTestingSetup* g_testing_setup = nullptr;
19+
1820
void benchmark::ConsolePrinter::header()
1921
{
2022
std::cout << "# Benchmark, evals, iterations, total, min, max, median" << std::endl;
@@ -113,6 +115,8 @@ void benchmark::BenchRunner::RunAll(Printer& printer, uint64_t num_evals, double
113115

114116
for (const auto& p : benchmarks()) {
115117
RegTestingSetup test{};
118+
assert(g_testing_setup == nullptr);
119+
g_testing_setup = &test;
116120
{
117121
LOCK(cs_main);
118122
assert(::ChainActive().Height() == 0);
@@ -133,6 +137,7 @@ void benchmark::BenchRunner::RunAll(Printer& printer, uint64_t num_evals, double
133137
p.second.func(state);
134138
}
135139
printer.result(state);
140+
g_testing_setup = nullptr;
136141
}
137142

138143
printer.footer();

‎src/bench/bench.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
#include <boost/preprocessor/cat.hpp>
1515
#include <boost/preprocessor/stringize.hpp>
1616

17+
struct RegTestingSetup;
18+
extern const RegTestingSetup* g_testing_setup; //!< A pointer to the current testing setup
19+
1720
// Simple micro-benchmarking framework; API mostly matches a subset of the Google Benchmark
1821
// framework (see https://github.com/google/benchmark)
1922
// Why not use the Google Benchmark framework? Because adding Yet Another Dependency

‎src/bench/block_assemble.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <consensus/validation.h>
77
#include <crypto/sha256.h>
88
#include <test/util/mining.h>
9+
#include <test/util/setup_common.h>
910
#include <test/util/wallet.h>
1011
#include <txmempool.h>
1112
#include <validation.h>
@@ -29,7 +30,7 @@ static void AssembleBlock(benchmark::State& state)
2930
std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
3031
for (size_t b{0}; b < NUM_BLOCKS; ++b) {
3132
CMutableTransaction tx;
32-
tx.vin.push_back(MineBlock(SCRIPT_PUB));
33+
tx.vin.push_back(MineBlock(g_testing_setup->m_node, SCRIPT_PUB));
3334
tx.vin.back().scriptWitness = witness;
3435
tx.vout.emplace_back(1337, SCRIPT_PUB);
3536
if (NUM_BLOCKS - b >= COINBASE_MATURITY)
@@ -46,7 +47,7 @@ static void AssembleBlock(benchmark::State& state)
4647
}
4748

4849
while (state.KeepRunning()) {
49-
PrepareBlock(SCRIPT_PUB);
50+
PrepareBlock(g_testing_setup->m_node, SCRIPT_PUB);
5051
}
5152
}
5253

‎src/bench/wallet_balance.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <node/context.h>
88
#include <optional.h>
99
#include <test/util/mining.h>
10+
#include <test/util/setup_common.h>
1011
#include <test/util/wallet.h>
1112
#include <validationinterface.h>
1213
#include <wallet/wallet.h>
@@ -29,8 +30,8 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b
2930
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
3031

3132
for (int i = 0; i < 100; ++i) {
32-
generatetoaddress(address_mine.get_value_or(ADDRESS_WATCHONLY));
33-
generatetoaddress(ADDRESS_WATCHONLY);
33+
generatetoaddress(g_testing_setup->m_node, address_mine.get_value_or(ADDRESS_WATCHONLY));
34+
generatetoaddress(g_testing_setup->m_node, ADDRESS_WATCHONLY);
3435
}
3536
SyncWithValidationInterfaceQueue();
3637

‎src/miner.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ BlockAssembler::Options::Options() {
4545
nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
4646
}
4747

48-
BlockAssembler::BlockAssembler(const CChainParams& params, const Options& options) : chainparams(params)
48+
BlockAssembler::BlockAssembler(const CTxMemPool& mempool, const CChainParams& params, const Options& options)
49+
: chainparams(params),
50+
m_mempool(mempool)
4951
{
5052
blockMinFeeRate = options.blockMinFeeRate;
5153
// Limit weight to between 4K and MAX_BLOCK_WEIGHT-4K for sanity:
@@ -67,7 +69,8 @@ static BlockAssembler::Options DefaultOptions()
6769
return options;
6870
}
6971

70-
BlockAssembler::BlockAssembler(const CChainParams& params) : BlockAssembler(params, DefaultOptions()) {}
72+
BlockAssembler::BlockAssembler(const CTxMemPool& mempool, const CChainParams& params)
73+
: BlockAssembler(mempool, params, DefaultOptions()) {}
7174

7275
void BlockAssembler::resetBlock()
7376
{
@@ -103,7 +106,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
103106
pblocktemplate->vTxFees.push_back(-1); // updated at end
104107
pblocktemplate->vTxSigOpsCost.push_back(-1); // updated at end
105108

106-
LOCK2(cs_main, mempool.cs);
109+
LOCK2(cs_main, m_mempool.cs);
107110
CBlockIndex* pindexPrev = ::ChainActive().Tip();
108111
assert(pindexPrev != nullptr);
109112
nHeight = pindexPrev->nHeight + 1;
@@ -236,7 +239,7 @@ int BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& already
236239
int nDescendantsUpdated = 0;
237240
for (CTxMemPool::txiter it : alreadyAdded) {
238241
CTxMemPool::setEntries descendants;
239-
mempool.CalculateDescendants(it, descendants);
242+
m_mempool.CalculateDescendants(it, descendants);
240243
// Insert all descendants (not yet in block) into the modified set
241244
for (CTxMemPool::txiter desc : descendants) {
242245
if (alreadyAdded.count(desc))
@@ -268,7 +271,7 @@ int BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& already
268271
// cached size/sigops/fee values that are not actually correct.
269272
bool BlockAssembler::SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx)
270273
{
271-
assert (it != mempool.mapTx.end());
274+
assert(it != m_mempool.mapTx.end());
272275
return mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it);
273276
}
274277

@@ -305,7 +308,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
305308
// and modifying them for their already included ancestors
306309
UpdatePackagesForAdded(inBlock, mapModifiedTx);
307310

308-
CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = mempool.mapTx.get<ancestor_score>().begin();
311+
CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = m_mempool.mapTx.get<ancestor_score>().begin();
309312
CTxMemPool::txiter iter;
310313

311314
// Limit the number of attempts to add transactions to the block when it is
@@ -314,11 +317,10 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
314317
const int64_t MAX_CONSECUTIVE_FAILURES = 1000;
315318
int64_t nConsecutiveFailed = 0;
316319

317-
while (mi != mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty())
318-
{
320+
while (mi != m_mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty()) {
319321
// First try to find a new transaction in mapTx to evaluate.
320-
if (mi != mempool.mapTx.get<ancestor_score>().end() &&
321-
SkipMapTxEntry(mempool.mapTx.project<0>(mi), mapModifiedTx, failedTx)) {
322+
if (mi != m_mempool.mapTx.get<ancestor_score>().end() &&
323+
SkipMapTxEntry(m_mempool.mapTx.project<0>(mi), mapModifiedTx, failedTx)) {
322324
++mi;
323325
continue;
324326
}
@@ -328,13 +330,13 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
328330
bool fUsingModified = false;
329331

330332
modtxscoreiter modit = mapModifiedTx.get<ancestor_score>().begin();
331-
if (mi == mempool.mapTx.get<ancestor_score>().end()) {
333+
if (mi == m_mempool.mapTx.get<ancestor_score>().end()) {
332334
// We're out of entries in mapTx; use the entry from mapModifiedTx
333335
iter = modit->iter;
334336
fUsingModified = true;
335337
} else {
336338
// Try to compare the mapTx entry to the mapModifiedTx entry
337-
iter = mempool.mapTx.project<0>(mi);
339+
iter = m_mempool.mapTx.project<0>(mi);
338340
if (modit != mapModifiedTx.get<ancestor_score>().end() &&
339341
CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) {
340342
// The best entry in mapModifiedTx has higher score
@@ -389,7 +391,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
389391
CTxMemPool::setEntries ancestors;
390392
uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
391393
std::string dummy;
392-
mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
394+
m_mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
393395

394396
onlyUnconfirmed(ancestors);
395397
ancestors.insert(iter);

‎src/miner.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ class BlockAssembler
147147
int nHeight;
148148
int64_t nLockTimeCutoff;
149149
const CChainParams& chainparams;
150+
const CTxMemPool& m_mempool;
150151

151152
public:
152153
struct Options {
@@ -155,8 +156,8 @@ class BlockAssembler
155156
CFeeRate blockMinFeeRate;
156157
};
157158

158-
explicit BlockAssembler(const CChainParams& params);
159-
BlockAssembler(const CChainParams& params, const Options& options);
159+
explicit BlockAssembler(const CTxMemPool& mempool, const CChainParams& params);
160+
explicit BlockAssembler(const CTxMemPool& mempool, const CChainParams& params, const Options& options);
160161

161162
/** Construct a new block template with coinbase to scriptPubKeyIn */
162163
std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn);
@@ -175,7 +176,7 @@ class BlockAssembler
175176
/** Add transactions based on feerate including unconfirmed ancestors
176177
* Increments nPackagesSelected / nDescendantsUpdated with corresponding
177178
* statistics from the package selection (for logging statistics). */
178-
void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
179+
void addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
179180

180181
// helper functions for addPackageTxs()
181182
/** Remove confirmed (inBlock) entries from given set */
@@ -189,13 +190,13 @@ class BlockAssembler
189190
bool TestPackageTransactions(const CTxMemPool::setEntries& package);
190191
/** Return true if given transaction from mapTx has already been evaluated,
191192
* or if the transaction's cached data in mapTx is incorrect. */
192-
bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
193+
bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set& mapModifiedTx, CTxMemPool::setEntries& failedTx) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
193194
/** Sort the package in an order that is valid to appear in a block */
194195
void SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries);
195196
/** Add descendants of given transactions to mapModifiedTx with ancestor
196197
* state updated assuming given transactions are inBlock. Returns number
197198
* of updated descendants. */
198-
int UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set &mapModifiedTx) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
199+
int UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set& mapModifiedTx) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
199200
};
200201

201202
/** Modify the extranonce in a block */

‎src/rpc/mining.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request)
102102
return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1);
103103
}
104104

105-
static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries)
105+
static UniValue generateBlocks(const CTxMemPool& mempool, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries)
106106
{
107107
int nHeightEnd = 0;
108108
int nHeight = 0;
@@ -116,7 +116,7 @@ static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, ui
116116
UniValue blockHashes(UniValue::VARR);
117117
while (nHeight < nHeightEnd && !ShutdownRequested())
118118
{
119-
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(Params()).CreateNewBlock(coinbase_script));
119+
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(mempool, Params()).CreateNewBlock(coinbase_script));
120120
if (!pblocktemplate.get())
121121
throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
122122
CBlock *pblock = &pblocktemplate->block;
@@ -179,9 +179,11 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request)
179179
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys"));
180180
}
181181

182+
const CTxMemPool& mempool = EnsureMemPool();
183+
182184
CHECK_NONFATAL(coinbase_script.size() == 1);
183185

184-
return generateBlocks(coinbase_script.at(0), num_blocks, max_tries);
186+
return generateBlocks(mempool, coinbase_script.at(0), num_blocks, max_tries);
185187
}
186188

187189
static UniValue generatetoaddress(const JSONRPCRequest& request)
@@ -215,9 +217,11 @@ static UniValue generatetoaddress(const JSONRPCRequest& request)
215217
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Error: Invalid address");
216218
}
217219

220+
const CTxMemPool& mempool = EnsureMemPool();
221+
218222
CScript coinbase_script = GetScriptForDestination(destination);
219223

220-
return generateBlocks(coinbase_script, nGenerate, nMaxTries);
224+
return generateBlocks(mempool, coinbase_script, nGenerate, nMaxTries);
221225
}
222226

223227
static UniValue getmininginfo(const JSONRPCRequest& request)
@@ -548,7 +552,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
548552

549553
// Create new block
550554
CScript scriptDummy = CScript() << OP_TRUE;
551-
pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy);
555+
pblocktemplate = BlockAssembler(mempool, Params()).CreateNewBlock(scriptDummy);
552556
if (!pblocktemplate)
553557
throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory");
554558

‎src/test/blockfilter_index_tests.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818

1919
BOOST_AUTO_TEST_SUITE(blockfilter_index_tests)
2020

21+
struct BuildChainTestingSetup : public TestChain100Setup {
22+
CBlock CreateBlock(const CBlockIndex* prev, const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey);
23+
bool BuildChain(const CBlockIndex* pindex, const CScript& coinbase_script_pub_key, size_t length, std::vector<std::shared_ptr<CBlock>>& chain);
24+
};
25+
2126
static bool CheckFilterLookups(BlockFilterIndex& filter_index, const CBlockIndex* block_index,
2227
uint256& last_header)
2328
{
@@ -52,12 +57,12 @@ static bool CheckFilterLookups(BlockFilterIndex& filter_index, const CBlockIndex
5257
return true;
5358
}
5459

55-
static CBlock CreateBlock(const CBlockIndex* prev,
56-
const std::vector<CMutableTransaction>& txns,
57-
const CScript& scriptPubKey)
60+
CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev,
61+
const std::vector<CMutableTransaction>& txns,
62+
const CScript& scriptPubKey)
5863
{
5964
const CChainParams& chainparams = Params();
60-
std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey);
65+
std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler(*m_node.mempool, chainparams).CreateNewBlock(scriptPubKey);
6166
CBlock& block = pblocktemplate->block;
6267
block.hashPrevBlock = prev->GetBlockHash();
6368
block.nTime = prev->nTime + 1;
@@ -76,8 +81,10 @@ static CBlock CreateBlock(const CBlockIndex* prev,
7681
return block;
7782
}
7883

79-
static bool BuildChain(const CBlockIndex* pindex, const CScript& coinbase_script_pub_key,
80-
size_t length, std::vector<std::shared_ptr<CBlock>>& chain)
84+
bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex,
85+
const CScript& coinbase_script_pub_key,
86+
size_t length,
87+
std::vector<std::shared_ptr<CBlock>>& chain)
8188
{
8289
std::vector<CMutableTransaction> no_txns;
8390

@@ -95,7 +102,7 @@ static bool BuildChain(const CBlockIndex* pindex, const CScript& coinbase_script
95102
return true;
96103
}
97104

98-
BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, TestChain100Setup)
105+
BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
99106
{
100107
BlockFilterIndex filter_index(BlockFilterType::BASIC, 1 << 20, true);
101108

‎src/test/miner_tests.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ struct MinerTestingSetup : public TestingSetup {
3030
{
3131
return CheckSequenceLocks(*m_node.mempool, tx, flags);
3232
}
33+
BlockAssembler AssemblerForTest(const CChainParams& params);
3334
};
3435
} // namespace miner_tests
3536

@@ -48,12 +49,13 @@ class HasReason {
4849

4950
static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE);
5051

51-
static BlockAssembler AssemblerForTest(const CChainParams& params) {
52+
BlockAssembler MinerTestingSetup::AssemblerForTest(const CChainParams& params)
53+
{
5254
BlockAssembler::Options options;
5355

5456
options.nBlockMaxWeight = MAX_BLOCK_WEIGHT;
5557
options.blockMinFeeRate = blockMinFeeRate;
56-
return BlockAssembler(params, options);
58+
return BlockAssembler(*m_node.mempool, params, options);
5759
}
5860

5961
constexpr static struct {

‎src/test/util/mining.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,23 @@
88
#include <consensus/merkle.h>
99
#include <key_io.h>
1010
#include <miner.h>
11+
#include <node/context.h>
1112
#include <pow.h>
1213
#include <script/standard.h>
1314
#include <validation.h>
1415

15-
CTxIn generatetoaddress(const std::string& address)
16+
CTxIn generatetoaddress(const NodeContext& node, const std::string& address)
1617
{
1718
const auto dest = DecodeDestination(address);
1819
assert(IsValidDestination(dest));
1920
const auto coinbase_script = GetScriptForDestination(dest);
2021

21-
return MineBlock(coinbase_script);
22+
return MineBlock(node, coinbase_script);
2223
}
2324

24-
CTxIn MineBlock(const CScript& coinbase_scriptPubKey)
25+
CTxIn MineBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey)
2526
{
26-
auto block = PrepareBlock(coinbase_scriptPubKey);
27+
auto block = PrepareBlock(node, coinbase_scriptPubKey);
2728

2829
while (!CheckProofOfWork(block->GetHash(), block->nBits, Params().GetConsensus())) {
2930
++block->nNonce;
@@ -36,10 +37,11 @@ CTxIn MineBlock(const CScript& coinbase_scriptPubKey)
3637
return CTxIn{block->vtx[0]->GetHash(), 0};
3738
}
3839

39-
std::shared_ptr<CBlock> PrepareBlock(const CScript& coinbase_scriptPubKey)
40+
std::shared_ptr<CBlock> PrepareBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey)
4041
{
42+
assert(node.mempool);
4143
auto block = std::make_shared<CBlock>(
42-
BlockAssembler{Params()}
44+
BlockAssembler{*node.mempool, Params()}
4345
.CreateNewBlock(coinbase_scriptPubKey)
4446
->block);
4547

0 commit comments

Comments
 (0)