From f02c28a72e35f0d4175ac358d522f154073791ff Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Thu, 21 Jul 2022 14:07:41 +0200 Subject: [PATCH] docs: add ADR 057 App Wiring Part I (#11873) ## Description Closes: #XXXX --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- docs/architecture/README.md | 1 + docs/architecture/adr-057-app-wiring-1.md | 285 ++++++++++++++++++++++ 2 files changed, 286 insertions(+) create mode 100644 docs/architecture/adr-057-app-wiring-1.md diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 4257d1e87ba8..6cc9f3dea34d 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -79,6 +79,7 @@ When writing ADRs, follow the same best practices for writing RFCs. When writing * [ADR 039: Epoched Staking](./adr-039-epoched-staking.md) * [ADR 040: Storage and SMT State Commitments](./adr-040-storage-and-smt-state-commitments.md) * [ADR 046: Module Params](./adr-046-module-params.md) +* [ADR 057: App Wiring Part I](./adr-057-app-wiring-1.md) ### Draft diff --git a/docs/architecture/adr-057-app-wiring-1.md b/docs/architecture/adr-057-app-wiring-1.md new file mode 100644 index 000000000000..5287613d0045 --- /dev/null +++ b/docs/architecture/adr-057-app-wiring-1.md @@ -0,0 +1,285 @@ +# ADR 057: App Wiring Part I + +## Changelog + +* 2022-05-04: Initial Draft + +## Status + +PROPOSED Partially Implemented + +## Abstract + +In order to make it easier to build Cosmos SDK modules and apps, we propose a new app wiring system based on +dependency injection and declarative app configurations to replace the current `app.go` code. + +## Context + +A number of factors have made the SDK and SDK apps in their current state hard to maintain. A symptom of the current +state of complexity is [`simapp/app.go`](https://github.com/cosmos/cosmos-sdk/blob/c3edbb22cab8678c35e21fe0253919996b780c01/simapp/app.go) +which contains almost 100 lines of imports and is otherwise over 600 lines of mostly boilerplate code that is +generally copied to each new project. (Not to mention the additional boilerplate which gets copied in `simapp/simd`.) + +The large amount of boilerplate needed to bootstrap an app has made it hard to release independently versioned go +modules for Cosmos SDK modules as described in [ADR 053: Go Module Refactoring](./adr-053-go-module-refactoring.md). + +In addition to being very verbose and repetitive, `app.go` also exposes a large surface area for breaking changes +as most modules instantiate themselves with positional parameters which forces breaking changes anytime a new parameter +(even an optional one) is needed. + +Several attempts were made to improve the current situation including [ADR 033: Internal-Module Communication](./adr-033-protobuf-inter-module-comm.md) +and [a proof-of-concept of a new SDK](https://github.com/allinbits/cosmos-sdk-poc). The discussions around these +designs led to the current solution described here. + +## Decision + +In order to improve the current situation, a new "app wiring" paradigm has been designed to replace `app.go` which +involves: + +* declaration configuration of the modules in an app which can be serialized to JSON or YAML +* a dependency-injection (DI) framework for instantiating apps from the that configuration + +### Dependency Injection + +When examining the code in `app.go` most of the code simply instantiates modules with dependencies provided either +by the framework (such as store keys) or by other modules (such as keepers). It is generally pretty obvious given +the context what the correct dependencies actually should be, so dependency-injection is an obvious solution. Rather +than making developers manually resolve dependencies, a module will tell the DI container what dependency it needs +and the container will figure out how to provide it. + +We explored several existing DI solutions in golang and felt that the reflection-based approach in [uber/dig](https://github.com/uber-go/dig) +was closest to what we needed but not quite there. Assessing what we needed for the SDK, we designed and built +the Cosmos SDK [depinject module](https://pkg.go.dev/github.com/cosmos/cosmos-sdk/depinject), which has the following +features: + +* dependency resolution and provision through functional constructors, ex: `func(need SomeDep) (AnotherDep, error)` +* dependency injection `In` and `Out` structs which support `optional` dependencies +* grouped-dependencies (many-per-container) through the `AutoGroupType` tag interface +* module-scoped dependencies via `ModuleKey`s (where each module gets a unique dependency) +* one-per-module dependencies through the `OnePerModuleType` tag interface +* sophisticated debugging information and container visualization via GraphViz + +Here are some examples of how these would be used in an SDK module: + +* `StoreKey` could be a module-scoped dependency which is unique per module +* a module's `AppModule` instance (or the equivalent) could be a `OnePerModuleType` +* CLI commands could be provided with `AutoGroupType`s + +Note that even though dependency resolution is dynamic and based on reflection, which could be considered a pitfall +of this approach, the entire dependency graph should be resolved immediately on app startup and only gets resolved +once (except in the case of dynamic config reloading which is a separate topic). This means that if there are any +errors in the dependency graph, they will get reported immediately on startup so this approach is only slightly worse +than fully static resolution in terms of error reporting and much better in terms of code complexity. + +### Declarative App Config + +In order to compose modules into an app, a declarative app configuration will be used. This configuration is based off +of protobuf and its basic structure is very simple: + +```protobuf +package cosmos.app.v1; + +message Config { + repeated ModuleConfig modules = 1; +} + +message ModuleConfig { + string name = 1; + google.protobuf.Any config = 2; +} +``` + +(See also https://github.com/cosmos/cosmos-sdk/blob/6e18f582bf69e3926a1e22a6de3c35ea327aadce/proto/cosmos/app/v1alpha1/config.proto) + +The configuration for every module is itself a protobuf message and modules will be identified and loaded based +on the protobuf type URL of their config object (ex. `cosmos.bank.module.v1.Module`). Modules are given a unique short `name` +to share resources across different versions of the same module which might have a different protobuf package +versions (ex. `cosmos.bank.module.v2.Module`). + +An example app config in YAML might look like this: + +```yaml +modules: + - name: baseapp + config: + "@type": cosmos.baseapp.module.v1.Module + begin_blockers: [staking, auth, bank] + end_blockers: [bank, auth, staking] + init_genesis: [bank, auth, staking] + - name: auth + config: + "@type": cosmos.auth.module.v1.Module + bech32_prefix: "foo" + - name: bank + config: + "@type": cosmos.bank.module.v1.Module + - name: staking + config: + "@type": cosmos.staking.module.v1.Module +``` + +In the above example, there is a hypothetical `baseapp` module which contains the information around ordering of +begin blockers, end blockers, and init genesis. Rather than lifting these concerns up to the module config layer, +they are themselves handled by a module which could allow a convenient way of swapping out different versions of +baseapp (for instance to target different versions of tendermint), without needing to change the rest of the config. +The `baseapp` module would then provide to the server framework (which sort of sits outside the ABCI app) an instance +of `abci.Application`. + +In this model, an app is *modules all the way down* and the dependency injection/app config layer is very much +protocol-agnostic and can adapt to even major breaking changes at the protocol layer. + +### Module & Protobuf Registration + +In order for the two components of dependency injection and declarative configuration to work together as described, +we need a way for modules to actually register themselves and provide dependencies to the container. + +One additional complexity that needs to be handled at this layer is protobuf registry initialization. Recall that +in both the current SDK `codec` and the proposed [ADR 054: Protobuf Semver Compatible Codegen](https://github.com/cosmos/cosmos-sdk/pull/11802), +protobuf types need to be explicitly registered. Given that the app config itself is based on protobuf and +uses protobuf `Any` types, protobuf registration needs to happen before the app config itself can be decoded. Because +we don't know which protobuf `Any` types will be needed a priori and modules themselves define those types, we need +to decode the app config in separate phases: + +1. parse app config JSON/YAML as raw JSON and collect required module type URLs (without doing proto JSON decoding) +2. build a [protobuf type registry](https://pkg.go.dev/google.golang.org/protobuf@v1.28.0/reflect/protoregistry) based + on file descriptors and types provided by each required module +3. decode the app config as proto JSON using the protobuf type registry + +Because in [ADR 054: Protobuf Semver Compatible Codegen](https://github.com/cosmos/cosmos-sdk/pull/11802), each module +should use `internal` generated code which is not registered with the global protobuf registry, this code should provide +an alternate way to register protobuf types with a type registry. In the same way that `.pb.go` files currently have a +`var File_foo_proto protoreflect.FileDescriptor` for the file `foo.proto`, generated code should have a new member +`var Types_foo_proto TypeInfo` where `TypeInfo` is an interface or struct with all the necessary info to register both +the protobuf generated types and file descriptor. + +So a module must provide dependency injection providers and protobuf types, and takes as input its module +config object which uniquely identifies the module based on its type URL. + +With this in mind, we define a global module register which allows module implementations to register themselves +with the following API: + +```go +// Register registers a module with the provided type name (ex. cosmos.bank.module.v1.Module) +// and the provided options. +func Register(configTypeName protoreflect.FullName, option ...Option) { ... } + +type Option { /* private methods */ } + +// Provide registers dependency injection provider functions which work with the +// cosmos-sdk container module. These functions can also accept an additional +// parameter for the module's config object. +func Provide(providers ...interface{}) Option { ... } + +// Types registers protobuf TypeInfo's with the protobuf registry. +func Types(types ...TypeInfo) Option { ... } +``` + +Ex: + +```go +func init() { + module.Register("cosmos.bank.module.v1.Module", + module.Types( + types.Types_tx_proto, + types.Types_query_proto, + types.Types_types_proto, + ), + module.Provide( + provideBankModule, + ) + ) +} + +type inputs struct { + container.In + + AuthKeeper auth.Keeper + DB ormdb.ModuleDB +} + +type outputs struct { + Keeper bank.Keeper + Handler app.Handler // app.Handler is a hypothetical type which replaces the current AppModule +} + +func provideBankModule(config types.Module, inputs) (outputs, error) { ... } +``` + +Note that in this module, a module configuration object *cannot* register different dependency providers based on the +configuration. This is intentional because it allows us to know globally which modules provide which dependencies. This +can help us figure out issues with missing dependencies in an app config if the needed modules are loaded at runtime. +In cases where required modules are not loaded at runtime, it may be possible to guide users to the correct module if +through a global Cosmos SDK module registry. + +### New `app.go` + +With this setup, `app.go` might now look something like this: + +```go +package main + +import ( + // Each go package which registers a module must be imported just for side-effects + // so that module implementations are registered. + _ "github.com/cosmos/cosmos-sdk/x/auth/module" + _ "github.com/cosmos/cosmos-sdk/x/bank/module" + _ "github.com/cosmos/cosmos-sdk/x/staking/module" + "github.com/cosmos/cosmos-sdk/core/app" +) + +// go:embed app.yaml +var appConfigYAML []byte + +func main() { + app.Run(app.LoadYAML(appConfigYAML)) +} +``` + +### Application to existing SDK modules + +So far we have described a system which is largely agnostic to the specifics of the SDK such as store keys, `AppModule`, +`BaseApp`, etc. A second app wiring ADR will be created which outlines the details of how this app wiring system will +be applied to the existing SDK in a way that: + +1. is as easy to apply to existing modules as possible, +2. while also making it possible to improve existing APIs and minimize long-term technical debt + +## Consequences + +### Backwards Compatibility + +Modules which work with the new app wiring system do not need to drop their existing `AppModule` and `NewKeeper` +registration paradigms. These two methods can live side-by-side for as long as is needed. + +### Positive + +* wiring up new apps will be simpler, more succinct and less error-prone +* it will be easier to develop and test standalone SDK modules without needing to replicate all of simapp +* it may be possible to dynamically load modules and upgrade chains without needing to do a coordinated stop and binary + upgrade using this mechanism +* easier plugin integration +* easier way to manage app construction for tools like Ignite CLI +* dependency injection framework provides more automated reasoning about dependencies in the project, with a graph visualization. + +### Negative + +* it may be confusing when a dependency is missing although error messages, the GraphViz visualization, and global + module registration may help with that + +### Neutral + +* it will require work and education + +## Further Discussions + +As mentioned above, a second app wiring ADR will be created to describe more specifics than there is space to go +into here. Further discussions will also happen within the Cosmos SDK Framework Working Group and in https://github.com/cosmos/cosmos-sdk/discussions/10582. + +## References + +* https://github.com/cosmos/cosmos-sdk/blob/c3edbb22cab8678c35e21fe0253919996b780c01/simapp/app.go +* https://github.com/allinbits/cosmos-sdk-poc +* https://github.com/uber-go/dig +* https://github.com/google/wire +* https://pkg.go.dev/github.com/cosmos/cosmos-sdk/container +* https://github.com/cosmos/cosmos-sdk/pull/11802