From 1e82fa498cf4881466f0539146c101242b9dc30d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Sat, 7 Feb 2026 12:34:22 +0100 Subject: [PATCH] mining: add interrupt() Both waitTipChanged() and createNewBlock() can take a long time to return. Add a way for clients to interrupt them. The new m_interrupt_mining is safely accessed with a lock on m_tip_block_mutex, but it has no guard annotation. A more thorough solution is discussed here: https://github.com/bitcoin/bitcoin/pull/34184#discussion_r2743566474 --- src/interfaces/mining.h | 9 +++++++-- src/ipc/capnp/mining.capnp | 1 + src/node/interfaces.cpp | 13 ++++++++++--- src/node/miner.cpp | 25 +++++++++++++++++-------- src/node/miner.h | 11 +++++++---- test/functional/interface_ipc_mining.py | 16 ++++++++++++++++ 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index a39f7d3cf5a..72cc0bcb67b 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -116,7 +116,7 @@ public: * @param[in] timeout how long to wait for a new tip (default is forever) * * @retval BlockRef hash and height of the current chain tip after this call. - * @retval std::nullopt if the node is shut down. + * @retval std::nullopt if the node is shut down or interrupt() is called. */ virtual std::optional waitTipChanged(uint256 current_tip, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0; @@ -130,10 +130,15 @@ public: * regtest and signets with only one miner, as these * could stall. * @retval BlockTemplate a block template. - * @retval std::nullptr if the node is shut down. + * @retval std::nullptr if the node is shut down or interrupt() is called. */ virtual std::unique_ptr createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = true) = 0; + /** + * Interrupts createNewBlock and waitTipChanged. + */ + virtual void interrupt() = 0; + /** * Checks if a given block is valid. * diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 9bed2e2d9d2..64cad4d49f2 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -24,6 +24,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") { waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef); createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions, cooldown: Bool = true) -> (result: BlockTemplate); checkBlock @5 (context :Proxy.Context, block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool); + interrupt @6 () -> (); } interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f14a2eb1b3d..afcaeb20c3e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -950,7 +950,7 @@ public: std::optional waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override { - return WaitTipChanged(chainman(), notifications(), current_tip, timeout); + return WaitTipChanged(chainman(), notifications(), current_tip, timeout, m_interrupt_mining); } std::unique_ptr createNewBlock(const BlockCreateOptions& options, bool cooldown) override @@ -978,11 +978,11 @@ public: // forever if no block was mined in the past day. while (chainman().IsInitialBlockDownload()) { maybe_tip = waitTipChanged(maybe_tip->hash, MillisecondsDouble{1000}); - if (!maybe_tip) return {}; + if (!maybe_tip || chainman().m_interrupt || WITH_LOCK(notifications().m_tip_block_mutex, return m_interrupt_mining)) return {}; } // Also wait during the final catch-up moments after IBD. - if (!CooldownIfHeadersAhead(chainman(), notifications(), *maybe_tip)) return {}; + if (!CooldownIfHeadersAhead(chainman(), notifications(), *maybe_tip, m_interrupt_mining)) return {}; } BlockAssembler::Options assemble_options{options}; @@ -990,6 +990,11 @@ public: return std::make_unique(assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node); } + void interrupt() override + { + InterruptWait(notifications(), m_interrupt_mining); + } + bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) override { LOCK(chainman().GetMutex()); @@ -1002,6 +1007,8 @@ public: NodeContext* context() override { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } KernelNotifications& notifications() { return *Assert(m_node.notifications); } + // Treat as if guarded by notifications().m_tip_block_mutex + bool m_interrupt_mining{false}; NodeContext& m_node; }; } // namespace diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 6c23af4effd..7ea8d10b730 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -455,7 +455,7 @@ std::optional GetTip(ChainstateManager& chainman) return BlockRef{tip->GetBlockHash(), tip->nHeight}; } -bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip) +bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip, bool& interrupt_mining) { uint256 last_tip_hash{last_tip.hash}; @@ -467,9 +467,12 @@ bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& ke WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock); kernel_notifications.m_tip_block_cv.wait_until(lock, cooldown_deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { const auto tip_block = kernel_notifications.TipBlock(); - return chainman.m_interrupt || (tip_block && *tip_block != last_tip_hash); + return chainman.m_interrupt || interrupt_mining || (tip_block && *tip_block != last_tip_hash); }); - if (chainman.m_interrupt) return false; + if (chainman.m_interrupt || interrupt_mining) { + interrupt_mining = false; + return false; + } // If the tip changed during the wait, extend the deadline const auto tip_block = kernel_notifications.TipBlock(); @@ -486,7 +489,7 @@ bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& ke return true; } -std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout) +std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout, bool& interrupt) { Assume(timeout >= 0ms); // No internal callers should use a negative timeout if (timeout < 0ms) timeout = 0ms; @@ -499,16 +502,22 @@ std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifi // always returns valid tip information when possible and only // returns null when shutting down, not when timing out. kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { - return kernel_notifications.TipBlock() || chainman.m_interrupt; + return kernel_notifications.TipBlock() || chainman.m_interrupt || interrupt; }); - if (chainman.m_interrupt) return {}; + if (chainman.m_interrupt || interrupt) { + interrupt = false; + return {}; + } // At this point TipBlock is set, so continue to wait until it is // different then `current_tip` provided by caller. kernel_notifications.m_tip_block_cv.wait_until(lock, deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { - return Assume(kernel_notifications.TipBlock()) != current_tip || chainman.m_interrupt; + return Assume(kernel_notifications.TipBlock()) != current_tip || chainman.m_interrupt || interrupt; }); + if (chainman.m_interrupt || interrupt) { + interrupt = false; + return {}; + } } - if (chainman.m_interrupt) return {}; // Must release m_tip_block_mutex before getTip() locks cs_main, to // avoid deadlocks. diff --git a/src/node/miner.h b/src/node/miner.h index 5316ebdd046..5c8668771f5 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -141,7 +141,7 @@ void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& opti void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce); -/* Interrupt the current wait for the next block template. */ +/* Interrupt a blocking call. */ void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait); /** * Return a new block template when fees rise to a certain threshold or after a @@ -159,8 +159,10 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma std::optional GetTip(ChainstateManager& chainman); /* Waits for the connected tip to change until timeout has elapsed. During node initialization, this will wait until the tip is connected (regardless of `timeout`). - * Returns the current tip, or nullopt if the node is shutting down. */ -std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout); + * Returns the current tip, or nullopt if the node is shutting down or interrupt() + * is called. + */ +std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout, bool& interrupt); /** * Wait while the best known header extends the current chain tip AND at least @@ -177,10 +179,11 @@ std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifi * once per connected client. Subsequent templates are provided by waitNext(). * * @param last_tip tip at the start of the cooldown window. + * @param interrupt_mining set to true to interrupt the cooldown. * * @returns false if interrupted. */ -bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip); +bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip, bool& interrupt_mining); } // namespace node #endif // BITCOIN_NODE_MINER_H diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 6449b4fa16c..5095583a0e2 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -137,6 +137,14 @@ class IPCMiningTest(BitcoinTestFramework): assert_equal(oldblockref.hash, newblockref.hash) assert_equal(oldblockref.height, newblockref.height) + self.log.debug("interrupt() should abort waitTipChanged()") + async def wait_for_tip(): + long_timeout = 60000.0 # 1 minute + result = (await mining.waitTipChanged(ctx, newblockref.hash, long_timeout)).result + # Unlike a timeout, interrupt() returns an empty BlockRef. + assert_equal(len(result.hash), 0) + await wait_and_do(wait_for_tip(), mining.interrupt()) + asyncio.run(capnp.run(async_routine())) def run_block_template_test(self): @@ -163,6 +171,14 @@ class IPCMiningTest(BitcoinTestFramework): # spurious failures. assert_greater_than_or_equal(time.time() - start, 0.9) + self.log.debug("interrupt() should abort createNewBlock() during cooldown") + async def create_block(): + result = await mining.createNewBlock(ctx, self.default_block_create_options) + # interrupt() causes createNewBlock to return nullptr + assert_equal(result._has("result"), False) + + await wait_and_do(create_block(), mining.interrupt()) + header_only_peer.peer_disconnect() self.connect_nodes(0, 1) self.sync_all()