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

Osmosis Proto #236

Closed
Philipp-Sc opened this issue May 30, 2022 · 15 comments
Closed

Osmosis Proto #236

Philipp-Sc opened this issue May 30, 2022 · 15 comments

Comments

@Philipp-Sc
Copy link
Contributor

Hello,

thank you so much for working on this awesome repository.

How much effort would it be to add Osmosis and Terra?
I tried running the build, but something fails. I was able to manually add some Osmosis related messages.

If it is easy to archive by adding it to the main.rs file maybe I do not need to implement it manually.

Thank you in advance.
Best regards,
Philipp

@tony-iqlusion
Copy link
Member

There's some ongoing discussion about this here: informalsystems/tendermint-rs#654 (comment)

I suggested having a single repo where all of the protobufs could be managed consistently. So yes, I think it'd be a good idea to reuse the same infrastructure we have already for crates like tendermint-proto and cosmos-sdk-proto to be able to generate crates for the protos for other networks.

@Philipp-Sc
Copy link
Contributor Author

I think this makes a lot of sense. From a technical perspective, what exactly needs to be done to make the main.rs work for osmosis? Just adjust the paths or are blockchain specific customizations needed as well?

@tony-iqlusion
Copy link
Member

We could add a new osmosis-proto crate here (which I've gone ahead and registered).

It could be structured similarly to (and dependent on) cosmos-sdk-proto.

It will require modifying proto-build to pull in the protos from https://github.com/osmosis-labs/osmosis/tree/main/proto/osmosis as a git submodule (similar to the other protos)

@Philipp-Sc
Copy link
Contributor Author

I got the cargo run to finish without errors with added paths for osmosis.

  • '../cosmos-sdk-proto/src/prost/osmosisd'
    What about the process for the lib.rs and cargo.toml do they need to be edited manually, or is there a script for that as well?

@tony-iqlusion
Copy link
Member

Since Osmosis is a completely separate network, I'd suggest putting its protos in a completely separate crate: osmosis-proto

You can get started by doing cp -r cosmos-sdk-proto osmosis-proto and then editing the files accordingly. You'll also need to add osmosis-proto to the workspace definition in the toplevel Cargo.toml

@tony-iqlusion
Copy link
Member

@Philipp-Sc I pushed up a branch with some initial boilerplate for an osmosis-proto crate you can use to get started:

https://github.com/cosmos/cosmos-rust/tree/osmosis-proto

See this commit:

bfdf7a1

@Philipp-Sc
Copy link
Contributor Author

thank you, I just separated the code from the cosmos-sdk-proto into osmosis-proto. I tested it with a few queries and it seems to work fine with the Osmosis gRPC endpoint I have.

How Osmosis is setup, the endpoint sometimes return google.protobuf.Any for that I will need to add some handcrafted code, so it can easily be casted into the type it acutally represents.

I will fork the branch you created and try to do a pull request.

@Philipp-Sc
Copy link
Contributor Author

@tony-iqlusion pull request is out #239

Let me know if you like me to make any changes before you merge it.

@Philipp-Sc
Copy link
Contributor Author

Philipp-Sc commented Jun 3, 2022

@tony-iqlusion I also have some changes for cosmrs, I hope to add them.

Example for tx/msg.rs

#[cfg(feature = "osmosis")]
impl MsgProto for osmosis_proto::osmosis::gamm::v1beta1::Pool {
    const TYPE_URL: &'static str = "/osmosis.gamm.v1beta1.Pool";
}

(this is needed to be able to work with google.protobuf.Any types)

Is cosmrs the right place for this, or do you want me to create a new crate just for osmosis?

@Philipp-Sc
Copy link
Contributor Author

Update:
See added osmosis specific cosmrs customization

I think this is a good way to do it. Let me know if you have any feedback.

Thanks and best regards,
Philipp

@Philipp-Sc
Copy link
Contributor Author

On another note: tx/msg.rs it would be great if the MsgProto implementations get generated by a script if possible. There are many types missing.

@Philipp-Sc
Copy link
Contributor Author

Closing this issue.

@tony-iqlusion
Copy link
Member

tony-iqlusion commented Aug 2, 2022

The MsgProto trait is gone. I assume you're referring to the TypeUrl trait?

If so, yes, that sure would be nice. There's an upstream issue about it on prost here:

tokio-rs/prost#299

@Philipp-Sc
Copy link
Contributor Author

If there was a way to get a list of all the type urls we could write a script to write the matching implementations.
Might be something to look into in the future. For the time being I am okay, with adding the types I need manually.

@tony-iqlusion
Copy link
Member

It will hopefully get fixed upstream in the next release. There's already a few PRs.

If it really ends up continuing to be an issue we can look at automating it.

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

No branches or pull requests

2 participants