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

feat(rpc): Implement the getblocksubsidy RPC #6032

Merged
merged 15 commits into from
Jan 31, 2023
Merged

feat(rpc): Implement the getblocksubsidy RPC #6032

merged 15 commits into from
Jan 31, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jan 25, 2023

Motivation

Some mining pools use getblocksubsidy to generate their own coinbase transactions.

Closes #5302

Specifications

The specification is here:
https://zcash.github.io/rpc/getblocksubsidy.html

And the zcashd implementation of the RPC is:
https://github.com/zcash/zcash/blob/20996b4eb771efde6bcb57d5aa8749a5705cd493/src/rpc/mining.cpp#L999-L1054

Complex Code or Requirements

This RPC uses the state tip as the default height, but otherwise it is very simple - it just calls the existing subsidy and funding stream calculation functions directly.

Solution

Implement the getblocksubsidy RPC:

  • Implement the RPC return type and method
    • Use the state tip height if there is no height argument
    • Return an error for heights below the first halving (miners only use this RPC for new blocks at the chain tip)
    • Add a ZEC formatting wrapper for Amount in zebra-rpc
  • Put RPC fields and funding streams in the same order as zcashd

We will never be able to match zcashd's format exactly, because they manually format their ZEC values, but we use serde_json, which requires f64 values and formats them itself.

Related changes:

  • Make it clearer that Zebra only supports transparent funding streams

Testing

Review

This is ready for review, feel free to be very critical, there are parts of it that I don't like either!

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?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow-Up Work

Testing:

@teor2345 teor2345 added A-consensus Area: Consensus rule updates P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces C-feature Category: New features A-compatibility Area: Compatibility with other nodes or wallets, or standard rules labels Jan 25, 2023
@teor2345 teor2345 requested a review from a team as a code owner January 25, 2023 05:52
@teor2345 teor2345 self-assigned this Jan 25, 2023
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team January 25, 2023 05:52
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 25, 2023

The floating-point values are the same as zcashd, they just have a different format.

Here is a test of the default height:

$ zcash-rpc-diff 28232 getblocksubsidy                                             
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 28232) and zcashd (zcash-cli zcash.conf port).                                                                                                                                   

Checking zebrad network and tip height...
Checking zcashd network and tip height...

Request:
getblocksubsidy

Querying zebrad main chain at height >=1960493...
real    0m0.007s

Querying zcashd main chain at height >=1960493...
real    0m0.007s

Response diff between zebrad and zcashd:
--- /run/user/1000/tmp.hmnUkRISsJ.rpc-diff/zebrad-main-1960493-getblocksubsidy.json     2023-01-25 15:52:45.910417439 +1000
+++ /run/user/1000/tmp.hmnUkRISsJ.rpc-diff/zcashd-main-1960493-getblocksubsidy.json     2023-01-25 15:52:45.917417356 +1000
@@ -3,25 +3,25 @@
     {
       "recipient": "Electric Coin Company",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.21875,
+      "value": 0.21875000,
       "valueZat": 21875000,
       "address": "t3cKr4YxVPvPBG1mCvzaoTTdBNokohsRJ8n"
     },
     {
       "recipient": "Zcash Foundation",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.15625,
+      "value": 0.15625000,
       "valueZat": 15625000,
       "address": "t3dvVE3SQEi7kqNzwrfNePxZ1d4hUyztBA1"
     },
     {
       "recipient": "Major Grants",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.25,
+      "value": 0.25000000,
       "valueZat": 25000000,
       "address": "t3XyYW8yBFRuMnfvm5KLGFbEVz25kckZXym"
     }
   ],
-  "miner": 2.5,
-  "founders": 0.0
+  "miner": 2.50000000,
+  "founders": 0.00000000
 }

And for a future block height:

$ zcash-rpc-diff 28232 getblocksubsidy 2000000                                     
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 28232) and zcashd (zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

Request:
getblocksubsidy 2000000

Querying zebrad main chain at height >=1960494...
real    0m0.007s

Querying zcashd main chain at height >=1960494...
real    0m0.007s

Response diff between zebrad and zcashd:
--- /run/user/1000/tmp.CGdtQDkxbW.rpc-diff/zebrad-main-1960494-getblocksubsidy.json     2023-01-25 15:54:29.115202098 +1000
+++ /run/user/1000/tmp.CGdtQDkxbW.rpc-diff/zcashd-main-1960494-getblocksubsidy.json     2023-01-25 15:54:29.122202016 +1000
@@ -3,25 +3,25 @@
     {
       "recipient": "Electric Coin Company",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.21875,
+      "value": 0.21875000,
       "valueZat": 21875000,
       "address": "t3T3smGZn6BoSFXWWXa1RaoQdcyaFjMfuYK"
     },
     {
       "recipient": "Zcash Foundation",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.15625,
+      "value": 0.15625000,
       "valueZat": 15625000,
       "address": "t3dvVE3SQEi7kqNzwrfNePxZ1d4hUyztBA1"
     },
     {
       "recipient": "Major Grants",
       "specification": "https://zips.z.cash/zip-0214",
-      "value": 0.25,
+      "value": 0.25000000,
       "valueZat": 25000000,
       "address": "t3XyYW8yBFRuMnfvm5KLGFbEVz25kckZXym"
     }
   ],
-  "miner": 2.5,
-  "founders": 0.0
+  "miner": 2.50000000,
+  "founders": 0.00000000
 }

And for a height beyond the funding streams:

$ zcash-rpc-diff 28232 getblocksubsidy 3000000
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 28232) and zcashd (zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

Request:
getblocksubsidy 3000000

Querying zebrad main chain at height >=1960494...
real    0m0.007s

Querying zcashd main chain at height >=1960494...
real    0m0.007s

Response diff between zebrad and zcashd:
--- /run/user/1000/tmp.dz4eKfAlTi.rpc-diff/zebrad-main-1960494-getblocksubsidy.json     2023-01-25 15:55:24.174555683 +1000
+++ /run/user/1000/tmp.dz4eKfAlTi.rpc-diff/zcashd-main-1960494-getblocksubsidy.json     2023-01-25 15:55:24.182555590 +1000
@@ -1,6 +1,6 @@
 {
   "fundingstreams": [
   ],
-  "miner": 1.5625,
-  "founders": 0.0
+  "miner": 1.56250000,
+  "founders": 0.00000000
 }

(This far future response is likely to get changed in a future network upgrade. But it doesn't crash or do anything wrong.)

@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Merging #6032 (4c9d148) into main (cda2879) will decrease coverage by 0.07%.
The diff coverage is 42.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6032      +/-   ##
==========================================
- Coverage   78.22%   78.15%   -0.07%     
==========================================
  Files         312      312              
  Lines       39036    39439     +403     
==========================================
+ Hits        30536    30824     +288     
- Misses       8500     8615     +115     

@teor2345 teor2345 requested a review from arya2 January 26, 2023 19:18
oxarbitrage
oxarbitrage previously approved these changes Jan 27, 2023
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.

I think it looks really good, i also think we could try harder to make the valueZat field match but i understand it could not worth the effort.

zebra-rpc/src/methods/get_block_template_rpcs.rs Outdated Show resolved Hide resolved
zebra-rpc/src/methods/get_block_template_rpcs.rs Outdated Show resolved Hide resolved
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 29, 2023

i also think we could try harder to make the valueZat field match but i understand it could not worth the effort.

It's a number field, so we need to use serde_json::RawValue if we want to control the output format:
https://docs.rs/serde_json/latest/serde_json/value/struct.RawValue.html

I'll see how hard it is if I get time, but I think it might be a low priority, because this code works and contains the same values. (Only the formatting is different.)

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.

thanks for the changes.

mergify bot added a commit that referenced this pull request Jan 31, 2023
@mergify mergify bot merged commit 4dd9426 into main Jan 31, 2023
@mergify mergify bot deleted the get-block-subsidy branch January 31, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-consensus Area: Consensus rule updates A-rpc Area: Remote Procedure Call interfaces C-feature Category: New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for getblocksubsidy RPC call
2 participants