mining: clarify SubmitBlock result handling
What the developer wrote
Authored by w0xlt
mining: clarify SubmitBlock result handling
Make the submitBlock return value explicit and check that it stays
consistent with the BIP22 reason string, so future changes do not return
success with a reason or failure without one.
Report "inconclusive" when no specific block rejection reason is
available. This covers blocks accepted without being connected, and
processing failures where ProcessNewBlock returns false without an
invalid BlockChecked result, for example when ActivateBestChain fails
after BlockChecked reported a valid block.
Also document why no validation-interface queue drain is needed before
unregistering: BlockChecked is emitted synchronously by ProcessNewBlock,
unlike most validation signals.
What changed, and why it matters
This Bitcoin Core commit tightens how the mining interface reports whether a submitted block succeeded or failed. Previously, a block could be accepted but not connected to the chain (for example, a valid but lower-work 'stale' block), and the result could be ambiguous or inconsistent. The change makes the success/failure logic explicit, adds a safety check that success and a failure reason cannot both be returned, and introduces an 'inconclusive' reason when the node cannot give a clear validation verdict. It also documents that a particular notification is synchronous, so no extra wait step is needed. There is no direct evidence this fixes an exploitable vulnerability; it is primarily a robustness and clarity improvement for mining clients.
Treat as a defensive robustness improvement rather than an urgent security fix. Mining pool operators and users of the IPC/mining interface should review the new 'inconclusive' reason handling. No immediate upgrade is required solely for security reasons based on this commit alone.
Security signals we found
Adds explicit invariant check between success result and BIP22 rejection reason
Clarifies ambiguous block-submission outcomes that could mislead mining clients
Documents synchronous BlockChecked emission to prevent future unsafe unregistration assumptions
Adds functional test for stale-block submission behavior
Evidence from the diff
The patch modifies SubmitBlock in src/node/miner.cpp and the mining interface in src/node/interfaces.cpp. It adds a CHECK_NONFATAL assertion that the boolean result equals reason.empty(), ensuring BIP22 reason strings and success/failure are consistent. It introduces an ‘inconclusive’ reason for cases where ProcessNewBlock returns false but no invalid BlockChecked state was emitted, such as ActivateBestChain failures or other non-validation errors. It also documents that BlockChecked is emitted synchronously inside ProcessNewBlock, so UnregisterSharedValidationInterface does not require a queue drain. A functional test is added to verify that a valid stale block (accepted but not connected) returns result=false with reason=’inconclusive’.
Changed components
src/node/miner.cpp SubmitBlocksrc/node/interfaces.cpp mining submitBlock result handlingtest/functional/interface_ipc_mining.pyInspect captured patch +46 / −9
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 2f68f414..6747dd60 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -1031,7 +1031,9 @@ public:
// ProcessNewBlock() can accept and store a block before it is checked
// for validity. Treat duplicates as errors for mining clients, and only
// return success when validation completed without setting a reason.
- return accepted && new_block && reason.empty();
+ const bool result{accepted && new_block && reason.empty()};
+ CHECK_NONFATAL(result == reason.empty());
+ return result;
}
const NodeContext* context() override { return &m_node; }
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index ccd9cc7c..66ae870f 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -396,16 +396,21 @@ bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock
auto sc = std::make_shared<SubmitBlockStateCatcher>(block->GetHash());
CHECK_NONFATAL(chainman.m_options.signals)->RegisterSharedValidationInterface(sc);
bool accepted = chainman.ProcessNewBlock(block, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/new_block);
+ // No queue drain is needed. The BlockChecked notification used above is
+ // emitted synchronously by ProcessNewBlock, unlike most validation signals.
CHECK_NONFATAL(chainman.m_options.signals)->UnregisterSharedValidationInterface(sc);
if (new_block && !*new_block && accepted) {
reason = "duplicate";
+ } else if (!accepted && (!sc->m_found || sc->m_state.IsValid())) {
+ // ProcessNewBlock can fail without a validation result, for example
+ // from an activation or system error. It can also fail after a valid
+ // BlockChecked result. In these cases the validation result is
+ // inconclusive.
+ reason = "inconclusive";
} else if (!sc->m_found) {
- // A block can be accepted and stored without being connected, for
- // example if it does not have more work than the current tip. In that
- // case no BlockChecked callback is emitted, so the validation result is
- // inconclusive. Mining::submitBlock treats this as an error for mining
- // clients, but it does not mean the block is invalid.
+ // The block was accepted but not connected, for example if it does not
+ // have more work than the current tip.
reason = "inconclusive";
} else if (!sc->m_state.IsValid()) {
reason = sc->m_state.GetRejectReason();
diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py
index 4cd9c17c..8d2ee859 100755
--- a/test/functional/interface_ipc_mining.py
+++ b/test/functional/interface_ipc_mining.py
@@ -111,10 +111,15 @@ class IPCMiningTest(BitcoinTestFramework):
coinbase_tx.nLockTime = coinbase_res.lockTime
return coinbase_tx
- async def build_candidate_block(self, template, ctx):
- """Build a complete block from a remote BlockTemplate."""
+ async def build_candidate_block(self, template, ctx, extra_nonce=b""):
+ """Build a complete block from a remote BlockTemplate.
+
+ The returned block replaces the dummy coinbase from CreateNewBlock()
+ with one constructed from getCoinbaseTx().
+ """
block = await mining_get_block(template, ctx)
- coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)
+ coinbase = await self.build_coinbase_test(
+ template, ctx, self.miniwallet, extra_nonce=extra_nonce)
# Reduce payout for balance comparison simplicity.
coinbase.vout[0].nValue = COIN
block.vtx[0] = coinbase
@@ -623,6 +628,31 @@ class IPCMiningTest(BitcoinTestFramework):
assert_equal(submitted, True)
self.sync_all()
+ self.log.debug("submitBlock should report inconclusive for a valid stale block")
+ async with AsyncExitStack() as stack:
+ active_template = await mining_create_block_template(
+ mining2, stack, ctx2, self.default_block_create_options)
+ submit_block_template = await mining_create_block_template(
+ mining2, stack, ctx2, self.default_block_create_options)
+ assert active_template is not None
+ assert submit_block_template is not None
+
+ active_block = await self.build_candidate_block(
+ active_template, ctx2, extra_nonce=b"\x01")
+ submit_block = await self.build_candidate_block(
+ submit_block_template, ctx2, extra_nonce=b"\x02")
+ active_block.solve()
+ submit_block.solve()
+
+ # Both templates share a parent. The first block becomes active,
+ # so the remaining valid block is accepted as stale.
+ await self.assert_submit_block(
+ mining2, ctx2, active_block, result=True)
+
+ await self.assert_submit_block(
+ mining2, ctx2, submit_block, result=False, reason="inconclusive")
+ self.sync_all()
+
self.log.debug("Submit the same invalid block twice")
async with destroying((await mining2.createNewBlock(ctx2, self.default_block_create_options)).result, ctx2) as template2:
invalid_block = await self.build_candidate_block(template2, ctx2)
Why this scored 24/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.