[WIP] Add getCoinbaseMerklePath() to Mining interface#30441
Closed
Sjors wants to merge 6 commits intobitcoin:masterfrom
Closed
[WIP] Add getCoinbaseMerklePath() to Mining interface#30441Sjors wants to merge 6 commits intobitcoin:masterfrom
Sjors wants to merge 6 commits intobitcoin:masterfrom
Conversation
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.
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
This was referenced Jul 13, 2024
Member
Author
|
Folded into #30440. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #30440.
This adds
getCoinbaseMerklePath()to the Mining interface.Unlike the entire
Mininginterface 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_pathfield.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.