diff --git a/doc/release-notes-34184.md b/doc/release-notes-34184.md new file mode 100644 index 00000000000..c582023eea2 --- /dev/null +++ b/doc/release-notes-34184.md @@ -0,0 +1,8 @@ +Mining IPC +---------- + +- `Mining.createNewBlock` now has a `cooldown` behavior (enabled by default) + that waits for IBD to finish and for the tip to catch up. This usually + prevents a flood of templates during startup, but is not guaranteed. (#34184) +- `Mining.interrupt()` can be used to interrupt `Mining.waitTipChanged` and + `Mining.createNewBlock`. (#34184) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 42002284d11..72cc0bcb67b 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -116,20 +116,28 @@ 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; /** * 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. It's recommended to disable this on + * 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 = {}) = 0; + 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 47cd242b81d..64cad4d49f2 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -22,8 +22,9 @@ 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 = .maxDouble) -> (result: Common.BlockRef); - createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions) -> (result: BlockTemplate); + 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 8f5406ab3c0..afcaeb20c3e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -950,10 +950,10 @@ 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) override + std::unique_ptr createNewBlock(const BlockCreateOptions& options, bool cooldown) override { // Reject too-small values instead of clamping so callers don't silently // end up mining with different options than requested. This matches the @@ -966,13 +966,35 @@ public: } // Ensure m_tip_block is set so consumers of BlockTemplate can rely on that. - if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {}; + std::optional 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 || 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, m_interrupt_mining)) return {}; + } BlockAssembler::Options assemble_options{options}; ApplyArgsManOptions(*Assert(m_node.args), assemble_options); 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()); @@ -985,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 fe26794528b..7ea8d10b730 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -455,7 +455,41 @@ std::optional GetTip(ChainstateManager& chainman) return BlockRef{tip->GetBlockHash(), tip->nHeight}; } -std::optional WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout) +bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const BlockRef& last_tip, bool& interrupt_mining) +{ + uint256 last_tip_hash{last_tip.hash}; + + while (const std::optional remaining = chainman.BlocksAheadOfTip()) { + const int cooldown_seconds = std::clamp(*remaining, 3, 20); + const auto cooldown_deadline{MockableSteadyClock::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 || interrupt_mining || (tip_block && *tip_block != last_tip_hash); + }); + 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(); + if (tip_block && *tip_block != last_tip_hash) { + last_tip_hash = *tip_block; + continue; + } + } + + // No tip change and the cooldown window has expired. + if (MockableSteadyClock::now() >= cooldown_deadline) break; + } + + return true; +} + +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; @@ -468,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 5a338dfb917..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,9 +159,31 @@ 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 + * 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. + * @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& interrupt_mining); } // namespace node #endif // BITCOIN_NODE_MINER_H diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ce1bd743b46..043e11ff63a 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -165,7 +165,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const { UniValue blockHashes(UniValue::VARR); while (nGenerate > 0 && !chainman.m_interrupt) { - std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true })); + std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }, /*cooldown=*/false)); CHECK_NONFATAL(block_template); std::shared_ptr block_out; @@ -376,7 +376,7 @@ static RPCHelpMan generateblock() { LOCK(chainman.GetMutex()); { - std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true})}; + std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true}, /*cooldown=*/false)}; CHECK_NONFATAL(block_template); block = block_template->getBlock(); @@ -870,8 +870,11 @@ static RPCHelpMan getblocktemplate() CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip); time_start = GetTime(); - // Create new block - block_template = miner.createNewBlock({.include_dummy_extranonce = true}); + // Create new block. Opt-out of cooldown mechanism, because it would add + // a delay to each getblocktemplate call. This differs from typical + // long-lived IPC usage, where the overhead is paid only when creating + // the initial template. + block_template = miner.createNewBlock({.include_dummy_extranonce = true}, /*cooldown=*/false); CHECK_NONFATAL(block_template); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 3b4beebeff5..7f05e4e9d81 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -122,7 +122,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const BOOST_CHECK(tx_mempool.size() == 0); // Block template should only have a coinbase when there's nothing in the mempool - std::unique_ptr block_template = mining->createNewBlock(options); + std::unique_ptr block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); CBlock block{block_template->getBlock()}; BOOST_REQUIRE_EQUAL(block.vtx.size(), 1U); @@ -166,7 +166,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const const auto high_fee_tx{entry.Fee(50000).Time(Now()).SpendsCoinbase(false).FromTx(tx)}; TryAddToMempool(tx_mempool, high_fee_tx); - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); block = block_template->getBlock(); BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U); @@ -253,7 +253,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; Txid hashLowFeeTx2 = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); block = block_template->getBlock(); @@ -268,7 +268,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vin[0].prevout.n = 1; tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee TryAddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx)); - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); block = block_template->getBlock(); BOOST_REQUIRE_EQUAL(block.vtx.size(), 9U); @@ -342,7 +342,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: LOCK(tx_mempool.cs); // Just to make sure we can still make simple blocks - auto block_template{mining->createNewBlock(options)}; + auto block_template{mining->createNewBlock(options, /*cooldown=*/false)}; BOOST_REQUIRE(block_template); CBlock block{block_template->getBlock()}; @@ -358,7 +358,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: } assert(tx_mempool.mapTx.size() == 51); assert(legacy_sigops == 20001); - BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-blk-sigops")); + BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-blk-sigops")); } { @@ -369,7 +369,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: assert(tx_mempool.mapTx.empty()); // Just to make sure we can still make simple blocks - auto block_template{mining->createNewBlock(options)}; + auto block_template{mining->createNewBlock(options, /*cooldown=*/false)}; BOOST_REQUIRE(block_template); CBlock block{block_template->getBlock()}; @@ -384,7 +384,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: assert(tx_mempool.mapTx.size() == 51); assert(legacy_sigops == 20001); - BOOST_REQUIRE(mining->createNewBlock(options)); + BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false)); } { @@ -414,7 +414,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: BOOST_CHECK(tx_mempool.GetIter(hash).has_value()); tx.vin[0].prevout.hash = hash; } - BOOST_REQUIRE(mining->createNewBlock(options)); + BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false)); } { @@ -424,7 +424,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: // orphan in tx_mempool, template creation fails hash = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).FromTx(tx)); - BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); + BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } { @@ -445,7 +445,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee hash = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - BOOST_REQUIRE(mining->createNewBlock(options)); + BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false)); } { @@ -461,7 +461,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: // give it a fee so it'll get mined TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); // Should throw bad-cb-multiple - BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-cb-multiple")); + BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-cb-multiple")); } { @@ -478,7 +478,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].scriptPubKey = CScript() << OP_2; hash = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); + BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } { @@ -498,7 +498,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: next->BuildSkip(); m_node.chainman->ActiveChain().SetTip(*next); } - BOOST_REQUIRE(mining->createNewBlock(options)); + BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false)); // Extend to a 210000-long block chain. while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) { CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); @@ -510,7 +510,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: next->BuildSkip(); m_node.chainman->ActiveChain().SetTip(*next); } - BOOST_REQUIRE(mining->createNewBlock(options)); + BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false)); // invalid p2sh txn in tx_mempool, template creation fails tx.vin[0].prevout.hash = txFirst[0]->GetHash(); @@ -526,7 +526,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].nValue -= LOWFEE; hash = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); - BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("block-script-verify-flag-failed")); + BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("block-script-verify-flag-failed")); // Delete the dummy blocks again. while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { @@ -632,7 +632,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1; BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail - auto block_template = mining->createNewBlock(options); + auto block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); // None of the of the absolute height/time locked tx should have made @@ -649,7 +649,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: m_node.chainman->ActiveChain().Tip()->nHeight++; SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); block = block_template->getBlock(); BOOST_CHECK_EQUAL(block.vtx.size(), 5U); @@ -725,7 +725,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const Txid hashFreeGrandchild = tx.GetHash(); TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); - auto block_template = mining->createNewBlock(options); + auto block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); CBlock block{block_template->getBlock()}; BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U); @@ -755,7 +755,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) options.include_dummy_extranonce = true; // Create and check a simple template - std::unique_ptr block_template = mining->createNewBlock(options); + std::unique_ptr block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); { CBlock block{block_template->getBlock()}; @@ -806,7 +806,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) * set at the end of the previous loop. */ if (current_height % 2 == 0) { - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); } diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp index 33e028a5c9d..614d2fd63ac 100644 --- a/src/test/testnet4_miner_tests.cpp +++ b/src/test/testnet4_miner_tests.cpp @@ -42,7 +42,7 @@ BOOST_AUTO_TEST_CASE(MiningInterface) const int64_t genesis_time{WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip()->GetBlockTime())}; SetMockTime(genesis_time + 3 * 60); - block_template = mining->createNewBlock(options); + block_template = mining->createNewBlock(options, /*cooldown=*/false); BOOST_REQUIRE(block_template); // The template should use the mocked system time diff --git a/src/validation.cpp b/src/validation.cpp index 6e248c65525..8c4ae2e9b48 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6282,6 +6282,19 @@ void ChainstateManager::RecalculateBestHeader() } } +std::optional 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); diff --git a/src/validation.h b/src/validation.h index 58df66e3c1b..482772c0d6f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1355,6 +1355,10 @@ public: //! 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 BlocksAheadOfTip() const LOCKS_EXCLUDED(::cs_main); + CCheckQueue& GetCheckQueue() { return m_script_check_queue; } ~ChainstateManager(); diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 2221d462879..5095583a0e2 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -4,18 +4,22 @@ # 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 import re from test_framework.blocktools import NULL_OUTPOINT from test_framework.messages import ( MAX_BLOCK_WEIGHT, + CBlockHeader, CTransaction, CTxIn, CTxOut, CTxInWitness, ser_uint256, COIN, + from_hex, + msg_headers, ) from test_framework.script import ( CScript, @@ -24,9 +28,11 @@ from test_framework.script import ( 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, @@ -131,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): @@ -141,9 +155,34 @@ class IPCMiningTest(BitcoinTestFramework): 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)).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) + + 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() + self.log.debug("Create a template") template = await mining_create_block_template(mining, stack, ctx, self.default_block_create_options) assert template is not None @@ -152,8 +191,9 @@ class IPCMiningTest(BitcoinTestFramework): 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)