diff --git a/modules/core/02-client/keeper/migrations.go b/modules/core/02-client/keeper/migrations.go index c63a6762f5b..04b0b80cc8b 100644 --- a/modules/core/02-client/keeper/migrations.go +++ b/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) } diff --git a/modules/core/02-client/migrations/v7/expected_keepers.go b/modules/core/02-client/migrations/v7/expected_keepers.go new file mode 100644 index 00000000000..90f034f52c3 --- /dev/null +++ b/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 +} diff --git a/modules/core/02-client/migrations/v7/genesis_test.go b/modules/core/02-client/migrations/v7/genesis_test.go index 990fc7d367e..f758ed94aa9 100644 --- a/modules/core/02-client/migrations/v7/genesis_test.go +++ b/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) diff --git a/modules/core/02-client/migrations/v7/store.go b/modules/core/02-client/migrations/v7/store.go index 2be30467000..20cb2ee2fd0 100644 --- a/modules/core/02-client/migrations/v7/store.go +++ b/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()) diff --git a/modules/core/02-client/migrations/v7/store_test.go b/modules/core/02-client/migrations/v7/store_test.go index c67425ea8bd..006ab637531 100644 --- a/modules/core/02-client/migrations/v7/store_test.go +++ b/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) diff --git a/modules/core/migrations/v7/genesis_test.go b/modules/core/migrations/v7/genesis_test.go index 1ba9fcdf48d..82297d53cdf 100644 --- a/modules/core/migrations/v7/genesis_test.go +++ b/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)