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: align proto packages to rust crates #3077

Merged
merged 7 commits into from
Sep 22, 2023
Merged

proto: align proto packages to rust crates #3077

merged 7 commits into from
Sep 22, 2023

Conversation

hdevalence
Copy link
Member

@hdevalence hdevalence commented Sep 22, 2023

After reorganizing the Rust crates into a coherent heirarchy, we never did the same for the .proto files. This PR does that.

Currently, this just reorganizes the proto files, updates the proto-compiler tool, and regenerates the source code. What's remaining is to update all of the import paths in all of the domain type implementations that reference the proto structures. I'd suggest we swarm that part tomorrow morning, using live share + search&replace, since this PR will conflict with everything else (and break all of the protos used by other tooling like the web interfaces).

I don't think we should try to update all of the TYPE_URLs at this point; those are used for generic Any support, which we don't usually make use of, and landing the changes ASAP is more important than restoring that functionality (except where we know we need it because eg the smoke test fails).

@hdevalence hdevalence temporarily deployed to smoke-test September 22, 2023 03:52 — with GitHub Actions Inactive
@hdevalence
Copy link
Member Author

A follow-on step once this PR lands (which we should do as soon as the Rust code compiles again) is to split up the RPC definitions in the client proto package into per-component QueryServices. We might also be able to land that as part of the next release cycle.

VSCode Search&Replace

core::crypto::v1alpha1
to
penumbra::crypto::tct::v1alpha1

files to include
crates/crypto/tct
@hdevalence hdevalence temporarily deployed to smoke-test September 22, 2023 05:36 — with GitHub Actions Inactive
@hdevalence
Copy link
Member Author

It was easier than I expected to patch up the import paths; we'll still need to go through and fix up the TYPE_URL constants.

@hdevalence hdevalence marked this pull request as ready for review September 22, 2023 06:20
@hdevalence
Copy link
Member Author

It looks like prost 0.12 has a Name trait: https://docs.rs/prost/latest/prost/trait.Name.html so we could skip updating all the TYPE_URLs, switch to prost 0.12, and delete all the impls instead of updating them.

@hdevalence hdevalence merged commit b9f1a34 into main Sep 22, 2023
7 of 8 checks passed
@hdevalence hdevalence deleted the reorg-protos branch September 22, 2023 06:22
@conorsch
Copy link
Contributor

Refs #2288

@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.

3 participants