-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat(core): add migrations registering #19370
Changes from 10 commits
daf87aa
409a79b
72b2176
a4e7719
53dc459
3f40376
6153444
d973b87
ea80fb4
ac089cd
2e0b1d2
394a1e7
b001af0
e3b7e1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -74,10 +74,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i | |||||||||||||||||||
* (crypto | x/auth) [#14372](https://github.com/cosmos/cosmos-sdk/pull/18194) Key checks on signatures antehandle. | ||||||||||||||||||||
* (types) [#18963](https://github.com/cosmos/cosmos-sdk/pull/18963) Swap out amino json encoding of `ABCIMessageLogs` for std lib json encoding | ||||||||||||||||||||
|
||||||||||||||||||||
### Deprecated | ||||||||||||||||||||
|
||||||||||||||||||||
* (simapp) [#19146](https://github.com/cosmos/cosmos-sdk/pull/19146) Replace `--v` CLI option with `--validator-count`/`-n`. | ||||||||||||||||||||
|
||||||||||||||||||||
### Bug Fixes | ||||||||||||||||||||
|
||||||||||||||||||||
* (baseapp) [#19338](https://github.com/cosmos/cosmos-sdk/pull/19338) Set HeaderInfo in context when calling `setState`. | ||||||||||||||||||||
|
@@ -144,6 +140,11 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i | |||||||||||||||||||
|
||||||||||||||||||||
* (server) [#18303](https://github.com/cosmos/cosmos-sdk/pull/18303) `appd export` has moved with other genesis commands, use `appd genesis export` instead. | ||||||||||||||||||||
|
||||||||||||||||||||
### Deprecated | ||||||||||||||||||||
|
||||||||||||||||||||
* (simapp) [#19146](https://github.com/cosmos/cosmos-sdk/pull/19146) Replace `--v` CLI option with `--validator-count`/`-n`. | ||||||||||||||||||||
* (module) [#19370](https://github.com/cosmos/cosmos-sdk/pull/19370) Depcreate ` module.Configurator`, use `appmodule.HasMigrations` and `appmodule.HasServicecs` instead from Core API. | ||||||||||||||||||||
|
||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo in "Depcreate". It should be "Deprecate". - Depcreate ` module.Configurator`, use `appmodule.HasMigrations` and `appmodule.HasServicecs` instead from Core API.
+ Deprecate `module.Configurator`, use `appmodule.HasMigrations` and `appmodule.HasServices` instead from Core API. Committable suggestion
Suggested change
|
||||||||||||||||||||
## [v0.50.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.3) - 2023-01-15 | ||||||||||||||||||||
|
||||||||||||||||||||
### Features | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,7 +5,45 @@ Note, always read the **SimApp** section for more information on application wir | |||||||||||||||||||||
|
||||||||||||||||||||||
## [Unreleased] | ||||||||||||||||||||||
|
||||||||||||||||||||||
### Unordered Transactions | ||||||||||||||||||||||
### SimApp | ||||||||||||||||||||||
|
||||||||||||||||||||||
In this section we describe the changes made in Cosmos SDK' SimApp. | ||||||||||||||||||||||
**These changes are directly applicable to your application wiring.** | ||||||||||||||||||||||
|
||||||||||||||||||||||
#### AnteHandlers | ||||||||||||||||||||||
|
||||||||||||||||||||||
The GasConsumptionDecorator and IncreaseSequenceDecorator have been merged with the SigVerificationDecorator, so you'll | ||||||||||||||||||||||
need to remove them both from your app.go code, they will yield to unresolvable symbols when compiling. | ||||||||||||||||||||||
|
||||||||||||||||||||||
#### Client (`root.go`) | ||||||||||||||||||||||
|
||||||||||||||||||||||
The `client` package has been refactored to make use of the address codecs (address, validator address, consensus address, etc.). | ||||||||||||||||||||||
This is part of the work of abstracting the SDK from the global bech32 config. | ||||||||||||||||||||||
|
||||||||||||||||||||||
This means the address codecs must be provided in the `client.Context` in the application client (usually `root.go`). | ||||||||||||||||||||||
|
||||||||||||||||||||||
```diff | ||||||||||||||||||||||
clientCtx = clientCtx. | ||||||||||||||||||||||
+ WithAddressCodec(addressCodec). | ||||||||||||||||||||||
+ WithValidatorAddressCodec(validatorAddressCodec). | ||||||||||||||||||||||
+ WithConsensusAddressCodec(consensusAddressCodec) | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
Comment on lines
+25
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code snippet provided for updating the + Ensure this snippet is accompanied by a clear explanation of where to implement these changes in the application code. Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
**When using `depinject` / `app v2`, the client codecs can be provided directly from application config.** | ||||||||||||||||||||||
|
||||||||||||||||||||||
Refer to SimApp `root_v2.go` and `root.go` for an example with an app v2 and a legacy app. | ||||||||||||||||||||||
|
||||||||||||||||||||||
### Core | ||||||||||||||||||||||
|
||||||||||||||||||||||
`appmodule.Environment` interface was introduced to fetch different services from the application. This can be used as an alternative to using `sdk.UnwrapContext(ctx)` to fetch the services. It needs to be passed into a module at instantiation. | ||||||||||||||||||||||
|
||||||||||||||||||||||
Circuit Breaker is used as an example. | ||||||||||||||||||||||
|
||||||||||||||||||||||
```go | ||||||||||||||||||||||
app.CircuitKeeper = circuitkeeper.NewKeeper(runtime.NewEnvironment((keys[circuittypes.StoreKey]), nil), appCodec, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AuthKeeper.AddressCodec()) | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The example provided for using the + Consider adding additional examples or a more detailed guide on implementing the `appmodule.Environment` interface across various modules. Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
#### Unordered Transactions | ||||||||||||||||||||||
|
||||||||||||||||||||||
The Cosmos SDK now supports unordered transactions. This means that transactions | ||||||||||||||||||||||
can be executed in any order and doesn't require the client to deal with or manage | ||||||||||||||||||||||
|
@@ -80,57 +118,19 @@ used as a TTL for the transaction and is used to provide replay protection. See | |||||||||||||||||||||
[ADR-070](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-070-unordered-transactions.md) | ||||||||||||||||||||||
for more details. | ||||||||||||||||||||||
|
||||||||||||||||||||||
### Params | ||||||||||||||||||||||
|
||||||||||||||||||||||
* Params migrations were removed. It is required to migrate to 0.50 prior to upgrading to v0.51. | ||||||||||||||||||||||
|
||||||||||||||||||||||
### SimApp | ||||||||||||||||||||||
|
||||||||||||||||||||||
In this section we describe the changes made in Cosmos SDK' SimApp. | ||||||||||||||||||||||
**These changes are directly applicable to your application wiring.** | ||||||||||||||||||||||
|
||||||||||||||||||||||
#### AnteHandlers | ||||||||||||||||||||||
|
||||||||||||||||||||||
The GasConsumptionDecorator and IncreaseSequenceDecorator have been merged with the SigVerificationDecorator, so you'll | ||||||||||||||||||||||
need to remove them both from your app.go code, they will yield to unresolvable symbols when compiling. | ||||||||||||||||||||||
|
||||||||||||||||||||||
#### Client (`root.go`) | ||||||||||||||||||||||
|
||||||||||||||||||||||
The `client` package has been refactored to make use of the address codecs (address, validator address, consensus address, etc.). | ||||||||||||||||||||||
This is part of the work of abstracting the SDK from the global bech32 config. | ||||||||||||||||||||||
|
||||||||||||||||||||||
This means the address codecs must be provided in the `client.Context` in the application client (usually `root.go`). | ||||||||||||||||||||||
|
||||||||||||||||||||||
```diff | ||||||||||||||||||||||
clientCtx = clientCtx. | ||||||||||||||||||||||
+ WithAddressCodec(addressCodec). | ||||||||||||||||||||||
+ WithValidatorAddressCodec(validatorAddressCodec). | ||||||||||||||||||||||
+ WithConsensusAddressCodec(consensusAddressCodec) | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
|
||||||||||||||||||||||
**When using `depinject` / `app v2`, the client codecs can be provided directly from application config.** | ||||||||||||||||||||||
|
||||||||||||||||||||||
Refer to SimApp `root_v2.go` and `root.go` for an example with an app v2 and a legacy app. | ||||||||||||||||||||||
|
||||||||||||||||||||||
#### Dependency Injection | ||||||||||||||||||||||
|
||||||||||||||||||||||
<!-- explain app_config.go changes --> | ||||||||||||||||||||||
|
||||||||||||||||||||||
### Core | ||||||||||||||||||||||
### Modules | ||||||||||||||||||||||
|
||||||||||||||||||||||
`appmodule.Environment` interface was introduced to fetch different services from the application. This can be used as an alternative to using `sdk.UnwrapContext(ctx)` to fetch the services. It needs to be passed into a module at instantiation. | ||||||||||||||||||||||
#### `**all**` | ||||||||||||||||||||||
|
||||||||||||||||||||||
Circuit Breaker is used as an example. | ||||||||||||||||||||||
##### Params | ||||||||||||||||||||||
|
||||||||||||||||||||||
```go | ||||||||||||||||||||||
app.CircuitKeeper = circuitkeeper.NewKeeper(runtime.NewEnvironment((keys[circuittypes.StoreKey]), nil), appCodec, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AuthKeeper.AddressCodec()) | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
Old module migrations have been removed. It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any module migrations. | ||||||||||||||||||||||
|
||||||||||||||||||||||
### Modules | ||||||||||||||||||||||
##### Core API | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify the requirement for migrating to v0.50 before upgrading to v0.51 to ensure developers understand the importance of completing module migrations in the correct order. Providing additional resources or guidance on this process could be beneficial. + Add more detailed instructions or resources to assist developers with the migration process from v0.50 to v0.51. Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
#### `**all**` | ||||||||||||||||||||||
Core API has been introduces for modules in v0.47. With the deprecation of `sdk.Context`, we strongly recommend to use the `cosmossdk.io/core/appmodule` interfaces for the modules. This will allow the modules to work out of the box with server/v2 and baseapp, as well as limit their dependencies on the SDK. | ||||||||||||||||||||||
|
||||||||||||||||||||||
##### Dependency Injection | ||||||||||||||||||||||
##### Dependency Injection | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Highlight the significance of updating imports for dependency injection functions and ensure that developers are aware of the changes to + Include examples of updated imports for dependency injection functions to guide developers through the migration process. Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
Previously `cosmossdk.io/core` held functions `Invoke`, `Provide` and `Register` were moved to `cosmossdk.io/depinject/appconfig`. | ||||||||||||||||||||||
All modules using dependency injection must update their imports. | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -41,8 +41,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||||||
* [#18379](https://github.com/cosmos/cosmos-sdk/pull/18379) Add branch service. | ||||||||
* [#18457](https://github.com/cosmos/cosmos-sdk/pull/18457) Add branch.ExecuteWithGasLimit. | ||||||||
* [#19041](https://github.com/cosmos/cosmos-sdk/pull/19041) Add `appmodule.Environment` interface to fetch different services | ||||||||
* [#19370](https://github.com/cosmos/cosmos-sdk/pull/19370) Add `appmodule.Migrations` interface to handle migrations | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The entry for PR #19370 lacks a brief description of what the |
||||||||
|
||||||||
### API Breaking | ||||||||
### API Breaking Changes | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The URL in the entry for PR #18861 has a typo: - * [#18861](httpes://github.com/cosmos/cosmos-sdk/pull/18861) Moved `coin.ParseCoin` to `client/v2/internal`.
+ * [#18861](https://github.com/cosmos/cosmos-sdk/pull/18861) Moved `coin.ParseCoin` to `client/v2/internal`. Committable suggestion
Suggested change
The entry for PR #19041 under "API Breaking Changes" is duplicated. It's already mentioned under "Features". Consider removing the duplicate or clarifying if there are distinct aspects of the PR that belong to both categories. |
||||||||
|
||||||||
* [#18857](https://github.com/cosmos/cosmos-sdk/pull/18857) Moved `FormatCoins` to `x/tx`. | ||||||||
* [#18861](httpes://github.com/cosmos/cosmos-sdk/pull/18861) Moved `coin.ParseCoin` to `client/v2/internal`. | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,16 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
package appmodule | ||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing package documentation. + // Package appmodule provides interfaces and structures for managing application modules. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
import "context" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
type MigrationRegistrar interface { | ||||||||||||||||||||||||||||||||||||||||||||||||
// Register registers an in-place store migration for a module. The | ||||||||||||||||||||||||||||||||||||||||||||||||
// handler is a migration script to perform in-place migrations from version | ||||||||||||||||||||||||||||||||||||||||||||||||
// `fromVersion` to version `fromVersion+1`. | ||||||||||||||||||||||||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||||||||||||||||||||||||
// EACH TIME a module's ConsensusVersion increments, a new migration MUST | ||||||||||||||||||||||||||||||||||||||||||||||||
// be registered using this function. If a migration handler is missing for | ||||||||||||||||||||||||||||||||||||||||||||||||
// a particular function, the upgrade logic (see RunMigrations function) | ||||||||||||||||||||||||||||||||||||||||||||||||
// will panic. If the ConsensusVersion bump does not introduce any store | ||||||||||||||||||||||||||||||||||||||||||||||||
// changes, then a no-op function must be registered here. | ||||||||||||||||||||||||||||||||||||||||||||||||
Register(moduleName string, fromVersion uint64, handler func(context.Context) error) error | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+5
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The + // It is the caller's responsibility to ensure migrations are registered in a sequential manner without gaps. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package module | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/cosmos/gogoproto/grpc" | ||
|
@@ -20,6 +21,8 @@ import ( | |
// their services in the RegisterServices method. It is designed to eventually | ||
// support module object capabilities isolation as described in | ||
// https://github.com/cosmos/cosmos-sdk/issues/7093 | ||
// Deprecated: The Configurator is deprecated. | ||
// Preferably use core services for registering msg/query server and migrations. | ||
Comment on lines
+24
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deprecation of |
||
type Configurator interface { | ||
grpc.Server | ||
|
||
|
@@ -45,6 +48,10 @@ type Configurator interface { | |
// will panic. If the ConsensusVersion bump does not introduce any store | ||
// changes, then a no-op function must be registered here. | ||
RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error | ||
|
||
// Register registers an in-place store migration for a module. | ||
// It permits to register modules migrations that have migrated to serverv2 but still be compatible with baseapp. | ||
Register(moduleName string, fromVersion uint64, handler func(context.Context) error) error | ||
Comment on lines
+52
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
} | ||
|
||
type configurator struct { | ||
|
@@ -119,6 +126,14 @@ func (c *configurator) RegisterMigration(moduleName string, fromVersion uint64, | |
return nil | ||
} | ||
|
||
// Register implements the Configurator.Register method | ||
// It permis to register modules migrations that have migrated to serverv2 but still be compatible with baseapp. | ||
julienrbrt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (c *configurator) Register(moduleName string, fromVersion uint64, handler func(context.Context) error) error { | ||
return c.RegisterMigration(moduleName, fromVersion, func(sdkCtx sdk.Context) error { | ||
return handler(sdkCtx) | ||
}) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
// runModuleMigrations runs all in-place store migrations for one given module from a | ||
// version to another version. | ||
func (c *configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in "HasServicecs". It should be "HasServices".
Committable suggestion