Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change(rpc): generate coinbase transactions in the getblocktemplate RPC #5580

Merged
merged 14 commits into from
Nov 10, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 8, 2022

Motivation

This PR generates coinbase transactions when the getblocktemplate RPC is called.

Solution

Generate the coinbase transaction in the getblocktemplate RPC:

  • Add coinbase creation to zebra-chain
  • Add coinbase creation and miner subsidy to zebra-consensus

Related changes:

  • Add the miner config to the GetBlockTemplateRpcImpl
  • Add a getblocktemplate-rpcs feature to zebra-chain

Related fixes:

  • Fix missing feature dependencies in zebra-consensus
  • Provide fake valid block heights to getblocktemplate RPC tests
  • Rewrite check_script_form() so it is easier to read

Testing:

  • Add a getblocktemplate.coinbase_tx deserialized transaction snapshot
  • Update the getblocktemplate RPC snapshots

Closes #5453.

Review

This is a routine change, it might help getting two people to review it.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
  • How do you know it works? Does it have tests?

Follow Up Work

I split #5579 out of #5453, because the state request wasn't ready yet.

@teor2345 teor2345 added A-dependencies Area: Dependency file updates A-consensus Area: Consensus rule updates C-enhancement Category: This is an improvement P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces labels Nov 8, 2022
@teor2345 teor2345 requested review from a team as code owners November 8, 2022 03:22
@teor2345 teor2345 self-assigned this Nov 8, 2022
@teor2345 teor2345 requested review from dconnolly and oxarbitrage and removed request for a team November 8, 2022 03:22
@dconnolly dconnolly changed the title change(rpc): Generate coinbase transactions in the getblocktemplate RPC change(rpc): generate coinbase transactions in the getblocktemplate RPC Nov 8, 2022
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #5580 (6cbc9f9) into main (8d9c3ec) will decrease coverage by 0.04%.
The diff coverage is 76.74%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5580      +/-   ##
==========================================
- Coverage   78.83%   78.78%   -0.05%     
==========================================
  Files         305      305              
  Lines       38127    38160      +33     
==========================================
+ Hits        30056    30064       +8     
- Misses       8071     8096      +25     

Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good.

mergify bot added a commit that referenced this pull request Nov 9, 2022
mergify bot added a commit that referenced this pull request Nov 9, 2022
@mergify mergify bot merged commit 7e13677 into main Nov 10, 2022
@mergify mergify bot deleted the gbt-coinbase-tx branch November 10, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-dependencies Area: Dependency file updates A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Populate transparent coinbase transaction data for BlockTemplate
4 participants