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
This commit is contained in:
6 files changed
+58
-17
No files matched your search
@@ -116,7 +116,7 @@ public:
|
|||||||
* @param[in] timeout how long to wait for a new tip (default is forever)
|
* @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 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;
|
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
|
* regtest and signets with only one miner, as these
|
||||||
* could stall.
|
* could stall.
|
||||||
* @retval BlockTemplate a block template.
|
* @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;
|
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.
|
* Checks if a given block is valid.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
|
|||||||
waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef);
|
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);
|
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);
|
checkBlock @5 (context :Proxy.Context, block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
|
||||||
|
interrupt @6 () -> ();
|
||||||
}
|
}
|
||||||
|
|
||||||
interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") {
|
interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") {
|
||||||
|
|||||||
+10
-3
@@ -950,7 +950,7 @@ public:
|
|||||||
|
|
||||||
std::optional<BlockRef> waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override
|
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
|
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.
|
// forever if no block was mined in the past day.
|
||||||
while (chainman().IsInitialBlockDownload()) {
|
while (chainman().IsInitialBlockDownload()) {
|
||||||
maybe_tip = waitTipChanged(maybe_tip->hash, MillisecondsDouble{1000});
|
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.
|
// 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};
|
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);
|
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
|
bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) override
|
||||||
{
|
{
|
||||||
LOCK(chainman().GetMutex());
|
LOCK(chainman().GetMutex());
|
||||||
@@ -1002,6 +1007,8 @@ public:
|
|||||||
NodeContext* context() override { return &m_node; }
|
NodeContext* context() override { return &m_node; }
|
||||||
ChainstateManager& chainman() { return *Assert(m_node.chainman); }
|
ChainstateManager& chainman() { return *Assert(m_node.chainman); }
|
||||||
KernelNotifications& notifications() { return *Assert(m_node.notifications); }
|
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;
|
NodeContext& m_node;
|
||||||
};
|
};
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|||||||
+17
-8
@@ -455,7 +455,7 @@ std::optional<BlockRef> GetTip(ChainstateManager& chainman)
|
|||||||
return BlockRef{tip->GetBlockHash(), tip->nHeight};
|
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};
|
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);
|
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) {
|
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();
|
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
|
// If the tip changed during the wait, extend the deadline
|
||||||
const auto tip_block = kernel_notifications.TipBlock();
|
const auto tip_block = kernel_notifications.TipBlock();
|
||||||
@@ -486,7 +489,7 @@ bool CooldownIfHeadersAhead(ChainstateManager& chainman, KernelNotifications& ke
|
|||||||
return true;
|
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
|
Assume(timeout >= 0ms); // No internal callers should use a negative timeout
|
||||||
if (timeout < 0ms) timeout = 0ms;
|
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
|
// always returns valid tip information when possible and only
|
||||||
// returns null when shutting down, not when timing out.
|
// 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) {
|
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
|
// At this point TipBlock is set, so continue to wait until it is
|
||||||
// different then `current_tip` provided by caller.
|
// 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) {
|
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
|
// Must release m_tip_block_mutex before getTip() locks cs_main, to
|
||||||
// avoid deadlocks.
|
// avoid deadlocks.
|
||||||
|
|||||||
+7
-4
@@ -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);
|
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);
|
void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait);
|
||||||
/**
|
/**
|
||||||
* Return a new block template when fees rise to a certain threshold or after a
|
* 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);
|
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`).
|
/* 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. */
|
* Returns the current tip, or nullopt if the node is shutting down or interrupt()
|
||||||
std::optional<BlockRef> WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout);
|
* 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
|
* 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().
|
* once per connected client. Subsequent templates are provided by waitNext().
|
||||||
*
|
*
|
||||||
* @param last_tip tip at the start of the cooldown window.
|
* @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.
|
* @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
|
} // namespace node
|
||||||
|
|
||||||
#endif // BITCOIN_NODE_MINER_H
|
#endif // BITCOIN_NODE_MINER_H
|
||||||
@@ -137,6 +137,14 @@ class IPCMiningTest(BitcoinTestFramework):
|
|||||||
assert_equal(oldblockref.hash, newblockref.hash)
|
assert_equal(oldblockref.hash, newblockref.hash)
|
||||||
assert_equal(oldblockref.height, newblockref.height)
|
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()))
|
asyncio.run(capnp.run(async_routine()))
|
||||||
|
|
||||||
def run_block_template_test(self):
|
def run_block_template_test(self):
|
||||||
@@ -163,6 +171,14 @@ class IPCMiningTest(BitcoinTestFramework):
|
|||||||
# spurious failures.
|
# spurious failures.
|
||||||
assert_greater_than_or_equal(time.time() - start, 0.9)
|
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()
|
header_only_peer.peer_disconnect()
|
||||||
self.connect_nodes(0, 1)
|
self.connect_nodes(0, 1)
|
||||||
self.sync_all()
|
self.sync_all()
|
||||||
|
|||||||
Reference in new issue
Block a user