From 720f201e652885b9e0aec8e62a1bf9590052b320 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Mon, 28 Apr 2025 17:53:40 +0100 Subject: [PATCH] interfaces: refactor: move `waitNext` implementation to miner - We now assert for a valid chainman and notifications once when invoking WaitAndCreateNewBlock function. --- src/node/interfaces.cpp | 84 +------------------------------------ src/node/miner.cpp | 92 +++++++++++++++++++++++++++++++++++++++++ src/node/miner.h | 13 ++++++ 3 files changed, 107 insertions(+), 82 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 99a793e9a8..08594fe5af 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -935,87 +935,8 @@ public: std::unique_ptr waitNext(BlockWaitOptions options) override { - // Delay calculating the current template fees, just in case a new block - // comes in before the next tick. - CAmount current_fees = -1; - - // Alternate waiting for a new tip and checking if fees have risen. - // The latter check is expensive so we only run it once per second. - auto now{NodeClock::now()}; - const auto deadline = now + options.timeout; - const MillisecondsDouble tick{1000}; - const bool allow_min_difficulty{chainman().GetParams().GetConsensus().fPowAllowMinDifficultyBlocks}; - - do { - bool tip_changed{false}; - { - WAIT_LOCK(notifications().m_tip_block_mutex, lock); - // Note that wait_until() checks the predicate before waiting - notifications().m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { - AssertLockHeld(notifications().m_tip_block_mutex); - const auto tip_block{notifications().TipBlock()}; - // We assume tip_block is set, because this is an instance - // method on BlockTemplate and no template could have been - // generated before a tip exists. - tip_changed = Assume(tip_block) && tip_block != m_block_template->block.hashPrevBlock; - return tip_changed || chainman().m_interrupt; - }); - } - - if (chainman().m_interrupt) return nullptr; - // At this point the tip changed, a full tick went by or we reached - // the deadline. - - // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. - LOCK(::cs_main); - - // On test networks return a minimum difficulty block after 20 minutes - if (!tip_changed && allow_min_difficulty) { - const NodeClock::time_point tip_time{std::chrono::seconds{chainman().ActiveChain().Tip()->GetBlockTime()}}; - if (now > tip_time + 20min) { - tip_changed = true; - } - } - - /** - * We determine if fees increased compared to the previous template by generating - * a fresh template. There may be more efficient ways to determine how much - * (approximate) fees for the next block increased, perhaps more so after - * Cluster Mempool. - * - * We'll also create a new template if the tip changed during this iteration. - */ - if (options.fee_threshold < MAX_MONEY || tip_changed) { - auto tmpl{std::make_unique(m_assemble_options, - BlockAssembler{ - chainman().ActiveChainstate(), - context()->mempool.get(), - m_assemble_options} - .CreateNewBlock(), - m_node)}; - - // If the tip changed, return the new template regardless of its fees. - if (tip_changed) return tmpl; - - // Calculate the original template total fees if we haven't already - if (current_fees == -1) { - current_fees = 0; - for (CAmount fee : m_block_template->vTxFees) { - current_fees += fee; - } - } - - CAmount new_fees = 0; - for (CAmount fee : tmpl->m_block_template->vTxFees) { - new_fees += fee; - Assume(options.fee_threshold != MAX_MONEY); - if (new_fees >= current_fees + options.fee_threshold) return tmpl; - } - } - - now = NodeClock::now(); - } while (now < deadline); - + auto new_template = WaitAndCreateNewBlock(chainman(), notifications(), m_node.mempool.get(), m_block_template, options, m_assemble_options); + if (new_template) return std::make_unique(m_assemble_options, std::move(new_template), m_node); return nullptr; } @@ -1023,7 +944,6 @@ public: const std::unique_ptr m_block_template; - NodeContext* context() { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } KernelNotifications& notifications() { return *Assert(m_node.notifications); } NodeContext& m_node; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 0c982bedfe..213b46cd0b 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -16,11 +16,14 @@ #include #include #include +#include +#include #include #include #include #include #include +#include #include #include @@ -447,4 +450,93 @@ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t block.nNonce = nonce; block.hashMerkleRoot = BlockMerkleRoot(block); } + +std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainman, + KernelNotifications& kernel_notifications, + CTxMemPool* mempool, + const std::unique_ptr& block_template, + const BlockWaitOptions& options, + const BlockAssembler::Options& assemble_options) +{ + // Delay calculating the current template fees, just in case a new block + // comes in before the next tick. + CAmount current_fees = -1; + + // Alternate waiting for a new tip and checking if fees have risen. + // The latter check is expensive so we only run it once per second. + auto now{NodeClock::now()}; + const auto deadline = now + options.timeout; + const MillisecondsDouble tick{1000}; + const bool allow_min_difficulty{chainman.GetParams().GetConsensus().fPowAllowMinDifficultyBlocks}; + + do { + bool tip_changed{false}; + { + WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock); + // Note that wait_until() checks the predicate before waiting + kernel_notifications.m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { + AssertLockHeld(kernel_notifications.m_tip_block_mutex); + const auto tip_block{kernel_notifications.TipBlock()}; + // We assume tip_block is set, because this is an instance + // method on BlockTemplate and no template could have been + // generated before a tip exists. + tip_changed = Assume(tip_block) && tip_block != block_template->block.hashPrevBlock; + return tip_changed || chainman.m_interrupt; + }); + } + + if (chainman.m_interrupt) return nullptr; + // At this point the tip changed, a full tick went by or we reached + // the deadline. + + // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. + LOCK(::cs_main); + + // On test networks return a minimum difficulty block after 20 minutes + if (!tip_changed && allow_min_difficulty) { + const NodeClock::time_point tip_time{std::chrono::seconds{chainman.ActiveChain().Tip()->GetBlockTime()}}; + if (now > tip_time + 20min) { + tip_changed = true; + } + } + + /** + * We determine if fees increased compared to the previous template by generating + * a fresh template. There may be more efficient ways to determine how much + * (approximate) fees for the next block increased, perhaps more so after + * Cluster Mempool. + * + * We'll also create a new template if the tip changed during this iteration. + */ + if (options.fee_threshold < MAX_MONEY || tip_changed) { + auto new_tmpl{BlockAssembler{ + chainman.ActiveChainstate(), + mempool, + assemble_options} + .CreateNewBlock()}; + + // If the tip changed, return the new template regardless of its fees. + if (tip_changed) return new_tmpl; + + // Calculate the original template total fees if we haven't already + if (current_fees == -1) { + current_fees = 0; + for (CAmount fee : block_template->vTxFees) { + current_fees += fee; + } + } + + CAmount new_fees = 0; + for (CAmount fee : new_tmpl->vTxFees) { + new_fees += fee; + Assume(options.fee_threshold != MAX_MONEY); + if (new_fees >= current_fees + options.fee_threshold) return new_tmpl; + } + } + + now = NodeClock::now(); + } while (now < deadline); + + return nullptr; +} } // namespace node diff --git a/src/node/miner.h b/src/node/miner.h index 641f8a031b..5dcdca437a 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -32,6 +32,8 @@ class ChainstateManager; namespace Consensus { struct Params; }; namespace node { +class KernelNotifications; + static const bool DEFAULT_PRINT_MODIFIED_FEE = false; struct CBlockTemplate @@ -232,6 +234,17 @@ void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& opti /* Compute the block's merkle root, insert or replace the coinbase transaction and the merkle root into the block */ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce); + +/** + * Return a new block template when fees rise to a certain threshold or after a + * new tip; return nullopt if timeout is reached. + */ +std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainman, + KernelNotifications& kernel_notifications, + CTxMemPool* mempool, + const std::unique_ptr& block_template, + const BlockWaitOptions& options, + const BlockAssembler::Options& assemble_options); } // namespace node #endif // BITCOIN_NODE_MINER_H