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

add(rpc): Adds getblockheader RPC method #8967

Merged
merged 14 commits into from
Nov 18, 2024
Merged

add(rpc): Adds getblockheader RPC method #8967

merged 14 commits into from
Nov 18, 2024

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Oct 25, 2024

Motivation

This is needed for block explorer support and has been requested by a mining pool.

Closes #8442.
Part of #8446 (always includes height in response when called with verbosity = 2).

Specifications & References

https://zcash.github.io/rpc/getblockheader.html

Solution

  • Adds a getblockheader RPC method like the one in zcashd

Related changes:

  • Adds height field to getblock response when called with a height if verbosity == 2

Tests

  • Adds a snapshot test for querying by height/hash and with verbose being true/false
  • Adds a vectors test

PR Author's Checklist

  • The PR name will make sense to users.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added A-rpc Area: Remote Procedure Call interfaces P-Medium ⚡ labels Oct 25, 2024
@arya2 arya2 added this to the Zebra can replace `zcashd` milestone Oct 25, 2024
@arya2 arya2 self-assigned this Oct 25, 2024
@arya2 arya2 changed the title Add(rpc): Adds getblockheader RPC method add(rpc): Adds getblockheader RPC method Oct 25, 2024
… impl for sapling::tree::Root, adds snapshot and vector tests for new RPC method, adds snapshots.
@arya2 arya2 marked this pull request as ready for review October 25, 2024 20:53
@arya2 arya2 requested review from a team as code owners October 25, 2024 20:53
@arya2 arya2 requested review from upbqdn and removed request for a team October 25, 2024 20:53
@arya2 arya2 added the A-state Area: State / database changes label Oct 25, 2024
@arya2 arya2 marked this pull request as draft October 25, 2024 21:29
@arya2 arya2 marked this pull request as ready for review October 25, 2024 21:48
@arya2
Copy link
Contributor Author

arya2 commented Oct 25, 2024

zcash-rpc-diff util output with verbose = true:

 {
   "hash": "000000002c2063bab0acae547bab7e0309db8e1b7003e9c01eb423c0166d5ac9",
-  "confirmations": 2683651,
+  "confirmations": 169203,
   "height": 10000,
   "version": 4,
   "merkleroot": "e093171fc092c4bc0598c6d475489f892b177e3c6139ca680b064787093e2d1e",
-  "finalsaplingroot": "fbc2f4300c01f0b7820d00e3347c8da4ee614674376cbc45359daa54f9b5493e",
+  "finalsaplingroot": "0000000000000000000000000000000000000000000000000000000000000000",
   "time": 1479096667,
   "nonce": "000000000000000000000000000004cc00000000000000000000000000000000",
+  "solution": "00275a139867b3816117f666e4ba1442fb14dbdebd1301af4f33e934f5cc56eb0f1c70a562d9ec5fe0912126fd433fb55699e285b5689edf15fab1ac9dc853320a67930d13375df86cb39cdd385e6dbca3f909ac07a5cf1279637001afc1947c8f762121ad2772e1a74d16870881349c3bec75d7158965b17679055cb10b15df4a5152d806a96540b6eeced2d4272b501c69e62a300ae5c6a855f7870f02cf651863ea9f20765c3900772233d5061af31511e2da4eee4885176071804c1080f51f0e8f81abb745c22bd14adf5192d733d9b20f6e18ada5cfa7638d04e35c38f7a1613714fb26a84123cc5ae31e5fd1a4e816d7b254f9a9d22e9973630d7758d9c1092dac5a08831b44a5eac348d7dc7aca0ee37450324c9292e98751916dfebdcd3d250fa0db249fe703994cdf7a69cef57685562db17e3dd43cc442de94463e186174dc4a3454c4b8e4c65e30bff403024468345a216739a42c80d3695c951cc407f1efd81057c40847a23a11b7dc1448172fb2ad3ca9cb29cf057c6eb79ae56fa589e211d0f11fceff17c6be531f283ac21711902461046be5a642f9f8bf5406df53b602e82c3f4f95f06f4ee6254750cd5e862896fbe4602894d9c9914ba9a6f382197c5569e3428ffabb3d0c132bbb37826788abd107e274252edead9aac199e4574ca8ea16fa1358b4fa2a867d44b6af725555d56d7077358f3f5afea45d616314dcefa6b4df9c65b9a8e3cc832c8b2ebde9be1b5a6f081d81417197a7ca3ca0d7afec4a89e7bd915d8678edceae969ffe8b7da4515870f372f65bb05707dc5b6a03c44e6bca3ff211408e0c4c672c5266318cbf6372067aba9bedf8f5ea82e117688d1108cd5cef4c6065fe107e21334f5a78f1ec206bf21a14387472572ee6afd3d80ef11156b671f3ee5782f152a4b2671d20762cdd0a2680f16e74200360df7bb60797f4caea02dc053052c6e1235212006da6a978ab122d1d43ee2091e4299616b60b2f9b519c1111a3b62236f3e51c43d917007aedaa5ddcaa55591f7171962a79f39a5a6d67a39de6301187f2b6119ebc8f17e1afcc9b06aa495de49b45185d650507b1bb5e7346dcba10b91f2e57e9bc7522aabd71b88bd1d5016e1103bc4c9eba31237d1389b0e271d39400d3923bb06632727b9e367449992d16cc2529cbbca9103150366bc23213b8b735b7abee660ef9c893cf5ff0b3fcb1493dbb29b76bc62de035317052ea7ed059e06f21294e88241147163449adac480d249a8b455034295ce8248a5a9358a03c6d470f8ecda6848bf3f66074e3ade5a0a20a703df52a970e486121c3f1c23b82882fe7a9717e9a1566062cab854b6e147b8dc7de233c75e1c085436c6c259984602c5d4922d17137bdb383464cc11dcc5d3aadb4778b9f2f772c8075a57e001e0e9df36960e17c8cc12def8dd324e2bf4bee59e4fcc8c5718d862e3682367d948445b7698111836f1108d08fce80f13acb7045579d6bd4261b3b272a8d166dc5d93d7675fcb96b98929b667fc86583eb8533006ee106a0b516712f36529656066c63ee5179fbe6856613c16d9df7b7d173645ff42d9c63aaa8d5a5b650fa4263b3928d3c5cda0a1af938db8289adde889461473fa73671241bf5a8914436fb3c8812d776b49aa01ef759141b5a55bdc18b3015669e4ed09345bbb6e02f0c2ae6f3ac9e9f62db263ad39e3b295dbddc3ff15634deda2d71b2fd907c9cf25e74a76ccef379d1f1a3429a5f4298341e7c0529855c7c9c91cb15dd1d30cd9b627a6e4b1dd5cec759b7c6da1ce1b0731da1b13ef8b90d90a9b16d521017658ba2831a6895851db21d255d61bcb27912eb4f7ac0b7699f36333fbe5983cb1ba395f15456db1808ae0b4dd4913277f5a1443",
   "bits": "1d01bf9d",
-  "difficulty": "00000001bf9d0000000000000000000000000000000000000000000000000000",
+  "difficulty": 299851.4066097095,
+  "chainwork": "00000000000000000000000000000000000000000000000000000a13fdd5bc69",
   "previousblockhash": "000000001298a3685fbc493751ff2309a9bcef7cefad4565642bafb43f71b4c6",
   "nextblockhash": "00000000980d28a4bd926c4c553f7eaa5b29d3a78e8423f3863b72edd258839e"
 }

It matches exactly when verbose is false, but the difficulty and finalsaplingroot fields aren't matching. It's possible the finalsaplingroot field will match after Sapling activation (my zcashd is still syncing). zcashd also includes solution and chainwork fields, solution should be trivial to add, chainwork may be a little more complicated.

Update:

The finalsaplingroot does match after Sapling activation, though it was reversed.

Now it's just the difficulty field that's different, and the solution and chainwork fields that are missing.

Update 2:

The RPC is still missing the chainwork field but otherwise matches the zcashd output exactly, including while the Sapling note commitment tree is empty.

The chainwork field is confirmed to be the cumulative work in the chain, which Zebra doesn't store in the finalized state. It's also undocumented so we can probably leave it until/unless someone requests it. It could also still be calculated by the caller by summing up the work for the chain up to the requested block.

zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
@arya2 arya2 added the do-not-merge Tells Mergify not to merge this PR label Oct 26, 2024
@arya2 arya2 requested a review from natalieesk October 28, 2024 16:41
…t the `chainwork` field), updates snapshots, and avoids a possible panic when there's a chain reorg between state queries.
@arya2 arya2 removed the do-not-merge Tells Mergify not to merge this PR label Oct 31, 2024
@oxarbitrage oxarbitrage self-requested a review November 7, 2024 15:13
@oxarbitrage
Copy link
Contributor

The chainwork field is confirmed to be the cumulative work in the chain, which Zebra doesn't store in the finalized state. It's also undocumented so we can probably leave it until/unless someone requests it. It could also still be calculated by the caller by summing up the work for the chain up to the requested block.

The chainwork field was mentioned in the last Zcashd deprecation meeting and as it is an undocumented field it should be safe to ignore it in Zebra.

@oxarbitrage
Copy link
Contributor

The failing test seems to be unrelated however it will be good to check that out in another issue/context.

oxarbitrage
oxarbitrage previously approved these changes Nov 8, 2024
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 great to me, thanks!

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

@oxarbitrage can we get a final review here if it's all ok?

Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

Looks good, just FYI I made some small adjustments related to error codes in #9006 to align with zcashd behaviour, but we can merge this as is.

@gustavovalverde
Copy link
Member

@mergify refresh

Copy link
Contributor

mergify bot commented Nov 15, 2024

refresh

✅ Pull request refreshed

@gustavovalverde
Copy link
Member

@mergify update

Copy link
Contributor

mergify bot commented Nov 15, 2024

update

✅ Branch has been successfully updated

@mpguerra
Copy link
Contributor

@mergify refresh

Copy link
Contributor

mergify bot commented Nov 18, 2024

refresh

✅ Pull request refreshed

@gustavovalverde
Copy link
Member

@mergify requeue

Copy link
Contributor

mergify bot commented Nov 18, 2024

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@gustavovalverde
Copy link
Member

@mergify queue

Copy link
Contributor

mergify bot commented Nov 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 1e46914

mergify bot added a commit that referenced this pull request Nov 18, 2024
@mergify mergify bot merged commit 1e46914 into main Nov 18, 2024
170 of 172 checks passed
@mergify mergify bot deleted the get-block-header branch November 18, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement getblockheader RPC method
5 participants