From 3d0982997eadf1ba89abf42484fb750609f7a955 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 16 Apr 2024 09:18:32 +0800 Subject: [PATCH 01/12] fix: avoid panic when migrate param for newly added host --- CHANGELOG.md | 1 + .../apps/27-interchain-accounts/host/keeper/migrations.go | 3 +-- .../27-interchain-accounts/host/keeper/migrations_test.go | 5 +++++ .../apps/27-interchain-accounts/types/expected_keepers.go | 1 + 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf37396300c..0c7ff2b5d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#6055](https://github.com/cosmos/ibc-go/pull/6055) Introduce a new interface `ConsensusHost` used to validate an IBC `ClientState` and `ConsensusState` against the host chain's underlying consensus parameters. ### Bug Fixes +* (apps/27-interchain-accounts) [\#](https://github.com/cosmos/ibc-go/pull/) Avoid panic when migrate param for newly added host. ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 894018606c7..1953edbcdb8 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -22,8 +22,7 @@ func NewMigrator(k *Keeper) Migrator { func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { var params types.Params - m.keeper.legacySubspace.GetParamSet(ctx, ¶ms) - + m.keeper.legacySubspace.GetParamSetIfExists(ctx, ¶ms) if err := params.Validate(); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go index e47e40f100f..992b1c3bd72 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go @@ -22,6 +22,11 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { }, icahosttypes.DefaultParams(), }, + { + "success: no params", + func() {}, + icahosttypes.Params{HostEnabled: false}, + }, } for _, tc := range testCases { diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index de2eb9de245..7ea7bc795d7 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -37,4 +37,5 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) + GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet) } From 0f5258244c9e117c13179a73e3f7be1859eeff39 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 16 Apr 2024 09:22:44 +0800 Subject: [PATCH 02/12] keep default params --- CHANGELOG.md | 2 +- .../host/keeper/migrations.go | 17 ++++++++++++++++- .../host/keeper/migrations_test.go | 2 +- .../types/expected_keepers.go | 4 +++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c7ff2b5d59..5b9ff388e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#6055](https://github.com/cosmos/ibc-go/pull/6055) Introduce a new interface `ConsensusHost` used to validate an IBC `ClientState` and `ConsensusState` against the host chain's underlying consensus parameters. ### Bug Fixes -* (apps/27-interchain-accounts) [\#](https://github.com/cosmos/ibc-go/pull/) Avoid panic when migrate param for newly added host. +* (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Avoid panic when migrate param for newly added host. ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 1953edbcdb8..3378c6406ef 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -21,8 +21,23 @@ func NewMigrator(k *Keeper) Migrator { // MigrateParams migrates the host submodule's parameters from the x/params to self store. func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { + defaultParams := types.DefaultParams() var params types.Params - m.keeper.legacySubspace.GetParamSetIfExists(ctx, ¶ms) + ps := ¶ms + for _, pair := range ps.ParamSetPairs() { + if !m.keeper.legacySubspace.Has(ctx, pair.Key) { + var value interface{} + if string(pair.Key) == "HostEnabled" { + value = defaultParams.HostEnabled + } else if string(pair.Key) == "AllowMessages" { + value = defaultParams.AllowMessages + } else { + continue + } + m.keeper.legacySubspace.Set(ctx, pair.Key, value) + } + m.keeper.legacySubspace.Get(ctx, pair.Key, pair.Value) + } if err := params.Validate(); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go index 992b1c3bd72..0b9bdee9ee9 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go @@ -25,7 +25,7 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { { "success: no params", func() {}, - icahosttypes.Params{HostEnabled: false}, + icahosttypes.DefaultParams(), }, } diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 7ea7bc795d7..9fefc78c45f 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -37,5 +37,7 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) - GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet) + Has(ctx sdk.Context, key []byte) bool + Get(ctx sdk.Context, key []byte, ptr interface{}) + Set(ctx sdk.Context, key []byte, value interface{}) } From ef89bbe9a73a226a48cbeed90f24ce88fab81dbd Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 16 Apr 2024 11:04:12 +0800 Subject: [PATCH 03/12] Apply suggestions from code review --- .../apps/27-interchain-accounts/host/keeper/migrations.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 3378c6406ef..31a8c4f8ba2 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -27,11 +27,12 @@ func (m Migrator) MigrateParams(ctx sdk.Context) error { for _, pair := range ps.ParamSetPairs() { if !m.keeper.legacySubspace.Has(ctx, pair.Key) { var value interface{} - if string(pair.Key) == "HostEnabled" { + switch string(pair.Key) { + case "HostEnabled": value = defaultParams.HostEnabled - } else if string(pair.Key) == "AllowMessages" { + case "AllowMessages": value = defaultParams.AllowMessages - } else { + default: continue } m.keeper.legacySubspace.Set(ctx, pair.Key, value) From eb28d2cb18cffc09b6784d1135ac7dbba82b4571 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 17 Apr 2024 00:20:54 +0800 Subject: [PATCH 04/12] allow use default params when set nil legacySubspace --- .../host/keeper/migrations.go | 30 ++++++++----------- .../host/keeper/migrations_test.go | 12 ++++++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 31a8c4f8ba2..96028872f89 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -4,6 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" + icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" ) // Migrator is a struct for handling in-place state migrations. @@ -18,29 +19,22 @@ func NewMigrator(k *Keeper) Migrator { } } +// SetLegacySubspace sets the legacy parameter subspace for the Migrator. +func (m Migrator) SetLegacySubspace(legacySubspace icatypes.ParamSubspace) { + m.keeper.legacySubspace = legacySubspace +} + // MigrateParams migrates the host submodule's parameters from the x/params to self store. func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { - defaultParams := types.DefaultParams() var params types.Params - ps := ¶ms - for _, pair := range ps.ParamSetPairs() { - if !m.keeper.legacySubspace.Has(ctx, pair.Key) { - var value interface{} - switch string(pair.Key) { - case "HostEnabled": - value = defaultParams.HostEnabled - case "AllowMessages": - value = defaultParams.AllowMessages - default: - continue - } - m.keeper.legacySubspace.Set(ctx, pair.Key, value) + if m.keeper.legacySubspace == nil { + params = types.DefaultParams() + } else { + m.keeper.legacySubspace.GetParamSet(ctx, ¶ms) + if err := params.Validate(); err != nil { + return err } - m.keeper.legacySubspace.Get(ctx, pair.Key, pair.Value) - } - if err := params.Validate(); err != nil { - return err } m.keeper.SetParams(ctx, params) m.keeper.Logger(ctx).Info("successfully migrated ica/host submodule to self-manage params") diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go index 0b9bdee9ee9..224eec0ba3c 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go @@ -9,9 +9,10 @@ import ( func (suite *KeeperTestSuite) TestMigratorMigrateParams() { testCases := []struct { - msg string - malleate func() - expectedParams icahosttypes.Params + msg string + malleate func() + expectedParams icahosttypes.Params + useDefaultParams bool }{ { "success: default params", @@ -21,11 +22,13 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { subspace.SetParamSet(suite.chainA.GetContext(), ¶ms) // set params }, icahosttypes.DefaultParams(), + false, }, { "success: no params", func() {}, icahosttypes.DefaultParams(), + true, }, } @@ -38,6 +41,9 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { tc.malleate() // explicitly set params migrator := icahostkeeper.NewMigrator(&suite.chainA.GetSimApp().ICAHostKeeper) + if tc.useDefaultParams { + migrator.SetLegacySubspace(nil) + } err := migrator.MigrateParams(suite.chainA.GetContext()) suite.Require().NoError(err) From 215785d2b504bf5ea1cb20f584cb520e53fdb62a Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 18 Apr 2024 11:43:41 +0200 Subject: [PATCH 05/12] Update CHANGELOG.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 943e0922e3f..71f5add3fd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,8 +59,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#6055](https://github.com/cosmos/ibc-go/pull/6055) Introduce a new interface `ConsensusHost` used to validate an IBC `ClientState` and `ConsensusState` against the host chain's underlying consensus parameters. ### Bug Fixes + * (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Avoid panic when migrate param for newly added host. + ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 ### Dependencies From 3ffc86cab4f46ea6198d29c06a749b7be1763898 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 18 Apr 2024 11:43:59 +0200 Subject: [PATCH 06/12] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71f5add3fd4..dce2e3bd0d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,7 +62,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Avoid panic when migrate param for newly added host. - ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 ### Dependencies From 338df0b2610ed94cb6258e650301ad3cab856290 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 17 Apr 2024 08:25:40 +0800 Subject: [PATCH 07/12] cleanup --- modules/apps/27-interchain-accounts/types/expected_keepers.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 9fefc78c45f..de2eb9de245 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -37,7 +37,4 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) - Has(ctx sdk.Context, key []byte) bool - Get(ctx sdk.Context, key []byte, ptr interface{}) - Set(ctx sdk.Context, key []byte, value interface{}) } From 8cb18ce22674a7d6ad3b95fd0b62623a1fb1cdca Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 18 Apr 2024 15:00:37 +0200 Subject: [PATCH 08/12] refactor: rm setter in icahost migrator and adjust test case --- .../host/keeper/migrations.go | 6 ---- .../host/keeper/migrations_test.go | 33 ++++++++++++------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 96028872f89..1031bc9c532 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -4,7 +4,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" - icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" ) // Migrator is a struct for handling in-place state migrations. @@ -19,11 +18,6 @@ func NewMigrator(k *Keeper) Migrator { } } -// SetLegacySubspace sets the legacy parameter subspace for the Migrator. -func (m Migrator) SetLegacySubspace(legacySubspace icatypes.ParamSubspace) { - m.keeper.legacySubspace = legacySubspace -} - // MigrateParams migrates the host submodule's parameters from the x/params to self store. func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go index 224eec0ba3c..1f15f947aab 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations_test.go @@ -3,16 +3,18 @@ package keeper_test import ( "fmt" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + icahostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper" icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" ) func (suite *KeeperTestSuite) TestMigratorMigrateParams() { testCases := []struct { - msg string - malleate func() - expectedParams icahosttypes.Params - useDefaultParams bool + msg string + malleate func() + expectedParams icahosttypes.Params }{ { "success: default params", @@ -22,13 +24,25 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { subspace.SetParamSet(suite.chainA.GetContext(), ¶ms) // set params }, icahosttypes.DefaultParams(), - false, }, { - "success: no params", - func() {}, + "success: no legacy params pre-migration", + func() { + suite.chainA.GetSimApp().ICAHostKeeper = icahostkeeper.NewKeeper( + suite.chainA.Codec, + suite.chainA.GetSimApp().GetKey(icahosttypes.StoreKey), + nil, // assign a nil legacy param subspace + suite.chainA.GetSimApp().IBCFeeKeeper, + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper, + suite.chainA.GetSimApp().IBCKeeper.PortKeeper, + suite.chainA.GetSimApp().AccountKeeper, + suite.chainA.GetSimApp().ScopedICAHostKeeper, + suite.chainA.GetSimApp().MsgServiceRouter(), + suite.chainA.GetSimApp().GRPCQueryRouter(), + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + }, icahosttypes.DefaultParams(), - true, }, } @@ -41,9 +55,6 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { tc.malleate() // explicitly set params migrator := icahostkeeper.NewMigrator(&suite.chainA.GetSimApp().ICAHostKeeper) - if tc.useDefaultParams { - migrator.SetLegacySubspace(nil) - } err := migrator.MigrateParams(suite.chainA.GetContext()) suite.Require().NoError(err) From 5c1b9ec85936c81ea0180c47abc533f1af62b6eb Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 18 Apr 2024 15:03:08 +0200 Subject: [PATCH 09/12] chore: update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dce2e3bd0d7..7c7293f4970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Avoid panic when migrate param for newly added host. +* (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Fixed an edge case bug where migrating params for a pre-existing ica module which implemented controller functionality only could panic when migrating params for newly added host. ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 From 0c5c4402aadd351a2ef820171aa43f00a8f5b736 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 22 Apr 2024 17:14:36 +0800 Subject: [PATCH 10/12] Apply suggestions from code review --- .../host/keeper/migrations.go | 14 ++++++-------- .../types/expected_keepers.go | 1 + 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/keeper/migrations.go b/modules/apps/27-interchain-accounts/host/keeper/migrations.go index 1031bc9c532..992027d92f1 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/host/keeper/migrations.go @@ -21,14 +21,12 @@ func NewMigrator(k *Keeper) Migrator { // MigrateParams migrates the host submodule's parameters from the x/params to self store. func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { - var params types.Params - if m.keeper.legacySubspace == nil { - params = types.DefaultParams() - } else { - m.keeper.legacySubspace.GetParamSet(ctx, ¶ms) - if err := params.Validate(); err != nil { - return err - } + params := types.DefaultParams() + if m.keeper.legacySubspace != nil { + m.keeper.legacySubspace.GetParamSetIfExists(ctx, ¶ms) + } + if err := params.Validate(); err != nil { + return err } m.keeper.SetParams(ctx, params) m.keeper.Logger(ctx).Info("successfully migrated ica/host submodule to self-manage params") diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index de2eb9de245..7ea7bc795d7 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -37,4 +37,5 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) + GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet) } From 7f45153752d97a9c8279dda039be1d4771e81cc4 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 22 Apr 2024 17:16:49 +0800 Subject: [PATCH 11/12] Apply suggestions from code review --- CHANGELOG.md | 2 +- .../controller/keeper/migrations.go | 7 ++++--- .../controller/keeper/migrations_test.go | 17 +++++++++++++++++ .../types/expected_keepers.go | 1 - 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7293f4970..7d88d921e18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Fixed an edge case bug where migrating params for a pre-existing ica module which implemented controller functionality only could panic when migrating params for newly added host. +* (apps/27-interchain-accounts) [\#6167](https://github.com/cosmos/ibc-go/pull/6167) Fixed an edge case bug where migrating params for a pre-existing ica module which implemented controller functionality only could panic when migrating params for newly added host, and align controller param migration with host. ## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05 diff --git a/modules/apps/27-interchain-accounts/controller/keeper/migrations.go b/modules/apps/27-interchain-accounts/controller/keeper/migrations.go index 705d93cb4b4..77f62a73c5d 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/migrations.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/migrations.go @@ -55,9 +55,10 @@ func (m Migrator) AssertChannelCapabilityMigrations(ctx sdk.Context) error { // MigrateParams migrates the controller submodule's parameters from the x/params to self store. func (m Migrator) MigrateParams(ctx sdk.Context) error { if m.keeper != nil { - var params controllertypes.Params - m.keeper.legacySubspace.GetParamSet(ctx, ¶ms) - + params := controllertypes.DefaultParams() + if m.keeper.legacySubspace != nil { + m.keeper.legacySubspace.GetParamSetIfExists(ctx, ¶ms) + } m.keeper.SetParams(ctx, params) m.keeper.Logger(ctx).Info("successfully migrated ica/controller submodule to self-manage params") } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/migrations_test.go b/modules/apps/27-interchain-accounts/controller/keeper/migrations_test.go index da5be84b6f5..f0011439d14 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/migrations_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/migrations_test.go @@ -92,6 +92,23 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() { }, icacontrollertypes.DefaultParams(), }, + { + "success: no legacy params pre-migration", + func() { + suite.chainA.GetSimApp().ICAControllerKeeper = icacontrollerkeeper.NewKeeper( + suite.chainA.Codec, + suite.chainA.GetSimApp().GetKey(icacontrollertypes.StoreKey), + nil, // assign a nil legacy param subspace + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper, + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper, + suite.chainA.GetSimApp().IBCKeeper.PortKeeper, + suite.chainA.GetSimApp().ScopedICAControllerKeeper, + suite.chainA.GetSimApp().MsgServiceRouter(), + suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(), + ) + }, + icacontrollertypes.DefaultParams(), + }, } for _, tc := range testCases { diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 7ea7bc795d7..6dcb12a0bcf 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -36,6 +36,5 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { - GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet) } From c2d03976530ce48c003d5707bbb77e53ed138727 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 22 Apr 2024 19:42:44 +0800 Subject: [PATCH 12/12] Apply suggestions from code review --- modules/apps/27-interchain-accounts/types/expected_keepers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 6dcb12a0bcf..7ea7bc795d7 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -36,5 +36,6 @@ type PortKeeper interface { // ParamSubspace defines the expected Subspace interface for module parameters. type ParamSubspace interface { + GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet) }