Skip to content

mining: add reason/debug to submitSolution and unify with submitBlock#34672

Draft
w0xlt wants to merge 5 commits intobitcoin:masterfrom
w0xlt:ipc-submit-solution-new-fields
Draft

mining: add reason/debug to submitSolution and unify with submitBlock#34672
w0xlt wants to merge 5 commits intobitcoin:masterfrom
w0xlt:ipc-submit-solution-new-fields

Conversation

@w0xlt
Copy link
Contributor

@w0xlt w0xlt commented Feb 25, 2026

Built on top of #34644
Preferred to open a separate PR to keep the other one focused on submitBlock.

This PR:

  • Extracts a shared ProcessBlock helper that wraps ProcessNewBlock with SubmitBlockStateCatcher to capture BlockValidationState
  • Adds reason and debug output parameters to submitSolution, matching submitBlock
  • Makes both methods delegate to the same helper, eliminating duplicated logic

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.
Add reason and debug output parameters to submitSolution, matching
submitBlock. This relays the specific failure reason (e.g.
"bad-version(...)", "bad-witness-nonce-size", "duplicate") to callers
instead of just a bool.

This is a non-breaking capnp schema change: existing clients that
only read the bool result field continue to work.
Fold the duplicate/inconclusive/invalid result handling from
submitSolution and submitBlock into the ProcessBlock helper, so
both methods become simple one-line delegations.

No behavior change.
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 25, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #34662 (ci: use LLVM/Clang 22 in tidy job by fanquake)
  • #34422 (Update libmultiprocess subtree to be more stable with rust IPC client by ryanofsky)
  • #33922 (mining: add getMemoryLoad() and track template non-mempool memory footprint by Sjors)
  • #29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
  • #29409 (multiprocess: Add capnp wrapper for Chain interface by ryanofsky)
  • #10102 (Multiprocess bitcoin by ryanofsky)

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.

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:

  • test/functional/interface_ipc_mining.py:
    "assert len(block.vtx) >= 2, "Block should include at least the coinbase and the mempool tx"" -> use assert_greater_than_or_equal(len(block.vtx), 2, "Block should include at least the coinbase and the mempool tx")

2026-02-25 21:41:08

getCoinbaseTx @5 (context: Proxy.Context) -> (result: CoinbaseTx);
getCoinbaseMerklePath @6 (context: Proxy.Context) -> (result: List(Data));
submitSolution @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool);
submitSolution @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (reason: Text, debug: Text, result: Bool);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this not to be a breaking change, I suspect the new arguments need to go last?

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 3, 2026

🐙 This pull request conflicts with the target branch and needs rebase.

@DrahtBot DrahtBot mentioned this pull request Mar 4, 2026
@Sjors Sjors mentioned this pull request Mar 6, 2026
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants