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

rpc(getblock): Add value pools output #8765

Closed
wants to merge 1 commit into from
Closed

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Aug 14, 2024

Motivation

We want the lockbox value pool information in the getblock output for a given block

The lockbox information was added in zcashd at zcash/zcash#6912, It is part of a list of pools that this PR adds to Zebra.

#8730

Specifications & References

zcash/zcash#6912

Solution

The solution implements the valuePools object and some of the fields. In zcashd this is like:

...
  "valuePools": [
    {
      "id": "transparent",
      "monitored": true,
      "chainValue": 0.03437500,
      "chainValueZat": 3437500,
      "valueDelta": 0.00625000,
      "valueDeltaZat": 625000
    },
 ...

Where valueDelta is the value pool for the requested block but chainValue is the value pool up to the requested blocks. Zebra do not keep track of value pools at specific heights for the finalized chain so i think we can't have that value. I am open to suggestions here.

Solution implements id, valueDelta and valueDeltaZat for each pool including the lockbox

Tests

Snapshot tests added by now.

Follow-up Work

Need zcashd diff tests.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • 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.

@oxarbitrage oxarbitrage added NU-6 Network Upgrade: NU6 specific tasks A-rpc Area: Remote Procedure Call interfaces P-Medium ⚡ labels Aug 14, 2024
@oxarbitrage oxarbitrage requested a review from a team as a code owner August 14, 2024 22:55
@oxarbitrage oxarbitrage requested review from arya2 and removed request for a team August 14, 2024 22:55
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This PR looks great, though it seems zcashd is returning the total chain value balances at the provided heights, so we may need another PR to track those in Zebra first.

@@ -1548,6 +1574,7 @@ impl Default for GetBlock {
time: None,
tx: Vec::new(),
trees: GetBlockTrees::default(),
value_pools: vec![],
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional/Nitpick: This Default impl could be derived.

zebra-state/src/service.rs Show resolved Hide resolved
@oxarbitrage
Copy link
Contributor Author

Closed as it is not critical and much more work than expected.

@@ -10,6 +10,7 @@ use std::{collections::HashSet, fmt::Debug, sync::Arc};

use chrono::Utc;
use futures::{stream::FuturesOrdered, FutureExt, StreamExt, TryFutureExt};
use get_block_template_rpcs::types::zec::Zec;
Copy link
Contributor

Choose a reason for hiding this comment

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

This type needs to be moved out from behind the getblocktemplate-rpcs feature flag.

@arya2
Copy link
Contributor

arya2 commented Aug 15, 2024

This looks good to merge once the Zec type has been moved.

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 NU-6 Network Upgrade: NU6 specific tasks P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants