Skip to content

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Jul 12, 2024

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.

Sjors added 6 commits July 11, 2024 10:42
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.
These defaults are needed by the Mining interface in the next commit.
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.
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.
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 12, 2024

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
Copy link
Member Author

Sjors commented Jul 15, 2024

Folded into #30440.

@Sjors Sjors closed this Jul 15, 2024
@bitcoin bitcoin locked and limited conversation to collaborators Jul 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants