What changed, and why it matters
This commit adds a new 'interrupt()' button to Bitcoin Core's mining interface so outside callers can stop two long-running mining operations (waiting for a new chain tip and creating a new block) instead of waiting for them to finish on their own. It is a feature/refactoring change, not a fix for an active security bug. The only security-relevant note is that the new interrupt flag is accessed under a lock but is not formally marked as guarded by that lock, which the commit message itself flags as a known imperfection.
Treat as a routine feature commit. If reviewing for hardening, verify that the un-annotated m_interrupt_mining access is always performed under m_tip_block_mutex as claimed, and consider completing the follow-up discussed in PR #34184 to add the missing guard annotation. No emergency action is warranted.
Security signals we found
New interruptible control path added to long-running mining operations
Shared boolean flag (m_interrupt_mining) used without a Clang thread-safety guard annotation
Commit message self-discloses the guard-annotation gap and references follow-up discussion
No input validation changes, no privilege changes, no cryptographic changes
Evidence from the diff
The patch introduces an interrupt() method on interfaces::Mining and exposes it over the Cap’n Proto IPC interface. It threads a new m_interrupt_mining boolean through WaitTipChanged(), CooldownIfHeadersAhead(), and createNewBlock() so that callers can abort those waits. The flag is reset to false after an interrupt is consumed. The commit message explicitly notes that m_interrupt_mining has no guard annotation even though it is intended to be accessed under m_tip_block_mutex, and points to a PR discussion about a more thorough solution. No memory corruption, authentication bypass, consensus change, or denial-of-service vector is evident in the diff.
Changed components
src/interfaces/mining.hsrc/ipc/capnp/mining.capnpsrc/node/interfaces.cppsrc/node/miner.cppsrc/node/miner.htest/functional/interface_ipc_mining.pyInspect captured patch +58 / −17
diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
index a39f7d3c..72cc0bcb 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<BlockRef> 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<BlockTemplate> 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 9bed2e2d..64cad4d4 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 f14a2eb1..afcaeb20 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -950,7 +950,7 @@ public:
std::optional<BlockRef> 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<BlockTemplate> 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<BlockTemplateImpl>(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 6c23af4e..7ea8d10b 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -455,7 +455,7 @@ std::optional<BlockRef> 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<BlockRef> WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout)
+std::optional<BlockRef> 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<BlockRef> 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 5316ebdd..5c866877 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<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
std::optional<BlockRef> 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<BlockRef> 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<BlockRef> 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<BlockRef> 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 6449b4fa..5095583a 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()
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.