certusone
/
cosmos-sdk
mirror of https://github.com/certusone/cosmos-sdk.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

ADR 033: Inter-Module Communication (#7459) 

* Initial WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* Update Invoker

* WIP

* Tidying up

* Update title

* Cleanup diff

* cleanup

* Update context

* Add ClientConn impl

* WIP

* updates

* revert go.mod

* Describe internal methods and authorization

* Add comparison to x/capability

* Cleanup

* Apply suggestions from code review

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update docs/architecture/adr-033-protobuf-inter-module-comm.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update docs/architecture/adr-033-protobuf-inter-module-comm.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update docs/architecture/adr-033-protobuf-inter-module-comm.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update docs/architecture/adr-033-protobuf-inter-module-comm.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* ADR-033 Updates / Amendments (#8190)

* updates to ADR33 based on 12.04.20 architecture call

* add comments about improved devx

* Update docs/architecture/adr-033-protobuf-inter-module-comm.md

Co-authored-by: Aaron Craelius <aaron@regen.network>

* update with atomicity note

Co-authored-by: Aaron Craelius <aaron@regen.network>

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Cory <cjlevinson@gmail.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
This commit is contained in:
Aaron Craelius 2021-02-19 10:34:54 -05:00 committed by GitHub
parent e577378497
commit 7e481cc716
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 372 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
docs/architecture/README.md
View File

@ -72,6 +72,7 @@ Read about the [PROCESS](./PROCESS.md).
- [ADR 027: Deterministic Protobuf Serialization](./adr-027-deterministic-protobuf-serialization.md)
- [ADR 028: Public Key Addresses](./adr-028-public-key-addresses.md)
- [ADR 032: Typed Events](./adr-032-typed-events.md)
- [ADR 033: Inter-module RPC](architecture/adr-033-protobuf-inter-module-comm.md)
- [ADR 035: Rosetta API Support](./adr-035-rosetta-api-support.md)
- [ADR 037: Governance Split Votes](./adr-037-gov-split-vote.md)
- [ADR 038: State Listening](./adr-038-state-listening.md)

371
docs/architecture/adr-033-protobuf-inter-module-comm.md Normal file
View File

@ -0,0 +1,371 @@
# ADR 033: Protobuf-based Inter-Module Communication
## Changelog
- 2020-10-05: Initial Draft
## Status
Proposed
## Abstract
This ADR introduces a system for permissioned inter-module communication leveraging the protobuf `Query` and `Msg`
service definitions defined in [ADR 021](./adr-021-protobuf-query-encoding.md) and
[ADR 031](./adr-031-msg-service.md) which provides:
- stable protobuf based module interfaces to potentially later replace the keeper paradigm
- stronger inter-module object capabilities guarantees
- module accounts and sub-account authorization
## Context
In the current Cosmos SDK documentation on the [Object-Capability Model](../docs/core/ocap.md), it is state that:
> We assume that a thriving ecosystem of Cosmos-SDK modules that are easy to compose into a blockchain application will contain faulty or malicious modules.
There is currently not a thriving ecosystem of Cosmos SDK modules. We hypothesize that this is in part due to:
1. lack of a stable v1.0 Cosmos SDK to build modules off of. Module interfaces are changing, sometimes dramatically, from
point release to point release, often for good reasons, but this does not create a stable foundation to build on.
2. lack of a properly implemented object capability or even object-oriented encapsulation system which makes refactors
of module keeper interfaces inevitable because the current interfaces are poorly constrained.
### `x/bank` Case Study
Currently the `x/bank` keeper gives pretty much unrestricted access to any module which references it. For instance, the
`SetBalance` method allows the caller to set the balance of any account to anything, bypassing even proper tracking of supply.
There appears to have been some later attempts to implement some semblance of Ocaps using module-level minting, staking
and burning permissions. These permissions allow a module to mint, burn or delegate tokens with reference to the module’s
own account. These permissions are actually stored as a `[]string` array on the `ModuleAccount` type in state.
However, these permissions don’t really do much. They control what modules can be referenced in the `MintCoins`,
`BurnCoins` and `DelegateCoins***` methods, but for one there is no unique object capability token that controls access —
just a simple string. So the `x/upgrade` module could mint tokens for the `x/staking` module simple by calling
`MintCoins(“staking”)`. Furthermore, all modules which have access to these keeper methods, also have access to
`SetBalance` negating any other attempt at Ocaps and breaking even basic object-oriented encapsulation.
## Decision
Starting from the work in [ADR 021](./adr-021-protobuf-query-encoding.md) and [ADR 31](./adr-031-msg-service.md), we introduce
the following inter-module communication system as an new paradigm for secure module based authorization and OCAPS
framework. When implemented, this could also serve as an alternative the existing paradigm of passing keepers between
modules. The approach outlined here-in is intended to form the basis of a Cosmos SDK v1.0 that provides the necessary
stability and encapsulation guarantees that allow a thriving module ecosystem to emerge.
Of particular note — the decision is to _enable_ this functionality for modules to adopt at their own discretion.
Proposals to migrate existing modules to this new paradigm will have to be a separate conversation, potentially
addressed as amendments to this ADR.
### New "Keeper" Paradigm
In [ADR 021](./adr-021-protobuf-query-encoding.md), a mechanism for using protobuf service definitions to define queriers
was introduced and in [ADR 31](./adr-031-msg-service.md), a mechanism for using protobuf service to define `Msg`s was added.
Protobuf service definitions generate two golang interfaces representing the client and server sides of a service plus
some helper code. Here is a minimal example for the bank `Send` `Msg`.
```go
package bank
type MsgClient interface {
Send(context.Context, *MsgSend, opts ...grpc.CallOption) (*MsgSendResponse, error)
}
type MsgServer interface {
Send(context.Context, *MsgSend) (*MsgSendResponse, error)
}
```
[ADR 021](./adr-021-protobuf-query-encoding.md) and [ADR 31](./adr-031-msg-service.md) specifies how modules can implement the generated `QueryServer`
and `MsgServer` interfaces as replacements for the legacy queriers and `Msg` handlers respectively.
In this ADR we explain how modules can make queries and send `Msg`s to other modules using the generated `QueryClient`
and `MsgClient` interfaces and propose this mechanism as a replacement for the existing `Keeper` paradigm. To be clear,
this ADR does not necessitate the creation of new protobuf definitions or services. Rather, it leverages the same proto
based service interfaces already used by clients for inter-module communication.
Using this `QueryClient`/`MsgClient` approach has the following key benefits over keepers:
1. Protobuf types are checked for breaking changes using [buf](https://buf.build/docs/breaking-overview) and because of
the way protobuf is designed this will give us strong backwards compatibility guarantees while allowing for forward
evolution.
2. The separation between the client and server interfaces will allow us to insert permission checking code in between
the two which checks if one module is authorized to send the specified `Msg` to the other module providing a proper
object capability system.
3. The router for inter-module communication gives us a convenient place to handle rollback of transactions,
enabling atomicy of operations ([currently a problem](https://github.com/cosmos/cosmos-sdk/issues/8030)). Any failure within a module-to-module call would result in a failure of the entire
transaction
This mechanism has the added benefits of:
- reducing boilerplate through code generation, and
- allowing for modules in other languages either via a VM like CosmWasm or sub-processes using gRPC
### Inter-module Communication
To use the `Client` generated by the protobuf compiler we need a `grpc.ClientConn` implementation. For this we introduce
a new type, `ModuleKey`, which implements the `grpc.ClientConn` interface. `ModuleKey` can be thought of as the "private
key" corresponding to a module account, where authentication is provided through use of a special `Invoker()` function,
described in more detail below.
Whereas external clients use their account's private key to sign transactions containing `Msg`s where they are listed as signers,
modules use their `ModuleKey` to send `Msg`s where they are listed as the sole signer to other modules. For example, modules
could use their `ModuleKey` to "sign" a `/cosmos.bank.Msg/Send` transaction to send coins from the module's account to another
account.
Here's an example of a hypothetical module `foo` interacting with `x/bank`:
```go
package foo
func (fooMsgServer *MsgServer) Bar(ctx context.Context, req *MsgBarRequest) (*MsgBarResponse, error) {
bankQueryClient := bank.NewQueryClient(fooMsgServer.moduleKey)
balance, err := bankQueryClient.Balance(&bank.QueryBalanceRequest{Address: fooMsgServer.moduleKey.Address(), Denom: "foo"})
...
bankMsgClient := bank.NewMsgClient(fooMsgServer.moduleKey)
res, err := bankMsgClient.Send(ctx, &bank.MsgSendRequest{FromAddress: fooMsgServer.moduleKey.Address(), ...})
...
}
```
This design is also intended to be extensible to cover use cases of more fine grained permissioning like minting by
denom prefix being restricted to certain modules (as discussed in
[#7459](https://github.com/cosmos/cosmos-sdk/pull/7459#discussion_r529545528)).
### `ModuleKey`s and `ModuleID`s
A `ModuleKey` can be thought of as a "private key" for a module account and a `ModuleID` can be thought of as the
corresponding "public key". From the [ADR 028](./adr-028-public-key-addresses.md), modules can have both a root module account and any number of sub-accounts
or derived accounts that can be used for different pools (ex. staking pools) or managed accounts (ex. group
accounts). We can also think of module sub-accounts as similar to derived keys - there is a root key and then some
derivation path. `ModuleID` is a simple struct which contains the module name and optional "derivation" path,
and forms its address based on the `AddressHash` method from [the ADR 028 draft](https://github.com/cosmos/cosmos-sdk/pull/7086):
```go
type ModuleID struct {
ModuleName string
Path []byte
}
func (key ModuleID) Address() []byte {
return AddressHash(key.ModuleName, key.Path)
}
```
In addition to being able to generate a `ModuleID` and address, a `ModuleKey` contains a special function closure called
the `Invoker` which is the key to safe inter-module access. The `InvokeFn` closure corresponds to the `Invoke` method in
the `grpc.ClientConn` interface and under the hood is able to route messages to the appropriate `Msg` and `Query` handlers
performing appropriate security checks on `Msg`s. This allows for even safer inter-module access than keeper's whose
private member variables could be manipulated through reflection. Golang does not support reflection on a function
closure's captured variables and direct manipulation of memory would be needed for a truly malicious module to bypass
the `ModuleKey` security.
The two `ModuleKey` types are `RootModuleKey` and `DerivedModuleKey`:
```go
func Invoker(callInfo CallInfo) func(ctx context.Context, request, response interface{}, opts ...interface{}) error
type CallInfo {
Method string
Caller ModuleID
}
type RootModuleKey struct {
moduleName string
invoker Invoker
}
type DerivedModuleKey struct {
moduleName string
path []byte
invoker Invoker
}
```
A module can get access to a `DerivedModuleKey`, using the `Derive(path []byte)` method on `RootModuleKey` and then
would use this key to authenticate `Msg`s from a sub-account. Ex:
```go
package foo
func (fooMsgServer *MsgServer) Bar(ctx context.Context, req *MsgBar) (*MsgBarResponse, error) {
derivedKey := fooMsgServer.moduleKey.Derive(req.SomePath)
bankMsgClient := bank.NewMsgClient(derivedKey)
res, err := bankMsgClient.Balance(ctx, &bank.MsgSend{FromAddress: derivedKey.Address(), ...})
...
}
```
In this way, a module can gain permissioned access to a root account and any number of sub-accounts and send
authenticated `Msg`s from these accounts. The `Invoker` `callInfo.Caller` parameter is used under the hood to
distinguish between different module accounts, but either way the function returned by `Invoker` only allows `Msg`s
from either the root or a derived module account to pass through.
Note that `Invoker` itself returns a function closure based on the `CallInfo` passed in. This will allow client implementations
in the future that cache the invoke function for each method type avoiding the overhead of hash table lookup.
This would reduce the performance overhead of this inter-module communication method to the bare minimum required for
checking permissions.
To re-iterate, the closure only allows access to authorized calls. There is no access to anything else regardless of any
name impersonation.
Below is a rough sketch of the implementation of `grpc.ClientConn.Invoke` for `RootModuleKey`:
```go
func (key RootModuleKey) Invoke(ctx context.Context, method string, args, reply interface{}, opts ...grpc.CallOption) error {
f := key.invoker(CallInfo {Method: method, Caller: ModuleID {ModuleName: key.moduleName}})
return f(ctx, args, reply)
}
```
### `AppModule` Wiring and Requirements
In [ADR 031](./adr-031-msg-service.md), the `AppModule.RegisterService(Configurator)` method was introduced. To support
inter-module communication, we extend the `Configurator` interface to pass in the `ModuleKey` and to allow modules to
specify their dependencies on other modules using `RequireServer()`:
```go
type Configurator interface {
MsgServer() grpc.Server
QueryServer() grpc.Server
ModuleKey() ModuleKey
RequireServer(serverInterface interface{})
}
```
The `ModuleKey` is passed to modules in the `RegisterService` method itself so that `RegisterServices` serves as a single
entry point for configuring module services. This is intended to also have the side-effect of greatly reducing boilerplate in
`app.go`. For now, `ModuleKey`s will be created based on `AppModuleBasic.Name()`, but a more flexible system may be
introduced in the future. The `ModuleManager` will handle creation of module accounts behind the scenes.
Because modules do not get direct access to each other anymore, modules may have unfulfilled dependencies. To make sure
that module dependencies are resolved at startup, the `Configurator.RequireServer` method should be added. The `ModuleManager`
will make sure that all dependencies declared with `RequireServer` can be resolved before the app starts. An example
module `foo` could declare it's dependency on `x/bank` like this:
```go
package foo
func (am AppModule) RegisterServices(cfg Configurator) {
cfg.RequireServer((*bank.QueryServer)(nil))
cfg.RequireServer((*bank.MsgServer)(nil))
}
```
### Security Considerations
In addition to checking for `ModuleKey` permissions, a few additional security precautions will need to be taken by
the underlying router infrastructure.
#### Recursion and Re-entry
Recursive or re-entrant method invocations pose a potential security threat. This can be a problem if Module A
calls Module B and Module B calls module A again in the same call.
One basic way for the router system to deal with this is to maintain a call stack which prevents a module from
being referenced more than once in the call stack so that there is no re-entry. A `map[string]interface{}` table
in the router could be used to perform this security check.
#### Queries
Queries in Cosmos SDK are generally un-permissioned so allowing one module to query another module should not pose
any major security threats assuming basic precautions are taken. The basic precaution that the router system will
need to take is making sure that the `sdk.Context` passed to query methods does not allow writing to the store. This
can be done for now with a `CacheMultiStore` as is currently done for `BaseApp` queries.
### Internal Methods
In many cases, we may wish for modules to call methods on other modules which are not exposed to clients at all. For this
purpose, we add the `InternalServer` method to `Configurator`:
```go
type Configurator interface {
MsgServer() grpc.Server
QueryServer() grpc.Server
InternalServer() grpc.Server
}
```
As an example, x/slashing's Slash must call x/staking's Slash, but we don't want to expose x/staking's Slash to end users
and clients.
This wound also require creating a corresponding `internal.proto` file with a protobuf service in the given module's
proto package.
Services registered against `InternalServer` will be callable from other modules but not by external clients.
An alternative solution to internal-only methods could involve hooks / plugins as discussed [here](https://github.com/cosmos/cosmos-sdk/pull/7459#issuecomment-733807753).
A more detailed evaluation of a hooks / plugin system will be addressed later in follow-ups to this ADR or as a separate
ADR.
### Authorization
By default, the inter-module router requires that messages are sent by the first signer returned by `GetSigners`. The
inter-module router should also accept authorization middleware such as that provided by [ADR 030](https://github.com/cosmos/cosmos-sdk/pull/7105).
This middleware will allow accounts to otherwise specific module accounts to perform actions on their behalf.
Authorization middleware should take into account the need to grant certain modules effectively "admin" privileges to
other modules. This will be addressed in separate ADRs or updates to this ADR.
### Future Work
Other future improvements may include:
* custom code generation that:
* simplifies interfaces (ex. generates code with `sdk.Context` instead of `context.Context`)
* optimizes inter-module calls - for instance caching resolved methods after first invocation
* combining `StoreKey`s and `ModuleKey`s into a single interface so that modules have a single Ocaps handle
* code generation which makes inter-module communication more performant
* decoupling `ModuleKey` creation from `AppModuleBasic.Name()` so that app's can override root module account names
* inter-module hooks and plugins
## Alternatives
### MsgServices vs `x/capability`
The `x/capability` module does provide a proper object-capability implementation that can be used by any module in the
SDK and could even be used for inter-module Ocaps as described in [\#5931](https://github.com/cosmos/cosmos-sdk/issues/5931).
The advantages of the approach described in this ADR are mostly around how it integrates with other parts of the SDK,
specifically:
* protobuf so that:
* code generation of interfaces can be leveraged for a better dev UX
* module interfaces are versioned and checked for breakage using [buf](https://docs.buf.build/breaking-overview)
* sub-module accounts as per ADR 028
* the general `Msg` passing paradigm and the way signers are specified by `GetSigners`
Also, this is a complete replacement for keepers and could be applied to _all_ inter-module communication whereas the
`x/capability` approach in #5931 would need to be applied method by method.
## Consequences
### Backwards Compatibility
This ADR is intended to provide a pathway to a scenario where there is greater long term compatibility between modules.
In the short-term, this will likely result in breaking certain `Keeper` interfaces which are too permissive and/or
replacing `Keeper` interfaces altogether.
### Positive
- an alternative to keepers which can more easily lead to stable inter-module interfaces
- proper inter-module Ocaps
- improved module developer DevX, as commented on by several particpants on
[Architecture Review Call, Dec 3](https://hackmd.io/E0wxxOvRQ5qVmTf6N_k84Q)
- lays the groundwork for what can be a greatly simplified `app.go`
- router can be setup to enforce atomic transactions for moule-to-module calls
### Negative
- modules which adopt this will need significant refactoring
### Neutral
## Test Cases [optional]
## References
- [ADR 021](./adr-021-protobuf-query-encoding.md)
- [ADR 031](./adr-031-msg-service.md)
- [ADR 028](./adr-028-public-key-addresses.md)
- [ADR 030 draft](https://github.com/cosmos/cosmos-sdk/pull/7105)
- [Object-Capability Model](../docs/core/ocap.md)