mining: add precious option to IPC block submission #35300

w0xlt commented at 3:23 AM on May 16, 2026: contributor

This PR adds a precious flag to the IPC Mining submitBlock and submitSolution methods, as suggested in #34644 (review). Built on top #34644.

With this change, IPC mining clients can submit and prefer a block in one IPC call, instead of submitting it over IPC and then using the preciousblock RPC separately.

By default, same-work side blocks keep the existing behavior: submitBlock reports duplicate/inconclusive cases as failure with a reason, while submitSolution continues to return true when ProcessNewBlock accepts or already knows the block.

Includes unit and IPC functional coverage for default behavior, precious reorgs, active-tip no-op submissions, and invalid-block rejection.

DrahtBot added the label Mining on May 16, 2026

DrahtBot commented at 3:23 AM on May 16, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35300.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	Sjors, pablomartin4btc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35847 (test: move more tests to baseindex_tests and run them for all indexes by mzumsande)
#35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)

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 named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

[stale_template.submitSolution(ctx, side_solution_block.nVersion, side_solution_block.nTime, side_solution_block.nNonce, side_solution_coinbase.serialize(), False)] in test/functional/interface_ipc_mining.py
[stale_template.submitSolution(ctx, side_solution_block.nVersion, side_solution_block.nTime, side_solution_block.nNonce, side_solution_coinbase.serialize(), True)] in test/functional/interface_ipc_mining.py

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

[src/test/miner_tests.cpp] BOOST_CHECK_THROW(block_template->submitSolutionOld7(block.nVersion, block.nTime, block.nNonce, ...) -> Consider BOOST_CHECK_EXCEPTION with a matcher for the expected error text/reason instead of only checking the exception type.

2026-07-24 02:37:40

w0xlt marked this as a draft on May 16, 2026

w0xlt force-pushed on May 16, 2026

DrahtBot added the label CI failed on May 16, 2026

DrahtBot commented at 4:02 AM on May 16, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/25951528963/job/76290253269 LLM reason (✨ experimental): CI failed due to a C++ build error: src/node/miner.cpp would not compile (clang: passing CBlockIndex* where const CBlockIndex& is required in chainman.ActiveChain().Contains).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on May 16, 2026

Sjors commented at 9:39 AM on May 16, 2026: member

Maybe do this after #34672?

DrahtBot added the label Needs rebase on May 17, 2026

refactor: extract ActivateBestChain_ helper

Move the lock-held body of ActivateBestChain() into an internal helper without changing behavior. This allows callers that already hold m_chainstate_mutex to run activation without releasing and reacquiring it.

42d48e7760

mining: add precious IPC block submission option

Thread a precious flag through the Mining IPC submitBlock and submitSolution methods so callers can prefer an already-known same-work block, matching preciousblock-style behavior while preserving default duplicate handling.

Have PreciousBlock report whether the requested block is connected while still holding m_chainstate_mutex. This lets SubmitBlock return an atomic success result instead of racing a concurrent chain activation after PreciousBlock returns.

Prefer a captured validation failure over duplicate fallback so a known block that fails during precious activation returns its BIP22 reject reason.

f7a7b39e37

test: cover precious mining submissions

Add regtest unit coverage for the Mining interface precious flag, checking default duplicate and inconclusive behavior as well as submitBlock and submitSolution preferring a same-work side block when precious is set.

a1f8966316

test: deduplicate proof-of-work grinding 5299092c23

test: cover precious IPC mining submissions

Extend the IPC mining functional test to exercise the new precious argument for submitBlock and submitSolution, including default same-work side-block behavior and precious reorg behavior.

5d1bf44db5

doc: add IPC block submission release note 95f62bce86

w0xlt force-pushed on Jul 24, 2026

DrahtBot removed the label Needs rebase on Jul 24, 2026

in src/test/miner_tests.cpp:1029 in a1f8966316

1024 | +    BOOST_REQUIRE_EQUAL(reason, "");
1025 | +    BOOST_REQUIRE_EQUAL(debug, "");
1026 | +    BOOST_REQUIRE_EQUAL(ActiveTipHash(m_node), side_block.GetHash());
1027 | +
1028 | +    // Re-submitting the active tip with precious=true is a no-op reorg, but
1029 | +    // the block is validated/connected so submitBlock should report success.

Sjors commented at 12:47 PM on July 24, 2026:

In a1f8966316dcd0807d0ee612327d4a822c89177f test: cover precious mining submissions: mmm, I don't think precious should change the behavior if the block is duplicate, so just return duplicate?

Similarly, if we already had it, but it wasn't the tip, and we then switch the tip over, duplicate still seems like the correct response.

in src/test/miner_tests.cpp:1040 in a1f8966316

1035 | +    BOOST_REQUIRE_EQUAL(ActiveTipHash(m_node), side_block.GetHash());
1036 | +
1037 | +    // Submitting a brand-new same-work block with precious=true on the first
1038 | +    // try (no prior precious=false store) should still reorg to it. This
1039 | +    // exercises the path where AcceptBlock stores a fresh block and
1040 | +    // PreciousBlock immediately promotes it.

Sjors commented at 12:48 PM on July 24, 2026:

In a1f8966316dcd0807d0ee612327d4a822c89177f test: cover precious mining submissions: this seems to be the main use case, so maybe cover it at the start of the test?

In general this test is massive, so maybe split it?

in src/test/miner_tests.cpp:1145 in a1f8966316

1140 | +    BOOST_REQUIRE_EQUAL(reason, "bad-txnmrklroot");
1141 | +    BOOST_REQUIRE_EQUAL(debug, "hashMerkleRoot mismatch");
1142 | +    BOOST_REQUIRE_EQUAL(ActiveTipHash(m_node), tip_before_invalid);
1143 | +}
1144 | +
1145 | +BOOST_AUTO_TEST_CASE(SubmitSolution_precious)

Sjors commented at 12:50 PM on July 24, 2026:

In a1f8966316dcd0807d0ee612327d4a822c89177f test: cover precious mining submissions: since we expect more or less the same behavior as SubmitBlock, perhaps it's better to interleave the test? (and then have separate tests for each scenario, rather than for each method)

Sjors commented at 12:59 PM on July 24, 2026: member

Concept ACK

There's some game theory miners should contemplate before using this though:

It's clearly rational to prefer your own block for p2p relay[^0], but it's less obvious that you should mine the next block on it.

That might be a sunk-cost fallacy. If you saw the other block first, it's a good assumption that the rest of the network also saw it first. If the majority of hash rate is mining on the other side, and if you (the miner, not the pool) can't tip that majority over, it may be a waste of hash power.

In a Job Declaration scenario, the main criterion for which block to build on should be the side of the fork the pool picked. Unfortunately afaik the sv2 protocol doesn't currently communicate that (cc @plebhash).

[^0]: which could be achieved with some special handling where we briefly switch tip to validate, broadcast and then switch back to the first-seen tip.

in test/functional/interface_ipc_mining.py:133 in 95f62bce86

 129 | @@ -126,8 +130,29 @@ async def build_candidate_block(self, template, ctx, extra_nonce=b""):
 130 |          block.hashMerkleRoot = block.calc_merkle_root()
 131 |          return block
 132 |  
 133 | -    async def assert_submit_block(self, mining, ctx, block, *, result, reason="", debug=""):
 134 | -        submit = await mining.submitBlock(ctx, block.serialize())
 135 | +    def make_block_variant(self, block, coinbase, extra_nonce):

pablomartin4btc commented at 8:55 PM on July 24, 2026:

nit: make_block_variant, assert_chain_tip, and assert_submit_block none of them access self — all should be @staticmethod

in src/rpc/blockchain.cpp:1712 in 95f62bce86

1708 | @@ -1709,7 +1709,8 @@ static RPCMethod preciousblock()
1709 |      }
1710 |  
1711 |      BlockValidationState state;
1712 | -    chainman.ActiveChainstate().PreciousBlock(state, pblockindex);
1713 | +    bool connected{false};

pablomartin4btc commented at 9:05 PM on July 24, 2026:

nit: connected is silently unused. A [[maybe_unused]] attribute or a brief comment (// RPC callers don't need to distinguish connected vs not) would help the next reader I think...

in src/interfaces/mining.h:78 in 95f62bce86

  73 | @@ -72,9 +74,10 @@ class BlockTemplate
  74 |       *       is only one byte long, so the coinbase scriptSig needs at least
  75 |       *       one additional byte of data to avoid bad-cb-length.
  76 |       *
  77 | -     * @returns true if the block was accepted as a new block
  78 | +     * @returns true if the block was accepted as a new block. With precious,
  79 | +     *          an already-known block may return true if it is connected.

pablomartin4btc commented at 9:29 PM on July 24, 2026:

nit: perhaps worth clarifying that this includes blocks that are ancestors of the active tip, not just the tip itself, so callers don't assume result=true implies a reorg occurred? (If I understand it correctly)

in src/interfaces/mining.h:187 in 95f62bce86

 187 | +     * @returns           true if the block was accepted. By default, returns
 188 |       *                    false and sets reason if the block is a duplicate or
 189 | -     *                    the validation result is inconclusive.
 190 | +     *                    the validation result is inconclusive. With precious,
 191 | +     *                    an already-known block may return true if it is
 192 | +     *                    validated/connected.

pablomartin4btc commented at 9:29 PM on July 24, 2026:

nit: same as the above.

pablomartin4btc commented at 9:33 PM on July 24, 2026: member

Concept ACK

Adding precious=true to submitBlock and submitSolution, allows IPC mining clients atomically submit and prefer a same-work block in one go instead of a separate preciousblock RPC call.

The ActivateBestChain_ refactor is a clean prerequisite (so in commit 2 PreciousBlock can call it directly).

Left some nits (none blocking).