Skip to content

Merge bitcoin#25223: [kernel 2e/n] miner: Make mempool optional, st… #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 31, 2025
Merged

Merge bitcoin#25223: [kernel 2e/n] miner: Make mempool optional, st… #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
130981f
Merge bitcoin/bitcoin#25223: [kernel 2e/n] miner: Make `mempool` opti…
fanquake Jun 15, 2022
d1be47d
Fix remaining test files to use mempool pointer
May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 42 additions & 38 deletions src/node/miner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ BlockAssembler::Options::Options()
nBlockMaxSize = DEFAULT_BLOCK_MAX_SIZE;
}

BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool& mempool, const CChainParams& params, const Options& options) :
BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool* mempool, const CChainParams& params, const Options& options) :
m_blockman(chainstate.m_blockman),
m_cpoolman(*Assert(node.cpoolman)),
m_chain_helper(chainstate.ChainHelper()),
Expand Down Expand Up @@ -104,7 +104,7 @@ static BlockAssembler::Options DefaultOptions()
return options;
}

BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool& mempool, const CChainParams& params)
BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool* mempool, const CChainParams& params)
: BlockAssembler(chainstate, node, mempool, params, DefaultOptions()) {}

void BlockAssembler::resetBlock()
Expand Down Expand Up @@ -138,7 +138,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
pblocktemplate->vTxFees.push_back(-1); // updated at end
pblocktemplate->vTxSigOps.push_back(-1); // updated at end

LOCK2(cs_main, m_mempool.cs);
LOCK(::cs_main);
CBlockIndex* pindexPrev = m_chainstate.m_chain.Tip();
assert(pindexPrev != nullptr);
nHeight = pindexPrev->nHeight + 1;
Expand Down Expand Up @@ -181,8 +181,10 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc

int nPackagesSelected = 0;
int nDescendantsUpdated = 0;

addPackageTxs(nPackagesSelected, nDescendantsUpdated, pindexPrev);
if (m_mempool) {
LOCK(m_mempool->cs);
addPackageTxs(*m_mempool, nPackagesSelected, nDescendantsUpdated, pindexPrev);
}

int64_t nTime1 = GetTimeMicros();

Expand Down Expand Up @@ -347,15 +349,19 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter)
}
}

int BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded,
indexed_modified_transaction_set &mapModifiedTx)
/** Add descendants of given transactions to mapModifiedTx with ancestor
* state updated assuming given transactions are inBlock. Returns number
* of updated descendants. */
static int UpdatePackagesForAdded(const CTxMemPool& mempool,
const CTxMemPool::setEntries& alreadyAdded,
indexed_modified_transaction_set& mapModifiedTx) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs)
{
AssertLockHeld(m_mempool.cs);
AssertLockHeld(mempool.cs);

int nDescendantsUpdated = 0;
for (CTxMemPool::txiter it : alreadyAdded) {
CTxMemPool::setEntries descendants;
m_mempool.CalculateDescendants(it, descendants);
mempool.CalculateDescendants(it, descendants);
// Insert all descendants (not yet in block) into the modified set
for (CTxMemPool::txiter desc : descendants) {
if (alreadyAdded.count(desc)) {
Expand All @@ -377,23 +383,6 @@ int BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& already
return nDescendantsUpdated;
}

// Skip entries in mapTx that are already in a block or are present
// in mapModifiedTx (which implies that the mapTx ancestor state is
// stale due to ancestor inclusion in the block)
// Also skip transactions that we've already failed to add. This can happen if
// we consider a transaction in mapModifiedTx and it fails: we can then
// potentially consider it again while walking mapTx. It's currently
// guaranteed to fail again, but as a belt-and-suspenders check we put it in
// failedTx and avoid re-evaluation, since the re-evaluation would be using
// cached size/sigops/fee values that are not actually correct.
bool BlockAssembler::SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set& mapModifiedTx, CTxMemPool::setEntries& failedTx)
{
AssertLockHeld(m_mempool.cs);

assert(it != m_mempool.mapTx.end());
return mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it);
}

void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries)
{
// Sort package by ancestor count
Expand All @@ -415,9 +404,9 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve
// Each time through the loop, we compare the best transaction in
// mapModifiedTxs with the next transaction in the mempool to decide what
// transaction package to work on next.
void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated, const CBlockIndex* const pindexPrev)
void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated, const CBlockIndex* const pindexPrev)
{
AssertLockHeld(m_mempool.cs);
AssertLockHeld(mempool.cs);

// This credit pool is used only to check withdrawal limits and to find
// duplicates of indexes. There's used `BlockSubsidy` equaled to 0
Expand All @@ -436,7 +425,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
// Keep track of entries that failed inclusion, to avoid duplicate work
CTxMemPool::setEntries failedTx;

CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = m_mempool.mapTx.get<ancestor_score>().begin();
CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = mempool.mapTx.get<ancestor_score>().begin();
CTxMemPool::txiter iter;

// Limit the number of attempts to add transactions to the block when it is
Expand All @@ -445,26 +434,41 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
const int64_t MAX_CONSECUTIVE_FAILURES = 1000;
int64_t nConsecutiveFailed = 0;

while (mi != m_mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty()) {
while (mi != mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty()) {
// First try to find a new transaction in mapTx to evaluate.
if (mi != m_mempool.mapTx.get<ancestor_score>().end() &&
SkipMapTxEntry(m_mempool.mapTx.project<0>(mi), mapModifiedTx, failedTx)) {
++mi;
continue;
//
// Skip entries in mapTx that are already in a block or are present
// in mapModifiedTx (which implies that the mapTx ancestor state is
// stale due to ancestor inclusion in the block)
// Also skip transactions that we've already failed to add. This can happen if
// we consider a transaction in mapModifiedTx and it fails: we can then
// potentially consider it again while walking mapTx. It's currently
// guaranteed to fail again, but as a belt-and-suspenders check we put it in
// failedTx and avoid re-evaluation, since the re-evaluation would be using
// cached size/sigops/fee values that are not actually correct.
/** Return true if given transaction from mapTx has already been evaluated,
* or if the transaction's cached data in mapTx is incorrect. */
if (mi != mempool.mapTx.get<ancestor_score>().end()) {
auto it = mempool.mapTx.project<0>(mi);
assert(it != mempool.mapTx.end());
if (mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it)) {
++mi;
continue;
}
}

// Now that mi is not stale, determine which transaction to evaluate:
// the next entry from mapTx, or the best from mapModifiedTx?
bool fUsingModified = false;

modtxscoreiter modit = mapModifiedTx.get<ancestor_score>().begin();
if (mi == m_mempool.mapTx.get<ancestor_score>().end()) {
if (mi == mempool.mapTx.get<ancestor_score>().end()) {
// We're out of entries in mapTx; use the entry from mapModifiedTx
iter = modit->iter;
fUsingModified = true;
} else {
// Try to compare the mapTx entry to the mapModifiedTx entry
iter = m_mempool.mapTx.project<0>(mi);
iter = mempool.mapTx.project<0>(mi);
if (modit != mapModifiedTx.get<ancestor_score>().end() &&
CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) {
// The best entry in mapModifiedTx has higher score
Expand Down Expand Up @@ -547,7 +551,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
CTxMemPool::setEntries ancestors;
uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
std::string dummy;
m_mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);

onlyUnconfirmed(ancestors);
ancestors.insert(iter);
Expand Down Expand Up @@ -577,7 +581,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
++nPackagesSelected;

// Update transactions that depend on each of these
nDescendantsUpdated += UpdatePackagesForAdded(ancestors, mapModifiedTx);
nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);
}
}

Expand Down
17 changes: 5 additions & 12 deletions src/node/miner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class BlockAssembler
llmq::CInstantSendManager& m_isman;
llmq::CQuorumSnapshotManager& m_qsnapman;
const CChainParams& chainparams;
const CTxMemPool& m_mempool;
const CTxMemPool* const m_mempool;
const llmq::CQuorumBlockProcessor& m_quorum_block_processor;
const llmq::CQuorumManager& m_qman;

Expand All @@ -188,8 +188,8 @@ class BlockAssembler
CFeeRate blockMinFeeRate;
};

explicit BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool& mempool, const CChainParams& params);
explicit BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool& mempool, const CChainParams& params,
explicit BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool* mempool, const CChainParams& params);
explicit BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool* mempool, const CChainParams& params,
const Options& options);

/** Construct a new block template with coinbase to scriptPubKeyIn */
Expand All @@ -209,8 +209,8 @@ class BlockAssembler
/** Add transactions based on feerate including unconfirmed ancestors
* Increments nPackagesSelected / nDescendantsUpdated with corresponding
* statistics from the package selection (for logging statistics). */
void addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated,
const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
void addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated,
const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);

// helper functions for addPackageTxs()
/** Remove confirmed (inBlock) entries from given set */
Expand All @@ -222,15 +222,8 @@ class BlockAssembler
* These checks should always succeed, and they're here
* only as an extra check in case of suboptimal node configuration */
bool TestPackageTransactions(const CTxMemPool::setEntries& package) const;
/** Return true if given transaction from mapTx has already been evaluated,
* or if the transaction's cached data in mapTx is incorrect. */
bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set& mapModifiedTx, CTxMemPool::setEntries& failedTx) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
/** Sort the package in an order that is valid to appear in a block */
void SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries);
/** Add descendants of given transactions to mapModifiedTx with ancestor
* state updated assuming given transactions are inBlock. Returns number
* of updated descendants. */
int UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set& mapModifiedTx) EXCLUSIVE_LOCKS_REQUIRED(m_mempool.cs);
};

/** Modify the extranonce in a block */
Expand Down
7 changes: 3 additions & 4 deletions src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, const NodeContext& n
UniValue blockHashes(UniValue::VARR);
while (nHeight < nHeightEnd && !ShutdownRequested())
{
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(chainman.ActiveChainstate(), node, mempool, Params()).CreateNewBlock(coinbase_script));
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(chainman.ActiveChainstate(), node, &mempool, Params()).CreateNewBlock(coinbase_script));
if (!pblocktemplate.get())
throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
CBlock *pblock = &pblocktemplate->block;
Expand Down Expand Up @@ -375,8 +375,7 @@ static RPCHelpMan generateblock()
{
LOCK(cs_main);

CTxMemPool empty_mempool;
std::unique_ptr<CBlockTemplate> blocktemplate(BlockAssembler(active_chainstate, node, empty_mempool, chainparams).CreateNewBlock(coinbase_script));
std::unique_ptr<CBlockTemplate> blocktemplate(BlockAssembler(active_chainstate, node, nullptr, chainparams).CreateNewBlock(coinbase_script));
if (!blocktemplate) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
}
Expand Down Expand Up @@ -823,7 +822,7 @@ static RPCHelpMan getblocktemplate()
// Create new block
CScript scriptDummy = CScript() << OP_TRUE;
EnsureLLMQContext(node);
pblocktemplate = BlockAssembler(active_chainstate, node, mempool, Params()).CreateNewBlock(scriptDummy);
pblocktemplate = BlockAssembler(active_chainstate, node, &mempool, Params()).CreateNewBlock(scriptDummy);
if (!pblocktemplate)
throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory");

Expand Down
14 changes: 7 additions & 7 deletions src/test/block_reward_reallocation_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(tx.GetHash()));
BOOST_CHECK(tip->nHeight < Params().GetConsensus().BRRHeight);
// Creating blocks by different ways
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);
}
for ([[maybe_unused]] auto _ : irange::range(499)) {
CreateAndProcessBlock({}, coinbaseKey);
Expand Down Expand Up @@ -212,7 +212,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(tx.GetHash()));
const CAmount block_subsidy = GetBlockSubsidyInner(tip->nBits, tip->nHeight, consensus_params, isV20Active);
const CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);
BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment);
}

Expand All @@ -226,7 +226,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
const bool isV20Active{DeploymentActiveAfter(tip, consensus_params, Consensus::DEPLOYMENT_V20)};
const CAmount block_subsidy = GetBlockSubsidyInner(tip->nBits, tip->nHeight, consensus_params, isV20Active);
const CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx[0]->GetValueOut(), 28847249686);
BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment);
BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, 14423624841); // 0.4999999999
Expand All @@ -243,7 +243,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
const bool isV20Active{DeploymentActiveAfter(tip, consensus_params, Consensus::DEPLOYMENT_V20)};
const CAmount block_subsidy = GetBlockSubsidyInner(tip->nBits, tip->nHeight, consensus_params, isV20Active);
const CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);
BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment);
}
}
Expand All @@ -261,7 +261,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
CAmount expected_block_reward = block_subsidy_potential - block_subsidy_potential / 5;

const CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx[0]->GetValueOut(), expected_block_reward);
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx[0]->GetValueOut(), 141734128);
BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment);
Expand All @@ -281,7 +281,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
const bool isMNRewardReallocated{DeploymentActiveAfter(tip, consensus_params, Consensus::DEPLOYMENT_MN_RR)};
const CAmount block_subsidy = GetBlockSubsidyInner(tip->nBits, tip->nHeight, consensus_params, isV20Active);
CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);

if (isMNRewardReallocated) {
const CAmount platform_payment = PlatformShare(masternode_payment);
Expand All @@ -303,7 +303,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS
CAmount masternode_payment = GetMasternodePayment(tip->nHeight, block_subsidy, isV20Active);
const CAmount platform_payment = PlatformShare(masternode_payment);
masternode_payment -= platform_payment;
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, *m_node.mempool, Params()).CreateNewBlock(coinbasePubKey);
const auto pblocktemplate = BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), Params()).CreateNewBlock(coinbasePubKey);

CAmount block_subsidy_potential = block_subsidy + block_subsidy_sb;
BOOST_CHECK_EQUAL(tip->nHeight, 2358);
Expand Down
Loading
Loading