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

Consider protobuf service definitions for Msg's #7122

Closed
aaronc opened this issue Aug 20, 2020 · 4 comments · Fixed by #7458
Closed

Consider protobuf service definitions for Msg's #7122

aaronc opened this issue Aug 20, 2020 · 4 comments · Fixed by #7458

Comments

@aaronc
Copy link
Member

aaronc commented Aug 20, 2020

Summary

This issue proposes a complementary or alternative way of representing sdk.Msgs using protobuf service definitions. It’s just an idea, and something to think about after Stargate. We can totally throw it out if it makes no sense! But it might also provide some nice UX improvements...

Context

When we initially designed protobuf support for sdk.Msgs it was proposed that Msg return types be captured with a protobuf extension field, ex:

message MsgSubmitProposal
	option (cosmos_proto.msg_return) = “google.protobuf.Timestamp”;
	bytes delegator_address = 1;
	bytes validator_address = 2;
	repeated sdk.Coin amount = 3;
}

This was never adopted, however, and we currently don’t have a mechanism for specifying the return type of a Msg.

Having a well-specified return value for Msgs would improve client UX. For instance, in x/gov, MsgSubmitProposal returns the proposal ID as a big-endian uint64. This isn’t really documented anywhere and clients would need to know the internals of the SDK to parse that value and return it to users.

Also, there may be cases where we want to use these return values programatically. For instance, #7093 proposes a method for doing inter-module Ocaps using the Msg router. A well-defined return type would improve the developer UX for this approach.

Proposal

Protobuf service definitions could be used instead of concrete protobuf message types. This would have more or less the same over-the-wire representation but improve developer UX.

Ex:

package cosmos.gov;

service Msg {
  rpc SubmitProposal(MsgSubmitProposal) returns (MsgSubmitProposalResponse);
}

// Note that for backwards compatibility this uses MsgSubmitProposal as the request type instead of the more canonical MsgSubmitProposalRequest
message MsgSubmitProposal {
  google.protobuf.Any content = 1;
  bytes proposer = 2;
}

message MsgSubmitProposalResponse {
  uint64 proposal_id;
}

Note that overloading protobuf service definitions like this does not violate the intent of the protobuf spec which says:

If you don’t want to use gRPC, it’s also possible to use protocol buffers with your own RPC implementation.

Currently, we are encoding Msgs as Any in Txs which involves packing the binary-encoded Msg with its type URL.

The type URL for MsgSubmitProposal is /cosmos.gov.MsgSubmitProposal.

The fully-qualified RPC name for the SubmitProposal RPC call above is /cosmos.gov.Msg/SubmitProposal which is varies by a single / character. We could also pack this type URL into an Any with the same MsgSubmitProposal contents and handle it like a “packed RPC” call.

With this approach, we would get an auto-generated MsgServer interface:

type MsgServer interface {
  SubmitProposal(context.Context, *MsgSubmitProposal) (*MsgSubmitProposalResponse, error)
}

This is almost like an automatically generated Keeper interface. It does more or less the same thing… Instead of needing to manually register handlers with handler.go files and manually marshal return types, the MsgServer interface would just need to be implemented and registered and that’s it.

Also, MsgClient interfaces would be generated so theoretically some client code could send transactions simply by calling govClient.SubmitProposal(ctx, msg). Obviously a lot of transaction stuff would need to happen in the background… but it could be useful for client devs.

This approach could live alongside the current concrete Msg type approach, or even replace it if we wanted. In the example above, MsgSubmitProposal is used as the request type so that we can either send /cosmos.gov.MsgSubmitProposal as a concrete Msg or /cosmos.gov.Msg/SubmitProposal as an RPC call… (Hope that’s not too confusing…)

Consequences

Pros

  • communicates return type clearly
  • manual handler registration and return type marshaling is no longer needed, just implement the interface and register it
  • some keeper code could be automatically generate, would improve the UX of Do Proper Ocaps for x/bank (or not): A Case Study #7093 approach (1) if we chose to adopt that
  • generated client code could be useful for clients

Cons

  • supporting both this and the current concrete Msg type approach could be confusing
  • using service definitions outside the context of gRPC could be confusing (but doesn’t violate the proto3 spec)

References

@anilcse
Copy link
Collaborator

anilcse commented Oct 1, 2020

+1 This makes a lot of sense. If we can agree on replacing the concrete Msg types, it would be simpler and can totally avoid the confusion around it.

@aaronc
Copy link
Member Author

aaronc commented Oct 1, 2020

As I mentioned in #7421, I think this could play really nicely with #7093 and create a viable approach to stable v1.0 modules as well as have other benefits.

I wonder if you have any thoughts on this from either the cosmjs or cosmwasm perspectives @ethanfrey ?

@amaury1093
Copy link
Contributor

I love the auto-generated type MsgServer interface {...} which serves as a Keeper interface, it almost creates a canonical skeleton for Keepers to handle Msgs, and this will help stabilize modules towards a v1.0 API.

I have some doubts on the client side of things:

Also, MsgClient interfaces would be generated so theoretically some client code could send transactions simply by calling govClient.SubmitProposal(ctx, msg). Obviously a lot of transaction stuff would need to happen in the background… but it could be useful for client devs.

Does govClient.SubmitProposal(ctx, msg) actually send a network request e.g. using gRPC (assuming govClient handles tx signature and stuff)? What about txs with multiple messages? Auto-generated client code seems like a useful feature, but with limited scope.

If not using govClient.SubmitProposal(ctx, msg), I'm wondering how clients can get the response type back. If I'm sending a tx with two Any Msgs with typeUrl [/cosmos.gov.Msg/SubmitProposal, /cosmos.foo.Msg/Bar], the only way I can think of to retrieve the 2 Msg response types is to do some kind of proto service reflection on the client side, is that right?

@aaronc
Copy link
Member Author

aaronc commented Oct 2, 2020

Good questions @amaurymartiny

Does govClient.SubmitProposal(ctx, msg) actually send a network request e.g. using gRPC (assuming govClient handles tx signature and stuff)? What about txs with multiple messages? Auto-generated client code seems like a useful feature, but with limited scope.

No it wouldn't be a network request with gRPC directly. It would still go through the same broadcast Tx to tendermint pathway. Clients could use the auto-generated code by providing an RPC implementation that does that. With protobuf.js a custom RPC implementation is needed in general so this wouldn't be super awkward: https://github.com/protobufjs/protobuf.js#using-services.

And clients are of course not forced to use the generated client/server code, they can pack the Any with the method type and request body.

If not using govClient.SubmitProposal(ctx, msg), I'm wondering how clients can get the response type back. If I'm sending a tx with two Any Msgs with typeUrl [/cosmos.gov.Msg/SubmitProposal, /cosmos.foo.Msg/Bar], the only way I can think of to retrieve the 2 Msg response types is to do some kind of proto service reflection on the client side, is that right?

Yes, you would need to use some sort of reflection on the client interface if you're packing the Anys directly.

I think though, if your protobuf library generates asynchronous client interfaces (protobuf.js does this, but golang doesn't), you can use the generated code with multiple messages. You would just need some sort of batch tx RPC implementation. In protobuf.js this might look like:

var batchTx = newBatchTxRPCImpl()

var govClient = gov.Msg.create(batchTx)
govClient.submitProposal({...}, function(err, response) {
   ...
})

var fooClient = foo.Msg.create(batchTx)
fooClient.bar({...}, function(err, response) {
  ...
})

// broadcast both messages queued in the batch as a single tx
batchTx.broadcast(function(err, response) {
   ...
}

In this example, because it's an async API, each msg could receive a separate callback even though they're broadcast as a single tx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants