ADR 031: Protobuf Msg Services (#7458)
* Ideas * WIP * Add ADR 031 * WIP * WIP * Revert changes * Revert changes * Revert changes * Updates * Updates * Apply suggestions from code review Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Anil Kumar Kammari <anil@vitwit.com> * Add package names * Adding clarifications + Adding an abstract + Updating the first paragraph of decision * Update Context and wording * Update wording * Address proto naming Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Anil Kumar Kammari <anil@vitwit.com> Co-authored-by: Robert Zaremba <robert@zaremba.ch> Co-authored-by: Cory <cjlevinson@gmail.com>
This commit is contained in:
parent
c14a3a7cb2
commit
c7d926da2d
|
@ -55,3 +55,4 @@ Please add a entry below in your Pull Request for an ADR.
|
||||||
- [ADR 026: IBC Client Recovery Mechanisms](./adr-026-ibc-client-recovery-mechanisms.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 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)
|
||||||
|
|
|
@ -109,12 +109,12 @@ func (q Querier) QueryBalance(ctx context.Context, params *types.QueryBalancePar
|
||||||
### Custom Query Registration and Routing
|
### Custom Query Registration and Routing
|
||||||
|
|
||||||
Query server implementations as above would be registered with `AppModule`s using
|
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:
|
as below:
|
||||||
|
|
||||||
```go
|
```go
|
||||||
// x/bank/module.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})
|
types.RegisterQueryServer(server, keeper.Querier{am.keeper})
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
|
@ -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)
|
Loading…
Reference in New Issue