Skip to content

feat(rpc): add value pools to getblock - #9432

Merged
mergify[bot] merged 28 commits into
mainfrom
value-pools
Jun 20, 2025
Merged

feat(rpc): add value pools to getblock#9432
mergify[bot] merged 28 commits into
mainfrom
value-pools

Conversation

@conradoplg

@conradoplg conradoplg commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator

Motivation

We want to fill the valuePools and chainSupply fields for the getblock RPC.

Solution

The database part was included in #9539, I'll leave its description below for the record.

That requires keeping track of the value pools after each block; we only keep it for the entire chain.

This PR introduces a new column family named block_data. The idea behind the generic name is that we could add other data there like chainwork and block size. In fact, the main reason this is a draft PR is because we might want to add those in the same PR, but I'd like to get feedback on this first.

The database upgrade is slow (main reason) and takes ~2h to complete (it was 4h, I managed to improve a bit with rayon). I think this is harmless since it only impacts the RPC itself (it does not return the field if the data is not yet available).

To avoid a circular dependency, this also moves some subsidy-related functions from zebra-consensus to zebra-chain. I'm not sure if I should go ahead and move everything else related to that. Let me know if you prefer a separate PR with the move, but I also wanted feedback on this first.

Closes #5449

Tests

I adjusted the existing tests and used zcash-rpc-diff to compare with zcashd, and everything seems to match (with this exception)

Specifications & References

https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L766-L782

Follow-up Work

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.
  • If the PR shouldn't be in the release notes, it has the
    C-exclude-from-changelog label.

Comment thread zebra-chain/src/parameters/network/subsidy.rs
Comment thread zebra-chain/src/parameters/network/tests.rs
Comment thread zebra-consensus/src/block/subsidy.rs
Comment thread zebra-consensus/src/block/subsidy/general.rs Outdated
Comment thread zebra-utils/zcash-rpc-diff
Comment thread zebrad/tests/acceptance.rs
Comment thread .github/workflows/sub-deploy-integration-tests-gcp.yml Outdated
Comment thread .github/workflows/sub-ci-integration-tests-gcp.yml Outdated
@dismad

dismad commented May 15, 2025

Copy link
Copy Markdown
Contributor

Is there a way to show the status of the valuePool sync? That might be useful to add if it doesn't complicate things.

Screenshot_2025-05-15_14-57-18

I'm thinking in getinfo or getblockchaininfo a `isValuePoolsSynced' boolean, or even better, a % complete

@conradoplg

Copy link
Copy Markdown
Collaborator Author

Is there a way to show the status of the valuePool sync? That might be useful to add if it doesn't complicate things.

Screenshot_2025-05-15_14-57-18

I'm thinking in getinfo or getblockchaininfo a `isValuePoolsSynced' boolean, or even better, a % complete

I think it would be work that will be rarely used, since after the database is upgraded you'll never have to worry about it again. You can somewhat check the progress by calling getblock for some height and checking if it returns the pools

@conradoplg

Copy link
Copy Markdown
Collaborator Author

Moved to draft because @arya2 will try combining this and #9295

Comment thread zebra-state/src/service/finalized_state/disk_format/upgrade.rs
@mpguerra

mpguerra commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

@conradoplg can we close this one in favour of @arya2 's PR?

@conradoplg

Copy link
Copy Markdown
Collaborator Author

@conradoplg can we close this one in favour of @arya2 's PR?

I thin I'll repurpose it to contain the RPC call itself based on @arya2 's PR (which contains the DB changes but not the RPC change itself IIRC)

@conradoplg

Copy link
Copy Markdown
Collaborator Author

@Mergifyio update

@mergify

mergify Bot commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

update

✅ Branch has been successfully updated

@conradoplg

Copy link
Copy Markdown
Collaborator Author

@arya2 review reminder 🙏

arya2
arya2 previously approved these changes Jun 18, 2025

@arya2 arya2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good, I left some comments but none of them are blockers.

Comment thread zebra-rpc/src/methods/types/get_blockchain_info.rs Outdated
Comment thread zebrad/tests/common/checkpoints.rs Outdated
Comment thread zebra-rpc/src/methods.rs
Comment thread zebra-rpc/src/methods.rs
@arya2 arya2 mentioned this pull request Jun 19, 2025
3 tasks
@mpguerra

Copy link
Copy Markdown
Contributor

@Mergifyio queue

@mergify

mergify Bot commented Jun 20, 2025

Copy link
Copy Markdown
Contributor

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 9caa717

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-feature Category: New features P-Medium ⚡

Projects

No open projects
Status: Won't Fix

Development

Successfully merging this pull request may close these issues.

getblock: return the valuesPools and chainSupply fields

6 participants