refactor: centralize SubmitBlock result handling

Move the accepted/new-block/reason consistency check into SubmitBlock()
so submitBlock() and submitSolution() use the same success criteria.

This keeps duplicate and inconclusive handling in one place, removes the
new_block output parameter from the helper, and makes the helper return
whether the submitted block was accepted as a new valid block.

src/node/interfaces.cpp

@@ -920,9 +920,7 @@ public:
     bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase, std::string& reason, std::string& debug) override
     {
         AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce);
-        bool new_block;
-        const bool accepted = SubmitBlock(chainman(), std::make_shared<const CBlock>(m_block_template->block), &new_block, reason, debug);
-        return accepted && new_block && reason.empty();
+        return SubmitBlock(chainman(), std::make_shared<const CBlock>(m_block_template->block), reason, debug);
     }
 
     std::unique_ptr<BlockTemplate> waitNext(BlockWaitOptions options) override
@@ -1025,15 +1023,7 @@ public:
 
     bool submitBlock(const CBlock& block_in, std::string& reason, std::string& debug) override
     {
-        auto block = std::make_shared<const CBlock>(block_in);
-        bool new_block;
-        const bool accepted = SubmitBlock(chainman(), block, &new_block, reason, debug);
-        // 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.
-        const bool result{accepted && new_block && reason.empty()};
-        CHECK_NONFATAL(result == reason.empty());
-        return result;
+        return SubmitBlock(chainman(), std::make_shared<const CBlock>(block_in), reason, debug);
     }
 
     const NodeContext* context() override { return &m_node; }

src/node/miner.cpp

@@ -381,7 +381,7 @@ protected:
 };
 } // namespace
 
-bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock>& block, bool* new_block, std::string& reason, std::string& debug)
+bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock>& block, std::string& reason, std::string& debug)
 {
     reason.clear();
     debug.clear();
@@ -391,16 +391,16 @@ bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock
     // point decodes hex, formats BIP22/JSONRPC results, and calls
     // UpdateUncommittedBlockStructures() for legacy witness handling. IPC
     // callers submit already-formed blocks and need bool + reason/debug
-    // results, while submitSolution() preserves its duplicate-as-success
-    // behavior.
+    // results.
     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);
+    bool new_block;
+    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) {
+    if (!new_block && accepted) {
         reason = "duplicate";
     } else if (!accepted && (!sc->m_found || sc->m_state.IsValid())) {
         // ProcessNewBlock can fail without a validation result, for example
@@ -416,7 +416,9 @@ bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock
         reason = sc->m_state.GetRejectReason();
         debug = sc->m_state.GetDebugMessage();
     }
-    return accepted;
+    const bool result{accepted && new_block && reason.empty()};
+    CHECK_NONFATAL(result == reason.empty());
+    return result;
 }
 
 void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait)

src/node/miner.h

@@ -131,8 +131,8 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman);
 void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce);
 
 //! Submit a block and capture the validation state via the BlockChecked callback.
-//! Returns whether ProcessNewBlock accepted the block.
-bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock>& block, bool* new_block, std::string& reason, std::string& debug);
+//! Returns whether the block was accepted as a new valid block.
+bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr<const CBlock>& block, std::string& reason, std::string& debug);
 
 /* Interrupt a blocking call. */
 void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait);