Add checkblock RPC and checkBlock() to Mining interface #31564

Sjors · 2024-12-24T17:07:27Z

Rationale

Verifying block templates (no PoW)

Stratum v2 allows miners to generate their own block template. Pools may wish (or need) to verify these templates. This typically involves comparing mempools, asking miners to providing missing transactions and then reconstructing the proposed block.¹ This is not sufficient to ensure a proposed block is actually valid. In some schemes miners could take advantage of incomplete validation².

The Stratum Reference Implementation (SRI), currently the only Stratum v2 implementation, collects all missing mempool transactions, but does not yet fully verify the block.³ There's currently no way for it to do so, short of re-implementing Bitcoin Core validation code.

(although SRI could use this PR, the Template Provider role doesn't need it, so this is not part of #31098)

Verifying weak blocks (reduced PoW)

Stratum v2 decentralises block template generation, but still hinge on a centralised entity to approve these templates.

There used to be a decentralised mining protocol P2Pool⁴ which relied on (what we would today call) weak blocks. In such a system all participants perform verification and there is no privileged pool operator (that can reject a template).

P2Pool shares form a "sharechain" with each share referencing the previous share's hash. Each share contains a standard Bitcoin block header, some P2Pool-specific data that is used to compute the generation transaction (total subsidy, payout script of this share, a nonce, the previous share's hash, and the current target for shares), and a Merkle branch linking that generation transaction to the block header's Merkle hash.

IIUC only the header and coinbase of these shares / weak blocks were distributed amongst all pool participants and verified. This was sufficient at the time because fees were negligible, so payouts could simply be distributed based on work.

However, if P2Pool were to be resurrected today⁵, it would need to account for fees in a similar manner as PPLNS² above. In that case the share chain should include the full weak blocks⁶. The client software would need a way to verify those weak blocks.

A similar argument applies to Braidpool.

Enter `checkblock` and its IPC counterpart `checkBlock()`

Given a serialised block, it performs the same checks as submitblock, but without updating the block index, chainstate, etc. The proof-of-work check can be bypassed entirely, for the first use of verifying block templates. Alternatively a custom target can be provided for the use case of weak blocks.

When called with check_pow=false the checkblock RPC is (almost) the same as getblocktemplate in proposal mode, and indeed they both call checkBlock() on the Mining interface.

Implementation details:

Builds on:

The ChainstateManager::CheckNewBlock() implementation is based on ProcessNewBlock(), but refactored in a number of ways (see Sjors#75 for previous incarnations):

it drops the use of a StateCatcher
it requires building (directly) on the tip
if calls CheckWeakProofOfWork before the other checks
instead of AcceptBlock() it performs a (documented) subset of that function
it creates a throwaway CCoinsViewCache on top of the tip on which it connects the block (needed for full transaction verification)
it updates validation caches (and does not delete them). This should make subsequent validation (of a real block) faster if it contains overlapping transactions that are not in our mempool.

This CheckNewBlock() method is then called from the Mining interface createBlock() method. This in turn is called from the RPC checkblock code, which is a simplified clone of submitblock.

This method ended up being very similar to TestBlockValidity, so the last commit replaces the latter entirely (renaming CheckNewBlock).

This PR contains a simple unit test and a more elaborate functional test.

Potential followups:

if the block contains "better" transactions, (optionally) add them to our mempool ⁵
keep track of non-standard transactions⁷
allow rollback (one or two blocks) for pools to verify stale work / uncles

https://github.com/stratum-mining/sv2-spec/blob/main/06-Job-Declaration-Protocol.md ↩
https://delvingbitcoin.org/t/pplns-with-job-declaration/1099/45?u=sjors ↩ ↩²
https://github.com/stratum-mining/stratum/blob/v1.1.0/roles/jd-server/src/lib/job_declarator/message_handler.rs#L196 ↩
https://en.bitcoin.it/wiki/P2Pool ↩
this improves compact block relay once the real block comes in ↩ ↩²
The share chain client software could use compact block encoding to reduce its orphan / uncle rate. To make that easier, we could provide a reconstructblock RPC (reconstructBlock IPC) that takes the header and short ids, fills in a CBlock from the mempool (and jail) and returns a list of missing items. The share chain client then goes and fetches missing items and calls the method again. It then passes the complete block to checkblock. This avoids the need for others to fully implement compact block relay. Note that even if we implemented p2p weak block relay, it would not be useful for share chain clients, because they need to relay additional pool-specific data. ↩
https://delvingbitcoin.org/t/second-look-at-weak-blocks/805 ↩

DrahtBot · 2024-12-24T17:07:30Z

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/31564.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	tdb3

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

Sjors · 2024-12-24T17:09:59Z

I wrote on IRC:

Does anyone know what UpdateUncommittedBlockStructures is good for in the submitblock RPC?

To which @sipa responded:

Sjors[m]: it looks like it adds the "witness nonce" if missing (the witness for the coinbase input is a reserved field for adding future commitments, but it must be present, so that function adds a 0)

This PR omits UpdateUncommittedBlockStructures. It would be nice to come up with a test that shows what it does and fails on the current code.

mzumsande

The newly introduced CheckNewBlock() looks very similar to the existing TestBlockValidity() which was just removed from the mining interface in bfc4e02 - it just seems to be a bit more verbose and allows the multiplier for the PoW. Did you consider merging these functions, so that we don't have multiple functions in validation that basically do the same thing?

tdb3

Concept ACK

Really liking the idea of facilitating these type of systems.

Instead of a multiplier I could also allow a custom target. This would give weak block systems more flexibility on how to derive their difficulty.

I'd be in favor of this. It's more flexible, could prevent interface churn, and if the user wishes to create the custom target from a simple multiplication they could do the multiplication on their end.

tdb3 · 2024-12-26T23:13:07Z

src/pow.cpp

@@ -161,3 +161,19 @@ bool CheckProofOfWorkImpl(uint256 hash, unsigned int nBits, const Consensus::Par

    return true;
 }
+
+bool CheckWeakProofOfWork(uint256 hash, unsigned int nBits, unsigned int multiplier, const Consensus::Params& params)


Feel free to ignore before more concept acks arrive.
CheckWeakProofOfWork() seems really similar to CheckProofOfWorkImpl(). Maybe I'm missing something simple. What was the rationale for creating the new function vs adding an argument?

Just that I didn't want to touch CheckProofOfWorkImpl.

This also applies to the new DeriveTarget method. I added the motivation for not changing CheckProofOfWorkImpl to the commit message in: d3710b0

Sjors · 2024-12-27T14:32:16Z

The newly introduced CheckNewBlock() looks very similar to the existing TestBlockValidity()

It looks like I ended up with a similar design after a long detour. I'll look into combining these.

Instead of a multiplier I could also allow a custom target

@instagibbs is there any reason you went for a multiplier?

instagibbs · 2024-12-30T16:47:54Z

is there any reason you went for a multiplier?

nothing off the top of my head

Sjors · 2024-12-30T16:48:30Z

I refactored checkblock to take a target argument rather than a multiplier. In order to make this easier to implement and test, I also introduced a gettarget RPC and DeriveTarget() helper.

I then dropped the original TestBlockValidity and renamed CheckNewBlock to it. The generateblock and getblocktemplate RPC calls, as well
as BlockAssembler::CreateNewBlock use the new method.

If people prefer I could rewrite the git history to directly refactor TestBlockValidity, though I suspect the current approach is easier to review.

I split the test changes out into #31581, so will mark this PR as draft for now. While working on that I noticed a rather big difference between the original TestBlockValidity and my incarnation of it: the former required holding cs_main, which I think was wrong.

DrahtBot · 2024-12-30T16:58:19Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/34990349558}

Hints

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.

Sjors · 2024-12-30T18:27:18Z

I might make a separate PR to consider whether to drop AssertLockHeld(cs_main) from the original TestBlockValidity.

If that ends up rejected, then I'll keep the existing TestBlockValidity calls as they are, i.e. drop the last two commits from this PR and leave them as a followup.

Related: #31563

While working on that I noticed a rather big difference between the original TestBlockValidity and my incarnation of it: the former required holding cs_main, which I think was wrong.

Update: I was confused, CheckBlock needs to be called with a lock on cs_main, and there's no need to "drop AssertLockHeld(cs_main)". That should allow for some simplifications.

Sjors · 2024-12-30T20:13:15Z

Untangled the mess and rebased after #31562.

Also split out #31583.

0424968 test: use Mining interface in miner_tests (Sjors Provoost) Pull request description: Needed for both #31283 and #31564. By using the Mining interface in `miner_tests.cpp` we increase its coverage in unit tests. ACKs for top commit: achow101: ACK 0424968 ryanofsky: Code review ACK 0424968, just minor suggested changes (renames, comments, BOOST_REQUIREs) since last review and some more extra clarifications and checks added to the CreateNewBlock_validity test. The CreateNewBlock_validity changes seem clear and easy to understand now. vasild: ACK 0424968 tdb3: ACK 0424968 Tree-SHA512: 2761cb7555d759670e40d8f37b96a079f8e12a588ac43313b9e63c69afd478321515873a8896ea56784f0100dac4476b0c0e0ef8b5418f8aea24d9965cace4d4

luke-jr · 2025-01-07T06:15:57Z

The new RPC interface is redundant with the existing BIP 23 block proposals support.

P.S. DATUM does not provide a way for the pool to reject valid blocks. There is no approval of templates.

Split CheckProofOfWorkImpl() to introduce a helper function DeriveTarget() which converts the nBits value to the target. The function takes pow_limit as an argument so later commits can avoid having to pass ChainstateManager through the call stack. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

Sjors · 2025-01-07T08:36:14Z

Rebased after #31581 landed. I also cherry-picked the latest relevant commits from #31583.

@luke-jr I dropped DATUM from the description, since there's still no spec it's hard to understand what it actually does.

The checkblock RPC is indeed almost the same as getblocktemplate in proposal mode. I updated the description to point this out. The main difference is that it can check (reduced) proof-of-work.

My long term goal for this new method is to make it more powerful. It could optionally add new transactions to the mempool to improve compact block relay. It could perform small reorgs to validate forks, which can be useful for (stale) share accounting (and perhaps uncle and DAG schemes).

An alternative approach could be drop the checkblock RPC and leave (weak) PoW checking as an IPC-only feature. If people prefer that I can convert the tests in rpc_checkblock.py to instead increase coverage for getblocktemplate.

But I think it's nice to offer weak block checking support for mining pool projects that want to experiment with it, without forcing them to use IPC. The new RPC method is short and test covered.

(in the above I'm assuming that getblocktemplate should be considered more or less ossified)

DrahtBot · 2025-01-07T08:52:47Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/35241369689}

Hints

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.

Sjors · 2025-01-07T11:02:43Z

The weak block test was broken, because regtest uses the almost highest possible target. Using a multiplier would overflow it. I dropped 180ce81 and adjusted the tests. I also dropped b61f8cc and b74e145 because I don't need them for the test.

Delete the original TestBlockValidity and rename CheckNewBlock. Have the generateblock and getblocktemplate RPC calls, as well as BlockAssembler::CreateNewBlock use the new method.

luke-jr · 2025-01-07T12:28:54Z

What would use the "weak block" check that doesn't need the ability to do it independently from proposals anyway?

While the existing GBT BIPs are Final, the protocol was designed to allow for extensions. If there's a use case for this feature, doing it that way seems fine

Sjors · 2025-01-07T12:45:31Z

@luke-jr do you mean that every client that wants to call checkblock $block_hex or getblocktemplate '{"mode": "proposal", "data": "$block_hex"}' for a weak block, probably implements its own proof-of-work check? That's probably true, since it most likely needs the ability to parse the block anyway in order to inspect the coinbase. Compared to that calculating proof-of-work and comparing against a target is trivial.

What would an extension look like to pass check_pow and target arguments?

Sjors mentioned this pull request Dec 24, 2024

rpc: checkblock, add checkBlock() to Mining interface Sjors/bitcoin#75

Closed

This was referenced Dec 25, 2024

Add waitNext() to BlockTemplate interface #31283

Open

BlockAssembler: return selected packages virtual size and fee #30391

Merged

Broadcast own transactions only via short-lived Tor or I2P connections #29415

Open

mzumsande reviewed Dec 26, 2024

View reviewed changes

tdb3 reviewed Dec 27, 2024

View reviewed changes

Sjors mentioned this pull request Dec 30, 2024

test: have miner_tests use Mining interface #31581

Merged

Sjors force-pushed the 2024/12/check-block branch from 442f27a to 0182812 Compare December 30, 2024 16:46

Sjors force-pushed the 2024/12/check-block branch from 0182812 to 5ae0c47 Compare December 30, 2024 16:58

DrahtBot added the CI failed label Dec 30, 2024

Sjors force-pushed the 2024/12/check-block branch 3 times, most recently from c961e83 to 2985291 Compare December 30, 2024 18:18

Sjors marked this pull request as draft December 30, 2024 18:18

DrahtBot added the Needs rebase label Dec 30, 2024

Sjors force-pushed the 2024/12/check-block branch from 2985291 to ed301f0 Compare December 30, 2024 20:13

DrahtBot mentioned this pull request Dec 30, 2024

rpc: Extend scope of validation mutex in generateblock #31563

Merged

Sjors mentioned this pull request Dec 30, 2024

rpc: add gettarget , target getmininginfo field and show next block info #31583

Open

DrahtBot removed Needs rebase CI failed labels Dec 30, 2024

This was referenced Dec 31, 2024

miner: Reorg Testnet4 minimum difficulty blocks #31117

Draft

net: introduce block tracker to retry to download blocks after failure #27837

Draft

DrahtBot mentioned this pull request Jan 3, 2025

Fee Estimation via Fee rate Forecasters #30157

Draft

DrahtBot added the Needs rebase label Jan 6, 2025

Sjors force-pushed the 2024/12/check-block branch from ed301f0 to 37c73dc Compare January 7, 2025 08:36

Sjors force-pushed the 2024/12/check-block branch from 37c73dc to 26b505a Compare January 7, 2025 08:52

DrahtBot added the CI failed label Jan 7, 2025

DrahtBot removed the Needs rebase label Jan 7, 2025

Sjors added 2 commits January 7, 2025 11:58

validation: CheckNewBlock()

9cc580d

Add checkBlock() to Mining interface

953a4bd

Sjors force-pushed the 2024/12/check-block branch from 26b505a to 1705adf Compare January 7, 2025 11:01

Sjors added 2 commits January 7, 2025 12:04

rpc: add checkblock

2052141

validation: replace TestBlockValidity

8cd4e51

Delete the original TestBlockValidity and rename CheckNewBlock. Have the generateblock and getblocktemplate RPC calls, as well as BlockAssembler::CreateNewBlock use the new method.

Sjors force-pushed the 2024/12/check-block branch from 1705adf to 8cd4e51 Compare January 7, 2025 11:04

DrahtBot removed the CI failed label Jan 7, 2025

Sjors mentioned this pull request Jan 9, 2025

mining: bugfix: Fix duplicate coinbase tx weight reservation #31384

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add checkblock RPC and checkBlock() to Mining interface #31564

Add checkblock RPC and checkBlock() to Mining interface #31564

Sjors commented Dec 24, 2024 •

edited

Loading

DrahtBot commented Dec 24, 2024 •

edited

Loading

Sjors commented Dec 24, 2024

mzumsande left a comment

tdb3 left a comment •

edited

Loading

tdb3 Dec 26, 2024 •

edited

Loading

Sjors Dec 27, 2024

Sjors Dec 30, 2024

Sjors commented Dec 27, 2024

instagibbs commented Dec 30, 2024

Sjors commented Dec 30, 2024 •

edited

Loading

DrahtBot commented Dec 30, 2024

Sjors commented Dec 30, 2024 •

edited

Loading

Sjors commented Dec 30, 2024 •

edited

Loading

luke-jr commented Jan 7, 2025

Sjors commented Jan 7, 2025 •

edited

Loading

DrahtBot commented Jan 7, 2025

Sjors commented Jan 7, 2025 •

edited

Loading

luke-jr commented Jan 7, 2025

Sjors commented Jan 7, 2025

Add checkblock RPC and checkBlock() to Mining interface #31564

Are you sure you want to change the base?

Add checkblock RPC and checkBlock() to Mining interface #31564

Conversation

Sjors commented Dec 24, 2024 • edited Loading

Rationale

Verifying block templates (no PoW)

Verifying weak blocks (reduced PoW)

Enter checkblock and its IPC counterpart checkBlock()

Footnotes

DrahtBot commented Dec 24, 2024 • edited Loading

Code Coverage & Benchmarks

Reviews

Conflicts

Sjors commented Dec 24, 2024

mzumsande left a comment

Choose a reason for hiding this comment

tdb3 left a comment • edited Loading

Choose a reason for hiding this comment

tdb3 Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

Sjors Dec 27, 2024

Choose a reason for hiding this comment

Sjors Dec 30, 2024

Choose a reason for hiding this comment

Sjors commented Dec 27, 2024

instagibbs commented Dec 30, 2024

Sjors commented Dec 30, 2024 • edited Loading

DrahtBot commented Dec 30, 2024

Sjors commented Dec 30, 2024 • edited Loading

Sjors commented Dec 30, 2024 • edited Loading

luke-jr commented Jan 7, 2025

Sjors commented Jan 7, 2025 • edited Loading

DrahtBot commented Jan 7, 2025

Sjors commented Jan 7, 2025 • edited Loading

luke-jr commented Jan 7, 2025

Sjors commented Jan 7, 2025

Sjors commented Dec 24, 2024 •

edited

Loading

Enter `checkblock` and its IPC counterpart `checkBlock()`

DrahtBot commented Dec 24, 2024 •

edited

Loading

tdb3 left a comment •

edited

Loading

tdb3 Dec 26, 2024 •

edited

Loading

Sjors commented Dec 30, 2024 •

edited

Loading

Sjors commented Dec 30, 2024 •

edited

Loading

Sjors commented Dec 30, 2024 •

edited

Loading

Sjors commented Jan 7, 2025 •

edited

Loading

Sjors commented Jan 7, 2025 •

edited

Loading