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.

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; }

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();