cosmos-rust icon indicating copy to clipboard operation
cosmos-rust copied to clipboard

Add additional derive macros to Prost generated types

Open 0xForerunner opened this issue 2 years ago • 19 comments

If you take a look at osmosis-std You'll see they have derived some additional traits on many of their types, which allow you get access to type urls, and request responses automatically. This is super powerful, because then you can automatically generate utility functions to send/receive messages using a client. This is what the generated code looks like:

/// QueryParamsRequest is the request type for the Query/Params RPC method.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(
    Clone,
    PartialEq,
    Eq,
    ::prost::Message,
    ::serde::Serialize,
    ::serde::Deserialize,
    ::schemars::JsonSchema,
    CosmwasmExt,
)]
#[proto_message(type_url = "/cosmos.auth.v1beta1.QueryParamsRequest")]
#[proto_query(
    path = "/cosmos.auth.v1beta1.Query/Params",
    response_type = QueryParamsResponse
)]
pub struct QueryParamsRequest {}

The macros are already built so they could simply be used in this crate almost without changes.

0xForerunner avatar Jun 27 '23 05:06 0xForerunner

What's generating the type URLs?

tony-iqlusion avatar Jun 27 '23 17:06 tony-iqlusion

What's generating the type URLs?

I'm assuming they're just being scraped from the .proto files. Likely whatever template they're using to generate the code includes those extra derives

0xForerunner avatar Jun 27 '23 17:06 0xForerunner

I mainly ask because I've been very much interested in getting that upstreamed to Prost: https://github.com/tokio-rs/prost/pull/858

tony-iqlusion avatar Jun 27 '23 17:06 tony-iqlusion

Ah yes I see that would be pretty convenient. Perhaps you can comment more on this, but I've wondered why cosmrs redefines many of the types in cosmos-idk-proto. Adding these extra derives IMO would mean that a new version of cosmrs could directly use the types from cosmos-sdk-proto rather than redefining them.

0xForerunner avatar Jun 27 '23 17:06 0xForerunner

CosmRS defines "domain types" which wrap the protos and enforce various invariants, similar to how gogoproto is used in the upstream Cosmos SDK. It also has custom logic to handle field evolution between versions, as well as custom serializations for various types.

A similar pattern is used in tendermint-rs and ibc-rs (as well as various other Cosmos-related projects like Penumbra).

tony-iqlusion avatar Jun 27 '23 17:06 tony-iqlusion

Ah gotcha! I see how that could be useful. So currently I'm maintaining a crate called cosm-utils which consists of 80% boilerplate that could be completely removed if we were able to get these response and type_url traits on all of the proto types. It would make maintaining it so much easier.

0xForerunner avatar Jun 29 '23 16:06 0xForerunner

I see that cosmos_sdk_proto has a TypeUrl trait. Is there any chance we could get a Request trait that looks something like this?

pub trait Request {
    type Response: TypeUrl + MessageExt
}

This would be massively powerful. Adding a macro to make these for us wouldn't be too difficult either. What do you think?

0xForerunner avatar Aug 12 '23 01:08 0xForerunner

@ewoolsey FWIW I'd like to move to a trait like this in the next release: https://github.com/tokio-rs/prost/pull/896

tony-iqlusion avatar Aug 14 '23 16:08 tony-iqlusion

@ewoolsey FWIW I'd like to move to a trait like this in the next release: tokio-rs/prost#896

Awesome yeah I just saw that PR go up. That looks like a perfect solution. Any thoughts on a Request style trait that provides a Response associated type?

0xForerunner avatar Aug 14 '23 17:08 0xForerunner

What's the use case?

tony-iqlusion avatar Aug 14 '23 20:08 tony-iqlusion

@tony-iqlusion It allows you to build queries automatically. Essentially you could write a single function like this (coded from my head please ignore errors):

impl Client {
    pub async fn query<T: Request>(&self, msg: T) -> Result<<T as Request>::Response> {
        let res = self.query(msg)?;
        let response: <T as Request>::Response = serde_json::from_bytes(res.value)?;
        Ok(response)
    }
}

You could do something similar with execute message as well, imagine a message like MsgStoreCode, the Response could be

pub struct MsgStoreCodeResponse {
    code_id: u32
}

impl TryFrom<Vec<Events>> for MsgStoreCodeResponse {
    ...
}

0xForerunner avatar Aug 14 '23 20:08 0xForerunner

That's what tonic already does with gRPC endpoints

tony-iqlusion avatar Aug 14 '23 20:08 tony-iqlusion

That's what tonic already does with gRPC endpoints

Yes that's true for tonic and gRPC, but not true for other clients(tendermint_rpc, http, websocket, etc.)

0xForerunner avatar Aug 14 '23 20:08 0xForerunner

We don't even support gRPC yet. That seems like the logical place to start.

(Note: it's already implemented downstream in Ocular)

tony-iqlusion avatar Aug 14 '23 20:08 tony-iqlusion

(Note: it's already implemented downstream in Ocular)

Yeah I also have support for it in the library I'm maintaining for my work. It's currently very cumbersome to maintain manually, hence the request for this trait which would make that work a breeze.

0xForerunner avatar Aug 14 '23 21:08 0xForerunner

I have similar need where I'd like to get JSON version of cosmrs::bank::MsgSend and other proto related structs and having serde derives on those would make my life way easier.

What's the best way for now to do that?

penso avatar Sep 14 '23 18:09 penso

See #83 for JSON-related discussion

tony-iqlusion avatar Sep 14 '23 20:09 tony-iqlusion

In #432 we migrated to the new prost::Name trait.

It should eventually get first-class support in prost-build: https://github.com/tokio-rs/prost/issues/926

tony-iqlusion avatar Oct 03 '23 16:10 tony-iqlusion

In #432 we migrated to the new prost::Name trait.

It should eventually get first-class support in prost-build: tokio-rs/prost#926

This is fantastic news! Thanks for the hard work. I'll look into migrating over to this soon.

0xForerunner avatar Oct 03 '23 16:10 0xForerunner