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

Split out tendermint-proto into separate repository #1462

Open
soareschen opened this issue Aug 12, 2024 · 10 comments
Open

Split out tendermint-proto into separate repository #1462

soareschen opened this issue Aug 12, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@soareschen
Copy link
Contributor

Description

Downstream projects such as ibc-proto and ibc-rs depends only on tendermint-proto, but not the other tendermint crates. By having its own repository and release cycle, we won’t have to depend on the tendermint libraries directly, making it easier to keep tendermint-proto, ibc-proto-rs, and ibc-rs in sync. Plus, we’re aiming to fully decouple the ibc-rs libraries from tendermint (except for the ICS-07 implementation). These together would give us more flexibility, even allowing us to apply upgrades asynchronously.

@soareschen soareschen added the enhancement New feature or request label Aug 12, 2024
@romac
Copy link
Member

romac commented Aug 12, 2024

@tony-iqlusion From the cosmos-rust side, any objections with doing this or does it sound good to you?

@tony-iqlusion
Copy link
Collaborator

That sounds fine to me. In the past I’ve suggested getting all of the proto crates into a single repo so cross-cutting changes are easier

@informalsystems informalsystems deleted a comment from tarcieri Aug 15, 2024
@romac
Copy link
Member

romac commented Aug 16, 2024

I may be wrong, but I don't expect there should be many such cross-cutting changes when a new version of tendermint-proto is released. It's gonna be a whole other story when cometbft-proto v1 is released, but it'll only be a one-time cost I think.

That said I am not opposed the idea of having a single cosmos/rust-proto repository, it's just that it's a lot more initial work to get everything lined up etc.

@Farhad-Shabani @soareschen What do you think?

@Farhad-Shabani
Copy link
Member

Farhad-Shabani commented Aug 17, 2024

From ibc-rs perspective, putting all the proto crates into a single repo, if with a shared workspace where version bumps happen together, is not desirable. The proto level version upgrades are breaking changes for ibc-rs. Therefore, changes in such a repo that don’t directly relate to ibc-rs still impacts the project. Specially, we'd like to decouple ibc-rs (except for ICS-07) from any tendermint dependencies.

At the very least, I recommend keeping tendermint-proto and ibc-proto-rs workspaces and versions separate. Though, to me, generally keeping each workspace in separate repo feels like a simpler way to handle changelog, releases, and maintenance.

@soareschen
Copy link
Contributor Author

soareschen commented Aug 21, 2024

To me, the main considerations for whether to put code in the same repository are as follows:

  • Atomic upgrade - If there is a frequent need for multiple parts of the code to be updated at the same time, it is more efficient to update it in a monorepo with a single PR, as compared to spreading it across multiple PRs in multiple repositories with temporary git revision tags in dependencies.
  • Issue management - If the code is maintained by the same group of people within the same project scope, it is better to track all issues and PRs at the same place. But if there are multiple groups of people loosely collaborating, it might be better to maintain the code across multiple repositories.
  • Mixing of versions - If there is a need for one part of the code base to be used with multiple versions of another part of the code base, it may be better to split them into multiple repositories. This may be important not for development, but for testing and providing backward compatibility guarantees.

If we choose to go with monorepo, then there is a secondary concern of whether to put the code in the same workspace:

  • Dependency management - If the code shares a lot of common dependencies, it may help manage the dependencies in a single workspace. This would help reduce dependency hell, where different parts of the code depend on different versions of dependencies. But if one part of the code base need to have dependencies that may be incompatible with the dependencies of another part of the code base, then it may be better to put them in separate workspaces.
  • Release cycle - If one part of the code base has significantly different release cycle, or needs to provide different levels of backward compatibility, it may be better to put them in multiple workspaces.

@tony-iqlusion
Copy link
Collaborator

Right now cosmos-sdk-proto and ibc-proto both need a release after tendermint-proto. Soon it will be tendermint-proto -> cosmos-sdk-proto -> ibc-proto.

They all share the same dependencies (e.g. prost and tonic) and each have their own artisanal tool for ETLing protobuf schemas into Rust code.

Just the latter alone seems like a lot of duplicated effort.

@tony-iqlusion
Copy link
Collaborator

The other argument for consolidating into a single repo is they could share the "proto build" ETL logic for transforming protos into Rust source code, so we can actually share what cool tricks we've developed between them.

Here's one example: cosmos/ibc-proto-rs#240

@dakom
Copy link

dakom commented Sep 19, 2024

from the outside, building application/tooling that uses these crates, I'm very +1 having it all in one repo. It's been painful with some version conflicts and not knowing where to look, so punting with functions like any_to_any() and rpc_to_grpc_but_it's_really_the_same_type() if y'all catch my drift.

Tbh what I'd really like is one crate focused only on the tonic_build generation, with features so it's all there and kept in sync and I don't even need to think of which Coin or Any or whatever type is the one to use, since there would only be one of those types exported.

Also agree with the benefit of having the build logic in one place for things like that transport feature

Ofc that's just a view from the outside of wanting to import it all, not necessarily maintain it all, and I don't know the whole scope involved with such a request - please take my vote with a grain of salt :)

@tony-iqlusion
Copy link
Collaborator

@dakom as of the latest releases of tendermint-proto, cosmos-sdk-proto, and the forthcoming release of ibc-proto there should (hopefully) no longer be duplication between types.

tendermint-proto now contains its own versions of the google.protobuf well-known types which cosmos-sdk-proto and ibc-proto now use, and duplication between cosmos-sdk-proto and ibc-proto has been eliminated.

If you see duplicate types at this point, it's a bug.

@dakom
Copy link

dakom commented Sep 25, 2024

thanks @tony-iqlusion - sorry I missed the notification. most likely I need to update some dependency, or updated and didn't notice that this had been fixed. thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants