ADR 031: Protobuf Msg Services

docs/architecture/README.md

@@ -54,4 +54,5 @@ Please add a entry below in your Pull Request for an ADR.
 - [ADR 025: IBC Passive Channels](./adr-025-ibc-passive-channels.md)
 - [ADR 026: IBC Client Recovery Mechanisms](./adr-026-ibc-client-recovery-mechanisms.md)
 - [ADR 027: Deterministic Protobuf Serialization](./adr-027-deterministic-protobuf-serialization.md)
-- [ADR 029: Fee Grant Module](./adr-029-fee-grant-module.md)
+- [ADR 029: Fee Grant Module](./adr-029-fee-grant-module.md)
+- [ADR 031: Protobuf Msg Services](./adr-031-msg-service.md)

docs/architecture/adr-021-protobuf-query-encoding.md

@@ -109,12 +109,12 @@ func (q Querier) QueryBalance(ctx context.Context, params *types.QueryBalancePar
 ### Custom Query Registration and Routing
 
 Query server implementations as above would be registered with `AppModule`s using
-a new method `RegisterQueryServer(grpc.Server)` which could be implemented simply
+a new method `RegisterQueryService(grpc.Server)` which could be implemented simply
 as below:
 
 ```go
 // x/bank/module.go
-func (am AppModule) RegisterQueryServer(server grpc.Server) {
+func (am AppModule) RegisterQueryService(server grpc.Server) {
 	types.RegisterQueryServer(server, keeper.Querier{am.keeper})
 }
 ```

docs/architecture/adr-031-msg-service.md

@@ -0,0 +1,238 @@
+# ADR 031: Protobuf Msg Services
+
+## Changelog
+
+- 2020-10-05: Initial Draft
+
+## Status
+
+Proposed
+
+## Abstract
+
+We want to leverage protobuf `service` definitions for defining `Msg`s which will give us significant developer UX
+improvements in terms of the code that is generated and the fact that return types will now be well defined.
+
+## Context
+
+Currently `Msg` handlers in the Cosmos SDK do have return values that are placed in the `data` field of the response.
+These return values, however, are not specified anywhere except in the golang handler code.
+
+In early conversations [it was proposed](https://docs.google.com/document/d/1eEgYgvgZqLE45vETjhwIw4VOqK-5hwQtZtjVbiXnIGc/edit)
+that `Msg` return types be captured using a protobuf extension field, ex:
+
+```protobuf
+package cosmos.gov;
+
+message MsgSubmitProposal
+	option (cosmos_proto.msg_return) = “uint64”;
+	string delegator_address = 1;
+	string validator_address = 2;
+	repeated sdk.Coin amount = 3;
+}
+```
+
+This was never adopted, however.
+
+Having a well-specified return value for `Msg`s 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, https://github.com/cosmos/cosmos-sdk/issues/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.
+
+In addition, handler registration of `Msg` types tends to add a bit of
+boilerplate on top of keepers and is usually done through manual type switches.
+This isn't necessarily bad, but it does add overhead to creating modules.
+
+## Decision
+
+We decide to use protobuf `service` definitions for defining `Msg`s as well as
+the code generated by them as a replacement for `Msg` handlers.
+
+Below we define how this will look for the `SubmitProposal` message from `x/gov` module. 
+We start with a `Msg` `service` definition:
+
+```proto
+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;
+  string proposer = 2;
+}
+
+message MsgSubmitProposalResponse {
+  uint64 proposal_id;
+}
+```
+
+While this is most commonly used for gRPC, overloading protobuf `service` definitions like this does not violate
+the intent of the [protobuf spec](https://developers.google.com/protocol-buffers/docs/proto3#services) which says:
+> If you don’t want to use gRPC, it’s also possible to use protocol buffers with your own RPC implementation.
+With this approach, we would get an auto-generated `MsgServer` interface:
+
+In addition to clearly specifying return types, this has the benefit of generating client and server code. On the server
+side, this is almost like an automatically generated keeper method and could maybe be used intead of keepers eventually
+(see [\#7093](https://github.com/cosmos/cosmos-sdk/issues/7093)):
+
+```go
+package gov
+
+type MsgServer interface {
+  SubmitProposal(context.Context, *MsgSubmitProposal) (*MsgSubmitProposalResponse, error)
+}
+```
+
+On the client side, developers could take advantage of this by creating RPC implementations that encapsulate transaction
+logic. Protobuf libraries that use asynchronous callbacks, like [protobuf.js](https://github.com/protobufjs/protobuf.js#using-services)
+could use this to register callbacks for specific messages even for transactions that include multiple `Msg`s.
+
+For backwards compatibility, existing `Msg` types should be used as the request parameter
+for `service` definitions. Newer `Msg` types which only support `service` definitions
+should use the more canonical `Msg...Request` names.
+
+### Encoding
+
+Currently, we are encoding `Msg`s as `Any` in `Tx`s which involves packing the
+binary-encoded `Msg` with its type URL.
+
+The type URL for `MsgSubmitProposal` based on the proto3 spec is `/cosmos.gov.MsgSubmitProposal`. 
+
+The fully-qualified name for the `SubmitProposal` service method above (also
+based on the proto3 and gRPC specs) is `/cosmos.gov.Msg/SubmitProposal` which varies
+by a single `/` character. The generated `.pb.go` files for protobuf `service`s
+include names of this form and any compliant protobuf/gRPC code generator will
+generate the same name.
+
+In order to encode service methods in transactions, we encode them as `Any`s in
+the same `TxBody.messages` field as other `Msg`s. We simply set `Any.type_url`
+to the full-qualified method name (ex. `/cosmos.gov.Msg/SubmitProposal`) and
+set `Any.value` to the protobuf encoding of the request message
+(`MsgSubmitProposal` in this case). 
+
+### Decoding
+
+When decoding, `TxBody.UnpackInterfaces` will need a special case
+to detect if `Any` type URLs match the service method format (ex. `/cosmos.gov.Msg/SubmitProposal`)
+by checking for two `/` characters. Messages that are method names plus request parameters
+instead of a normal `Any` messages will get unpacked into the `ServiceMsg` struct:
+
+```go
+type ServiceMsg struct {
+  // MethodName is the fully-qualified service name
+  MethodName string
+  // Request is the request payload
+  Request MsgRequest
+}
+```
+
+### Routing
+
+In the future, `service` definitions may become the primary method for defining
+`Msg`s. As a starting point, we need to integrate with the SDK's existing routing
+and `Msg` interface. 
+
+To do this, `ServiceMsg` implements the `sdk.Msg` interface and its handler does the
+actual method routing, allowing this feature to be added incrementally on top of
+existing functionality.
+
+### `MsgRequest` interface
+
+All request messages will need to implement the `MsgRequest` interface which is a
+simplified version of `Msg`, without `Route()`, `Type()` and `GetSignBytes()` which
+are no longer needed:
+
+```go
+type MsgRequest interface {
+  proto.Message
+  ValidateBasic() error
+  GetSigners() []AccAddress
+}
+```
+
+`ServiceMsg` will forward its `ValidateBasic` and `GetSigners` methods to the `MsgRequest`
+methods.
+
+### Module Configuration
+
+In [ADR 021](./adr-021-protobuf-query-encoding.md), we introduced a method `RegisterQueryService`
+to `AppModule` which allows for modules to register gRPC queriers.
+
+To register `Msg` services, we attempt a more extensible approach by converting `RegisterQueryService`
+to a more generic `RegisterServices` method:
+
+```go
+type AppModule interface {
+  RegisterServices(Configurator)
+  ...
+}
+
+type Configurator interface {
+  QueryServer() grpc.Server
+  MsgServer() grpc.Server
+}
+
+// example module:
+func (am AppModule) RegisterServices(cfg Configurator) {
+	types.RegisterQueryServer(cfg.QueryServer(), keeper)
+	types.RegisterMsgServer(cfg.MsgServer(), keeper)
+}
+```
+
+The `RegisterServices` method and the `Configurator` interface are intended to
+evolve to satisfy the use cases discussed in [\#7093](https://github.com/cosmos/cosmos-sdk/issues/7093)
+and [\#7122](https://github.com/cosmos/cosmos-sdk/issues/7421).
+
+When `Msg` services are registered, the framework _should_ verify that all `Msg...Request` types
+implement the `MsgRequest` interface described above and throw an error during initialization rather
+than later when transactions are processed.
+
+### `Msg` Service Implementation
+
+Just like query services, `Msg` service methods can retrieve the `sdk.Context`
+from the `context.Context` parameter method using the `sdk.UnwrapSDKContext`
+method:
+
+```go
+package gov
+
+func (k Keeper) SubmitProposal(goCtx context.Context, params *types.MsgSubmitProposal) (*MsgSubmitProposalResponse, error) {
+	ctx := sdk.UnwrapSDKContext(goCtx)
+    ...
+}
+```
+
+The `sdk.Context` should have an `EventManager` already attached by the `ServiceMsg`
+router.
+
+Separate handler definition is no longer needed with this approach.
+
+## 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, this would improve the UX of [\#7093](https://github.com/cosmos/cosmos-sdk/issues/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 simultaneously could be confusing
+(we could choose to deprecate the current approach)
+- using `service` definitions outside the context of gRPC could be confusing (but doesn’t violate the proto3 spec)
+
+## References
+
+- [Initial Github Issue \#7122](https://github.com/cosmos/cosmos-sdk/issues/7122)
+- [proto 3 Language Guide: Defining Services](https://developers.google.com/protocol-buffers/docs/proto3#services)
+- [Initial pre-`Any` `Msg` designs](https://docs.google.com/document/d/1eEgYgvgZqLE45vETjhwIw4VOqK-5hwQtZtjVbiXnIGc)
+- [ADR 020](./adr-020-protobuf-transaction-encoding.md)
+- [ADR 021](./adr-021-protobuf-query-encoding.md)