From 813b4a80d7f8c2d7cc6cf7b09fb74039b7e28b55 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Thu, 14 May 2026 13:17:06 -0700 Subject: [PATCH 1/3] refactor: introduce SubmitBlock helper Introduce a SubmitBlock() helper in node/miner.cpp that wraps ProcessNewBlock submission and captures validation state through the BlockChecked callback. Route submitSolution through the helper before adding any new IPC method. No behavior change. --- src/node/interfaces.cpp | 4 ++- src/node/miner.cpp | 57 +++++++++++++++++++++++++++++++++++++++++ src/node/miner.h | 4 +++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index edb73628484..28f8f2e88ee 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -920,7 +920,9 @@ public: bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) override { AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce); - return chainman().ProcessNewBlock(std::make_shared(m_block_template->block), /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/nullptr); + std::string reason; + std::string debug; + return SubmitBlock(chainman(), std::make_shared(m_block_template->block), /*new_block=*/nullptr, reason, debug); } std::unique_ptr waitNext(BlockWaitOptions options) override diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 5c0df502e83..ccd9cc7c57a 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -357,6 +358,62 @@ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t block.fChecked = false; } +namespace { +class SubmitBlockStateCatcher final : public CValidationInterface +{ +public: + uint256 m_hash; + bool m_found{false}; + BlockValidationState m_state; + + explicit SubmitBlockStateCatcher(const uint256& hash) : m_hash{hash} {} + +protected: + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) override + { + if (block->GetHash() != m_hash) return; + // ProcessNewBlock emits BlockChecked synchronously while holding cs_main, + // so SubmitBlock can read these fields after ProcessNewBlock returns + // without extra synchronization. + m_found = true; + m_state = state; + } +}; +} // namespace + +bool SubmitBlock(ChainstateManager& chainman, const std::shared_ptr& block, bool* new_block, std::string& reason, std::string& debug) +{ + reason.clear(); + debug.clear(); + + // This follows the submitblock RPC's validation-state capture pattern, but + // is intentionally kept separate from the RPC implementation. The RPC entry + // 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. + auto sc = std::make_shared(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); + CHECK_NONFATAL(chainman.m_options.signals)->UnregisterSharedValidationInterface(sc); + + if (new_block && !*new_block && accepted) { + reason = "duplicate"; + } 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. + reason = "inconclusive"; + } else if (!sc->m_state.IsValid()) { + reason = sc->m_state.GetRejectReason(); + debug = sc->m_state.GetDebugMessage(); + } + return accepted; +} + void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait) { LOCK(kernel_notifications.m_tip_block_mutex); diff --git a/src/node/miner.h b/src/node/miner.h index 14780833c02..af327307329 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -18,6 +18,7 @@ #include #include #include +#include #include class CBlockIndex; @@ -129,6 +130,9 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman); /* Compute the block's merkle root, insert or replace the coinbase transaction and the merkle root into the block */ 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& block, bool* new_block, std::string& reason, std::string& debug); /* Interrupt a blocking call. */ void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wait); From 5b60f69e40ac1ec2b2f4276e2529914813cd22e5 Mon Sep 17 00:00:00 2001 From: woltx <94266259+w0xlt@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:30:09 -0800 Subject: [PATCH 2/3] mining: add submitBlock IPC method to Mining interface Add a submitBlock method to the Mining IPC interface, similar to the submitblock RPC. This accepts a fully assembled block, validates it, and if accepted as new, processes it into chainstate. This is needed for Stratum v2 Job Declarator Server (JDS), where accepted solutions may correspond to jobs not tied to a Bitcoin Core BlockTemplate. JDS receives PushSolution fields and reconstructs full blocks; without an IPC submitBlock method, final submission requires the submitblock RPC. The method returns detailed status (reason/debug strings) matching the checkBlock pattern, giving callers enough information to handle validation failures. --- src/interfaces/mining.h | 27 +++++++++++++++++++++++++-- src/ipc/capnp/mining.capnp | 1 + src/node/interfaces.cpp | 11 +++++++++++ src/test/miner_tests.cpp | 18 +++++++++++++----- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index a95bc220185..ff4f87109f9 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -60,8 +60,10 @@ public: * @param[in] nonce nonce block header field * @param[in] coinbase complete coinbase transaction (including witness) * - * @note unlike the submitblock RPC, this method does NOT add the - * coinbase witness automatically. + * @note Unlike the submitblock RPC, this method does not call + * UpdateUncommittedBlockStructures to add a missing coinbase witness + * reserved value. Callers must provide a complete coinbase transaction, + * including the witness when a witness commitment is present. * * @note for heights <= 16, the BIP34 height push in getCoinbaseTx().script_sig_prefix * is only one byte long, so the coinbase scriptSig needs at least @@ -157,6 +159,27 @@ public: */ virtual bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) = 0; + /** + * Process a fully assembled block. + * + * Similar to the submitblock RPC. Accepts a complete block, validates + * it, and if accepted as new, processes it into chainstate. Accepted + * blocks may then be announced to peers through normal validation signals. + * + * @param[in] block the complete block to submit + * @param[out] reason failure reason (BIP22) + * @param[out] debug more detailed rejection reason + * @returns true if the block was accepted as a new block. Returns + * false and sets reason if the block is a duplicate or + * the validation result is inconclusive. + * + * @note Unlike the submitblock RPC, this method does not call + * UpdateUncommittedBlockStructures to add a missing coinbase witness + * reserved value. Callers must submit a fully formed block, including + * the coinbase witness when a witness commitment is present. + */ + virtual bool submitBlock(const CBlock& block, std::string& reason, std::string& debug) = 0; + //! Get internal node context. Useful for RPC and testing, //! but not accessible across processes. virtual const node::NodeContext* context() { return nullptr; } diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 64cad4d49f2..a6dd8d71f31 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -25,6 +25,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") { 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); interrupt @6 () -> (); + submitBlock @7 (context :Proxy.Context, block: Data) -> (reason: Text, debug: Text, result: Bool); } interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 28f8f2e88ee..2f68f414f0d 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1023,6 +1023,17 @@ public: return state.IsValid(); } + bool submitBlock(const CBlock& block_in, std::string& reason, std::string& debug) override + { + auto block = std::make_shared(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. + return accepted && new_block && reason.empty(); + } + const NodeContext* context() override { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } KernelNotifications& notifications() { return *Assert(m_node.notifications); } diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 2ddc1307cba..fd9559b543c 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -858,12 +858,20 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) block.hashMerkleRoot = BlockMerkleRoot(block); block.nNonce = bi.nonce; } - std::shared_ptr shared_pblock = std::make_shared(block); - // Alternate calls between Chainman's ProcessNewBlock and submitSolution - // via the Mining interface. The former is used by net_processing as well - // as the submitblock RPC. + // Alternate calls between submitBlock and submitSolution via the + // Mining interface. if (current_height % 2 == 0) { - BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)); + std::string reason{"stale reason"}; + std::string debug{"stale debug"}; + BOOST_REQUIRE(mining->submitBlock(block, reason, debug)); + BOOST_REQUIRE_EQUAL(reason, ""); + BOOST_REQUIRE_EQUAL(debug, ""); + + reason = "stale reason"; + debug = "stale debug"; + BOOST_REQUIRE(!mining->submitBlock(block, reason, debug)); + BOOST_REQUIRE_EQUAL(reason, "duplicate"); + BOOST_REQUIRE_EQUAL(debug, ""); } else { BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(txCoinbase))); } From 3962138cc036578926d901e6ff4ff807243bbb4e Mon Sep 17 00:00:00 2001 From: woltx <94266259+w0xlt@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:41:49 -0800 Subject: [PATCH 3/3] test: add IPC submitBlock functional test Test the new Mining.submitBlock IPC method: - Invalid block (bad version) returns failure with reason - Valid block (with a real mempool tx) is accepted and propagates - Duplicate block returns failure with "duplicate" reason - Witness commitment without coinbase witness nonce is rejected (bad-witness-nonce-size), confirming no auto-fix behavior - submitBlock then submitSolution: duplicate is accepted (submitSolution returns true for already-known blocks) - submitSolution then submitBlock interaction (duplicate) Build candidate blocks from BlockTemplate data in the existing coinbase and submission test, then exercise checkBlock(), submitSolution(), and submitBlock() against those candidates. submitBlock() uses an isolated IPC node for cases that would otherwise affect the main submitSolution() and checkBlock() assertions. --- test/functional/interface_ipc_mining.py | 143 +++++++++++++++++++-- test/functional/test_framework/ipc_util.py | 8 +- 2 files changed, 138 insertions(+), 13 deletions(-) diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index b59ffaab653..4cd9c17c99f 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -6,9 +6,14 @@ import asyncio import time from contextlib import AsyncExitStack +from copy import deepcopy from decimal import Decimal from io import BytesIO -from test_framework.blocktools import NULL_OUTPOINT, script_BIP34_coinbase_height +from test_framework.blocktools import ( + NULL_OUTPOINT, + script_BIP34_coinbase_height, + WITNESS_COMMITMENT_HEADER, +) from test_framework.messages import ( CBlockHeader, COIN, @@ -58,10 +63,10 @@ class IPCMiningTest(BitcoinTestFramework): self.skip_if_no_py_capnp() def set_test_params(self): - self.num_nodes = 2 + self.num_nodes = 3 def setup_nodes(self): - self.extra_init = [{"ipcbind": True}, {}] + self.extra_init = [{"ipcbind": True}, {}, {"ipcbind": True}] super().setup_nodes() # Use this function to also load the capnp modules (we cannot use set_test_params for this, # as it is being called before knowing whether capnp is available). @@ -106,6 +111,22 @@ 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.""" + block = await mining_get_block(template, ctx) + coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet) + # Reduce payout for balance comparison simplicity. + coinbase.vout[0].nValue = COIN + block.vtx[0] = coinbase + block.hashMerkleRoot = block.calc_merkle_root() + return block + + async def assert_submit_block(self, mining, ctx, block, *, result, reason="", debug=""): + submit = await mining.submitBlock(ctx, block.serialize()) + assert_equal(submit.result, result) + assert_equal(submit.reason, reason) + assert_equal(submit.debug, debug) + def run_mining_interface_test(self): """Test Mining interface methods.""" self.log.info("Running Mining interface test") @@ -167,6 +188,10 @@ class IPCMiningTest(BitcoinTestFramework): # Reconnect nodes so next tests are happy node.wait_for_rpc_connection() self.connect_nodes(1, 0) + # Restarting node 0 drops its P2P connection to the rest of the test + # chain. Restore the synced 0-1-2 topology before later tests split + # node 2 off for submitBlock checks. + self.sync_all() def run_block_template_test(self): """Test BlockTemplate interface methods.""" @@ -461,20 +486,31 @@ class IPCMiningTest(BitcoinTestFramework): async def async_routine(): ctx, mining = await make_mining_ctx(self) + # Node 0 drives the checkBlock() and submitSolution() checks. Node + # 2 has a separate IPC interface and starts synced through node 1, + # so it can be isolated below to test submitBlock() without + # changing node 0's template and chain state first. + ctx2, mining2 = await make_mining_ctx(self, node_index=2) current_block_height = self.nodes[0].getchaintips()[0]["height"] check_opts = self.capnp_modules['mining'].BlockCheckOptions() + # Send a real transaction so the template includes it. + self.miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0]) + async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template: - block = await mining_get_block(template, ctx) + block = await self.build_candidate_block(template, ctx) + coinbase = block.vtx[0] + self.log.debug("Template should include a mempool transaction") + assert len(block.vtx) >= 2, "Block should include at least the coinbase and the mempool tx" balance = self.miniwallet.get_balance() - coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet) - # Reduce payout for balance comparison simplicity - coinbase.vout[0].nValue = COIN - block.vtx[0] = coinbase - block.hashMerkleRoot = block.calc_merkle_root() original_version = block.nVersion + self.log.debug("Disconnect node 2 before block submission tests") + # The default topology is 2 -> 1 -> 0. Splitting the 1-2 edge + # lets node 2 accept/reject complete blocks independently. + self.disconnect_nodes(1, 2) + self.log.debug("Submit solution that can't be deserialized") try: await template.submitSolution(ctx, 0, 0, 0, b"") @@ -488,8 +524,19 @@ class IPCMiningTest(BitcoinTestFramework): check = await mining.checkBlock(ctx, block.serialize(), check_opts) assert_equal(check.result, False) assert_equal(check.reason, "bad-version(0x00000000)") + assert_equal(check.debug, "rejected nVersion=0x00000000 block") + self.log.debug("submitSolution should reject a bad-version block") submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result assert_equal(submitted, False) + self.log.debug("submitBlock should reject a bad-version block") + await self.assert_submit_block( + mining2, + ctx2, + block, + result=False, + reason="bad-version(0x00000000)", + debug="rejected nVersion=0x00000000 block", + ) self.log.debug("Submit a valid block") block.nVersion = original_version block.solve() @@ -503,6 +550,16 @@ class IPCMiningTest(BitcoinTestFramework): self.log.debug("Submitted coinbase must include witness") assert_not_equal(coinbase.serialize_without_witness().hex(), coinbase.serialize().hex()) + missing_witness_block = deepcopy(block) + missing_witness_block.vtx[0].wit.vtxinwit = [] + has_witness_commitment = any( + len(o.scriptPubKey) >= 38 and o.scriptPubKey[2:6] == WITNESS_COMMITMENT_HEADER + for o in missing_witness_block.vtx[0].vout + ) + assert has_witness_commitment, "Coinbase should have a witness commitment output" + missing_witness_block.hashMerkleRoot = missing_witness_block.calc_merkle_root() + missing_witness_block.solve() + self.log.debug("submitSolution should reject a coinbase missing witness") submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness())).result assert_equal(submitted, False) @@ -512,14 +569,34 @@ class IPCMiningTest(BitcoinTestFramework): remote_block_after = await mining_get_block(template, ctx) assert_not_equal(remote_block_before.serialize().hex(), remote_block_after.serialize().hex()) + self.log.debug("submitBlock should reject a block missing coinbase witness") + await self.assert_submit_block( + mining2, + ctx2, + missing_witness_block, + result=False, + reason="bad-witness-nonce-size", + debug="CheckWitnessMalleation : invalid witness reserved value size", + ) + self.log.debug("Submit again, with the witness") submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result assert_equal(submitted, True) + self.log.debug("Submit a valid complete block through the disconnected node") + await self.assert_submit_block(mining2, ctx2, block, result=True) + assert_equal(self.nodes[2].getchaintips()[0]["height"], current_block_height + 1) + self.log.debug("submitBlock should reject the duplicate complete block") + await self.assert_submit_block(mining2, ctx2, block, result=False, reason="duplicate") + self.log.debug("Block should propagate") # Check that the IPC node actually updates its own chain assert_equal(self.nodes[0].getchaintips()[0]["height"], current_block_height + 1) # Stalls if a regression causes submitSolution() to accept an invalid block: + self.sync_blocks(self.nodes[:2]) + # Rejoin node 2 and verify the submitBlock result converges with + # the submitSolution result from node 0. + self.connect_nodes(1, 2) self.sync_all() # Check that the other node accepts the block assert_equal(self.nodes[0].getchaintips()[0], self.nodes[1].getchaintips()[0]) @@ -530,6 +607,54 @@ class IPCMiningTest(BitcoinTestFramework): check = await mining.checkBlock(ctx, block.serialize(), check_opts) assert_equal(check.result, False) assert_equal(check.reason, "inconclusive-not-best-prevblk") + self.log.debug("submitBlock on the same node should fail with duplicate after submitSolution succeeds") + await self.assert_submit_block(mining, ctx, block, result=False, reason="duplicate") + + self.log.debug("submitSolution should still return True for a duplicate after submitBlock succeeds") + async with destroying((await mining2.createNewBlock(ctx2, self.default_block_create_options)).result, ctx2) as template2: + duplicate_block = await self.build_candidate_block(template2, ctx2) + duplicate_coinbase = duplicate_block.vtx[0] + duplicate_block.solve() + self.log.debug("Submit a valid complete block before duplicate submitSolution") + await self.assert_submit_block(mining2, ctx2, duplicate_block, result=True) + self.nodes[2].waitforblockheight(current_block_height + 2) + self.log.debug("submitSolution should accept the duplicate block") + submitted = (await template2.submitSolution(ctx2, duplicate_block.nVersion, duplicate_block.nTime, duplicate_block.nNonce, duplicate_coinbase.serialize())).result + assert_equal(submitted, True) + 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) + invalid_block.vtx[0].nLockTime = 2**32 - 1 + invalid_block.hashMerkleRoot = invalid_block.calc_merkle_root() + invalid_block.solve() + self.log.debug("submitBlock should reject the non-final block") + await self.assert_submit_block( + mining2, + ctx2, + invalid_block, + result=False, + reason="bad-txns-nonfinal", + debug="non-final transaction", + ) + self.log.debug("submitBlock should report duplicate-invalid for the same block") + await self.assert_submit_block( + mining2, + ctx2, + invalid_block, + result=False, + reason="duplicate-invalid", + debug=f"block {invalid_block.hash_hex} was previously marked invalid", + ) + + self.log.debug("Submit a malformed complete block") + try: + await mining2.submitBlock(ctx2, block.serialize()[:-15]) + raise AssertionError("submitBlock unexpectedly succeeded") + except capnp.lib.capnp.KjException as e: + assert_capnp_failed(e, "remote exception: std::exception: SpanReader::read(): end of data:") + assert_equal(self.nodes[2].is_node_stopped(), False) asyncio.run(capnp.run(async_routine())) diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py index 5ce582372f8..a4ebd091e8e 100644 --- a/test/functional/test_framework/ipc_util.py +++ b/test/functional/test_framework/ipc_util.py @@ -94,8 +94,8 @@ def load_capnp_modules(config): } -async def make_capnp_init_ctx(self): - node = self.nodes[0] +async def make_capnp_init_ctx(self, node_index=0): + node = self.nodes[node_index] # Establish a connection, and create Init proxy object. connection = await capnp.AsyncIoStream.create_unix_connection(node.ipc_socket_path) client = capnp.TwoPartyClient(connection) @@ -152,9 +152,9 @@ async def mining_get_coinbase_tx(block_template, ctx) -> CoinbaseTxData: lockTime=int(template_capnp.lockTime), ) -async def make_mining_ctx(self): +async def make_mining_ctx(self, node_index=0): """Create IPC context and Mining proxy object.""" - ctx, init = await make_capnp_init_ctx(self) + ctx, init = await make_capnp_init_ctx(self, node_index) self.log.debug("Create Mining proxy object") mining = init.makeMining(ctx).result return ctx, mining