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

proto: split RPCs into per-component QueryServices #3091

Merged
merged 2 commits into from
Sep 23, 2023

Conversation

hdevalence
Copy link
Member

@hdevalence hdevalence commented Sep 23, 2023

Closes #3083.

Still outstanding work to do:

  • Split up the server-side implementations currently in pd.
  • Resolve the governance issues.

On the latter point, I discovered that large parts of the governance code don't have any RPCs defined, and instead rely on having knowledge of the Rust code to layer together raw state accesses. This is practically impossible for any third party client to do, so it's not an acceptable solution. The shim code that enabled this was removed as part of the refactoring, so we'll either need to define the relevant RPCs, or leave that functionality disabled until we do.

Closes #3083.

Still outstanding work to do:

- [ ] Split up the server-side implementations currently in `pd`.
- [ ] Resolve the governance issues.

On the latter point, I discovered that large parts of the governance code don't
have any RPCs defined, and instead rely on having knowledge of the Rust code to
layer together raw state accesses.  This is practically impossible for any
third party client to do, so it's not an acceptable solution.  The shim code
that enabled this was removed as part of the refactoring, so we'll either need
to define the relevant RPCs, or leave that functionality disabled until we do.
@hdevalence
Copy link
Member Author

I ran into another difficulty while making these changes. We have an RPC to provide raw K/V queries to the underlying storage, and it seemed like this should conceptually be attached to the penumbra-storage crate. Because penumbra-storage (intentionally) doesn't depend on penumbra-proto (so that it can be more independent of the Penumbra crates and reusable by other projects), doing this meant duplicating parts of our proto compilation logic to have a parallel production of proto codegen in the penumbra-storage crate.

However, the RPC isn't actually really generic to the storage system, for the following reason: it returns K/V entries with ICS23 proofs up to the apphash, and the Penumbra apphash isn't the JMT merkle root. We add an extra layer of hashing to have a second "stub" proof to satisfy requirements of counterparty chains. So I put the RPCs in the app package's QueryService for now.

@hdevalence hdevalence temporarily deployed to smoke-test September 23, 2023 23:36 — with GitHub Actions Inactive
@hdevalence hdevalence marked this pull request as ready for review September 23, 2023 23:37
@hdevalence hdevalence merged commit 1b09e41 into main Sep 23, 2023
7 of 8 checks passed
@hdevalence hdevalence deleted the split-rpc-by-component branch September 23, 2023 23:55
@conorsch
Copy link
Contributor

Refs #2288

grod220 pushed a commit that referenced this pull request Oct 6, 2023
* modify scan_block() function

* modify get_updates() function

* fmt

* fix

* fix docs

* flush_update and _inner refactoring

* clears state in flush_updates

* fmt

* fix docs
@cratelyn cratelyn added the protobuf-changes Makes changes to the protobuf definitions. label Mar 5, 2024
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.

Split existing GRPC services into per-component QueryServices.
3 participants