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

view(rpc): add DelegationsByAddressIndex query #3984

Merged
merged 5 commits into from
Mar 11, 2024

Conversation

jessepinho
Copy link
Contributor

@jessepinho jessepinho commented Mar 9, 2024

Describe your changes

Right now, the StakingService has a method, validatorInfo, that returns a series of ValidatorInfos.

As of penumbra-zone/web#604, we've refactored the minifront staking page to treat validators as tokens that an address may have a balance of. To achieve that, we're fetching validator infos, then generating ValueViews from those validators that include the address's balance of those tokens.

This logic should be moved into a new RPC method on the ViewService called DelegationsByAddressIndex, since it's a common enough use case that clients shouldn't all have to do it themselves.

Relevant changes are to view.proto and ViewService; the rest is just codegen changes.

Issue ticket number and link

Closes penumbra-zone/web#612

Checklist before requesting a review

  • (n/a) If this code contains consensus-breaking changes, I have added the "consensus-breaking" label.

@jessepinho jessepinho force-pushed the jessepinho/delegation-balances-rpc-web-612 branch from 5c6251a to aa568ef Compare March 9, 2024 02:55
@jessepinho jessepinho force-pushed the jessepinho/delegation-balances-rpc-web-612 branch from 82cd410 to 097dd6d Compare March 9, 2024 02:57
@jessepinho jessepinho force-pushed the jessepinho/delegation-balances-rpc-web-612 branch from 097dd6d to 83339b6 Compare March 9, 2024 03:00
@jessepinho jessepinho marked this pull request as ready for review March 9, 2024 03:22
@cratelyn cratelyn added the protobuf-changes Makes changes to the protobuf definitions. label Mar 9, 2024
@erwanor erwanor marked this pull request as draft March 11, 2024 09:29
@erwanor erwanor changed the title Create a new DelegationsByAddressIndex RPC method staking(rpc): add DelegationsByAddressIndex to query service Mar 11, 2024
@erwanor erwanor changed the title staking(rpc): add DelegationsByAddressIndex to query service view(rpc): add DelegationsByAddressIndex query Mar 11, 2024
&self,
_request: tonic::Request<pb::DelegationsByAddressIndexRequest>,
) -> Result<tonic::Response<Self::DelegationsByAddressIndexStream>, tonic::Status> {
unimplemented!("delegations_by_address_index")
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this PR isn't ready yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the idea is that it could be implemented by other implementations of ViewService, such as in the web app. (Note that fn authorize_and_build above is also unimplemented.) I had to add this method here or else the Rust compiler would complain that this method was missing.

@jessepinho jessepinho marked this pull request as ready for review March 11, 2024 20:49
Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

My bad, I also thought this was a WIP. This makes sense and I don't think we need to spend time chasing feature parity across view service impls (in this case this seem onerous even).

@jessepinho jessepinho merged commit eb4f5e4 into main Mar 11, 2024
6 checks passed
@jessepinho jessepinho deleted the jessepinho/delegation-balances-rpc-web-612 branch March 11, 2024 21:24
@jessepinho
Copy link
Contributor Author

@erwanor I'd accidentally left this as a draft PR — that's prob why! :D

conorsch pushed a commit that referenced this pull request Mar 13, 2024
## Describe your changes
Right now, the `StakingService` has a method, `validatorInfo`, that
returns a series of `ValidatorInfo`s.

As of penumbra-zone/web#604, we've refactored
the minifront staking page to treat validators as tokens that an address
may have a balance of. To achieve that, [we're fetching validator infos,
then generating `ValueView`s from those
validators](https://github.com/penumbra-zone/web/pull/604/files#diff-9bd971ea2f5dadcc555a9a848d1cea2a0d6df1a0069b9e5e490218058ac5adbc)
that include the address's balance of those tokens.

This logic should be moved into a new RPC method on the `ViewService`
called `DelegationsByAddressIndex`, since it's a common enough use case
that clients shouldn't all have to do it themselves.

Relevant changes are to
[`view.proto`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-03b7341d5bf81ab9c8d8542d220d5ba4ae122bd3837531309742b685d7ec1619)
and
[`ViewService`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-36188e6ab5083e8be9d29039370cc523cfcfede683b223327b2d90e3f75d40c7);
the rest is just codegen changes.

## Issue ticket number and link
Closes penumbra-zone/web#612

## Checklist before requesting a review

- [x] (n/a) If this code contains consensus-breaking changes, I have
added the "consensus-breaking" label.

(cherry picked from commit eb4f5e4)
conorsch pushed a commit that referenced this pull request Mar 13, 2024
Right now, the `StakingService` has a method, `validatorInfo`, that
returns a series of `ValidatorInfo`s.

As of penumbra-zone/web#604, we've refactored
the minifront staking page to treat validators as tokens that an address
may have a balance of. To achieve that, [we're fetching validator infos,
then generating `ValueView`s from those
validators](https://github.com/penumbra-zone/web/pull/604/files#diff-9bd971ea2f5dadcc555a9a848d1cea2a0d6df1a0069b9e5e490218058ac5adbc)
that include the address's balance of those tokens.

This logic should be moved into a new RPC method on the `ViewService`
called `DelegationsByAddressIndex`, since it's a common enough use case
that clients shouldn't all have to do it themselves.

Relevant changes are to
[`view.proto`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-03b7341d5bf81ab9c8d8542d220d5ba4ae122bd3837531309742b685d7ec1619)
and
[`ViewService`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-36188e6ab5083e8be9d29039370cc523cfcfede683b223327b2d90e3f75d40c7);
the rest is just codegen changes.

Closes penumbra-zone/web#612

- [x] (n/a) If this code contains consensus-breaking changes, I have
added the "consensus-breaking" label.

(cherry picked from commit eb4f5e4)

Also includes a manual regen of the proto files, post-cherry-pick onto
the release branch.
conorsch pushed a commit that referenced this pull request Mar 13, 2024
Right now, the `StakingService` has a method, `validatorInfo`, that
returns a series of `ValidatorInfo`s.

As of penumbra-zone/web#604, we've refactored
the minifront staking page to treat validators as tokens that an address
may have a balance of. To achieve that, [we're fetching validator infos,
then generating `ValueView`s from those
validators](https://github.com/penumbra-zone/web/pull/604/files#diff-9bd971ea2f5dadcc555a9a848d1cea2a0d6df1a0069b9e5e490218058ac5adbc)
that include the address's balance of those tokens.

This logic should be moved into a new RPC method on the `ViewService`
called `DelegationsByAddressIndex`, since it's a common enough use case
that clients shouldn't all have to do it themselves.

Relevant changes are to
[`view.proto`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-03b7341d5bf81ab9c8d8542d220d5ba4ae122bd3837531309742b685d7ec1619)
and
[`ViewService`](https://github.com/penumbra-zone/penumbra/pull/3984/files#diff-36188e6ab5083e8be9d29039370cc523cfcfede683b223327b2d90e3f75d40c7);
the rest is just codegen changes.

Closes penumbra-zone/web#612

- [x] (n/a) If this code contains consensus-breaking changes, I have
added the "consensus-breaking" label.

(cherry picked from commit eb4f5e4)

Also includes a manual regen of the proto files, post-cherry-pick onto
the release branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protobuf-changes Makes changes to the protobuf definitions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ViewService#delegationsByAddressIndex method that returns validators as ValueViews
4 participants