This PR adds a submitBlock method to the IPC Mining interface, equivalent to the submitblock RPC. It accepts a serialized block over IPC, validates/processes it via the normal block-processing path.
The method uses the same result shape as checkBlock: bool + reason/debug out-params. It reports duplicate, inconclusive, and invalid-block rejection details, and initializes reason/debug on every call.
Closes #34626
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
153+ * it, and if accepted as new, processes it into chainstate. Accepted
154+ * blocks may then be announced to peers through normal validation signals.
155+ *
156+ * Like the submitblock RPC, this may add the coinbase witness nonce if
157+ * missing, if the block already has a witness commitment and segwit is
158+ * active for the previous block (via UpdateUncommittedBlockStructures).
7a2d88c: I would prefer to mimic the behavior of submitSolution() here, and not magically modify the block.
It was useful for backwards compatibility when SegWit was newly introduced, but since IPC is new and SegWit has been active for a decade, there’s no need for it. And I find the behavior confusing.
Removed the UpdateUncommittedBlockStructures call - the block is now passed as std::make_shared<const CBlock>(block_in) directly (the const enforces this at the type level).
The doc comment now notes: “unlike the submitblock RPC, this method does NOT add the coinbase witness automatically.”
Added a test that submits a block with a witness commitment but no coinbase witness nonce and confirms it’s rejected with bad-witness-nonce-size.
927@@ -928,6 +928,24 @@ class BlockTemplateImpl : public BlockTemplate
928 NodeContext& m_node;
929 };
930
931+class SubmitBlockStateCatcher final : public CValidationInterface
submitSolution can do without.
submitSolution returns just a bool (and passes nullptr for new_block), so it doesn’t need to distinguish failure reasons.
submitBlock returns (reason, debug, result) and to populate reason and debug with the specific rejection string (e.g. bad-version(...), bad-witness-nonce-size), we need the BlockValidationState, which is only available through the BlockChecked validation interface callback.
checkBlock with submitSolution.
Yes, checkBlock calls TestBlockValidity() which returns BlockValidationState directly as its return value.
submitBlock calls ProcessNewBlock() which only returns a bool. The BlockValidationState with the specific rejection reason is only available asynchronously through the CValidationInterface::BlockChecked callback.
146@@ -147,10 +147,11 @@ class Mining
147 virtual bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) = 0;
148
149 /**
150- * Process and relay a fully assembled block.
151+ * Process a fully assembled block.
346+ async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template:
347+ block = await mining_get_block(template, ctx)
348+ balance = self.miniwallet.get_balance()
349+ coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)
350+ coinbase.vout[0].nValue = COIN
351+ block.vtx[0] = coinbase
b72108fac82c38ced57aeded8725aec59ec52304: let’s add at least one real transaction to the block.
It’s also useful, see my earlier comment, to test:
373+ assert_equal(self.nodes[0].getchaintips()[0], self.nodes[1].getchaintips()[0])
374+
375+ self.miniwallet.rescan_utxos()
376+ assert_equal(self.miniwallet.get_balance(), balance + 1)
377+
378+ self.log.debug("submitBlock duplicate should fail")
b72108fac82c38ced57aeded8725aec59ec52304: it might be useful to add a test for the interaction between submitSolution and submitBlock(). In either order this should result in a duplicate.
Calling submitSolution after submitBlock() shouldn’t result in a crash or missing object error, because we hold on to BlockTemplate as long as the client has a reference to it. The sv2-tp implementation waits 10 seconds after a new block before releasing stale templates.
Concept ACK
But let’s make it more similar to the submitSolution() IPC rather than the getblocktemplate RPC, in terms of behavior and implementation (which can be much smaller).
999@@ -982,6 +1000,44 @@ class MinerImpl : public Mining
1000 return state.IsValid();
1001 }
1002
1003+ bool submitBlock(const CBlock& block_in, std::string& reason, std::string& debug) override
submitSolution reconstructs the CBlock it should be able to use the same code path as submitBlock.
That’s true in principle, but submitSolution currently returns a plain bool (and true for duplicates), while submitBlock returns (reason, debug, result) and false for duplicates.
Sharing the code path would change submitSolution’s return semantics, which would be a breaking change for existing IPC clients. Might be worth doing as a follow-up if the IPC schema for submitSolution is updated to also return reason/debug strings.
Adding fields to .capnp is not a breaking change for clients. It makes sense to relay the failure reason.
I’m not saying that submitSolution should call submitBlock directly, it can be a common helper. So it shouldn’t be necessary to change the submitSolution signature, that can be left to a followup.
utACK 9e12fb4989bbac68d2ec4d13fcace56fd0b0c445
New method added to the mining IPC interface. New method, so no behavior change or compatibility issues. The implementation builds on submitSolution method and “submitblock” RPC, so it is rather small. Tests are added, including for typical error cases. Follow-up in #34672.
857@@ -858,6 +858,35 @@ class ChainImpl : public Chain
858 NodeContext& m_node;
859 };
860
861+class SubmitBlockStateCatcher final : public CValidationInterface
In commit “refactor: extract ProcessBlock helper for submitSolution and submitBlock” (https://github.com/bitcoin/bitcoin/pull/34644/changes/9e12fb4989bbac68d2ec4d13fcace56fd0b0c445):
SubmitBlockStateCatcher is essentially a 1:1 copy of submitblock_StateCatcher in src/rpc/mining.cpp. Since commit this commit already refactored how this is used locally, does it make sense to deduplicate it entirely and move it to a shared location so that it can be used in src/rpc/mining.cpp as well?
This can be done in a follow-up PR. It’s a refactor that increases scope. Moving the class to a shared header would touch src/rpc/mining.cpp which is unrelated to this PR’s purpose (adding IPC submitBlock).
And also it is only 10 lines of boilerplate. Deduplicating it would require a new shared header, an include in both files, and coordination between the RPC and IPC layers that currently don’t depend on each other. The “abstraction cost” exceeds the “duplication cost” for something this small.
493+ assert_equal(submitted, True)
494+
495+ self.sync_all()
496+
497+ asyncio.run(capnp.run(async_routine()))
498+
nit suggestion:
In commit “test: add IPC submitBlock functional test” (https://github.com/bitcoin/bitcoin/pull/34644/changes/354538db36dc8f04a547b6cbb9159d0a94834901):
The current tests check what happens when you submit the same valid block twice (returning “duplicate”). It would be great to also test what happens when you submit the same invalid block twice.
The IPC test should return “duplicate-invalid” the second time, proving that the node correctly remembered the block was bad and rejected it.
Here is a quick test snippet you can drop in to cover this
0index 032a696353..f5cc9c3d56 100755
1--- a/test/functional/interface_ipc_mining.py
2+++ b/test/functional/interface_ipc_mining.py
3@@ -530,6 +530,43 @@ class IPCMiningTest(BitcoinTestFramework):
4
5 asyncio.run(capnp.run(async_routine()))
6
7+ def run_submit_block_duplicate_invalid_test(self):
8+ """Test submitting the exact same invalid block twice to check StateCatcher behavior."""
9+ self.log.info("Running submitBlock duplicate-invalid test")
10+
11+ async def async_routine():
12+ ctx, mining = await self.make_mining_ctx()
13+
14+ async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as
15 template:
16+ block = await mining_get_block(template, ctx)
17+ coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)
18+ coinbase.vout[0].nValue = COIN
19+ block.vtx[0] = coinbase
20+ block.hashMerkleRoot = block.calc_merkle_root()
21+
22+ # Make the block definitively invalid (bad locktime)
23+ block.vtx[0].nLockTime = 2**32 - 1
24+ block.hashMerkleRoot = block.calc_merkle_root()
25+ block.solve()
26+ serialized_block = block.serialize()
27+
28+ self.log.debug("Submit invalid block first time")
29+ result1 = await mining.submitBlock(ctx, serialized_block)
30+ assert_equal(result1.result, False)
31+ assert_equal(result1.reason, "bad-txns-nonfinal")
32+
33+
34+ self.log.info(f"First submission reason was: '{result1.reason}'")
35+
36+ self.log.debug("Submit the exact same invalid block a second time")
37+ result2 = await mining.submitBlock(ctx, serialized_block)
38+ assert_equal(result2.result, False)
39+
40+ self.log.info(f"Second submission reason was: '{result2.reason}'")
41+ assert_equal(result2.reason, "duplicate-invalid")
42+
43+ asyncio.run(capnp.run(async_routine()))
44+
45 def run_test(self):
46 self.miniwallet = MiniWallet(self.nodes[0])
47 self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions()
48@@ -540,6 +577,7 @@ class IPCMiningTest(BitcoinTestFramework):
49 self.run_submit_block_witness_test()
50 self.run_submit_block_then_solution_test()
51 self.run_submit_solution_then_block_test()
52+ self.run_submit_block_duplicate_invalid_test()
53 self.run_ipc_option_override_test()
527+ assert_equal(result.reason, "duplicate")
528+
529+ self.sync_all()
530+
531+ asyncio.run(capnp.run(async_routine()))
532+
In commit “test: add IPC submitBlock functional test” (https://github.com/bitcoin/bitcoin/pull/34644/changes/354538db36dc8f04a547b6cbb9159d0a94834901):
I was testing the boundary conditions of the new IPC interface and noticed that sending malformed or truncated block data causes the entire node to crash.
Unlike the standard submitblock RPC which safely catches deserialization errors and returns a -22 Block decode failed error. The IPC implementation seems to let the deserialization exception bubble up, resulting in a SIGABRT (exit code -6).
Since a sv2 client could potentially send bad wire data, this looks like a DoS vector that should be caught at the boundary.
You can reproduce the crash by adding this test to test/functional/interface_ipc_mining.py:
0def run_submit_block_malformed_test(self):
1 """Test that submitting truncated/garbage bytes does not crash the node."""
2 self.log.info("Running submitBlock malformed data test")
3
4 async def async_routine():
5 ctx, mining = await self.make_mining_ctx()
6
7 async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template:
8 block = await mining_get_block(template, ctx)
9
10 # Truncate the last 15 bytes to create an invalid serialization
11 bad_bytes = block.serialize()[:-15]
12
13 self.log.debug("Submitting truncated block bytes via IPC")
14
15 try:
16 result = await mining.submitBlock(ctx, bad_bytes)
17 assert_equal(result.result, False)
18 except Exception as e:
19 self.log.info(f"IPC exception caught in test: {e}")
20
21 # The node should handle the bad stream gracefully and remain alive
22 assert_equal(self.nodes[0].is_node_stopped(), False)
23
24 asyncio.run(capnp.run(async_routine()))
Running this results in the peer disconnecting and the node crashing:
0AssertionError: [node 0] Node returned unexpected exit code (-6) vs ((0,)) when stopping
1capnp.lib.capnp.KjException: capnp/rpc.c++:2778: disconnected: Peer disconnected.
I suspect the C++ handler unpacking the Data blob into a CBlock needs to be wrapped in a try...catch block to handle the std::ios_base::failure and return a safe rejection reason (e.g., "inconclusive" or a specific decode error), similar to how src/rpc/mining.cpp handles it.
I ran the test but I wasn’t able to reproduce the error you mentioned.
As I understand it, Proxy.Context already handles it (via kj::runCatchingExceptions()), catching std::exception and sends it back to the client as kj::Exception rather than crashing the node.
I added the suggested functional test for this (truncated block bytes via submitBlock, asserting the node stays alive) so we can verify this on CI. Could you share which platform/configuration you tested on and which capnp version you were using? Is it possible there’s a platform-specific edge case ?
Here is my environment
The kj::runCatchingExceptions() safety net might be failing locally because Fedora’s capnproto package could be compiled with flags that alter standard exception handling. This could cause it to bypass the catch block and crash.
I couldn’t find any information about flags that alter standard exception handling. Standard C++ try/catch for std::exception is fundamental. Distro packages don’t break this.
Even if Cap’n Proto were built with -fno-exceptions, it would use a completely different error path.
The information provided so far are vague and insufficient to reproduce the error.
Anyway, what you are reporting - assuming it is reproducible - affects the IPC as overall (it would affect checkBlock and any other IPC method that deserializes Data → C++ types, not just submitBlock) and should be handled in a specific PR.
checkBlock or other IPC methods, then it is a broader issue with the IPC layer itself. It definitely shouldn’t hold up this specific PR.
Concept ACK
I reviewed the code and ran some boundary tests. Left some comments below along with a couple of suggestions for test coverage and code cleanup
I opened https://github.com/2140-dev/bitcoin-capnp-types/pull/12 for easier testing from Rust, cc @plebhash.
It would be good to rebase this now that #34422 landed, to prevent confusion while testing from Rust.
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.
Unlike the submitblock RPC, this method does NOT auto-add the coinbase
witness nonce (via UpdateUncommittedBlockStructures). Since this is a new
interface and SegWit has been active for years, callers must provide a
fully-formed block including the coinbase witness nonce when a witness
commitment is present.
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.
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)
The test bootstraps a candidate block with createNewBlock(), mutates and
serializes it, then submits it through Mining.submitBlock instead of
BlockTemplate.submitSolution. This exercises the complete-block submission
path and approximates the Sv2 JDS use case.
Move SubmitBlockStateCatcher and the ProcessNewBlock + validation
interface registration logic into a shared ProcessBlock() helper.
Both submitSolution and submitBlock now use this common code path.
No behavior change.
It would be good to rebase this
Done. Thanks.