Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add in-place store migrations #8485

Merged
merged 33 commits into from Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
10d72d7
Add 1st version of migrate
amaury1093 Feb 1, 2021
8036fca
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-8…
amaury1093 Feb 2, 2021
a9a406e
Put migration logic into Configurator
amaury1093 Feb 2, 2021
9ed3987
add test to bank store migration
amaury1093 Feb 2, 2021
a095d3a
add test for configurator
amaury1093 Feb 2, 2021
d550bff
Merge branch 'master' into am-8345-migration
amaury1093 Feb 2, 2021
b7141f9
Error if no migration found
amaury1093 Feb 3, 2021
2d554ce
Remove RunMigrations from Configurator interface
amaury1093 Feb 3, 2021
3df3f44
Update spec
amaury1093 Feb 3, 2021
ba6bd44
Rename folders
amaury1093 Feb 3, 2021
197b7ce
Merge branch 'master' into am-8345-migration
amaury1093 Feb 3, 2021
424932c
copy-paste from keys.go
amaury1093 Feb 3, 2021
254a71f
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 Feb 3, 2021
b6c0714
Fix nil map
amaury1093 Feb 3, 2021
9ab7f13
rename function
amaury1093 Feb 3, 2021
fc076f5
Update simapp/app.go
amaury1093 Feb 5, 2021
ae7b7dc
Update simapp/app_test.go
amaury1093 Feb 5, 2021
2c26734
Adderss reviews
amaury1093 Feb 8, 2021
eb15047
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 Feb 8, 2021
291a9e6
Fix tests
amaury1093 Feb 8, 2021
41d29ed
Update testutil/context.go
amaury1093 Feb 8, 2021
33494dc
Update docs for ConsensusVersion
amaury1093 Feb 8, 2021
e3a2412
Rename to forVersion
amaury1093 Feb 8, 2021
6db31d4
Merge branch 'am-8345-migration' of ssh://github.com/cosmos/cosmos-sd…
amaury1093 Feb 8, 2021
55f547e
Fix tests
amaury1093 Feb 8, 2021
8f3b9d4
Check error early
amaury1093 Feb 8, 2021
29b85b7
Return 1 for intiial version
amaury1093 Feb 8, 2021
41e0339
Use MigrationKeeper
amaury1093 Feb 10, 2021
d922003
Fix test
amaury1093 Feb 10, 2021
f422b93
Revert adding marshaler to Configurator
amaury1093 Feb 10, 2021
8cf88fe
Godoc updates
amaury1093 Feb 10, 2021
5add13a
Update docs
amaury1093 Feb 10, 2021
2cef291
Merge branch 'master' into am-8345-migration
aaronc Feb 10, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 28 additions & 1 deletion simapp/app.go
Expand Up @@ -198,6 +198,9 @@ type SimApp struct {

// simulation manager
sm *module.SimulationManager

// the configurator
configurator module.Configurator
}

func init() {
Expand Down Expand Up @@ -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.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{})
Expand Down Expand Up @@ -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.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
//
// `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()
Expand Down
84 changes: 83 additions & 1 deletion simapp/app_test.go
Expand Up @@ -6,10 +6,13 @@ import (
"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) {
Expand Down Expand Up @@ -45,3 +48,82 @@ 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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a functional tests. Shall we add:

    if testing.Short() {
        t.Skip("skipping test in short mode.")
    }

Copy link
Member

Choose a reason for hiding this comment

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

not a bad idea to distinguish short and verbose tests going forward...

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. Though the Makefile currently does not support -short

Copy link
Contributor

Choose a reason for hiding this comment

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

We should park this aside for a second, identify all the various test types (short, verbose, etc), introduce the necessary changes in the Makefile (I'm happy to take care of it), and take it from there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alessio thanks, sounds good. In this case, I didn't add testing.Short() in this test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this tests runs below 0.1s then fine. Otherwise let's add TODO: check testing.Short() flag. 100 tests of 0.1 already give 10s ;)

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 // errors while registering migration
expRegErrMsg string
expRunErr bool // errors while running migration
expRunErrMsg string
robert-zaremba marked this conversation as resolved.
Show resolved Hide resolved
expCalled int
}{
{
"cannot register migration for non-existant module",
"foo",
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, "", 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", false, "", 0,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
var err error

// 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

if tc.moduleName != "" {
err = app.configurator.RegisterMigration(tc.moduleName, 0, func(sdk.KVStore) error {
called++

return nil
})
}

if tc.expRegErr {
require.Error(t, err)
require.Equal(t, tc.expRegErrMsg, err.Error())

return
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
}
require.NoError(t, err)

err = app.RunMigrations(
app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}),
map[string]uint64{"bank": 0},
)
if tc.expRunErr {
require.Error(t, err)
require.Equal(t, tc.expRunErrMsg, err.Error())
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
} else {
require.NoError(t, err)
require.Equal(t, tc.expCalled, called)
}
})
}
}
3 changes: 3 additions & 0 deletions tests/mocks/types_module_module.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions 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.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
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
}
17 changes: 3 additions & 14 deletions types/context_test.go
Expand Up @@ -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"
)

Expand All @@ -26,23 +24,14 @@ 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")
v1 := []byte("world")
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))
Expand All @@ -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)

Expand Down
45 changes: 42 additions & 3 deletions 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
Expand All @@ -15,16 +19,31 @@ 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) error
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
Marshaler() codec.Marshaler
}


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,
migrations: map[string]map[uint64]func(store sdk.KVStore) error{},
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
}
}

var _ Configurator = configurator{}
Expand All @@ -38,3 +57,23 @@ 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) error {
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
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
}
56 changes: 56 additions & 0 deletions types/module/module.go
Expand Up @@ -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"
)

//__________________________________________________________________________________________
Expand Down Expand Up @@ -174,6 +175,9 @@ type AppModule interface {
// RegisterServices allows a module to register services
RegisterServices(Configurator)

// ConsensusVersion tracks state-breaking versions of the module
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
ConsensusVersion() uint64

// ABCI
BeginBlock(sdk.Context, abci.RequestBeginBlock)
EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate
Expand Down Expand Up @@ -208,6 +212,9 @@ 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.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
func (gam GenesisOnlyAppModule) ConsensusVersion() uint64 { return 0 }
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

// BeginBlock returns an empty module begin-block
func (gam GenesisOnlyAppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {}

Expand Down Expand Up @@ -328,6 +335,23 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) map[st
return genesisData
}

// 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 := runMigrations(c, 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.
Expand Down Expand Up @@ -369,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
}
2 changes: 1 addition & 1 deletion types/module/module_test.go
Expand Up @@ -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)

Expand Down