refactor: simplify automatic migration code by using client keeper fu…

…nctions (cosmos#2864)

modules/core/02-client/keeper/migrations.go

@@ -34,5 +34,5 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error {
 // - removes the localhost client
 // - asserts that existing tendermint clients are properly registered on the chain codec
 func (m Migrator) Migrate2to3(ctx sdk.Context) error {
-	return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
+	return v7.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.keeper)
 }

modules/core/02-client/migrations/v7/expected_keepers.go

@@ -0,0 +1,14 @@
+package v7
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/v6/modules/core/exported"
+)
+
+// ClientKeeper expected IBC client keeper
+type ClientKeeper interface {
+	GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool)
+	SetClientState(ctx sdk.Context, clientID string, clientState exported.ClientState)
+	ClientStore(ctx sdk.Context, clientID string) sdk.KVStore
+}

modules/core/02-client/migrations/v7/genesis_test.go

@@ -101,7 +101,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
 	// migrate store get expected genesis
 	// store migration and genesis migration should produce identical results
 	// NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients
-	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec())
+	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper)
 	suite.Require().NoError(err)
 	expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)
 

modules/core/02-client/migrations/v7/store.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
-	"github.com/cosmos/cosmos-sdk/store/prefix"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -29,18 +28,18 @@ const Localhost string = "09-localhost"
 // - Pruning all solo machine consensus states
 // - Removing the localhost client
 // - Asserting existing tendermint clients are properly registered on the chain codec
-func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
+func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
 	store := ctx.KVStore(storeKey)
 
-	if err := handleSolomachineMigration(ctx, store, cdc); err != nil {
+	if err := handleSolomachineMigration(ctx, store, cdc, clientKeeper); err != nil {
 		return err
 	}
 
-	if err := handleTendermintMigration(ctx, store, cdc); err != nil {
+	if err := handleTendermintMigration(ctx, store, cdc, clientKeeper); err != nil {
 		return err
 	}
 
-	if err := handleLocalhostMigration(ctx, store, cdc); err != nil {
+	if err := handleLocalhostMigration(ctx, store, cdc, clientKeeper); err != nil {
 		return err
 	}
 
@@ -49,15 +48,14 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar
 
 // handleSolomachineMigration iterates over the solo machine clients and migrates client state from
 // protobuf definition v2 to v3. All consensus states stored outside of the client state are pruned.
-func handleSolomachineMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec) error {
+func handleSolomachineMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
 	clients, err := collectClients(ctx, store, exported.Solomachine)
 	if err != nil {
 		return err
 	}
 
 	for _, clientID := range clients {
-		clientPrefix := []byte(fmt.Sprintf("%s/%s/", host.KeyClientStorePrefix, clientID))
-		clientStore := prefix.NewStore(store, clientPrefix)
+		clientStore := clientKeeper.ClientStore(ctx, clientID)
 
 		bz := clientStore.Get(host.ClientStateKey())
 		if bz == nil {
@@ -76,13 +74,8 @@ func handleSolomachineMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.Bi
 
 		updatedClientState := migrateSolomachine(clientState)
 
-		bz, err := clienttypes.MarshalClientState(cdc, &updatedClientState)
-		if err != nil {
-			return sdkerrors.Wrap(err, "failed to unmarshal client state bytes into solo machine client state")
-		}
-
 		// update solomachine in store
-		clientStore.Set(host.ClientStateKey(), bz)
+		clientKeeper.SetClientState(ctx, clientID, &updatedClientState)
 
 		removeAllClientConsensusStates(clientStore)
 	}
@@ -92,7 +85,7 @@ func handleSolomachineMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.Bi
 
 // handlerTendermintMigration asserts that the tendermint client in state can be decoded properly.
 // This ensures the upgrading chain properly registered the tendermint client types on the chain codec.
-func handleTendermintMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec) error {
+func handleTendermintMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
 	clients, err := collectClients(ctx, store, exported.Tendermint)
 	if err != nil {
 		return err
@@ -104,20 +97,14 @@ func handleTendermintMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.Bin
 
 	clientID := clients[0]
 
-	clientPrefix := []byte(fmt.Sprintf("%s/%s/", host.KeyClientStorePrefix, clientID))
-	clientStore := prefix.NewStore(store, clientPrefix)
-
-	bz := clientStore.Get(host.ClientStateKey())
-	if bz == nil {
-		return clienttypes.ErrClientNotFound
-	}
-
-	var clientState exported.ClientState
-	if err := cdc.UnmarshalInterface(bz, &clientState); err != nil {
-		return sdkerrors.Wrap(err, "failed to unmarshal client state bytes into tendermint client state")
+	// unregistered tendermint client types will panic when unmarshaling the client state
+	// in GetClientState
+	clientState, ok := clientKeeper.GetClientState(ctx, clientID)
+	if !ok {
+		return sdkerrors.Wrapf(clienttypes.ErrClientNotFound, "clientID %s", clientID)
 	}
 
-	_, ok := clientState.(*ibctm.ClientState)
+	_, ok = clientState.(*ibctm.ClientState)
 	if !ok {
 		return sdkerrors.Wrap(clienttypes.ErrInvalidClient, "client state is not tendermint even though client id contains 07-tendermint")
 	}
@@ -126,15 +113,14 @@ func handleTendermintMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.Bin
 }
 
 // handleLocalhostMigration removes all client and consensus states associated with the localhost client type.
-func handleLocalhostMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec) error {
+func handleLocalhostMigration(ctx sdk.Context, store sdk.KVStore, cdc codec.BinaryCodec, clientKeeper ClientKeeper) error {
 	clients, err := collectClients(ctx, store, Localhost)
 	if err != nil {
 		return err
 	}
 
 	for _, clientID := range clients {
-		clientPrefix := []byte(fmt.Sprintf("%s/%s/", host.KeyClientStorePrefix, clientID))
-		clientStore := prefix.NewStore(store, clientPrefix)
+		clientStore := clientKeeper.ClientStore(ctx, clientID)
 
 		// delete the client state
 		clientStore.Delete(host.ClientStateKey())

modules/core/02-client/migrations/v7/store_test.go

@@ -4,15 +4,11 @@ import (
 	"strconv"
 	"testing"
 
-	"github.com/cosmos/cosmos-sdk/codec"
-	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/ibc-go/v6/modules/core/02-client/migrations/v7"
 	"github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
 	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
-	solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine"
-	ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint"
 	ibctesting "github.com/cosmos/ibc-go/v6/testing"
 )
 
@@ -40,23 +36,6 @@ func TestIBCTestSuite(t *testing.T) {
 	suite.Run(t, new(MigrationsV7TestSuite))
 }
 
-// test that MigrateStore returns an error if the codec used doesn't register tendermint types
-func (suite *MigrationsV7TestSuite) TestMigrateStoreTendermint() {
-	path := ibctesting.NewPath(suite.chainA, suite.chainB)
-	suite.coordinator.SetupClients(path)
-
-	registry := codectypes.NewInterfaceRegistry()
-	cdc := codec.NewProtoCodec(registry)
-
-	solomachine.RegisterInterfaces(registry)
-	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), cdc)
-	suite.Require().Error(err)
-
-	ibctm.RegisterInterfaces(registry)
-	err = v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), cdc)
-	suite.Require().NoError(err)
-}
-
 // create multiple solo machine clients, tendermint and localhost clients
 // ensure that solo machine clients are migrated and their consensus states are removed
 // ensure the localhost is deleted entirely.
@@ -79,7 +58,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateStore() {
 	suite.createSolomachineClients(solomachines)
 	suite.createLocalhostClients()
 
-	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec())
+	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper)
 	suite.Require().NoError(err)
 
 	suite.assertSolomachineClients(solomachines)

modules/core/migrations/v7/genesis_test.go

@@ -128,7 +128,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
 	// migrate store get expected genesis
 	// store migration and genesis migration should produce identical results
 	// NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients
-	err := clientv7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec())
+	err := clientv7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(host.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper)
 	suite.Require().NoError(err)
 	expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)