Add migration for checksums to collections.KeySet (#5324) (#5328)

* wip, we're migrating

* add first draft of tests.

* Add Logger, log in migrations, use more specific name, handle store errors.

* Update modules/light-clients/08-wasm/keeper/migrations.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Mark key, Checksums msg as deprecated.

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit 2a39b26)

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

modules/light-clients/08-wasm/keeper/keeper.go

@@ -11,21 +11,24 @@ import (
 
 	storetypes "cosmossdk.io/core/store"
 	errorsmod "cosmossdk.io/errors"
+	"cosmossdk.io/log"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
 	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
+	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )
 
 // Keeper defines the 08-wasm keeper
 type Keeper struct {
 	// implements gRPC QueryServer interface
 	types.QueryServer
 
-	cdc codec.BinaryCodec
+	cdc          codec.BinaryCodec
+	storeService storetypes.KVStoreService
 
 	clientKeeper types.ClientKeeper
 
@@ -65,6 +68,7 @@ func NewKeeperWithVM(
 
 	return Keeper{
 		cdc:          cdc,
+		storeService: storeService,
 		clientKeeper: clientKeeper,
 		authority:    authority,
 	}
@@ -94,6 +98,11 @@ func (k Keeper) GetAuthority() string {
 	return k.authority
 }
 
+// Logger returns a module-specific logger.
+func (Keeper) Logger(ctx sdk.Context) log.Logger {
+	return ctx.Logger().With("module", "x/"+exported.ModuleName+"-"+types.ModuleName)
+}
+
 func (Keeper) storeWasmCode(ctx sdk.Context, code []byte, storeFn func(code wasmvm.WasmCode) (wasmvm.Checksum, error)) ([]byte, error) {
 	var err error
 	if types.IsGzip(code) {

modules/light-clients/08-wasm/keeper/migrations.go

@@ -0,0 +1,73 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
+	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
+)
+
+// Migrator is a struct for handling in-place store migrations.
+type Migrator struct {
+	keeper Keeper
+}
+
+// NewMigrator returns a new Migrator.
+func NewMigrator(keeper Keeper) Migrator {
+	return Migrator{
+		keeper: keeper,
+	}
+}
+
+// MigrateChecksums migrates the wasm store from using a single key to
+// store a list of checksums to using a collections.KeySet to store the checksums.
+//
+// It grabs the checksums stored previously under the old key and stores
+// them in the global KeySet collection. It then deletes the old key and
+// the checksums stored under it.
+func (m Migrator) MigrateChecksums(ctx sdk.Context) error {
+	checksums, err := m.getStoredChecksums(ctx)
+	if err != nil {
+		return err
+	}
+
+	for _, hash := range checksums {
+		if err := ibcwasm.Checksums.Set(ctx, hash); err != nil {
+			return err
+		}
+	}
+
+	// delete the previously stored checksums
+	if err := m.deleteChecksums(ctx); err != nil {
+		return err
+	}
+
+	m.keeper.Logger(ctx).Info("successfully migrated Checksums to collections")
+	return nil
+}
+
+// getStoredChecksums returns the checksums stored under the KeyChecksums key.
+func (m Migrator) getStoredChecksums(ctx sdk.Context) ([][]byte, error) {
+	store := m.keeper.storeService.OpenKVStore(ctx)
+
+	bz, err := store.Get([]byte(types.KeyChecksums))
+	if err != nil {
+		return [][]byte{}, err
+	}
+
+	var hashes types.Checksums
+	err = m.keeper.cdc.Unmarshal(bz, &hashes)
+	if err != nil {
+		return [][]byte{}, err
+	}
+
+	return hashes.Checksums, nil
+}
+
+// deleteChecksums deletes the checksums stored under the KeyChecksums key.
+func (m Migrator) deleteChecksums(ctx sdk.Context) error {
+	store := m.keeper.storeService.OpenKVStore(ctx)
+	err := store.Delete([]byte(types.KeyChecksums))
+
+	return err
+}

modules/light-clients/08-wasm/keeper/migrations_test.go

@@ -0,0 +1,61 @@
+package keeper_test
+
+import (
+	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
+	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/keeper"
+	"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
+)
+
+func (suite *KeeperTestSuite) TestMigrateWasmStore() {
+	testCases := []struct {
+		name      string
+		checksums [][]byte
+	}{
+		{
+			"success: empty checksums",
+			[][]byte{},
+		},
+		{
+			"success: multiple checksums",
+			[][]byte{[]byte("hash1"), []byte("hash2"), []byte("hash3")},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			suite.storeChecksums(tc.checksums)
+
+			// run the migration
+			wasmKeeper := GetSimApp(suite.chainA).WasmClientKeeper
+			m := keeper.NewMigrator(wasmKeeper)
+
+			err := m.MigrateChecksums(suite.chainA.GetContext())
+			suite.Require().NoError(err)
+
+			// check that they were stored in KeySet
+			for _, hash := range tc.checksums {
+				suite.Require().True(ibcwasm.Checksums.Has(suite.chainA.GetContext(), hash))
+			}
+
+			// check that the data under the old key was deleted
+			store := suite.chainA.GetContext().KVStore(GetSimApp(suite.chainA).GetKey(types.StoreKey))
+			suite.Require().Nil(store.Get([]byte(types.KeyChecksums)))
+		})
+	}
+}
+
+// storeChecksums stores the given checksums under the KeyChecksums key, it runs
+// each time on an empty store so we don't need to read the previous checksums.
+func (suite *KeeperTestSuite) storeChecksums(checksums [][]byte) {
+	ctx := suite.chainA.GetContext()
+
+	store := ctx.KVStore(GetSimApp(suite.chainA).GetKey(types.StoreKey))
+	checksum := types.Checksums{Checksums: checksums}
+	bz, err := GetSimApp(suite.chainA).AppCodec().Marshal(&checksum)
+	suite.Require().NoError(err)
+
+	store.Set([]byte(types.KeyChecksums), bz)
+}

modules/light-clients/08-wasm/module.go

@@ -111,10 +111,15 @@ func NewAppModule(k keeper.Keeper) AppModule {
 func (am AppModule) RegisterServices(cfg module.Configurator) {
 	types.RegisterMsgServer(cfg.MsgServer(), am.keeper)
 	types.RegisterQueryServer(cfg.QueryServer(), am.keeper)
+
+	wasmMigrator := keeper.NewMigrator(am.keeper)
+	if err := cfg.RegisterMigration(types.ModuleName, 1, wasmMigrator.MigrateChecksums); err != nil {
+		panic(fmt.Errorf("failed to migrate checksums from version 1 to 2: %v", err))
+	}
 }
 
 // ConsensusVersion implements AppModule/ConsensusVersion.
-func (AppModule) ConsensusVersion() uint64 { return 1 }
+func (AppModule) ConsensusVersion() uint64 { return 2 }
 
 // ProposalMsgs returns msgs used for governance proposals for simulations.
 func (AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.WeightedProposalMsg {

modules/light-clients/08-wasm/types/keys.go

@@ -9,4 +9,8 @@ const (
 
 	// Wasm is the client type for IBC light clients created using 08-wasm
 	Wasm = ModuleName
+
+	// KeyChecksums is the key under which all checksums are stored
+	// Deprecated: in favor of collections.KeySet
+	KeyChecksums = "checksums"
 )