From 10d72d71f1a07bc8d158faa8a692f5ed552690ad Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 1 Feb 2021 16:18:22 +0100 Subject: [PATCH 01/26] Add 1st version of migrate --- simapp/app.go | 26 ++++++++++++ types/module/module.go | 14 +++++++ x/auth/legacy/v040/store.go | 4 ++ x/auth/module.go | 8 ++++ x/auth/vesting/module.go | 8 ++++ x/authz/module.go | 8 ++++ x/bank/legacy/v040/store.go | 25 ++++++++++++ x/bank/legacy/v042/store.go | 59 +++++++++++++++++++++++++++ x/bank/legacy/v042/store_test.go | 7 ++++ x/bank/legacy/v042/types.go | 5 +++ x/bank/module.go | 25 ++++++++++++ x/capability/module.go | 8 ++++ x/crisis/module.go | 8 ++++ x/distribution/module.go | 8 ++++ x/evidence/module.go | 8 ++++ x/feegrant/module.go | 8 ++++ x/genutil/module.go | 8 ++++ x/gov/module.go | 8 ++++ x/ibc/applications/transfer/module.go | 8 ++++ x/ibc/core/module.go | 8 ++++ x/mint/module.go | 8 ++++ x/params/module.go | 8 ++++ x/slashing/module.go | 8 ++++ x/staking/module.go | 8 ++++ x/upgrade/module.go | 8 ++++ 25 files changed, 301 insertions(+) create mode 100644 x/auth/legacy/v040/store.go create mode 100644 x/bank/legacy/v040/store.go create mode 100644 x/bank/legacy/v042/store.go create mode 100644 x/bank/legacy/v042/store_test.go create mode 100644 x/bank/legacy/v042/types.go diff --git a/simapp/app.go b/simapp/app.go index 23ced043d047..9fc5b32dc959 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -598,6 +598,32 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry) } +// MigrateStore performs in-place store migrations. This function is not called +// automatically, it is meant to be called from an x/upgrade UpgradeHandler. +// `migrationsMap` is a map of moduleName to fromVersion (unit64), where +// fromVersion denotes the version from which we should migrate the module. +// +// Example: +// app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { +// err := app.MigrateStore(ctx, map[string]unint64{ +// "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion +// "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion +// }) +// if err != nil { +// panic(err) +// } +// }) +func (app *SimApp) MigrateStore(ctx sdk.Context, migrationsMap map[string]uint64) error { + for moduleName, module := range app.mm.Modules { + err := module.MigrateStore(ctx, app.keys[moduleName], migrationsMap[moduleName]) + if err != nil { + return err + } + } + + return nil +} + // RegisterSwaggerAPI registers swagger route with API Server func RegisterSwaggerAPI(ctx client.Context, rtr *mux.Router) { statikFS, err := fs.New() diff --git a/types/module/module.go b/types/module/module.go index 2379c93d5ebd..e02a85a66a9e 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -174,6 +174,12 @@ type AppModule interface { // RegisterServices allows a module to register services RegisterServices(Configurator) + // ConsensusVersion tracks state-breaking versions of the module + ConsensusVersion() uint64 + + // MigrateStore performs in-place store migrations. + MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error + // ABCI BeginBlock(sdk.Context, abci.RequestBeginBlock) EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate @@ -208,6 +214,14 @@ func (gam GenesisOnlyAppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Que // RegisterServices registers all services. func (gam GenesisOnlyAppModule) RegisterServices(Configurator) {} +// ConsensusVersion tracks state-breaking versions of the module. +func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (gam GenesisOnlyAppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns an empty module begin-block func (gam GenesisOnlyAppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} diff --git a/x/auth/legacy/v040/store.go b/x/auth/legacy/v040/store.go new file mode 100644 index 000000000000..9fb81b60a749 --- /dev/null +++ b/x/auth/legacy/v040/store.go @@ -0,0 +1,4 @@ +package v040 + +// AddrLen defines a valid address length +const AddrLen = 20 diff --git a/x/auth/module.go b/x/auth/module.go index 73aa9a1066a9..998af5809e97 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -147,6 +147,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the auth module. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index 3cc579a40e37..0e20cc4d800c 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -127,3 +127,11 @@ func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.Valid func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.RawMessage { return am.DefaultGenesis(cdc) } + +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} diff --git a/x/authz/module.go b/x/authz/module.go index 1bcd388e4df9..faf1c842604f 100644 --- a/x/authz/module.go +++ b/x/authz/module.go @@ -152,6 +152,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} // EndBlock does nothing diff --git a/x/bank/legacy/v040/store.go b/x/bank/legacy/v040/store.go new file mode 100644 index 000000000000..6ff84a34082c --- /dev/null +++ b/x/bank/legacy/v040/store.go @@ -0,0 +1,25 @@ +package v040 + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" +) + +// KVStore keys +var ( + BalancesPrefix = []byte("balances") +) + +// AddressFromBalancesStore returns an account address from a balances prefix +// store. The key must not contain the perfix BalancesPrefix as the prefix store +// iterator discards the actual prefix. +func AddressFromBalancesStore(key []byte) sdk.AccAddress { + addr := key[:v040auth.AddrLen] + if len(addr) != v040auth.AddrLen { + panic(fmt.Sprintf("unexpected account address key length; got: %d, expected: %d", len(addr), v040auth.AddrLen)) + } + + return sdk.AccAddress(addr) +} diff --git a/x/bank/legacy/v042/store.go b/x/bank/legacy/v042/store.go new file mode 100644 index 000000000000..42665801ea2e --- /dev/null +++ b/x/bank/legacy/v042/store.go @@ -0,0 +1,59 @@ +package v042 + +import ( + "github.com/cosmos/cosmos-sdk/store/prefix" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" + v040bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v040" +) + +// KVStore keys +var ( + // BalancesPrefix is the for the account balances store. We use a byte + // (instead of say `[]]byte("balances")` to save some disk space). + BalancesPrefix = []byte{0x02} +) + +// AddressFromBalancesStore returns an account address from a balances prefix +// store. The key must not contain the perfix BalancesPrefix as the prefix store +// iterator discards the actual prefix. +func AddressFromBalancesStore(key []byte) sdk.AccAddress { + addrLen := key[0] + addr := key[1 : addrLen+1] + + return sdk.AccAddress(addr) +} + +// CreateAccountBalancesPrefix creates the prefix for an account's balances. +func CreateAccountBalancesPrefix(addr []byte) []byte { + return append(BalancesPrefix, address.MustLengthPrefix(addr)...) +} + +// StoreMigration performs in-place store migrations from v0.40 to v0.42. The +// migration includes: +// +// - Change addresses to be length-prefixed. +// - Change balances prefix to 1 byte +func StoreMigration(store sdk.KVStore) error { + // old key is of format: + // prefix ("balances") || addrBytes (20 bytes) || denomBytes + // new key is of format + // prefix (0x02) || addrLen (1 byte) || addrBytes || denomBytes + oldStore := prefix.NewStore(store, v040bank.BalancesPrefix) + newStore := prefix.NewStore(store, BalancesPrefix) + + oldStoreIter := oldStore.Iterator(nil, nil) + defer oldStoreIter.Close() + + for ; oldStoreIter.Valid(); oldStoreIter.Next() { + addr := v040bank.AddressFromBalancesStore(oldStoreIter.Key()) + denom := oldStoreIter.Key()[1+v040auth.AddrLen:] + newStoreKey := append(CreateAccountBalancesPrefix(addr), denom...) + + newStore.Set(newStoreKey, oldStoreIter.Value()) // Values don't change. + oldStore.Delete(oldStoreIter.Key()) + } + + return nil +} diff --git a/x/bank/legacy/v042/store_test.go b/x/bank/legacy/v042/store_test.go new file mode 100644 index 000000000000..61f87e8b97e6 --- /dev/null +++ b/x/bank/legacy/v042/store_test.go @@ -0,0 +1,7 @@ +package v042_test + +import "testing" + +func TestStoreMigration(t *testing.T) { + // TODO +} diff --git a/x/bank/legacy/v042/types.go b/x/bank/legacy/v042/types.go new file mode 100644 index 000000000000..22534d31f878 --- /dev/null +++ b/x/bank/legacy/v042/types.go @@ -0,0 +1,5 @@ +package v042 + +const ( + ModuleName = "bank" +) diff --git a/x/bank/module.go b/x/bank/module.go index f271fa21975c..547023bcae39 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -22,6 +22,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank/client/cli" "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/x/bank/keeper" + v042bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" "github.com/cosmos/cosmos-sdk/x/bank/simulation" "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -151,6 +152,30 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 1 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + // Map of version n -> migrate function from version n to version n+1. + migrationsMap := map[uint64]func(store sdk.KVStore) error{ + 0: v042bank.StoreMigration, + } + + // Run in-place migrations sequentially until current ConsensusVersion. + for i := fromVersion; i < am.ConsensusVersion(); i++ { + migrateFn, found := migrationsMap[i] + if found { + err := migrateFn(ctx.KVStore(storeKey)) + if err != nil { + return err + } + } + } + + return nil +} + // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/capability/module.go b/x/capability/module.go index 7957f57747d6..94809d2cbd99 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -136,6 +136,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(genState) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock executes all ABCI BeginBlock logic respective to the capability module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/crisis/module.go b/x/crisis/module.go index 5d34c1ce28f9..dbe1e2c28da4 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -158,6 +158,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/distribution/module.go b/x/distribution/module.go index 034be6d9651d..962345df8900 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -161,6 +161,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the distribution module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/evidence/module.go b/x/evidence/module.go index 4367fe8d58ce..ba6b80933b18 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -175,6 +175,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, am.keeper)) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/feegrant/module.go b/x/feegrant/module.go index 5f4ba807d895..99fd5c6a12ab 100644 --- a/x/feegrant/module.go +++ b/x/feegrant/module.go @@ -167,6 +167,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the feegrant module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/genutil/module.go b/x/genutil/module.go index bfaeb0c59168..ce59f1f59b7a 100644 --- a/x/genutil/module.go +++ b/x/genutil/module.go @@ -110,3 +110,11 @@ func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONMarshaler, data j func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.RawMessage { return am.DefaultGenesis(cdc) } + +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} diff --git a/x/gov/module.go b/x/gov/module.go index ad2191660c08..36d9a1b29dbb 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -177,6 +177,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go index 67c736555b8a..5762cc765e55 100644 --- a/x/ibc/applications/transfer/module.go +++ b/x/ibc/applications/transfer/module.go @@ -145,6 +145,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { } diff --git a/x/ibc/core/module.go b/x/ibc/core/module.go index 3371dc88a446..67f1b276ab55 100644 --- a/x/ibc/core/module.go +++ b/x/ibc/core/module.go @@ -156,6 +156,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, *am.keeper)) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the ibc module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { ibcclient.BeginBlocker(ctx, am.keeper.ClientKeeper) diff --git a/x/mint/module.go b/x/mint/module.go index 44e96ce74bda..aa0ad4a794e7 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -146,6 +146,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the mint module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) diff --git a/x/params/module.go b/x/params/module.go index b0a4584129ef..3b1582a739b9 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -139,6 +139,14 @@ func (am AppModule) ExportGenesis(_ sdk.Context, _ codec.JSONMarshaler) json.Raw return nil } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/slashing/module.go b/x/slashing/module.go index 91ad472e90df..32ce7cffae97 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -159,6 +159,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the slashing module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/staking/module.go b/x/staking/module.go index f2e422117476..27ee2d4a0044 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -157,6 +157,14 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock returns the begin blocker for the staking module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 4e4982a324cf..03cdaed2ebaa 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -120,6 +120,14 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } +// ConsensusVersion tracks state-breaking versions of the module. +func (AppModule) ConsensusVersion() uint64 { return 0 } + +// MigrateStore performs in-place store migrations. +func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { + return nil +} + // BeginBlock calls the upgrade module hooks // // CONTRACT: this is registered in BeginBlocker *before* all other modules' BeginBlock functions From a9a406ee9a3074f33073b1855ccfe978e86a6ce0 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Tue, 2 Feb 2021 14:53:11 +0100 Subject: [PATCH 02/26] Put migration logic into Configurator --- simapp/app.go | 28 +-------------- types/module/configurator.go | 51 +++++++++++++++++++++++++-- types/module/module.go | 35 +++++++++++++----- x/auth/module.go | 5 --- x/auth/vesting/module.go | 5 --- x/authz/module.go | 5 --- x/bank/module.go | 24 ++----------- x/capability/module.go | 5 --- x/crisis/module.go | 5 --- x/distribution/module.go | 5 --- x/evidence/module.go | 5 --- x/feegrant/module.go | 5 --- x/genutil/module.go | 5 --- x/gov/module.go | 5 --- x/ibc/applications/transfer/module.go | 5 --- x/ibc/core/module.go | 5 --- x/mint/module.go | 5 --- x/params/module.go | 5 --- x/slashing/module.go | 5 --- x/staking/module.go | 5 --- x/upgrade/module.go | 5 --- 21 files changed, 78 insertions(+), 145 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 9fc5b32dc959..a56c67f32242 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -393,7 +393,7 @@ func NewSimApp( app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) - app.mm.RegisterServices(module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())) + app.mm.RegisterServices(module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys)) // add test gRPC service for testing gRPC queries in isolation testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{}) @@ -598,32 +598,6 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry) } -// MigrateStore performs in-place store migrations. This function is not called -// automatically, it is meant to be called from an x/upgrade UpgradeHandler. -// `migrationsMap` is a map of moduleName to fromVersion (unit64), where -// fromVersion denotes the version from which we should migrate the module. -// -// Example: -// app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { -// err := app.MigrateStore(ctx, map[string]unint64{ -// "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion -// "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion -// }) -// if err != nil { -// panic(err) -// } -// }) -func (app *SimApp) MigrateStore(ctx sdk.Context, migrationsMap map[string]uint64) error { - for moduleName, module := range app.mm.Modules { - err := module.MigrateStore(ctx, app.keys[moduleName], migrationsMap[moduleName]) - if err != nil { - return err - } - } - - return nil -} - // RegisterSwaggerAPI registers swagger route with API Server func RegisterSwaggerAPI(ctx client.Context, rtr *mux.Router) { statikFS, err := fs.New() diff --git a/types/module/configurator.go b/types/module/configurator.go index d561dd9eef0d..e40c064835ce 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -1,6 +1,10 @@ package module -import "github.com/gogo/protobuf/grpc" +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/gogo/protobuf/grpc" +) // Configurator provides the hooks to allow modules to configure and register // their services in the RegisterServices method. It is designed to eventually @@ -15,16 +19,30 @@ type Configurator interface { // QueryServer returns a grpc.Server instance which allows registering services // that will be exposed as gRPC services as well as ABCI query handlers. QueryServer() grpc.Server + + // RegisterMigration 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`. + RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) + + // RunMigration runs all in-place store migrations for a module from a + // given version to another version. + RunMigration(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error } type configurator struct { msgServer grpc.Server queryServer grpc.Server + + // storeKeys is used to access module stores inside in-place store migrations. + storeKeys map[string]*sdk.KVStoreKey + // migrations is a map of moduleName -> fromVersion -> migration script handler + migrations map[string]map[uint64]func(store sdk.KVStore) error } // NewConfigurator returns a new Configurator instance -func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server) Configurator { - return configurator{msgServer: msgServer, queryServer: queryServer} +func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server, storeKeys map[string]*sdk.KVStoreKey) Configurator { + return configurator{msgServer: msgServer, queryServer: queryServer, storeKeys: storeKeys} } var _ Configurator = configurator{} @@ -38,3 +56,30 @@ func (c configurator) MsgServer() grpc.Server { func (c configurator) QueryServer() grpc.Server { return c.queryServer } + +// RegisterMigration implements the Configurator.RegisterMigration method +func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) { + c.migrations[moduleName][fromVersion] = handler +} + +// RunMigration implements the Configurator.RunMigration method +func (c configurator) RunMigration(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { + storeKey, found := c.storeKeys[moduleName] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) + } + + // Run in-place migrations for the module sequentially until toVersion. + for i := fromVersion; i < toVersion; i++ { + migrateFn, found := c.migrations[moduleName][i] + // If no migrations has been registered for this module, we just skip. + if found { + err := migrateFn(ctx.KVStore(storeKey)) + if err != nil { + return err + } + } + } + + return nil +} diff --git a/types/module/module.go b/types/module/module.go index e02a85a66a9e..2216b8c221bd 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -177,9 +177,6 @@ type AppModule interface { // ConsensusVersion tracks state-breaking versions of the module ConsensusVersion() uint64 - // MigrateStore performs in-place store migrations. - MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error - // ABCI BeginBlock(sdk.Context, abci.RequestBeginBlock) EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate @@ -217,11 +214,6 @@ func (gam GenesisOnlyAppModule) RegisterServices(Configurator) {} // ConsensusVersion tracks state-breaking versions of the module. func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (gam GenesisOnlyAppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns an empty module begin-block func (gam GenesisOnlyAppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} @@ -342,6 +334,33 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st return genesisData } +// MigrateStore performs in-place store migrations. This function is not called +// automatically, it is meant to be called from an x/upgrade UpgradeHandler. +// `migrationsMap` is a map of moduleName to fromVersion (unit64), where +// fromVersion denotes the version from which we should migrate the module. +// +// Example: +// cfg := module.NewConfigurator(...) +// app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { +// err := app.mm.MigrateStore(ctx, cfg, map[string]unint64{ +// "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion +// "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion +// }) +// if err != nil { +// panic(err) +// } +// }) +func (m Manager) MigrateStore(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { + for moduleName, module := range m.Modules { + err := cfg.RunMigration(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + if err != nil { + return err + } + } + + return nil +} + // BeginBlock performs begin block functionality for all modules. It creates a // child context with an event manager to aggregate events emitted from all // modules. diff --git a/x/auth/module.go b/x/auth/module.go index 998af5809e97..9ed1138c9ff1 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -150,11 +150,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the auth module. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index 0e20cc4d800c..06a947721ba8 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -130,8 +130,3 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } - -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} diff --git a/x/authz/module.go b/x/authz/module.go index faf1c842604f..a8088656a7b7 100644 --- a/x/authz/module.go +++ b/x/authz/module.go @@ -155,11 +155,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} // EndBlock does nothing diff --git a/x/bank/module.go b/x/bank/module.go index 547023bcae39..0016a5d76438 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -22,7 +22,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank/client/cli" "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/x/bank/keeper" - v042bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" + v042 "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" "github.com/cosmos/cosmos-sdk/x/bank/simulation" "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -101,6 +101,7 @@ type AppModule struct { func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) + cfg.RegisterMigration(types.ModuleName, 0, v042.StoreMigration) } // NewAppModule creates a new AppModule object @@ -155,27 +156,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 1 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - // Map of version n -> migrate function from version n to version n+1. - migrationsMap := map[uint64]func(store sdk.KVStore) error{ - 0: v042bank.StoreMigration, - } - - // Run in-place migrations sequentially until current ConsensusVersion. - for i := fromVersion; i < am.ConsensusVersion(); i++ { - migrateFn, found := migrationsMap[i] - if found { - err := migrateFn(ctx.KVStore(storeKey)) - if err != nil { - return err - } - } - } - - return nil -} - // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/capability/module.go b/x/capability/module.go index 94809d2cbd99..8a1189400c7f 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -139,11 +139,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock executes all ABCI BeginBlock logic respective to the capability module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/crisis/module.go b/x/crisis/module.go index dbe1e2c28da4..4ef39afb4772 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -161,11 +161,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/distribution/module.go b/x/distribution/module.go index 962345df8900..1830d08c583a 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -164,11 +164,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the distribution module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/evidence/module.go b/x/evidence/module.go index ba6b80933b18..7de6a10c4604 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -178,11 +178,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/feegrant/module.go b/x/feegrant/module.go index 99fd5c6a12ab..b61dd4855436 100644 --- a/x/feegrant/module.go +++ b/x/feegrant/module.go @@ -170,11 +170,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the feegrant module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/genutil/module.go b/x/genutil/module.go index ce59f1f59b7a..09b2f8e5c1b2 100644 --- a/x/genutil/module.go +++ b/x/genutil/module.go @@ -113,8 +113,3 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } - -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} diff --git a/x/gov/module.go b/x/gov/module.go index 36d9a1b29dbb..0a8ce0c24a7f 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -180,11 +180,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go index 5762cc765e55..987b73470b4f 100644 --- a/x/ibc/applications/transfer/module.go +++ b/x/ibc/applications/transfer/module.go @@ -148,11 +148,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { } diff --git a/x/ibc/core/module.go b/x/ibc/core/module.go index 67f1b276ab55..53c39b508d27 100644 --- a/x/ibc/core/module.go +++ b/x/ibc/core/module.go @@ -159,11 +159,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the ibc module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { ibcclient.BeginBlocker(ctx, am.keeper.ClientKeeper) diff --git a/x/mint/module.go b/x/mint/module.go index aa0ad4a794e7..6f408e6763a7 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -149,11 +149,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the mint module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) diff --git a/x/params/module.go b/x/params/module.go index 3b1582a739b9..4a8f4ff5dcff 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -142,11 +142,6 @@ func (am AppModule) ExportGenesis(_ sdk.Context, _ codec.JSONMarshaler) json.Raw // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/slashing/module.go b/x/slashing/module.go index 32ce7cffae97..a1cac6ae864f 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -162,11 +162,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the slashing module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) diff --git a/x/staking/module.go b/x/staking/module.go index 27ee2d4a0044..5a1880055c3d 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -160,11 +160,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock returns the begin blocker for the staking module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 03cdaed2ebaa..268ba817979f 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -123,11 +123,6 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R // ConsensusVersion tracks state-breaking versions of the module. func (AppModule) ConsensusVersion() uint64 { return 0 } -// MigrateStore performs in-place store migrations. -func (am AppModule) MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, fromVersion uint64) error { - return nil -} - // BeginBlock calls the upgrade module hooks // // CONTRACT: this is registered in BeginBlocker *before* all other modules' BeginBlock functions From 9ed398766574f0fb0ce87d36c2cf40828e7ce134 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Tue, 2 Feb 2021 16:33:18 +0100 Subject: [PATCH 03/26] add test to bank store migration --- testutil/context.go | 25 ++++++++++++++++++++++++ types/context_test.go | 17 +++------------- types/module/configurator.go | 6 +++--- types/module/module.go | 8 ++++---- x/bank/legacy/v042/store.go | 6 +++--- x/bank/legacy/v042/store_test.go | 33 ++++++++++++++++++++++++++++++-- x/params/keeper/common_test.go | 24 +++-------------------- 7 files changed, 72 insertions(+), 47 deletions(-) create mode 100644 testutil/context.go diff --git a/testutil/context.go b/testutil/context.go new file mode 100644 index 000000000000..b7fd34b686cb --- /dev/null +++ b/testutil/context.go @@ -0,0 +1,25 @@ +package testutil + +import ( + "github.com/tendermint/tendermint/libs/log" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + dbm "github.com/tendermint/tm-db" + + "github.com/cosmos/cosmos-sdk/store" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// DefaultContext creates a sdk.Context that can be used in tests. +func DefaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context { + db := dbm.NewMemDB() + cms := store.NewCommitMultiStore(db) + cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db) + cms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db) + err := cms.LoadLatestVersion() + if err != nil { + panic(err) + } + ctx := sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger()) + + return ctx +} diff --git a/types/context_test.go b/types/context_test.go index 018bd6a25792..4af5d8390f2f 100644 --- a/types/context_test.go +++ b/types/context_test.go @@ -8,13 +8,11 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/libs/log" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - dbm "github.com/tendermint/tm-db" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/store" "github.com/cosmos/cosmos-sdk/tests/mocks" + "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/types" ) @@ -26,15 +24,6 @@ func TestContextTestSuite(t *testing.T) { suite.Run(t, new(contextTestSuite)) } -func (s *contextTestSuite) defaultContext(key types.StoreKey) types.Context { - db := dbm.NewMemDB() - cms := store.NewCommitMultiStore(db) - cms.MountStoreWithDB(key, types.StoreTypeIAVL, db) - s.Require().NoError(cms.LoadLatestVersion()) - ctx := types.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger()) - return ctx -} - func (s *contextTestSuite) TestCacheContext() { key := types.NewKVStoreKey(s.T().Name() + "_TestCacheContext") k1 := []byte("hello") @@ -42,7 +31,7 @@ func (s *contextTestSuite) TestCacheContext() { k2 := []byte("key") v2 := []byte("value") - ctx := s.defaultContext(key) + ctx := testutil.DefaultContext(key, types.NewTransientStoreKey("transient_"+s.T().Name())) store := ctx.KVStore(key) store.Set(k1, v1) s.Require().Equal(v1, store.Get(k1)) @@ -64,7 +53,7 @@ func (s *contextTestSuite) TestCacheContext() { func (s *contextTestSuite) TestLogContext() { key := types.NewKVStoreKey(s.T().Name()) - ctx := s.defaultContext(key) + ctx := testutil.DefaultContext(key, types.NewTransientStoreKey("transient_"+s.T().Name())) ctrl := gomock.NewController(s.T()) s.T().Cleanup(ctrl.Finish) diff --git a/types/module/configurator.go b/types/module/configurator.go index e40c064835ce..59f43cd8ae22 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -25,9 +25,9 @@ type Configurator interface { // `fromVersion` to version `fromVersion+1`. RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) - // RunMigration runs all in-place store migrations for a module from a + // RunMigrations runs all in-place store migrations for a module from a // given version to another version. - RunMigration(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error + RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error } type configurator struct { @@ -63,7 +63,7 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h } // RunMigration implements the Configurator.RunMigration method -func (c configurator) RunMigration(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { +func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { storeKey, found := c.storeKeys[moduleName] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) diff --git a/types/module/module.go b/types/module/module.go index 2216b8c221bd..6af76d80fdf7 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -334,7 +334,7 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st return genesisData } -// MigrateStore performs in-place store migrations. This function is not called +// RunMigrations performs in-place store migrations. This function is not called // automatically, it is meant to be called from an x/upgrade UpgradeHandler. // `migrationsMap` is a map of moduleName to fromVersion (unit64), where // fromVersion denotes the version from which we should migrate the module. @@ -342,7 +342,7 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st // Example: // cfg := module.NewConfigurator(...) // app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { -// err := app.mm.MigrateStore(ctx, cfg, map[string]unint64{ +// err := app.mm.RunMigrations(ctx, cfg, map[string]unint64{ // "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion // "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion // }) @@ -350,9 +350,9 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st // panic(err) // } // }) -func (m Manager) MigrateStore(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { +func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { for moduleName, module := range m.Modules { - err := cfg.RunMigration(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + err := cfg.RunMigrations(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) if err != nil { return err } diff --git a/x/bank/legacy/v042/store.go b/x/bank/legacy/v042/store.go index 42665801ea2e..a4e51f28d65e 100644 --- a/x/bank/legacy/v042/store.go +++ b/x/bank/legacy/v042/store.go @@ -41,17 +41,17 @@ func StoreMigration(store sdk.KVStore) error { // new key is of format // prefix (0x02) || addrLen (1 byte) || addrBytes || denomBytes oldStore := prefix.NewStore(store, v040bank.BalancesPrefix) - newStore := prefix.NewStore(store, BalancesPrefix) oldStoreIter := oldStore.Iterator(nil, nil) defer oldStoreIter.Close() for ; oldStoreIter.Valid(); oldStoreIter.Next() { addr := v040bank.AddressFromBalancesStore(oldStoreIter.Key()) - denom := oldStoreIter.Key()[1+v040auth.AddrLen:] + denom := oldStoreIter.Key()[v040auth.AddrLen:] newStoreKey := append(CreateAccountBalancesPrefix(addr), denom...) - newStore.Set(newStoreKey, oldStoreIter.Value()) // Values don't change. + // Set new key on store. Values don't change. + store.Set(newStoreKey, oldStoreIter.Value()) oldStore.Delete(oldStoreIter.Key()) } diff --git a/x/bank/legacy/v042/store_test.go b/x/bank/legacy/v042/store_test.go index 61f87e8b97e6..8690291b31a6 100644 --- a/x/bank/legacy/v042/store_test.go +++ b/x/bank/legacy/v042/store_test.go @@ -1,7 +1,36 @@ package v042_test -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/testutil" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + v040bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v040" + v042bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" +) func TestStoreMigration(t *testing.T) { - // TODO + bankKey := sdk.NewKVStoreKey("bank") + ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test")) + store := ctx.KVStore(bankKey) + + _, _, addr := testdata.KeyTestPubAddr() + denom := []byte("foo") + value := []byte("bar") + + oldKey := append(append(v040bank.BalancesPrefix, addr...), denom...) + store.Set(oldKey, value) + + err := v042bank.StoreMigration(store) + require.NoError(t, err) + + newKey := append(v042bank.CreateAccountBalancesPrefix(addr), denom...) + // -7 because we replaced "balances" with 0x02, + // +1 because we added length-prefix to address. + require.Equal(t, len(oldKey)-7+1, len(newKey)) + require.Nil(t, store.Get(oldKey)) + require.Equal(t, value, store.Get(newKey)) } diff --git a/x/params/keeper/common_test.go b/x/params/keeper/common_test.go index f6d567db11f7..3ba444173588 100644 --- a/x/params/keeper/common_test.go +++ b/x/params/keeper/common_test.go @@ -1,14 +1,9 @@ package keeper_test import ( - "github.com/tendermint/tendermint/libs/log" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - dbm "github.com/tendermint/tm-db" - - "github.com/cosmos/cosmos-sdk/simapp" - "github.com/cosmos/cosmos-sdk/codec" - "github.com/cosmos/cosmos-sdk/store" + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper" ) @@ -18,7 +13,7 @@ func testComponents() (*codec.LegacyAmino, sdk.Context, sdk.StoreKey, sdk.StoreK legacyAmino := createTestCodec() mkey := sdk.NewKVStoreKey("test") tkey := sdk.NewTransientStoreKey("transient_test") - ctx := defaultContext(mkey, tkey) + ctx := testutil.DefaultContext(mkey, tkey) keeper := paramskeeper.NewKeeper(marshaler, legacyAmino, mkey, tkey) return legacyAmino, ctx, mkey, tkey, keeper @@ -37,16 +32,3 @@ func createTestCodec() *codec.LegacyAmino { cdc.RegisterConcrete(invalid{}, "test/invalid", nil) return cdc } - -func defaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context { - db := dbm.NewMemDB() - cms := store.NewCommitMultiStore(db) - cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db) - cms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db) - err := cms.LoadLatestVersion() - if err != nil { - panic(err) - } - ctx := sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger()) - return ctx -} From a095d3acb11555544e9d4c8c1398c86bb3f819ec Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Tue, 2 Feb 2021 18:21:40 +0100 Subject: [PATCH 04/26] add test for configurator --- simapp/app.go | 29 +++++++++++- simapp/app_test.go | 71 +++++++++++++++++++++++++++++- tests/mocks/types_module_module.go | 3 ++ types/module/configurator.go | 34 ++++++++++++-- types/module/module.go | 17 +------ types/module/module_test.go | 2 +- 6 files changed, 133 insertions(+), 23 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index a56c67f32242..d2b59cf3581c 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -198,6 +198,9 @@ type SimApp struct { // simulation manager sm *module.SimulationManager + + // the configurator + configurator module.Configurator } func init() { @@ -393,7 +396,8 @@ func NewSimApp( app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) - app.mm.RegisterServices(module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys)) + app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + app.mm.RegisterServices(app.configurator) // add test gRPC service for testing gRPC queries in isolation testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{}) @@ -598,6 +602,29 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry) } +// RunMigrations performs in-place store migrations for all modules. This +// function is not called automatically, it is meant to be called from an +// x/upgrade UpgradeHandler. +// +// `migrationsMap` is a map of moduleName to fromVersion (unit64), where +// fromVersion denotes the version from which we should migrate the module, the +// target version being the module's latest ConsensusVersion. +// +// Example: +// cfg := module.NewConfigurator(...) +// app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { +// err := app.RunMigrations(ctx, map[string]unint64{ +// "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion +// "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion +// }) +// if err != nil { +// panic(err) +// } +// }) +func (app *SimApp) RunMigrations(ctx sdk.Context, migrationsMap map[string]uint64) error { + return app.mm.RunMigrations(ctx, app.configurator, migrationsMap) +} + // RegisterSwaggerAPI registers swagger route with API Server func RegisterSwaggerAPI(ctx client.Context, rtr *mux.Router) { statikFS, err := fs.New() diff --git a/simapp/app_test.go b/simapp/app_test.go index 6543f94fd437..bbd76ba6edcb 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -2,14 +2,18 @@ package simapp import ( "encoding/json" + "fmt" "os" "testing" "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" - abci "github.com/tendermint/tendermint/abci/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" ) func TestSimAppExportAndBlockedAddrs(t *testing.T) { @@ -45,3 +49,68 @@ func TestGetMaccPerms(t *testing.T) { dup := GetMaccPerms() require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions") } + +func TestRunMigrations(t *testing.T) { + db := dbm.NewMemDB() + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) + + // Create a new configurator for the purpose of this test. + app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + + testCases := []struct { + name string + moduleName string + expRegErr bool + expRegErrMsg string + expCalled int + }{ + { + "cannot register migration for non-existant module", + "foo", + true, "store key for module foo not found: not found", 0, + }, + { + "can register and run migration handler for x/bank", + "bank", + false, "", 1, + }, + { + "cannot register migration handler for same module & fromVersion", + "bank", + true, "another migration for module bank and version 0 already exists: internal logic error", 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + // Since it's very hard to test in-place store migrations in tests (due + // to the difficulty of maintaing multiple versions of a module), we're + // just testing here that the migration logic is called or not for our + // custom module. + called := 0 + + err := app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.KVStore) error { + fmt.Println("HELLO") + called++ + + return nil + }) + + if tc.expRegErr { + require.Error(t, err) + require.Equal(t, tc.expRegErrMsg, err.Error()) + + return + } + require.NoError(t, err) + + err = app.RunMigrations( + app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}), + map[string]uint64{"bank": 0}, + ) + require.NoError(t, err) + require.Equal(t, tc.expCalled, called) + }) + } +} diff --git a/tests/mocks/types_module_module.go b/tests/mocks/types_module_module.go index 41ded4d7a2c0..e465e115423b 100644 --- a/tests/mocks/types_module_module.go +++ b/tests/mocks/types_module_module.go @@ -492,6 +492,9 @@ func (m *MockAppModule) ExportGenesis(arg0 types0.Context, arg1 codec.JSONMarsha return ret0 } +// ConsensusVersion mocks base method +func (m *MockAppModule) ConsensusVersion() uint64 { return 0 } + // ExportGenesis indicates an expected call of ExportGenesis func (mr *MockAppModuleMockRecorder) ExportGenesis(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() diff --git a/types/module/configurator.go b/types/module/configurator.go index 59f43cd8ae22..eacf8ded7c2b 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -23,7 +23,7 @@ type Configurator interface { // RegisterMigration 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`. - RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) + RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error // RunMigrations runs all in-place store migrations for a module from a // given version to another version. @@ -42,7 +42,12 @@ type configurator struct { // NewConfigurator returns a new Configurator instance func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server, storeKeys map[string]*sdk.KVStoreKey) Configurator { - return configurator{msgServer: msgServer, queryServer: queryServer, storeKeys: storeKeys} + return configurator{ + msgServer: msgServer, + queryServer: queryServer, + storeKeys: storeKeys, + migrations: map[string]map[uint64]func(store sdk.KVStore) error{}, + } } var _ Configurator = configurator{} @@ -58,12 +63,33 @@ func (c configurator) QueryServer() grpc.Server { } // RegisterMigration implements the Configurator.RegisterMigration method -func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) { +func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error { + if c.migrations[moduleName][fromVersion] != nil { + return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, fromVersion) + } + + _, found := c.storeKeys[moduleName] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) + } + + if c.migrations[moduleName] == nil { + c.migrations[moduleName] = map[uint64]func(store sdk.KVStore) error{} + } + c.migrations[moduleName][fromVersion] = handler + + return nil } // RunMigration implements the Configurator.RunMigration method func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { + _, found := c.migrations[moduleName] + if !found { + // If no migrations has been registered for this module, we just skip. + return nil + } + storeKey, found := c.storeKeys[moduleName] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) @@ -72,7 +98,7 @@ func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVers // Run in-place migrations for the module sequentially until toVersion. for i := fromVersion; i < toVersion; i++ { migrateFn, found := c.migrations[moduleName][i] - // If no migrations has been registered for this module, we just skip. + // If no migrations has been registered for this version, we just skip. if found { err := migrateFn(ctx.KVStore(storeKey)) if err != nil { diff --git a/types/module/module.go b/types/module/module.go index 6af76d80fdf7..b3172728dbb8 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -334,22 +334,7 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st return genesisData } -// RunMigrations performs in-place store migrations. This function is not called -// automatically, it is meant to be called from an x/upgrade UpgradeHandler. -// `migrationsMap` is a map of moduleName to fromVersion (unit64), where -// fromVersion denotes the version from which we should migrate the module. -// -// Example: -// cfg := module.NewConfigurator(...) -// app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { -// err := app.mm.RunMigrations(ctx, cfg, map[string]unint64{ -// "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion -// "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion -// }) -// if err != nil { -// panic(err) -// } -// }) +// RunMigrations performs in-place store migrations for all modules. func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { for moduleName, module := range m.Modules { err := cfg.RunMigrations(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) diff --git a/types/module/module_test.go b/types/module/module_test.go index 630c57619245..fa11d7a49603 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -179,7 +179,7 @@ func TestManager_RegisterQueryServices(t *testing.T) { msgRouter := mocks.NewMockServer(mockCtrl) queryRouter := mocks.NewMockServer(mockCtrl) - cfg := module.NewConfigurator(msgRouter, queryRouter) + cfg := module.NewConfigurator(msgRouter, queryRouter, nil) mockAppModule1.EXPECT().RegisterServices(cfg).Times(1) mockAppModule2.EXPECT().RegisterServices(cfg).Times(1) From b7141f990988ea994788c1e350e6b334004bd868 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 11:31:23 +0100 Subject: [PATCH 05/26] Error if no migration found --- simapp/app_test.go | 45 +++++++++++++++++++++++------------- types/module/configurator.go | 21 +++++++++-------- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index bbd76ba6edcb..88455a272304 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -2,7 +2,6 @@ package simapp import ( "encoding/json" - "fmt" "os" "testing" @@ -60,42 +59,51 @@ func TestRunMigrations(t *testing.T) { testCases := []struct { name string moduleName string - expRegErr bool + expRegErr bool // errors while registering migration expRegErrMsg string + expRunErr bool // errors while running migration + expRunErrMsg string expCalled int }{ { "cannot register migration for non-existant module", "foo", - true, "store key for module foo not found: not found", 0, + true, "store key for module foo not found: not found", false, "", 0, + }, + { + "throws error on RunMigrations if no migration registered for bank", + "", + false, "", true, "no migration found for module bank from version 0 to version 1: not found", 0, }, { "can register and run migration handler for x/bank", "bank", - false, "", 1, + false, "", false, "", 1, }, { "cannot register migration handler for same module & fromVersion", "bank", - true, "another migration for module bank and version 0 already exists: internal logic error", 0, + true, "another migration for module bank and version 0 already exists: internal logic error", false, "", 0, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + var err error - // Since it's very hard to test in-place store migrations in tests (due - // to the difficulty of maintaing multiple versions of a module), we're - // just testing here that the migration logic is called or not for our - // custom module. + // Since it's very hard to test actual in-place store migrations in + // tests (due to the difficulty of maintaing multiple versions of a + // module), we're just testing here that the migration logic is + // called. called := 0 - err := app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.KVStore) error { - fmt.Println("HELLO") - called++ + if tc.moduleName != "" { + err = app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.KVStore) error { + called++ - return nil - }) + return nil + }) + } if tc.expRegErr { require.Error(t, err) @@ -109,8 +117,13 @@ func TestRunMigrations(t *testing.T) { app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}), map[string]uint64{"bank": 0}, ) - require.NoError(t, err) - require.Equal(t, tc.expCalled, called) + if tc.expRunErr { + require.Error(t, err) + require.Equal(t, tc.expRunErrMsg, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.expCalled, called) + } }) } } diff --git a/types/module/configurator.go b/types/module/configurator.go index eacf8ded7c2b..d53180ccea50 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -84,9 +84,11 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h // RunMigration implements the Configurator.RunMigration method func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { - _, found := c.migrations[moduleName] - if !found { - // If no migrations has been registered for this module, we just skip. + // No-op if toVersion is the initial version. + // Some modules don't have a store key (e.g. vesting), in this case, their + // ConsensusVersion will always stay at 0, and running migrations on + // those modules will be skipped on this line. + if toVersion == 0 { return nil } @@ -98,12 +100,13 @@ func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVers // Run in-place migrations for the module sequentially until toVersion. for i := fromVersion; i < toVersion; i++ { migrateFn, found := c.migrations[moduleName][i] - // If no migrations has been registered for this version, we just skip. - if found { - err := migrateFn(ctx.KVStore(storeKey)) - if err != nil { - return err - } + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) + } + + err := migrateFn(ctx.KVStore(storeKey)) + if err != nil { + return err } } From 2d554cefef3be4964a1a15c788075f8ee711d31d Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 11:41:48 +0100 Subject: [PATCH 06/26] Remove RunMigrations from Configurator interface --- types/module/configurator.go | 35 ------------------------------- types/module/module.go | 40 +++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/types/module/configurator.go b/types/module/configurator.go index d53180ccea50..1a18880e3255 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -24,10 +24,6 @@ type Configurator interface { // handler is a migration script to perform in-place migrations from version // `fromVersion` to version `fromVersion+1`. RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error - - // RunMigrations runs all in-place store migrations for a module from a - // given version to another version. - RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error } type configurator struct { @@ -81,34 +77,3 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h return nil } - -// RunMigration implements the Configurator.RunMigration method -func (c configurator) RunMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { - // No-op if toVersion is the initial version. - // Some modules don't have a store key (e.g. vesting), in this case, their - // ConsensusVersion will always stay at 0, and running migrations on - // those modules will be skipped on this line. - if toVersion == 0 { - return nil - } - - storeKey, found := c.storeKeys[moduleName] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) - } - - // Run in-place migrations for the module sequentially until toVersion. - for i := fromVersion; i < toVersion; i++ { - migrateFn, found := c.migrations[moduleName][i] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) - } - - err := migrateFn(ctx.KVStore(storeKey)) - if err != nil { - return err - } - } - - return nil -} diff --git a/types/module/module.go b/types/module/module.go index b3172728dbb8..8018bb751c42 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -40,6 +40,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) //__________________________________________________________________________________________ @@ -336,8 +337,13 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st // RunMigrations performs in-place store migrations for all modules. func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { + c, ok := cfg.(configurator) + if !ok { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg) + } + for moduleName, module := range m.Modules { - err := cfg.RunMigrations(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + err := runMigrations(c, ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) if err != nil { return err } @@ -387,3 +393,35 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo Events: ctx.EventManager().ABCIEvents(), } } + +// runMigrations runs all in-place store migrations for one given module from a +// version to another version. +func runMigrations(cfg configurator, ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { + // No-op if toVersion is the initial version. + // Some modules don't have a store key (e.g. vesting), in this case, their + // ConsensusVersion will always stay at 0, and running migrations on + // those modules will be skipped on this line. + if toVersion == 0 { + return nil + } + + storeKey, found := cfg.storeKeys[moduleName] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) + } + + // Run in-place migrations for the module sequentially until toVersion. + for i := fromVersion; i < toVersion; i++ { + migrateFn, found := cfg.migrations[moduleName][i] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) + } + + err := migrateFn(ctx.KVStore(storeKey)) + if err != nil { + return err + } + } + + return nil +} From 3df3f44734ad9b6e115242ff328565e7b1543275 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 11:55:23 +0100 Subject: [PATCH 07/26] Update spec --- x/bank/spec/01_state.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/bank/spec/01_state.md b/x/bank/spec/01_state.md index f744e2e779a1..6ca6b97e8fc1 100644 --- a/x/bank/spec/01_state.md +++ b/x/bank/spec/01_state.md @@ -7,5 +7,5 @@ order: 1 The `x/bank` module keeps state of two primary objects, account balances and the total supply of all balances. -- Balances: `[]byte("balances") | []byte(address) / []byte(balance.Denom) -> ProtocolBuffer(balance)` - Supply: `0x0 -> ProtocolBuffer(Supply)` +- Balances: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)` From ba6bd446b8da68cae18d324c07a19f666ac229bd Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 12:04:59 +0100 Subject: [PATCH 08/26] Rename folders --- x/bank/legacy/v040/{store.go => keys.go} | 0 x/bank/legacy/v042/keys.go | 28 ++++++++++++++++++++++++ x/bank/legacy/v042/store.go | 27 ++--------------------- x/bank/legacy/v042/store_test.go | 2 +- x/bank/module.go | 2 +- 5 files changed, 32 insertions(+), 27 deletions(-) rename x/bank/legacy/v040/{store.go => keys.go} (100%) create mode 100644 x/bank/legacy/v042/keys.go diff --git a/x/bank/legacy/v040/store.go b/x/bank/legacy/v040/keys.go similarity index 100% rename from x/bank/legacy/v040/store.go rename to x/bank/legacy/v040/keys.go diff --git a/x/bank/legacy/v042/keys.go b/x/bank/legacy/v042/keys.go new file mode 100644 index 000000000000..1f526f7faffe --- /dev/null +++ b/x/bank/legacy/v042/keys.go @@ -0,0 +1,28 @@ +package v042 + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" +) + +// KVStore keys +var ( + // BalancesPrefix is the for the account balances store. We use a byte + // (instead of say `[]]byte("balances")` to save some disk space). + BalancesPrefix = []byte{0x02} +) + +// AddressFromBalancesStore returns an account address from a balances prefix +// store. The key must not contain the perfix BalancesPrefix as the prefix store +// iterator discards the actual prefix. +func AddressFromBalancesStore(key []byte) sdk.AccAddress { + addrLen := key[0] + addr := key[1 : addrLen+1] + + return sdk.AccAddress(addr) +} + +// CreateAccountBalancesPrefix creates the prefix for an account's balances. +func CreateAccountBalancesPrefix(addr []byte) []byte { + return append(BalancesPrefix, address.MustLengthPrefix(addr)...) +} diff --git a/x/bank/legacy/v042/store.go b/x/bank/legacy/v042/store.go index a4e51f28d65e..a83521182088 100644 --- a/x/bank/legacy/v042/store.go +++ b/x/bank/legacy/v042/store.go @@ -3,39 +3,16 @@ package v042 import ( "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" v040bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v040" ) -// KVStore keys -var ( - // BalancesPrefix is the for the account balances store. We use a byte - // (instead of say `[]]byte("balances")` to save some disk space). - BalancesPrefix = []byte{0x02} -) - -// AddressFromBalancesStore returns an account address from a balances prefix -// store. The key must not contain the perfix BalancesPrefix as the prefix store -// iterator discards the actual prefix. -func AddressFromBalancesStore(key []byte) sdk.AccAddress { - addrLen := key[0] - addr := key[1 : addrLen+1] - - return sdk.AccAddress(addr) -} - -// CreateAccountBalancesPrefix creates the prefix for an account's balances. -func CreateAccountBalancesPrefix(addr []byte) []byte { - return append(BalancesPrefix, address.MustLengthPrefix(addr)...) -} - -// StoreMigration performs in-place store migrations from v0.40 to v0.42. The +// MigrateStore performs in-place store migrations from v0.40 to v0.42. The // migration includes: // // - Change addresses to be length-prefixed. // - Change balances prefix to 1 byte -func StoreMigration(store sdk.KVStore) error { +func MigrateStore(store sdk.KVStore) error { // old key is of format: // prefix ("balances") || addrBytes (20 bytes) || denomBytes // new key is of format diff --git a/x/bank/legacy/v042/store_test.go b/x/bank/legacy/v042/store_test.go index 8690291b31a6..aeb02485f7f4 100644 --- a/x/bank/legacy/v042/store_test.go +++ b/x/bank/legacy/v042/store_test.go @@ -24,7 +24,7 @@ func TestStoreMigration(t *testing.T) { oldKey := append(append(v040bank.BalancesPrefix, addr...), denom...) store.Set(oldKey, value) - err := v042bank.StoreMigration(store) + err := v042bank.MigrateStore(store) require.NoError(t, err) newKey := append(v042bank.CreateAccountBalancesPrefix(addr), denom...) diff --git a/x/bank/module.go b/x/bank/module.go index 0016a5d76438..e68c908ec8a8 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -101,7 +101,7 @@ type AppModule struct { func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) - cfg.RegisterMigration(types.ModuleName, 0, v042.StoreMigration) + cfg.RegisterMigration(types.ModuleName, 0, v042.MigrateStore) } // NewAppModule creates a new AppModule object From 424932cbf103e926c307696f8b8b49519d09befd Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 15:45:46 +0100 Subject: [PATCH 09/26] copy-paste from keys.go --- x/bank/legacy/v040/keys.go | 24 +++++++++++++++++++++++- x/bank/legacy/v040/types.go | 5 ----- x/bank/legacy/v042/keys.go | 24 +++++++++++++++++++++++- x/bank/legacy/v042/types.go | 5 ----- 4 files changed, 46 insertions(+), 12 deletions(-) delete mode 100644 x/bank/legacy/v040/types.go delete mode 100644 x/bank/legacy/v042/types.go diff --git a/x/bank/legacy/v040/keys.go b/x/bank/legacy/v040/keys.go index 6ff84a34082c..bd1035599164 100644 --- a/x/bank/legacy/v040/keys.go +++ b/x/bank/legacy/v040/keys.go @@ -7,11 +7,33 @@ import ( v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" ) +const ( + // ModuleName defines the module name + ModuleName = "bank" + + // StoreKey defines the primary module store key + StoreKey = ModuleName + + // RouterKey defines the module's message routing key + RouterKey = ModuleName + + // QuerierRoute defines the module's query routing key + QuerierRoute = ModuleName +) + // KVStore keys var ( - BalancesPrefix = []byte("balances") + BalancesPrefix = []byte("balances") + SupplyKey = []byte{0x00} + DenomMetadataPrefix = []byte{0x1} ) +// DenomMetadataKey returns the denomination metadata key. +func DenomMetadataKey(denom string) []byte { + d := []byte(denom) + return append(DenomMetadataPrefix, d...) +} + // AddressFromBalancesStore returns an account address from a balances prefix // store. The key must not contain the perfix BalancesPrefix as the prefix store // iterator discards the actual prefix. diff --git a/x/bank/legacy/v040/types.go b/x/bank/legacy/v040/types.go deleted file mode 100644 index 04ac05f9fcd3..000000000000 --- a/x/bank/legacy/v040/types.go +++ /dev/null @@ -1,5 +0,0 @@ -package v040 - -const ( - ModuleName = "bank" -) diff --git a/x/bank/legacy/v042/keys.go b/x/bank/legacy/v042/keys.go index 1f526f7faffe..8d718b1c5d3c 100644 --- a/x/bank/legacy/v042/keys.go +++ b/x/bank/legacy/v042/keys.go @@ -5,13 +5,35 @@ import ( "github.com/cosmos/cosmos-sdk/types/address" ) +const ( + // ModuleName defines the module name + ModuleName = "bank" + + // StoreKey defines the primary module store key + StoreKey = ModuleName + + // RouterKey defines the module's message routing key + RouterKey = ModuleName + + // QuerierRoute defines the module's query routing key + QuerierRoute = ModuleName +) + // KVStore keys var ( // BalancesPrefix is the for the account balances store. We use a byte // (instead of say `[]]byte("balances")` to save some disk space). - BalancesPrefix = []byte{0x02} + BalancesPrefix = []byte{0x02} + SupplyKey = []byte{0x00} + DenomMetadataPrefix = []byte{0x1} ) +// DenomMetadataKey returns the denomination metadata key. +func DenomMetadataKey(denom string) []byte { + d := []byte(denom) + return append(DenomMetadataPrefix, d...) +} + // AddressFromBalancesStore returns an account address from a balances prefix // store. The key must not contain the perfix BalancesPrefix as the prefix store // iterator discards the actual prefix. diff --git a/x/bank/legacy/v042/types.go b/x/bank/legacy/v042/types.go deleted file mode 100644 index 22534d31f878..000000000000 --- a/x/bank/legacy/v042/types.go +++ /dev/null @@ -1,5 +0,0 @@ -package v042 - -const ( - ModuleName = "bank" -) From b6c0714320eddfa625a42e05767e90bf860e5c3e Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 16:00:38 +0100 Subject: [PATCH 10/26] Fix nil map --- types/module/configurator.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/types/module/configurator.go b/types/module/configurator.go index 1a18880e3255..d2dde870026f 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -60,6 +60,10 @@ func (c configurator) QueryServer() grpc.Server { // RegisterMigration implements the Configurator.RegisterMigration method func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error { + if c.migrations[moduleName] == nil { + c.migrations[moduleName] = map[uint64]func(store sdk.KVStore) error{} + } + if c.migrations[moduleName][fromVersion] != nil { return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, fromVersion) } @@ -69,10 +73,6 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) } - if c.migrations[moduleName] == nil { - c.migrations[moduleName] = map[uint64]func(store sdk.KVStore) error{} - } - c.migrations[moduleName][fromVersion] = handler return nil From 9ab7f138c6c8fafad6ee0b9f6da14ea64f63ea9c Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 3 Feb 2021 16:04:11 +0100 Subject: [PATCH 11/26] rename function --- types/module/module.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index 8018bb751c42..e1cf7c56fbca 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -343,7 +343,7 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap } for moduleName, module := range m.Modules { - err := runMigrations(c, ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + err := runModuleMigrations(c, ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) if err != nil { return err } @@ -396,7 +396,7 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo // runMigrations runs all in-place store migrations for one given module from a // version to another version. -func runMigrations(cfg configurator, ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { +func runModuleMigrations(cfg configurator, ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { // No-op if toVersion is the initial version. // Some modules don't have a store key (e.g. vesting), in this case, their // ConsensusVersion will always stay at 0, and running migrations on From fc076f5906a7803f9903ef4068e3ca1a3234ae6c Mon Sep 17 00:00:00 2001 From: Amaury Date: Fri, 5 Feb 2021 16:11:13 +0100 Subject: [PATCH 12/26] Update simapp/app.go Co-authored-by: Robert Zaremba --- simapp/app.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index d2b59cf3581c..eec692e982e0 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -603,8 +603,7 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { } // RunMigrations performs in-place store migrations for all modules. This -// function is not called automatically, it is meant to be called from an -// x/upgrade UpgradeHandler. +// function MUST be only called by x/upgrade UpgradeHandler. // // `migrationsMap` is a map of moduleName to fromVersion (unit64), where // fromVersion denotes the version from which we should migrate the module, the From ae7b7dcf3b87eb683182b27cc05485d12d3e8187 Mon Sep 17 00:00:00 2001 From: Amaury Date: Fri, 5 Feb 2021 16:11:23 +0100 Subject: [PATCH 13/26] Update simapp/app_test.go Co-authored-by: Robert Zaremba --- simapp/app_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 88455a272304..371bfce7cdf6 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -103,13 +103,9 @@ func TestRunMigrations(t *testing.T) { return nil }) - } - if tc.expRegErr { - require.Error(t, err) - require.Equal(t, tc.expRegErrMsg, err.Error()) - - return + assert.ErrorEqual(t, err, tc.expRegErrMsg) + } } require.NoError(t, err) From 2c26734678b9dbee5ca57b71a1a864eaca26ebbb Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 14:38:55 +0100 Subject: [PATCH 14/26] Adderss reviews --- simapp/app.go | 2 +- simapp/app_test.go | 8 +++-- types/module/configurator.go | 50 +++++++++++++++++++++++++++----- types/module/module.go | 41 +++++--------------------- x/bank/legacy/v042/store.go | 5 +++- x/bank/legacy/v042/store_test.go | 2 +- 6 files changed, 62 insertions(+), 46 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index d2b59cf3581c..eefbe0496f21 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -396,7 +396,7 @@ func NewSimApp( app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) - app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + app.configurator = module.NewConfigurator(app.AppCodec(), app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) app.mm.RegisterServices(app.configurator) // add test gRPC service for testing gRPC queries in isolation diff --git a/simapp/app_test.go b/simapp/app_test.go index 88455a272304..9fd00f339e02 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -11,6 +11,7 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -51,10 +52,11 @@ func TestGetMaccPerms(t *testing.T) { func TestRunMigrations(t *testing.T) { db := dbm.NewMemDB() - app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeTestEncodingConfig(), EmptyAppOptions{}) + encCfg := MakeTestEncodingConfig() + app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) // Create a new configurator for the purpose of this test. - app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + app.configurator = module.NewConfigurator(encCfg.Marshaler, app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) testCases := []struct { name string @@ -98,7 +100,7 @@ func TestRunMigrations(t *testing.T) { called := 0 if tc.moduleName != "" { - err = app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.KVStore) error { + err = app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.Context, sdk.StoreKey, codec.Marshaler) error { called++ return nil diff --git a/types/module/configurator.go b/types/module/configurator.go index d2dde870026f..15c50c1465e7 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -1,9 +1,11 @@ package module import ( + "github.com/gogo/protobuf/grpc" + + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/gogo/protobuf/grpc" ) // Configurator provides the hooks to allow modules to configure and register @@ -23,26 +25,28 @@ type Configurator interface { // RegisterMigration 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`. - RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error + RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error } type configurator struct { + cdc codec.Marshaler msgServer grpc.Server queryServer grpc.Server // storeKeys is used to access module stores inside in-place store migrations. storeKeys map[string]*sdk.KVStoreKey // migrations is a map of moduleName -> fromVersion -> migration script handler - migrations map[string]map[uint64]func(store sdk.KVStore) error + migrations map[string]map[uint64]MigrationHandler } // NewConfigurator returns a new Configurator instance -func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server, storeKeys map[string]*sdk.KVStoreKey) Configurator { +func NewConfigurator(cdc codec.Marshaler, msgServer grpc.Server, queryServer grpc.Server, storeKeys map[string]*sdk.KVStoreKey) Configurator { return configurator{ + cdc: cdc, msgServer: msgServer, queryServer: queryServer, storeKeys: storeKeys, - migrations: map[string]map[uint64]func(store sdk.KVStore) error{}, + migrations: map[string]map[uint64]MigrationHandler{}, } } @@ -59,9 +63,9 @@ func (c configurator) QueryServer() grpc.Server { } // RegisterMigration implements the Configurator.RegisterMigration method -func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler func(store sdk.KVStore) error) error { +func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error { if c.migrations[moduleName] == nil { - c.migrations[moduleName] = map[uint64]func(store sdk.KVStore) error{} + c.migrations[moduleName] = map[uint64]MigrationHandler{} } if c.migrations[moduleName][fromVersion] != nil { @@ -77,3 +81,35 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h return nil } + +// 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 { + // No-op if toVersion is the initial version. + // Some modules don't have a store key (e.g. vesting), in this case, their + // ConsensusVersion will always stay at 0, and running migrations on + // those modules will be skipped on this line. + if toVersion == 0 { + return nil + } + + storeKey, found := c.storeKeys[moduleName] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) + } + + // Run in-place migrations for the module sequentially until toVersion. + for i := fromVersion; i < toVersion; i++ { + migrateFn, found := c.migrations[moduleName][i] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) + } + + err := migrateFn(ctx, storeKey, c.cdc) + if err != nil { + return err + } + } + + return nil +} diff --git a/types/module/module.go b/types/module/module.go index e1cf7c56fbca..4e3308204b47 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -335,6 +335,13 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st return genesisData } +// MigrationHandler is the migration function that each module registers. +type MigrationHandler func(store sdk.Context, storeKey sdk.StoreKey, cdc codec.Marshaler) error + +// MigrationMap is a map of moduleName -> version, where version denotes the +// version from which we should perform the migration for each module. +type MigrationMap map[string]uint64 + // RunMigrations performs in-place store migrations for all modules. func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { c, ok := cfg.(configurator) @@ -343,7 +350,7 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap } for moduleName, module := range m.Modules { - err := runModuleMigrations(c, ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + err := c.runModuleMigrations(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) if err != nil { return err } @@ -393,35 +400,3 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo Events: ctx.EventManager().ABCIEvents(), } } - -// runMigrations runs all in-place store migrations for one given module from a -// version to another version. -func runModuleMigrations(cfg configurator, ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { - // No-op if toVersion is the initial version. - // Some modules don't have a store key (e.g. vesting), in this case, their - // ConsensusVersion will always stay at 0, and running migrations on - // those modules will be skipped on this line. - if toVersion == 0 { - return nil - } - - storeKey, found := cfg.storeKeys[moduleName] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) - } - - // Run in-place migrations for the module sequentially until toVersion. - for i := fromVersion; i < toVersion; i++ { - migrateFn, found := cfg.migrations[moduleName][i] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) - } - - err := migrateFn(ctx.KVStore(storeKey)) - if err != nil { - return err - } - } - - return nil -} diff --git a/x/bank/legacy/v042/store.go b/x/bank/legacy/v042/store.go index a83521182088..4511ba6159fc 100644 --- a/x/bank/legacy/v042/store.go +++ b/x/bank/legacy/v042/store.go @@ -1,6 +1,7 @@ package v042 import ( + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" @@ -12,7 +13,9 @@ import ( // // - Change addresses to be length-prefixed. // - Change balances prefix to 1 byte -func MigrateStore(store sdk.KVStore) error { +func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, _ codec.Marshaler) error { + store := ctx.KVStore(storeKey) + // old key is of format: // prefix ("balances") || addrBytes (20 bytes) || denomBytes // new key is of format diff --git a/x/bank/legacy/v042/store_test.go b/x/bank/legacy/v042/store_test.go index aeb02485f7f4..19067497b2be 100644 --- a/x/bank/legacy/v042/store_test.go +++ b/x/bank/legacy/v042/store_test.go @@ -24,7 +24,7 @@ func TestStoreMigration(t *testing.T) { oldKey := append(append(v040bank.BalancesPrefix, addr...), denom...) store.Set(oldKey, value) - err := v042bank.MigrateStore(store) + err := v042bank.MigrateStore(ctx, bankKey, nil) require.NoError(t, err) newKey := append(v042bank.CreateAccountBalancesPrefix(addr), denom...) From 291a9e6bd45d49ed21d6f9299bf46fc3dfc016ee Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 14:40:54 +0100 Subject: [PATCH 15/26] Fix tests --- simapp/app_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 4445abf6f9c1..3d81bc691469 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -105,9 +105,9 @@ func TestRunMigrations(t *testing.T) { return nil }) - if tc.expRegErr { - assert.ErrorEqual(t, err, tc.expRegErrMsg) - } + if tc.expRegErr { + require.EqualError(t, err, tc.expRegErrMsg) + } } require.NoError(t, err) @@ -116,8 +116,7 @@ func TestRunMigrations(t *testing.T) { map[string]uint64{"bank": 0}, ) if tc.expRunErr { - require.Error(t, err) - require.Equal(t, tc.expRunErrMsg, err.Error()) + require.EqualError(t, err, tc.expRunErrMsg) } else { require.NoError(t, err) require.Equal(t, tc.expCalled, called) From 41d29ed7a4231597603ce4b9d4236418a7c7d071 Mon Sep 17 00:00:00 2001 From: Amaury Date: Mon, 8 Feb 2021 14:41:31 +0100 Subject: [PATCH 16/26] Update testutil/context.go Co-authored-by: Robert Zaremba --- testutil/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/context.go b/testutil/context.go index b7fd34b686cb..2fb9865a2696 100644 --- a/testutil/context.go +++ b/testutil/context.go @@ -9,7 +9,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// DefaultContext creates a sdk.Context that can be used in tests. +// DefaultContext creates a sdk.Context with a fresh MemDB that can be used in tests. func DefaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context { db := dbm.NewMemDB() cms := store.NewCommitMultiStore(db) From 33494dcdb89f970def4979b2a98d5e44b6e68bf6 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 14:47:10 +0100 Subject: [PATCH 17/26] Update docs for ConsensusVersion --- types/module/module.go | 10 ++++++++-- types/module/module_test.go | 4 +++- x/auth/module.go | 7 +++++-- x/auth/vesting/module.go | 7 +++++-- x/authz/module.go | 7 +++++-- x/bank/module.go | 7 +++++-- x/capability/module.go | 7 +++++-- x/crisis/module.go | 7 +++++-- x/distribution/module.go | 7 +++++-- x/evidence/module.go | 7 +++++-- x/feegrant/module.go | 7 +++++-- x/genutil/module.go | 7 +++++-- x/gov/module.go | 7 +++++-- x/ibc/applications/transfer/module.go | 7 +++++-- x/ibc/core/module.go | 7 +++++-- x/mint/module.go | 7 +++++-- x/params/module.go | 7 +++++-- x/slashing/module.go | 7 +++++-- x/staking/module.go | 7 +++++-- x/upgrade/module.go | 7 +++++-- 20 files changed, 101 insertions(+), 39 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index 4e3308204b47..26a2972c93c3 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -175,7 +175,10 @@ type AppModule interface { // RegisterServices allows a module to register services RegisterServices(Configurator) - // ConsensusVersion tracks state-breaking versions of the module + // ConsensusVersion is a sequence number for state-breaking change of the + // module. It should be incremented on each consensus-breaking change + // introduced by the module. To avoid wrong/empty versions, the initial version + // is set to 1. ConsensusVersion() uint64 // ABCI @@ -212,7 +215,10 @@ func (gam GenesisOnlyAppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Que // RegisterServices registers all services. func (gam GenesisOnlyAppModule) RegisterServices(Configurator) {} -// ConsensusVersion tracks state-breaking versions of the module. +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 0 } // BeginBlock returns an empty module begin-block diff --git a/types/module/module_test.go b/types/module/module_test.go index fa11d7a49603..9dafb37566c2 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/simapp" "github.com/golang/mock/gomock" "github.com/gorilla/mux" @@ -168,6 +169,7 @@ func TestManager_RegisterRoutes(t *testing.T) { func TestManager_RegisterQueryServices(t *testing.T) { mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) + encCfg := simapp.MakeTestEncodingConfig() mockAppModule1 := mocks.NewMockAppModule(mockCtrl) mockAppModule2 := mocks.NewMockAppModule(mockCtrl) @@ -179,7 +181,7 @@ func TestManager_RegisterQueryServices(t *testing.T) { msgRouter := mocks.NewMockServer(mockCtrl) queryRouter := mocks.NewMockServer(mockCtrl) - cfg := module.NewConfigurator(msgRouter, queryRouter, nil) + cfg := module.NewConfigurator(encCfg.Marshaler, msgRouter, queryRouter, nil) mockAppModule1.EXPECT().RegisterServices(cfg).Times(1) mockAppModule2.EXPECT().RegisterServices(cfg).Times(1) diff --git a/x/auth/module.go b/x/auth/module.go index 9ed1138c9ff1..338ee70e7ccb 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -147,8 +147,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the auth module. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index 06a947721ba8..ecc5a77baf5a 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -128,5 +128,8 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } diff --git a/x/authz/module.go b/x/authz/module.go index a8088656a7b7..c94aaa871f4a 100644 --- a/x/authz/module.go +++ b/x/authz/module.go @@ -152,8 +152,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} diff --git a/x/bank/module.go b/x/bank/module.go index e68c908ec8a8..8e7b11070da5 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -153,8 +153,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 1 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/capability/module.go b/x/capability/module.go index 8a1189400c7f..e9a17d9b2dbf 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -136,8 +136,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(genState) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the capability module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/crisis/module.go b/x/crisis/module.go index 4ef39afb4772..ca41e9bc08de 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -158,8 +158,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/distribution/module.go b/x/distribution/module.go index 1830d08c583a..658781b103cf 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -161,8 +161,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the distribution module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { diff --git a/x/evidence/module.go b/x/evidence/module.go index 7de6a10c4604..d72a693a1277 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -175,8 +175,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, am.keeper)) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { diff --git a/x/feegrant/module.go b/x/feegrant/module.go index b61dd4855436..4a222b3cee09 100644 --- a/x/feegrant/module.go +++ b/x/feegrant/module.go @@ -167,8 +167,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the feegrant module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/genutil/module.go b/x/genutil/module.go index 09b2f8e5c1b2..becca6a950c4 100644 --- a/x/genutil/module.go +++ b/x/genutil/module.go @@ -111,5 +111,8 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } diff --git a/x/gov/module.go b/x/gov/module.go index 0a8ce0c24a7f..f47fe91ff3ed 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -177,8 +177,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go index 987b73470b4f..74ff5e57b339 100644 --- a/x/ibc/applications/transfer/module.go +++ b/x/ibc/applications/transfer/module.go @@ -145,8 +145,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { diff --git a/x/ibc/core/module.go b/x/ibc/core/module.go index 53c39b508d27..f2b2318771c8 100644 --- a/x/ibc/core/module.go +++ b/x/ibc/core/module.go @@ -156,8 +156,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, *am.keeper)) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the ibc module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { diff --git a/x/mint/module.go b/x/mint/module.go index 6f408e6763a7..cf9e5e8999ff 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -146,8 +146,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the mint module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { diff --git a/x/params/module.go b/x/params/module.go index 4a8f4ff5dcff..88205c65f4af 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -139,8 +139,11 @@ func (am AppModule) ExportGenesis(_ sdk.Context, _ codec.JSONMarshaler) json.Raw return nil } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/slashing/module.go b/x/slashing/module.go index a1cac6ae864f..52c78a626231 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -159,8 +159,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the slashing module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { diff --git a/x/staking/module.go b/x/staking/module.go index 5a1880055c3d..0ec94958a299 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -157,8 +157,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the staking module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 268ba817979f..ccf7b077e65e 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -120,8 +120,11 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion tracks state-breaking versions of the module. -func (AppModule) ConsensusVersion() uint64 { return 0 } +// ConsensusVersion is a sequence number for state-breaking change of the +// module. It should be incremented on each consensus-breaking change +// introduced by the module. To avoid wrong/empty versions, the initial version +// is set to 1. +func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock calls the upgrade module hooks // From e3a241226a79d64a5cdfd4776e3bec72e0e2c312 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 14:50:34 +0100 Subject: [PATCH 18/26] Rename to forVersion --- simapp/app_test.go | 18 ++++++++++++------ types/module/configurator.go | 22 +++++++++++++--------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 3d81bc691469..3be13a14f8f2 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -61,6 +61,7 @@ func TestRunMigrations(t *testing.T) { testCases := []struct { name string moduleName string + forVersion uint64 expRegErr bool // errors while registering migration expRegErrMsg string expRunErr bool // errors while running migration @@ -69,22 +70,27 @@ func TestRunMigrations(t *testing.T) { }{ { "cannot register migration for non-existant module", - "foo", + "foo", 0, + true, "store key for module foo not found: not found", false, "", 0, + }, + { + "cannot register migration for version 0", + "foo", 1, true, "store key for module foo not found: not found", false, "", 0, }, { "throws error on RunMigrations if no migration registered for bank", - "", + "", 1, false, "", true, "no migration found for module bank from version 0 to version 1: not found", 0, }, { "can register and run migration handler for x/bank", - "bank", + "bank", 1, false, "", false, "", 1, }, { - "cannot register migration handler for same module & fromVersion", - "bank", + "cannot register migration handler for same module & forVersion", + "bank", 1, true, "another migration for module bank and version 0 already exists: internal logic error", false, "", 0, }, } @@ -100,7 +106,7 @@ func TestRunMigrations(t *testing.T) { called := 0 if tc.moduleName != "" { - err = app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.Context, sdk.StoreKey, codec.Marshaler) error { + err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context, sdk.StoreKey, codec.Marshaler) error { called++ return nil diff --git a/types/module/configurator.go b/types/module/configurator.go index 15c50c1465e7..a096cacf9564 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -24,8 +24,8 @@ type Configurator interface { // RegisterMigration 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`. - RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error + // `forVersion` to version `forVersion+1`. + RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error } type configurator struct { @@ -35,7 +35,7 @@ type configurator struct { // storeKeys is used to access module stores inside in-place store migrations. storeKeys map[string]*sdk.KVStoreKey - // migrations is a map of moduleName -> fromVersion -> migration script handler + // migrations is a map of moduleName -> forVersion -> migration script handler migrations map[string]map[uint64]MigrationHandler } @@ -63,13 +63,17 @@ func (c configurator) QueryServer() grpc.Server { } // RegisterMigration implements the Configurator.RegisterMigration method -func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error { +func (c configurator) RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error { + if forVersion == 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidVersion, "module migration versions should start at 1") + } + if c.migrations[moduleName] == nil { c.migrations[moduleName] = map[uint64]MigrationHandler{} } - if c.migrations[moduleName][fromVersion] != nil { - return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, fromVersion) + if c.migrations[moduleName][forVersion] != nil { + return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, forVersion) } _, found := c.storeKeys[moduleName] @@ -77,14 +81,14 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) } - c.migrations[moduleName][fromVersion] = handler + c.migrations[moduleName][forVersion] = handler return nil } // 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 { +func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, forVersion, toVersion uint64) error { // No-op if toVersion is the initial version. // Some modules don't have a store key (e.g. vesting), in this case, their // ConsensusVersion will always stay at 0, and running migrations on @@ -99,7 +103,7 @@ func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fr } // Run in-place migrations for the module sequentially until toVersion. - for i := fromVersion; i < toVersion; i++ { + for i := forVersion; i < toVersion; i++ { migrateFn, found := c.migrations[moduleName][i] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) From 55f547e9f3658b49b02855119ce16bae8d089eeb Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 15:15:29 +0100 Subject: [PATCH 19/26] Fix tests --- simapp/app.go | 4 ++-- simapp/app_test.go | 20 ++++++++++++++------ types/module/configurator.go | 6 +++--- types/module/module.go | 4 ++-- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 511d93a4036e..444e2007a95b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -612,7 +612,7 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { // Example: // cfg := module.NewConfigurator(...) // app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan) { -// err := app.RunMigrations(ctx, map[string]unint64{ +// err := app.RunMigrations(ctx, module.MigrationMap{ // "bank": 1, // Migrate x/bank from v1 to current x/bank's ConsensusVersion // "staking": 8, // Migrate x/staking from v8 to current x/staking's ConsensusVersion // }) @@ -620,7 +620,7 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { // panic(err) // } // }) -func (app *SimApp) RunMigrations(ctx sdk.Context, migrationsMap map[string]uint64) error { +func (app *SimApp) RunMigrations(ctx sdk.Context, migrationsMap module.MigrationMap) error { return app.mm.RunMigrations(ctx, app.configurator, migrationsMap) } diff --git a/simapp/app_test.go b/simapp/app_test.go index 3be13a14f8f2..55799ddc62f5 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -70,18 +70,18 @@ func TestRunMigrations(t *testing.T) { }{ { "cannot register migration for non-existant module", - "foo", 0, + "foo", 1, true, "store key for module foo not found: not found", false, "", 0, }, { "cannot register migration for version 0", - "foo", 1, - true, "store key for module foo not found: not found", false, "", 0, + "bank", 0, + true, "module migration versions should start at 1: invalid version", false, "", 0, }, { "throws error on RunMigrations if no migration registered for bank", "", 1, - false, "", true, "no migration found for module bank from version 0 to version 1: not found", 0, + false, "", true, "no migration found for module bank from version 1 to version 2: not found", 0, }, { "can register and run migration handler for x/bank", @@ -91,7 +91,7 @@ func TestRunMigrations(t *testing.T) { { "cannot register migration handler for same module & forVersion", "bank", 1, - true, "another migration for module bank and version 0 already exists: internal logic error", false, "", 0, + true, "another migration for module bank and version 1 already exists: internal logic error", false, "", 0, }, } @@ -106,20 +106,28 @@ func TestRunMigrations(t *testing.T) { called := 0 if tc.moduleName != "" { + // Register migration for module from version `forVersion` to `forVersion+1`. err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context, sdk.StoreKey, codec.Marshaler) error { called++ return nil }) + if tc.expRegErr { require.EqualError(t, err, tc.expRegErrMsg) + + return } } require.NoError(t, err) err = app.RunMigrations( app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}), - map[string]uint64{"bank": 0}, + module.MigrationMap{ + "auth": 1, "authz": 1, "bank": 1, "staking": 1, "mint": 1, "distribution": 1, + "slashing": 1, "gov": 1, "params": 1, "ibc": 1, "upgrade": 1, "vesting": 1, + "feegrant": 1, "transfer": 1, "evidence": 1, "crisis": 1, "genutil": 1, "capability": 1, + }, ) if tc.expRunErr { require.EqualError(t, err, tc.expRunErrMsg) diff --git a/types/module/configurator.go b/types/module/configurator.go index a096cacf9564..f6fba83a6115 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -88,12 +88,12 @@ func (c configurator) RegisterMigration(moduleName string, forVersion uint64, ha // 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, forVersion, toVersion uint64) error { +func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { // No-op if toVersion is the initial version. // Some modules don't have a store key (e.g. vesting), in this case, their // ConsensusVersion will always stay at 0, and running migrations on // those modules will be skipped on this line. - if toVersion == 0 { + if toVersion <= 1 { return nil } @@ -103,7 +103,7 @@ func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fo } // Run in-place migrations for the module sequentially until toVersion. - for i := forVersion; i < toVersion; i++ { + for i := fromVersion; i < toVersion; i++ { migrateFn, found := c.migrations[moduleName][i] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) diff --git a/types/module/module.go b/types/module/module.go index 26a2972c93c3..91ecec90741e 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -349,14 +349,14 @@ type MigrationHandler func(store sdk.Context, storeKey sdk.StoreKey, cdc codec.M type MigrationMap map[string]uint64 // RunMigrations performs in-place store migrations for all modules. -func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationsMap map[string]uint64) error { +func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationMap MigrationMap) error { c, ok := cfg.(configurator) if !ok { return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg) } for moduleName, module := range m.Modules { - err := c.runModuleMigrations(ctx, moduleName, migrationsMap[moduleName], module.ConsensusVersion()) + err := c.runModuleMigrations(ctx, moduleName, migrationMap[moduleName], module.ConsensusVersion()) if err != nil { return err } From 8f3b9d4e10b0c17ea1bbdd8346d2fb98377c9b00 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 15:19:15 +0100 Subject: [PATCH 20/26] Check error early --- simapp/app_test.go | 2 +- types/module/configurator.go | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 55799ddc62f5..768c00a7394a 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -81,7 +81,7 @@ func TestRunMigrations(t *testing.T) { { "throws error on RunMigrations if no migration registered for bank", "", 1, - false, "", true, "no migration found for module bank from version 1 to version 2: not found", 0, + false, "", true, "no migrations found for module bank: not found", 0, }, { "can register and run migration handler for x/bank", diff --git a/types/module/configurator.go b/types/module/configurator.go index f6fba83a6115..e1e856c6d337 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -91,7 +91,7 @@ func (c configurator) RegisterMigration(moduleName string, forVersion uint64, ha func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { // No-op if toVersion is the initial version. // Some modules don't have a store key (e.g. vesting), in this case, their - // ConsensusVersion will always stay at 0, and running migrations on + // ConsensusVersion will always stay at 1, and running migrations on // those modules will be skipped on this line. if toVersion <= 1 { return nil @@ -102,9 +102,14 @@ func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fr return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) } + moduleMigrationsMap, found := c.migrations[moduleName] + if !found { + return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migrations found for module %s", moduleName) + } + // Run in-place migrations for the module sequentially until toVersion. for i := fromVersion; i < toVersion; i++ { - migrateFn, found := c.migrations[moduleName][i] + migrateFn, found := moduleMigrationsMap[i] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) } From 29b85b7a63b02a0fea2d5a40d7b283d9f43aa854 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Mon, 8 Feb 2021 15:20:08 +0100 Subject: [PATCH 21/26] Return 1 for intiial version --- tests/mocks/types_module_module.go | 2 +- types/module/module.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mocks/types_module_module.go b/tests/mocks/types_module_module.go index e465e115423b..0d00584ab9a8 100644 --- a/tests/mocks/types_module_module.go +++ b/tests/mocks/types_module_module.go @@ -493,7 +493,7 @@ func (m *MockAppModule) ExportGenesis(arg0 types0.Context, arg1 codec.JSONMarsha } // ConsensusVersion mocks base method -func (m *MockAppModule) ConsensusVersion() uint64 { return 0 } +func (m *MockAppModule) ConsensusVersion() uint64 { return 1 } // ExportGenesis indicates an expected call of ExportGenesis func (mr *MockAppModuleMockRecorder) ExportGenesis(arg0, arg1 interface{}) *gomock.Call { diff --git a/types/module/module.go b/types/module/module.go index 91ecec90741e..3733892a247a 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -219,7 +219,7 @@ func (gam GenesisOnlyAppModule) RegisterServices(Configurator) {} // module. It should be incremented on each consensus-breaking change // introduced by the module. To avoid wrong/empty versions, the initial version // is set to 1. -func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 0 } +func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns an empty module begin-block func (gam GenesisOnlyAppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} From 41e0339b8256ac99f348ea7825b000b0895f1c67 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 10 Feb 2021 16:21:37 +0100 Subject: [PATCH 22/26] Use MigrationKeeper --- simapp/app_test.go | 8 +------- types/module/configurator.go | 15 +-------------- types/module/module.go | 2 +- x/bank/keeper/migrations.go | 20 ++++++++++++++++++++ x/bank/legacy/v042/store.go | 3 +-- x/bank/module.go | 5 +++-- 6 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 x/bank/keeper/migrations.go diff --git a/simapp/app_test.go b/simapp/app_test.go index 768c00a7394a..aeabe3132f44 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -11,7 +11,6 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -68,11 +67,6 @@ func TestRunMigrations(t *testing.T) { expRunErrMsg string expCalled int }{ - { - "cannot register migration for non-existant module", - "foo", 1, - true, "store key for module foo not found: not found", false, "", 0, - }, { "cannot register migration for version 0", "bank", 0, @@ -107,7 +101,7 @@ func TestRunMigrations(t *testing.T) { if tc.moduleName != "" { // Register migration for module from version `forVersion` to `forVersion+1`. - err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context, sdk.StoreKey, codec.Marshaler) error { + err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context) error { called++ return nil diff --git a/types/module/configurator.go b/types/module/configurator.go index e1e856c6d337..a2de025836e3 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -33,8 +33,6 @@ type configurator struct { msgServer grpc.Server queryServer grpc.Server - // storeKeys is used to access module stores inside in-place store migrations. - storeKeys map[string]*sdk.KVStoreKey // migrations is a map of moduleName -> forVersion -> migration script handler migrations map[string]map[uint64]MigrationHandler } @@ -45,7 +43,6 @@ func NewConfigurator(cdc codec.Marshaler, msgServer grpc.Server, queryServer grp cdc: cdc, msgServer: msgServer, queryServer: queryServer, - storeKeys: storeKeys, migrations: map[string]map[uint64]MigrationHandler{}, } } @@ -76,11 +73,6 @@ func (c configurator) RegisterMigration(moduleName string, forVersion uint64, ha return sdkerrors.Wrapf(sdkerrors.ErrLogic, "another migration for module %s and version %d already exists", moduleName, forVersion) } - _, found := c.storeKeys[moduleName] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) - } - c.migrations[moduleName][forVersion] = handler return nil @@ -97,11 +89,6 @@ func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fr return nil } - storeKey, found := c.storeKeys[moduleName] - if !found { - return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "store key for module %s not found", moduleName) - } - moduleMigrationsMap, found := c.migrations[moduleName] if !found { return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migrations found for module %s", moduleName) @@ -114,7 +101,7 @@ func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fr return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no migration found for module %s from version %d to version %d", moduleName, i, i+1) } - err := migrateFn(ctx, storeKey, c.cdc) + err := migrateFn(ctx) if err != nil { return err } diff --git a/types/module/module.go b/types/module/module.go index 3733892a247a..64e7a68d8406 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -342,7 +342,7 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st } // MigrationHandler is the migration function that each module registers. -type MigrationHandler func(store sdk.Context, storeKey sdk.StoreKey, cdc codec.Marshaler) error +type MigrationHandler func(store sdk.Context) error // MigrationMap is a map of moduleName -> version, where version denotes the // version from which we should perform the migration for each module. diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go new file mode 100644 index 000000000000..de6b61a9c2b6 --- /dev/null +++ b/x/bank/keeper/migrations.go @@ -0,0 +1,20 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + v042 "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" +) + +// MigrationKeeper is an interface that the keeper implements for handling +// in-place store migrations. +type MigrationKeeper interface { + // Migrate1 migrates the store from version 1 to 2. + Migrate1(ctx sdk.Context) error +} + +var _ MigrationKeeper = (*BaseKeeper)(nil) + +// Migrate1 implements MigrationKeeper.Migrate1 method. +func (keeper BaseKeeper) Migrate1(ctx sdk.Context) error { + return v042.MigrateStore(ctx, keeper.storeKey) +} diff --git a/x/bank/legacy/v042/store.go b/x/bank/legacy/v042/store.go index 4511ba6159fc..9295d8d2ff77 100644 --- a/x/bank/legacy/v042/store.go +++ b/x/bank/legacy/v042/store.go @@ -1,7 +1,6 @@ package v042 import ( - "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040" @@ -13,7 +12,7 @@ import ( // // - Change addresses to be length-prefixed. // - Change balances prefix to 1 byte -func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, _ codec.Marshaler) error { +func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey) error { store := ctx.KVStore(storeKey) // old key is of format: diff --git a/x/bank/module.go b/x/bank/module.go index 8e7b11070da5..e1a1a3adbf0b 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -22,7 +22,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank/client/cli" "github.com/cosmos/cosmos-sdk/x/bank/client/rest" "github.com/cosmos/cosmos-sdk/x/bank/keeper" - v042 "github.com/cosmos/cosmos-sdk/x/bank/legacy/v042" "github.com/cosmos/cosmos-sdk/x/bank/simulation" "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -101,7 +100,9 @@ type AppModule struct { func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) - cfg.RegisterMigration(types.ModuleName, 0, v042.MigrateStore) + cfg.RegisterMigration(types.ModuleName, 0, func(ctx sdk.Context) error { + return am.keeper.(keeper.MigrationKeeper).Migrate1(ctx) + }) } // NewAppModule creates a new AppModule object From d9220031d8953ad01178108cf4d796705093ce74 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 10 Feb 2021 16:23:15 +0100 Subject: [PATCH 23/26] Fix test --- x/bank/legacy/v042/store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/bank/legacy/v042/store_test.go b/x/bank/legacy/v042/store_test.go index 19067497b2be..e1b6a7987a04 100644 --- a/x/bank/legacy/v042/store_test.go +++ b/x/bank/legacy/v042/store_test.go @@ -24,7 +24,7 @@ func TestStoreMigration(t *testing.T) { oldKey := append(append(v040bank.BalancesPrefix, addr...), denom...) store.Set(oldKey, value) - err := v042bank.MigrateStore(ctx, bankKey, nil) + err := v042bank.MigrateStore(ctx, bankKey) require.NoError(t, err) newKey := append(v042bank.CreateAccountBalancesPrefix(addr), denom...) From f422b93afc40bdb055af3b628f78b72bbe86f3e6 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 10 Feb 2021 16:27:44 +0100 Subject: [PATCH 24/26] Revert adding marshaler to Configurator --- simapp/app.go | 2 +- simapp/app_test.go | 2 +- types/module/configurator.go | 5 +---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 444e2007a95b..688aa429fd7b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -396,7 +396,7 @@ func NewSimApp( app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) - app.configurator = module.NewConfigurator(app.AppCodec(), app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter()) app.mm.RegisterServices(app.configurator) // add test gRPC service for testing gRPC queries in isolation diff --git a/simapp/app_test.go b/simapp/app_test.go index aeabe3132f44..2adcf2e7e8d2 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -55,7 +55,7 @@ func TestRunMigrations(t *testing.T) { app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{}) // Create a new configurator for the purpose of this test. - app.configurator = module.NewConfigurator(encCfg.Marshaler, app.MsgServiceRouter(), app.GRPCQueryRouter(), app.keys) + app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter()) testCases := []struct { name string diff --git a/types/module/configurator.go b/types/module/configurator.go index a2de025836e3..36092d7b5a98 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -3,7 +3,6 @@ package module import ( "github.com/gogo/protobuf/grpc" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -29,7 +28,6 @@ type Configurator interface { } type configurator struct { - cdc codec.Marshaler msgServer grpc.Server queryServer grpc.Server @@ -38,9 +36,8 @@ type configurator struct { } // NewConfigurator returns a new Configurator instance -func NewConfigurator(cdc codec.Marshaler, msgServer grpc.Server, queryServer grpc.Server, storeKeys map[string]*sdk.KVStoreKey) Configurator { +func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server) Configurator { return configurator{ - cdc: cdc, msgServer: msgServer, queryServer: queryServer, migrations: map[string]map[uint64]MigrationHandler{}, From 8cf88fe48acb241a128d523b5f36af62217e35db Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 10 Feb 2021 16:58:30 +0100 Subject: [PATCH 25/26] Godoc updates --- simapp/app.go | 6 +++--- types/module/module.go | 11 ++++------- x/auth/module.go | 5 +---- x/auth/vesting/module.go | 5 +---- x/authz/module.go | 5 +---- x/bank/module.go | 5 +---- x/capability/module.go | 5 +---- x/crisis/module.go | 5 +---- x/distribution/module.go | 5 +---- x/evidence/module.go | 5 +---- x/feegrant/module.go | 5 +---- x/genutil/module.go | 5 +---- x/gov/module.go | 5 +---- x/ibc/applications/transfer/module.go | 5 +---- x/ibc/core/module.go | 5 +---- x/mint/module.go | 5 +---- x/params/module.go | 5 +---- x/slashing/module.go | 5 +---- x/staking/module.go | 5 +---- x/upgrade/module.go | 5 +---- 20 files changed, 25 insertions(+), 82 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 688aa429fd7b..4db7d53d7755 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -605,7 +605,7 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { // RunMigrations performs in-place store migrations for all modules. This // function MUST be only called by x/upgrade UpgradeHandler. // -// `migrationsMap` is a map of moduleName to fromVersion (unit64), where +// `migrateFromVersions` is a map of moduleName to fromVersion (unit64), where // fromVersion denotes the version from which we should migrate the module, the // target version being the module's latest ConsensusVersion. // @@ -620,8 +620,8 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { // panic(err) // } // }) -func (app *SimApp) RunMigrations(ctx sdk.Context, migrationsMap module.MigrationMap) error { - return app.mm.RunMigrations(ctx, app.configurator, migrationsMap) +func (app *SimApp) RunMigrations(ctx sdk.Context, migrateFromVersions module.MigrationMap) error { + return app.mm.RunMigrations(ctx, app.configurator, migrateFromVersions) } // RegisterSwaggerAPI registers swagger route with API Server diff --git a/types/module/module.go b/types/module/module.go index 64e7a68d8406..58bb4927b29a 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -178,7 +178,7 @@ type AppModule interface { // ConsensusVersion is a sequence number for state-breaking change of the // module. It should be incremented on each consensus-breaking change // introduced by the module. To avoid wrong/empty versions, the initial version - // is set to 1. + // should be set to 1. ConsensusVersion() uint64 // ABCI @@ -215,10 +215,7 @@ func (gam GenesisOnlyAppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Que // RegisterServices registers all services. func (gam GenesisOnlyAppModule) RegisterServices(Configurator) {} -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns an empty module begin-block @@ -349,14 +346,14 @@ type MigrationHandler func(store sdk.Context) error type MigrationMap map[string]uint64 // RunMigrations performs in-place store migrations for all modules. -func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrationMap MigrationMap) error { +func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, migrateFromVersions MigrationMap) error { c, ok := cfg.(configurator) if !ok { return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg) } for moduleName, module := range m.Modules { - err := c.runModuleMigrations(ctx, moduleName, migrationMap[moduleName], module.ConsensusVersion()) + err := c.runModuleMigrations(ctx, moduleName, migrateFromVersions[moduleName], module.ConsensusVersion()) if err != nil { return err } diff --git a/x/auth/module.go b/x/auth/module.go index 338ee70e7ccb..9e4af71ff037 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -147,10 +147,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the auth module. diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index ecc5a77baf5a..259143469388 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -128,8 +128,5 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } diff --git a/x/authz/module.go b/x/authz/module.go index c94aaa871f4a..b39c02487fe0 100644 --- a/x/authz/module.go +++ b/x/authz/module.go @@ -152,10 +152,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} diff --git a/x/bank/module.go b/x/bank/module.go index e1a1a3adbf0b..77bff6b73b61 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -154,10 +154,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock performs a no-op. diff --git a/x/capability/module.go b/x/capability/module.go index e9a17d9b2dbf..06bee1e2d029 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -136,10 +136,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(genState) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the capability module. diff --git a/x/crisis/module.go b/x/crisis/module.go index ca41e9bc08de..66d829be810e 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -158,10 +158,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. diff --git a/x/distribution/module.go b/x/distribution/module.go index 658781b103cf..0f0c0d358a91 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -161,10 +161,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the distribution module. diff --git a/x/evidence/module.go b/x/evidence/module.go index d72a693a1277..9143e208e716 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -175,10 +175,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, am.keeper)) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module. diff --git a/x/feegrant/module.go b/x/feegrant/module.go index 4a222b3cee09..31b08e6baab1 100644 --- a/x/feegrant/module.go +++ b/x/feegrant/module.go @@ -167,10 +167,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the feegrant module. diff --git a/x/genutil/module.go b/x/genutil/module.go index becca6a950c4..7a65531e453f 100644 --- a/x/genutil/module.go +++ b/x/genutil/module.go @@ -111,8 +111,5 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } diff --git a/x/gov/module.go b/x/gov/module.go index f47fe91ff3ed..b72a4bfe5f5c 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -177,10 +177,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go index 74ff5e57b339..25290d69a64c 100644 --- a/x/ibc/applications/transfer/module.go +++ b/x/ibc/applications/transfer/module.go @@ -145,10 +145,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock implements the AppModule interface diff --git a/x/ibc/core/module.go b/x/ibc/core/module.go index f2b2318771c8..6527ab71eb2f 100644 --- a/x/ibc/core/module.go +++ b/x/ibc/core/module.go @@ -156,10 +156,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(ExportGenesis(ctx, *am.keeper)) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the ibc module. diff --git a/x/mint/module.go b/x/mint/module.go index cf9e5e8999ff..d33043b36a2e 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -146,10 +146,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the mint module. diff --git a/x/params/module.go b/x/params/module.go index 88205c65f4af..c1a115b210c1 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -139,10 +139,7 @@ func (am AppModule) ExportGenesis(_ sdk.Context, _ codec.JSONMarshaler) json.Raw return nil } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock performs a no-op. diff --git a/x/slashing/module.go b/x/slashing/module.go index 52c78a626231..ba5cfb60adb9 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -159,10 +159,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the slashing module. diff --git a/x/staking/module.go b/x/staking/module.go index 0ec94958a299..48c67e6f5cd1 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -157,10 +157,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json return cdc.MustMarshalJSON(gs) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock returns the begin blocker for the staking module. diff --git a/x/upgrade/module.go b/x/upgrade/module.go index ccf7b077e65e..23311c22d5f3 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -120,10 +120,7 @@ func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONMarshaler) json.R return am.DefaultGenesis(cdc) } -// ConsensusVersion is a sequence number for state-breaking change of the -// module. It should be incremented on each consensus-breaking change -// introduced by the module. To avoid wrong/empty versions, the initial version -// is set to 1. +// ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock calls the upgrade module hooks From 5add13a2fa1c38bbff91c03a5477edec872bc8af Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 10 Feb 2021 17:56:55 +0100 Subject: [PATCH 26/26] Update docs --- types/module/configurator.go | 9 ++++++--- types/module/module_test.go | 4 +--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/types/module/configurator.go b/types/module/configurator.go index 36092d7b5a98..0e766df2a13d 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -24,6 +24,12 @@ type Configurator interface { // RegisterMigration registers an in-place store migration for a module. The // handler is a migration script to perform in-place migrations from version // `forVersion` to version `forVersion+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. RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error } @@ -79,9 +85,6 @@ func (c configurator) RegisterMigration(moduleName string, forVersion uint64, ha // version to another version. func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error { // No-op if toVersion is the initial version. - // Some modules don't have a store key (e.g. vesting), in this case, their - // ConsensusVersion will always stay at 1, and running migrations on - // those modules will be skipped on this line. if toVersion <= 1 { return nil } diff --git a/types/module/module_test.go b/types/module/module_test.go index 9dafb37566c2..630c57619245 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/cosmos/cosmos-sdk/simapp" "github.com/golang/mock/gomock" "github.com/gorilla/mux" @@ -169,7 +168,6 @@ func TestManager_RegisterRoutes(t *testing.T) { func TestManager_RegisterQueryServices(t *testing.T) { mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) - encCfg := simapp.MakeTestEncodingConfig() mockAppModule1 := mocks.NewMockAppModule(mockCtrl) mockAppModule2 := mocks.NewMockAppModule(mockCtrl) @@ -181,7 +179,7 @@ func TestManager_RegisterQueryServices(t *testing.T) { msgRouter := mocks.NewMockServer(mockCtrl) queryRouter := mocks.NewMockServer(mockCtrl) - cfg := module.NewConfigurator(encCfg.Marshaler, msgRouter, queryRouter, nil) + cfg := module.NewConfigurator(msgRouter, queryRouter) mockAppModule1.EXPECT().RegisterServices(cfg).Times(1) mockAppModule2.EXPECT().RegisterServices(cfg).Times(1)