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

Prepare changeset for 0.69.1 #4019

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Conversation

conorsch
Copy link
Contributor

This PR includes cherry-picked commits intended for inclusion in v0.69.1, targeting the release branch for same. These commits are based on the following PRs:

jessepinho and others added 3 commits March 13, 2024 13:22
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.
…zation (#4008)

## Describe your changes

This pulls a proto type out of the state and passes it to the RPC
directly, rather than deserializing through a domain type (which
involves another round of allocations and some crypto operations to
parse field elements). This should help reduce load for RPC servers
streaming compact blocks to clients.

We could in principle go further and pull bytes directly out of the
state and pass them directly to the client (avoiding the
bytes>proto>bytes conversion) but that would be more work (I'm not sure
exactly how or if Tonic supports that), and this is lower-hanging fruit.

## Issue ticket number and link

No issue, just an observation after I looked at Grafana and took a few
minutes to change

## Checklist before requesting a review

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

> This only changes the RPC implementation, not any consensus logic; it
should be safe to cherry-pick and deploy on a release branch.

(cherry picked from commit 3a9db32)
Closes #4007

The bug results from a hazardous API I created. The `apply` method does _not_
apply the events to the underlying state, it extracts them and returns them to
the caller, who can silently discard them.

A better fix is outlined in the comment:

```
// We have to manually extract events and push them down to the state to avoid losing them.
// TODO: in a commit not intended to be cherry-picked, we should fix this hazardous API:
// - rename `StateDelta::apply` to `StateDelta::apply_extracting_events`
// - add `StateDelta::apply_with_events` that pushes the events down.
// - go through all uses of `apply_extracting_events` and determine what behavior is correct
```

However, I didn't do this in this commit, because my hope is that we can have
something easily cherry-pickable into a point release, which can be deployed to
unblock the DEX work. After applying it to both `main` and a release branch, we
can then do the fix for the bug class.

(cherry picked from commit 318d3d1)
@conorsch conorsch force-pushed the prepare-changeset-for-0.69.1 branch from bfa9687 to 6c1c82e Compare March 13, 2024 20:22
@conorsch conorsch merged commit a562eb1 into release/v0.69.x Mar 13, 2024
4 checks passed
@conorsch conorsch deleted the prepare-changeset-for-0.69.1 branch March 13, 2024 21:03
conorsch added a commit that referenced this pull request Mar 14, 2024
Follow up to [0]. Now that v0.69.1 is live [1], we should also bump the
docs, which are currently only built from main, not from release
branches.

[0] #4019
[1] https://github.com/penumbra-zone/penumbra/releases/tag/v0.69.1
conorsch added a commit that referenced this pull request Mar 15, 2024
Follow up to [0]. Now that v0.69.1 is live [1], we should also bump the
docs, which are currently only built from main, not from release
branches.

[0] #4019
[1] https://github.com/penumbra-zone/penumbra/releases/tag/v0.69.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants