mining: add cooldown argument to createNewBlock()

At startup, if the needs to catch up, connected mining clients will
receive a flood of new templates as new blocks are connected.

Fix this by adding an optional cooldown argument to createNewBlock().
When set to true, block template creation is briefly paused while the
best header chain is ahead of the tip.

This wait only happens when the best header extends the current tip,
to ignore competing branches.

Additionally, cooldown waits for isInitialBlockDownload() to latch to
false, which happens when there is less than a day of blocks left to sync.

When cooldown is false, the default and used by all internal callers,
createNewBlock() returns immediately. The argument is optional,
because the functional test suite is negatively impacted by this
mechanism, and single miner signets could end up stuck if no block
was mined for a day.

Fixes bitcoin#33994

Author: Sjors

src/interfaces/mining.h

@@ -145,13 +145,15 @@ class Mining
    /**
      * Construct a new block template.
      *
-     * During node initialization, this will wait until the tip is connected.
-     *
      * @param[in] options options for creating the block
+     * @param[in] cooldown wait for tip to be connected and IBD to complete.
+     *                     If the best header is ahead of the tip, wait for the
+     *                     tip to catch up. Recommended, except for regtest and
+     *                     signets with only one miner.
      * @retval BlockTemplate a block template.
      * @retval std::nullptr if the node is shut down.
      */
-    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}) = 0;
+    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = false) = 0;
 
     /**
      * Checks if a given block is valid.

src/ipc/capnp/mining.capnp

@@ -17,7 +17,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
     isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool);
     getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool);
     waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
-    createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions) -> (result: BlockTemplate);
+    createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions, cooldown: Bool) -> (result: BlockTemplate);
     checkBlock @5 (context :Proxy.Context, block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
 }
 

src/node/interfaces.cpp

@@ -967,10 +967,27 @@ class MinerImpl : public Mining
         return WaitTipChanged(chainman(), notifications(), current_tip, timeout);
     }
 
-    std::unique_ptr<BlockTemplate> createNewBlock(const BlockCreateOptions& options) override
+    std::unique_ptr<BlockTemplate> createNewBlock(const BlockCreateOptions& options, bool cooldown) override
     {
         // Ensure m_tip_block is set so consumers of BlockTemplate can rely on that.
-        if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {};
+        std::optional<BlockRef> maybe_tip{waitTipChanged(uint256::ZERO, MillisecondsDouble::max())};
+
+        if (!maybe_tip) return {};
+
+        if (cooldown) {
+            // Do not return a template during IBD, because it can have long
+            // pauses and sometimes takes a while to get started. Although this
+            // is useful in general, it's gated behind the cooldown argument,
+            // because on regtest and single miner signets this would wait
+            // 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 {};
+            }
+
+            // Also wait during the final catch-up moments after IBD.
+            if (!CooldownIfHeadersAhead(chainman(), notifications(), *maybe_tip)) return {};
+        }
 
         BlockAssembler::Options assemble_options{options};
         ApplyArgsManOptions(*Assert(m_node.args), assemble_options);

src/node/miner.cpp

@@ -452,6 +452,38 @@ std::optional<BlockRef> GetTip(ChainstateManager& chainman)
     return BlockRef{tip->GetBlockHash(), tip->nHeight};
 }
 
+bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip)
+{
+    uint256 last_tip_hash{last_tip.hash};
+
+    while (const std::optional<int> remaining = chainman.BlocksAheadOfTip()) {
+        const int cooldown_seconds = std::clamp(*remaining, 3, 20);
+        // Use a steady clock so cooldown is independent of mocktime.
+        const auto cooldown_deadline{SteadyClock::now() + std::chrono::seconds{cooldown_seconds}};
+
+        {
+            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);
+            });
+            if (chainman.m_interrupt) return false;
+
+            // If the tip changed during the wait, extend the deadline
+            const auto tip_block = kernel_notifications.TipBlock();
+            if (tip_block && *tip_block != last_tip_hash) {
+                last_tip_hash = *tip_block;
+                continue;
+            }
+        }
+
+        // No tip change and the cooldown window has expired.
+        if (SteadyClock::now() >= cooldown_deadline) break;
+    }
+
+    return true;
+}
+
 std::optional<BlockRef> WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout)
 {
     Assume(timeout >= 0ms); // No internal callers should use a negative timeout

src/node/miner.h

@@ -163,6 +163,25 @@ std::optional<BlockRef> GetTip(ChainstateManager& chainman);
  * 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);
 
+/**
+ * Wait while the best known header extends the current chain tip AND at least
+ * one block is being added to the tip every 3 seconds. If the tip is
+ * sufficiently far behind, allow up to 20 seconds for the next tip update.
+ *
+ * It’s not safe to keep waiting, because a malicious miner could announce a
+ * header and delay revealing the block, causing all other miners using this
+ * software to stall. At the same time, we need to balance between the default
+ * waiting time being brief, but not ending the cooldown prematurely when a
+ * random block is slow to download (or process).
+ *
+ * The cooldown only applies to createNewBlock(), which is typically called
+ * once per connected client. Subsequent templates are provided by waitNext().
+ *
+ * @param last_tip tip at the start of the cooldown window.
+ *
+ * @returns false if interrupted.
+ */
+bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip);
 } // namespace node
 
 #endif // BITCOIN_NODE_MINER_H

src/validation.cpp

@@ -6299,6 +6299,19 @@ void ChainstateManager::RecalculateBestHeader()
     }
 }
 
+std::optional<int> ChainstateManager::BlocksAheadOfTip() const
+{
+    LOCK(::cs_main);
+    const CBlockIndex* best_header{m_best_header};
+    const CBlockIndex* tip{ActiveChain().Tip()};
+    // Only consider headers that extend the active tip; ignore competing branches.
+    if (best_header && tip && best_header->nChainWork > tip->nChainWork &&
+        best_header->GetAncestor(tip->nHeight) == tip) {
+        return best_header->nHeight - tip->nHeight;
+    }
+    return std::nullopt;
+}
+
 bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs)
 {
     AssertLockHeld(::cs_main);

src/validation.h

@@ -1354,6 +1354,10 @@ class ChainstateManager
     //! header in our block-index not known to be invalid, recalculate it.
     void RecalculateBestHeader() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    //! Returns how many blocks the best header is ahead of the current tip,
+    //! or nullopt if the best header does not extend the tip.
+    std::optional<int> BlocksAheadOfTip() const LOCKS_EXCLUDED(::cs_main);
+
     CCheckQueue<CScriptCheck>& GetCheckQueue() { return m_script_check_queue; }
 
     ~ChainstateManager();

test/functional/interface_ipc_mining.py

@@ -4,16 +4,20 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the IPC (multiprocess) Mining interface."""
 import asyncio
+import time
 from contextlib import AsyncExitStack
 from io import BytesIO
 from test_framework.blocktools import NULL_OUTPOINT
 from test_framework.messages import (
+    CBlockHeader,
     CTransaction,
     CTxIn,
     CTxOut,
     CTxInWitness,
     ser_uint256,
     COIN,
+    from_hex,
+    msg_headers,
 )
 from test_framework.script import (
     CScript,
@@ -22,9 +26,11 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_greater_than_or_equal,
     assert_not_equal
 )
 from test_framework.wallet import MiniWallet
+from test_framework.p2p import P2PInterface
 from test_framework.ipc_util import (
     destroying,
     mining_create_block_template,
@@ -155,18 +161,36 @@ def run_block_template_test(self):
 
         async def async_routine():
             ctx, mining = await self.make_mining_ctx()
-            blockref = await mining.getTip(ctx)
 
             async with AsyncExitStack() as stack:
+                self.log.debug("createNewBlock() should wait if tip is still updating")
+                self.disconnect_nodes(0, 1)
+                node1_block_hash = self.generate(self.nodes[1], 1, sync_fun=self.no_op)[0]
+                header = from_hex(CBlockHeader(), self.nodes[1].getblockheader(node1_block_hash, False))
+                header_only_peer = self.nodes[0].add_p2p_connection(P2PInterface())
+                header_only_peer.send_and_ping(msg_headers([header]))
+                start = time.time()
+                async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options, cooldown=True)).result, ctx):
+                    pass
+                # Lower-bound only: a heavily loaded CI host might still exceed 0.9s
+                # even without the cooldown, so this can miss regressions but avoids
+                # spurious failures.
+                assert_greater_than_or_equal(time.time() - start, 0.9)
+
+                header_only_peer.peer_disconnect()
+                self.connect_nodes(0, 1)
+                self.sync_all()
+
                 self.log.debug("Create a template")
                 template = await mining_create_block_template(mining, stack, ctx, self.default_block_create_options)
 
                 self.log.debug("Test some inspectors of Template")
                 header = (await template.getBlockHeader(ctx)).result
                 assert_equal(len(header), block_header_size)
                 block = await mining_get_block(template, ctx)
-                assert_equal(ser_uint256(block.hashPrevBlock), blockref.result.hash)
-                assert len(block.vtx) >= 1
+                current_tip = self.nodes[0].getbestblockhash()
+                assert_equal(ser_uint256(block.hashPrevBlock), ser_uint256(int(current_tip, 16)))
+                assert_greater_than_or_equal(len(block.vtx), 1)
                 txfees = await template.getTxFees(ctx)
                 assert_equal(len(txfees.result), 0)
                 txsigops = await template.getTxSigops(ctx)
@@ -232,7 +256,7 @@ async def async_routine():
             current_block_height = self.nodes[0].getchaintips()[0]["height"]
             check_opts = self.capnp_modules['mining'].BlockCheckOptions()
 
-            async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template:
+            async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options, cooldown=True)).result, ctx) as template:
                 block = await mining_get_block(template, ctx)
                 balance = self.miniwallet.get_balance()
                 coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)

test/functional/test_framework/ipc_util.py

@@ -108,7 +108,7 @@ async def make_capnp_init_ctx(self):
 
 async def mining_create_block_template(mining, stack, ctx, opts):
     """Call mining.createNewBlock() and return template, then call template.destroy() when stack exits."""
-    return await stack.enter_async_context(destroying((await mining.createNewBlock(ctx, opts)).result, ctx))
+    return await stack.enter_async_context(destroying((await mining.createNewBlock(ctx, opts, cooldown=True)).result, ctx))
 
 
 async def mining_wait_next_template(template, stack, ctx, opts):