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:
bitcoin#34184 (comment)

Author: Sjors

src/interfaces/mining.h

@@ -138,7 +138,7 @@ class Mining
      * @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;
 
@@ -152,10 +152,15 @@ class Mining
      *                     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.
      *

src/ipc/capnp/mining.capnp

@@ -19,6 +19,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
     waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (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") {

src/node/interfaces.cpp

@@ -965,7 +965,7 @@ class MinerImpl : public Mining
 
     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
@@ -993,18 +993,23 @@ class MinerImpl : public Mining
             // 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};
         ApplyArgsManOptions(*Assert(m_node.args), assemble_options);
         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());
@@ -1017,6 +1022,8 @@ class MinerImpl : public Mining
     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

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.

src/node/miner.h

@@ -142,7 +142,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
@@ -160,8 +160,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
@@ -178,10 +180,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

test/functional/interface_ipc_mining.py

@@ -153,6 +153,14 @@ async def async_routine():
             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):
@@ -179,6 +187,14 @@ async def async_routine():
                 # 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, cooldown=True)
+                    # 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()