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

ADR-031: elaborate consequences and encapsulation of module client-se… #7839

Merged
merged 4 commits into from
Nov 18, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions docs/architecture/adr-031-msg-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ This isn't necessarily bad, but it does add overhead to creating modules.
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.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whitespaces were removed automatically.

Below we define how this will look for the `SubmitProposal` message from `x/gov` module.
We start with a `Msg` `service` definition:

```proto
Expand Down Expand Up @@ -105,7 +105,7 @@ should use the more canonical `Msg...Request` names.
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 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
Expand All @@ -117,15 +117,15 @@ 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).
(`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
Expand All @@ -139,7 +139,7 @@ type ServiceMsg struct {

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.
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
Expand Down Expand Up @@ -218,17 +218,25 @@ Separate handler definition is no longer needed with this approach.

## Consequences

This design changes how a module functionality is exposed and accessed. It deprecates the existing `Handler` interface and `AppModule.Route` in favor of [Protocol Buffer Services](https://developers.google.com/protocol-buffers/docs/proto3#services) and Service Routing described above. This dramatically simplifies the code. We don't need to create handlers and keepers any more. Use of Protocol Buffer auto-generated clients clearly separates the communication interfaces between the module and a modules user. The control logic (aka handlers and keepers) is not exposed any more. A module interface can be seen as a black box accessible through a client API. It's worth to note that the client interfaces are also generated by Protocol Buffers.

This also changes how we will perform functional tests. Instead of mocking AppModules and Router, we will mock a client (server will stay hidden). More specifically: we will never mock `x/moduleA/server` in `x/moduleB`. One can think about it as working with external services (eg DBs, or online servers...). We assume that the transmission between clients and servers is correctly handled by generated Protocol Buffers.
robert-zaremba marked this conversation as resolved.
Show resolved Hide resolved

Finally, closing a module to client API opens desirable OCAP patterns discussed in ADR-033. Since server implementation and interface is hidden, nobody can hold "keepers"/servers and will be forced to relay on the client interface, which will drive developers for correct encapsulation and software engineering patterns.

### 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
- communication interface is automatically generated, the developer can now focus only on the state transition methods - 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 and tests
- dramatically reduces and simplifies the code

### 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)
Expand Down