[WIP] Add getCoinbaseMerklePath() to Mining interface

Sjors commented at 2:22 pm on July 12, 2024: member

Builds on #30440.

This adds getCoinbaseMerklePath() to the Mining interface.

Unlike the entire Mining interface so far, this is not a refactor. It adds new functionality.

The Stratum v2 NewTemplate message is a short message intended to give miners something to do as quickly as possible. It does not include the serialized block transactions. In lieu of that it contains a merkle_path field.

The reason this PR is WIP is that I haven’t studied this protocol message in any detail. The implementation is copy-pasted from earlier versions of #29432 and it may be possible to reuse parts of merkle_tests.cpp.

The main purpose of this PR is to provide a complete and stable Mining interface for #30437 to build on. This can then be used to develop a “sidecar” alternative to #29432. I’ll get to improving the implementation later.

refactor: use CHECK_NONFATAL to avoid single-use symbol 7d76589ace

refactor: add coinbase constraints to BlockAssembler::Options

When generating a block template through e.g. getblocktemplate RPC,
we reserve 4000 weight units and 400 sigops. Pools use this space
for their coinbase outputs.

At least one pool patched their Bitcoin Core node to adjust
these hardcoded values. They eventually produced an invalid
block which exceeded the sigops limit.
https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426

The existince of such patches suggests it may be useful to
make this value configurable. This commit would make such a
change easier.

The main motivation however is that the Stratum v2 spec
requires the pool to communicate the maximum bytes they intend
to add to the coinbase outputs. A proposed change to the spec
would also require them to communicate the maximum number of sigops.

This commit adds both to BlockAssembler::Options. The default
is the same as the previously hardcode values.

af3f85fb0f

Add util/mining.h for default coinbase constraints

These defaults are needed by the Mining interface in the next commit.

7e1d7cd1aa

Pass coinbase constraints to createNewBlock

Existing callers of createNewBlock use the defaults,
so this commit does not change behavior and the
Assume(s) won't be false.

This commit also documents what happens when
-blockmaxweight is lower than the coinbase
reserved value.

e67e4669a9

Have createNewBlock return BlockTemplate interface

An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data.

This would be the case for a Stratum v2 Template Provider which needs to send a NewTemplate message (which doesn't include transactions) as quickly as possible.

b13eedab3e

Add getCoinbaseMerklePath() to Mining interface e71b59c97e

DrahtBot commented at 2:22 pm on July 12, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

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:

#30440 (Have createNewBlock() return a BlockTemplate interface by Sjors)
#30409 (Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals by Sjors)
#29432 (Stratum v2 Template Provider (take 3) by Sjors)
#27433 (getblocktemplate improvements for segwit and sigops by Sjors)

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.

Sjors commented at 12:17 pm on July 15, 2024: member

Folded into #30440.

Sjors closed this on Jul 15, 2024

[WIP] Add getCoinbaseMerklePath() to Mining interface #30441

Code Coverage

Reviews

Conflicts