diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2f68f414f0d..6747dd60aaa 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 ccd9cc7c57a..66ae870f844 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -396,16 +396,21 @@ bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr(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 4cd9c17c99f..8d2ee85964c 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)