diff --git a/src/node/miner.cpp b/src/node/miner.cpp index e75bc3a603b..fce0afb140e 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -106,8 +106,6 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio void BlockAssembler::resetBlock() { - inBlock.clear(); - // Reserve space for fixed-size block header, txs count, and coinbase tx. nBlockWeight = m_options.block_reserved_weight; nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops; @@ -145,10 +143,11 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblock->nTime = TicksSinceEpoch(NodeClock::now()); m_lock_time_cutoff = pindexPrev->GetMedianTimePast(); - int nPackagesSelected = 0; - int nDescendantsUpdated = 0; if (m_mempool) { - addPackageTxs(nPackagesSelected, nDescendantsUpdated); + LOCK(m_mempool->cs); + m_mempool->StartBlockBuilding(); + addChunks(); + m_mempool->StopBlockBuilding(); } const auto time_1{SteadyClock::now()}; @@ -185,30 +184,17 @@ std::unique_ptr BlockAssembler::CreateNewBlock() } const auto time_2{SteadyClock::now()}; - LogDebug(BCLog::BENCH, "CreateNewBlock() packages: %.2fms (%d packages, %d updated descendants), validity: %.2fms (total %.2fms)\n", - Ticks(time_1 - time_start), nPackagesSelected, nDescendantsUpdated, + LogDebug(BCLog::BENCH, "CreateNewBlock() chunks: %.2fms, validity: %.2fms (total %.2fms)\n", + Ticks(time_1 - time_start), Ticks(time_2 - time_1), Ticks(time_2 - time_start)); return std::move(pblocktemplate); } -void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet) +bool BlockAssembler::TestPackage(FeePerWeight package_feerate, int64_t packageSigOpsCost) const { - for (CTxMemPool::setEntries::iterator iit = testSet.begin(); iit != testSet.end(); ) { - // Only test txs not already in the block - if (inBlock.count((*iit)->GetSharedTx()->GetHash())) { - testSet.erase(iit++); - } else { - iit++; - } - } -} - -bool BlockAssembler::TestPackage(uint64_t packageSize, int64_t packageSigOpsCost) const -{ - // TODO: switch to weight-based accounting for packages instead of vsize-based accounting. - if (nBlockWeight + WITNESS_SCALE_FACTOR * packageSize >= m_options.nBlockMaxWeight) { + if (nBlockWeight + package_feerate.size >= m_options.nBlockMaxWeight) { return false; } if (nBlockSigOpsCost + packageSigOpsCost >= MAX_BLOCK_SIGOPS_COST) { @@ -219,99 +205,35 @@ bool BlockAssembler::TestPackage(uint64_t packageSize, int64_t packageSigOpsCost // Perform transaction-level checks before adding to block: // - transaction finality (locktime) -bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& package) const +bool BlockAssembler::TestPackageTransactions(const std::vector& txs) const { - for (CTxMemPool::txiter it : package) { - if (!IsFinalTx(it->GetTx(), nHeight, m_lock_time_cutoff)) { + for (const auto tx : txs) { + if (!IsFinalTx(tx.get().GetTx(), nHeight, m_lock_time_cutoff)) { return false; } } return true; } -void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) +void BlockAssembler::AddToBlock(const CTxMemPoolEntry& entry) { - pblocktemplate->block.vtx.emplace_back(iter->GetSharedTx()); - pblocktemplate->vTxFees.push_back(iter->GetFee()); - pblocktemplate->vTxSigOpsCost.push_back(iter->GetSigOpCost()); - nBlockWeight += iter->GetTxWeight(); + pblocktemplate->block.vtx.emplace_back(entry.GetSharedTx()); + pblocktemplate->vTxFees.push_back(entry.GetFee()); + pblocktemplate->vTxSigOpsCost.push_back(entry.GetSigOpCost()); + nBlockWeight += entry.GetTxWeight(); ++nBlockTx; - nBlockSigOpsCost += iter->GetSigOpCost(); - nFees += iter->GetFee(); - inBlock.insert(iter->GetSharedTx()->GetHash()); + nBlockSigOpsCost += entry.GetSigOpCost(); + nFees += entry.GetFee(); if (m_options.print_modified_fee) { LogPrintf("fee rate %s txid %s\n", - CFeeRate(iter->GetModifiedFee(), iter->GetTxSize()).ToString(), - iter->GetTx().GetHash().ToString()); + CFeeRate(entry.GetModifiedFee(), entry.GetTxSize()).ToString(), + entry.GetTx().GetHash().ToString()); } } -/** 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) +void BlockAssembler::addChunks() { - AssertLockHeld(mempool.cs); - - int nDescendantsUpdated = 0; - for (CTxMemPool::txiter it : alreadyAdded) { - CTxMemPool::setEntries 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)) { - continue; - } - ++nDescendantsUpdated; - modtxiter mit = mapModifiedTx.find(desc); - if (mit == mapModifiedTx.end()) { - CTxMemPoolModifiedEntry modEntry(desc); - mit = mapModifiedTx.insert(modEntry).first; - } - mapModifiedTx.modify(mit, update_for_parent_inclusion(it)); - } - } - return nDescendantsUpdated; -} - -void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::vector& sortedEntries) -{ - // Sort package by ancestor count - // If a transaction A depends on transaction B, then A's ancestor count - // must be greater than B's. So this is sufficient to validly order the - // transactions for block inclusion. - sortedEntries.clear(); - sortedEntries.insert(sortedEntries.begin(), package.begin(), package.end()); - std::sort(sortedEntries.begin(), sortedEntries.end(), CompareTxIterByAncestorCount()); -} - -// This transaction selection algorithm orders the mempool based -// on feerate of a transaction including all unconfirmed ancestors. -// Since we don't remove transactions from the mempool as we select them -// for block inclusion, we need an alternate method of updating the feerate -// of a transaction with its not-yet-selected ancestors as we go. -// This is accomplished by walking the in-mempool descendants of selected -// transactions and storing a temporary modified state in mapModifiedTxs. -// 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 auto& mempool{*Assert(m_mempool)}; - LOCK(mempool.cs); - - // mapModifiedTx will store sorted packages after they are modified - // because some of their txs are already in the block - indexed_modified_transaction_set mapModifiedTx; - // Keep track of entries that failed inclusion, to avoid duplicate work - std::set failedTx; - - CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); - CTxMemPool::txiter iter; - // Limit the number of attempts to add transactions to the block when it is // close to full; this is just a simple heuristic to finish quickly if the // mempool has a lot of entries. @@ -319,124 +241,51 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda constexpr int32_t BLOCK_FULL_ENOUGH_WEIGHT_DELTA = 4000; int64_t nConsecutiveFailed = 0; - while (mi != mempool.mapTx.get().end() || !mapModifiedTx.empty()) { - // First try to find a new transaction in mapTx to evaluate. - // - // 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().end()) { - auto it = mempool.mapTx.project<0>(mi); - assert(it != mempool.mapTx.end()); - if (mapModifiedTx.count(it) || inBlock.count(it->GetSharedTx()->GetHash()) || failedTx.count(it->GetSharedTx()->GetHash())) { - ++mi; - continue; - } - } + std::vector selected_transactions; + selected_transactions.reserve(MAX_CLUSTER_COUNT_LIMIT); + FeePerWeight chunk_feerate; - // Now that mi is not stale, determine which transaction to evaluate: - // the next entry from mapTx, or the best from mapModifiedTx? - bool fUsingModified = false; + // This fills selected_transactions + chunk_feerate = m_mempool->GetBlockBuilderChunk(selected_transactions); + FeePerVSize chunk_feerate_vsize = ToFeePerVSize(chunk_feerate); - modtxscoreiter modit = mapModifiedTx.get().begin(); - if (mi == mempool.mapTx.get().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 = mempool.mapTx.project<0>(mi); - if (modit != mapModifiedTx.get().end() && - CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) { - // The best entry in mapModifiedTx has higher score - // than the one from mapTx. - // Switch which transaction (package) to consider - iter = modit->iter; - fUsingModified = true; - } else { - // Either no entry in mapModifiedTx, or it's worse than mapTx. - // Increment mi for the next loop iteration. - ++mi; - } - } - - // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't - // contain anything that is inBlock. - assert(!inBlock.count(iter->GetSharedTx()->GetHash())); - - uint64_t packageSize = iter->GetSizeWithAncestors(); - CAmount packageFees = iter->GetModFeesWithAncestors(); - int64_t packageSigOpsCost = iter->GetSigOpCostWithAncestors(); - if (fUsingModified) { - packageSize = modit->nSizeWithAncestors; - packageFees = modit->nModFeesWithAncestors; - packageSigOpsCost = modit->nSigOpCostWithAncestors; - } - - if (packageFees < m_options.blockMinFeeRate.GetFee(packageSize)) { - // Everything else we might consider has a lower fee rate + while (selected_transactions.size() > 0) { + // Check to see if min fee rate is still respected. + if (chunk_feerate.fee < m_options.blockMinFeeRate.GetFee(chunk_feerate_vsize.size)) { + // Everything else we might consider has a lower feerate return; } - if (!TestPackage(packageSize, packageSigOpsCost)) { - if (fUsingModified) { - // Since we always look at the best entry in mapModifiedTx, - // we must erase failed entries so that we can consider the - // next best entry on the next loop iteration - mapModifiedTx.get().erase(modit); - failedTx.insert(iter->GetSharedTx()->GetHash()); - } + int64_t package_sig_ops = 0; + for (const auto& tx : selected_transactions) { + package_sig_ops += tx.get().GetSigOpCost(); + } + // Check to see if this chunk will fit. + if (!TestPackage(chunk_feerate, package_sig_ops) || !TestPackageTransactions(selected_transactions)) { + // This chunk won't fit, so we skip it and will try the next best one. + m_mempool->SkipBuilderChunk(); ++nConsecutiveFailed; if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight + BLOCK_FULL_ENOUGH_WEIGHT_DELTA > m_options.nBlockMaxWeight) { // Give up if we're close to full and haven't succeeded in a while - break; + return; } - continue; - } + } else { + m_mempool->IncludeBuilderChunk(); - auto ancestors{mempool.AssumeCalculateMemPoolAncestors(__func__, *iter, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; - - onlyUnconfirmed(ancestors); - ancestors.insert(iter); - - // Test if all tx's are Final - if (!TestPackageTransactions(ancestors)) { - if (fUsingModified) { - mapModifiedTx.get().erase(modit); - failedTx.insert(iter->GetSharedTx()->GetHash()); + // This chunk will fit, so add it to the block. + nConsecutiveFailed = 0; + for (const auto& tx : selected_transactions) { + AddToBlock(tx); } - continue; + pblocktemplate->m_package_feerates.emplace_back(chunk_feerate_vsize); } - // This transaction will make it in; reset the failed counter. - nConsecutiveFailed = 0; - - // Package can be added. Sort the entries in a valid order. - std::vector sortedEntries; - SortForBlock(ancestors, sortedEntries); - - for (size_t i = 0; i < sortedEntries.size(); ++i) { - AddToBlock(sortedEntries[i]); - // Erase from the modified set, if present - mapModifiedTx.erase(sortedEntries[i]); - } - - ++nPackagesSelected; - pblocktemplate->m_package_feerates.emplace_back(packageFees, static_cast(packageSize)); - - // Update transactions that depend on each of these - nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx); + selected_transactions.clear(); + chunk_feerate = m_mempool->GetBlockBuilderChunk(selected_transactions); + chunk_feerate_vsize = ToFeePerVSize(chunk_feerate); } } diff --git a/src/node/miner.h b/src/node/miner.h index a9a88b39cf2..83e7c6b751f 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -49,101 +49,7 @@ struct CBlockTemplate std::vector vchCoinbaseCommitment; /* A vector of package fee rates, ordered by the sequence in which * packages are selected for inclusion in the block template.*/ - std::vector m_package_feerates; -}; - -// Container for tracking updates to ancestor feerate as we include (parent) -// transactions in a block -struct CTxMemPoolModifiedEntry { - explicit CTxMemPoolModifiedEntry(CTxMemPool::txiter entry) - { - iter = entry; - nSizeWithAncestors = entry->GetSizeWithAncestors(); - nModFeesWithAncestors = entry->GetModFeesWithAncestors(); - nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors(); - } - - CAmount GetModifiedFee() const { return iter->GetModifiedFee(); } - uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } - CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } - size_t GetTxSize() const { return iter->GetTxSize(); } - const CTransaction& GetTx() const { return iter->GetTx(); } - - CTxMemPool::txiter iter; - uint64_t nSizeWithAncestors; - CAmount nModFeesWithAncestors; - int64_t nSigOpCostWithAncestors; -}; - -/** Comparator for CTxMemPool::txiter objects. - * It simply compares the internal memory address of the CTxMemPoolEntry object - * pointed to. This means it has no meaning, and is only useful for using them - * as key in other indexes. - */ -struct CompareCTxMemPoolIter { - bool operator()(const CTxMemPool::txiter& a, const CTxMemPool::txiter& b) const - { - return &(*a) < &(*b); - } -}; - -struct modifiedentry_iter { - typedef CTxMemPool::txiter result_type; - result_type operator() (const CTxMemPoolModifiedEntry &entry) const - { - return entry.iter; - } -}; - -// A comparator that sorts transactions based on number of ancestors. -// This is sufficient to sort an ancestor package in an order that is valid -// to appear in a block. -struct CompareTxIterByAncestorCount { - bool operator()(const CTxMemPool::txiter& a, const CTxMemPool::txiter& b) const - { - if (a->GetCountWithAncestors() != b->GetCountWithAncestors()) { - return a->GetCountWithAncestors() < b->GetCountWithAncestors(); - } - return CompareIteratorByHash()(a, b); - } -}; - - -struct CTxMemPoolModifiedEntry_Indices final : boost::multi_index::indexed_by< - boost::multi_index::ordered_unique< - modifiedentry_iter, - CompareCTxMemPoolIter - >, - // sorted by modified ancestor fee rate - boost::multi_index::ordered_non_unique< - // Reuse same tag from CTxMemPool's similar index - boost::multi_index::tag, - boost::multi_index::identity, - CompareTxMemPoolEntryByAncestorFee - > -> -{}; - -typedef boost::multi_index_container< - CTxMemPoolModifiedEntry, - CTxMemPoolModifiedEntry_Indices -> indexed_modified_transaction_set; - -typedef indexed_modified_transaction_set::nth_index<0>::type::iterator modtxiter; -typedef indexed_modified_transaction_set::index::type::iterator modtxscoreiter; - -struct update_for_parent_inclusion -{ - explicit update_for_parent_inclusion(CTxMemPool::txiter it) : iter(it) {} - - void operator() (CTxMemPoolModifiedEntry &e) - { - e.nModFeesWithAncestors -= iter->GetModifiedFee(); - e.nSizeWithAncestors -= iter->GetTxSize(); - e.nSigOpCostWithAncestors -= iter->GetSigOpCost(); - } - - CTxMemPool::txiter iter; + std::vector m_package_feerates; }; /** Generate a new block, without valid proof-of-work */ @@ -158,7 +64,6 @@ private: uint64_t nBlockTx; uint64_t nBlockSigOpsCost; CAmount nFees; - std::unordered_set inBlock; // Chain context for the block int nHeight; @@ -195,29 +100,23 @@ private: /** Clear the block's state and prepare for assembling a new block */ void resetBlock(); /** Add a tx to the block */ - void AddToBlock(CTxMemPool::txiter iter); + void AddToBlock(const CTxMemPoolEntry& entry); // Methods for how to add transactions to a block. - /** Add transactions based on feerate including unconfirmed ancestors - * Increments nPackagesSelected / nDescendantsUpdated with corresponding - * statistics from the package selection (for logging statistics). + /** Add transactions based on chunk feerate * * @pre BlockAssembler::m_mempool must not be nullptr */ - void addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(!m_mempool->cs); + void addChunks() EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs); - // helper functions for addPackageTxs() - /** Remove confirmed (inBlock) entries from given set */ - void onlyUnconfirmed(CTxMemPool::setEntries& testSet); + // helper functions for addChunks() /** Test if a new package would "fit" in the block */ - bool TestPackage(uint64_t packageSize, int64_t packageSigOpsCost) const; + bool TestPackage(FeePerWeight package_feerate, int64_t packageSigOpsCost) const; /** Perform checks on each transaction in a package: * locktime, premature-witness, serialized size (if necessary) * 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; - /** Sort the package in an order that is valid to appear in a block */ - void SortForBlock(const CTxMemPool::setEntries& package, std::vector& sortedEntries); + bool TestPackageTransactions(const std::vector& txs) const; }; /** diff --git a/src/txmempool.h b/src/txmempool.h index 2b5882c8a4b..04f2fdd4f5a 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -372,6 +372,7 @@ public: */ mutable RecursiveMutex cs; std::unique_ptr m_txgraph GUARDED_BY(cs); + mutable std::unique_ptr m_builder GUARDED_BY(cs); indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; @@ -922,6 +923,24 @@ private: // callbacks). void addNewTransaction(CTxMemPool::txiter it) EXCLUSIVE_LOCKS_REQUIRED(cs); void addNewTransaction(CTxMemPool::txiter it, CTxMemPool::setEntries& setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); +public: + void StartBlockBuilding() const EXCLUSIVE_LOCKS_REQUIRED(cs) { assert(!m_builder); m_builder = m_txgraph->GetBlockBuilder(); } + FeePerWeight GetBlockBuilderChunk(std::vector& entries) const EXCLUSIVE_LOCKS_REQUIRED(cs) + { + if (!m_builder) { return {}; } + + auto res = m_builder->GetCurrentChunk(); + if (!res) { return {}; } + + auto [chunk_entries, chunk_feerate] = *res; + for (TxGraph::Ref* ref : chunk_entries) { + entries.emplace_back(static_cast(*ref)); + } + return chunk_feerate; + } + void IncludeBuilderChunk() const EXCLUSIVE_LOCKS_REQUIRED(cs) { m_builder->Include(); } + void SkipBuilderChunk() const EXCLUSIVE_LOCKS_REQUIRED(cs) { m_builder->Skip(); } + void StopBlockBuilding() const EXCLUSIVE_LOCKS_REQUIRED(cs) { m_builder.reset(); } }; /**