Skip to content

Commit 0b77b98

Browse files
mergify[bot]mmsqedamiannolan
authored
fix: avoid panic when migrate param for newly added host (backport #6167) (#6192)
* fix: avoid panic when migrate param for newly added host (#6167) * fix: avoid panic when migrate param for newly added host * keep default params * Apply suggestions from code review * allow use default params when set nil legacySubspace * Update CHANGELOG.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update CHANGELOG.md * cleanup * refactor: rm setter in icahost migrator and adjust test case * chore: update changelog * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Damian Nolan <damiannolan@gmail.com> (cherry picked from commit c4413c5) * fix: compiler error for NewKeeper args --------- Co-authored-by: mmsqe <mavis@crypto.com> Co-authored-by: Damian Nolan <damiannolan@gmail.com>
1 parent 41970f9 commit 0b77b98

File tree

6 files changed

+49
-6
lines changed

6 files changed

+49
-6
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
5353

5454
### Bug Fixes
5555

56+
* (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.
57+
5658
## [v8.2.0](https://github.com/cosmos/ibc-go/releases/tag/v8.2.0) - 2024-04-05
5759

5860
### Dependencies

modules/apps/27-interchain-accounts/controller/keeper/migrations.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,10 @@ func (m Migrator) AssertChannelCapabilityMigrations(ctx sdk.Context) error {
5555
// MigrateParams migrates the controller submodule's parameters from the x/params to self store.
5656
func (m Migrator) MigrateParams(ctx sdk.Context) error {
5757
if m.keeper != nil {
58-
var params controllertypes.Params
59-
m.keeper.legacySubspace.GetParamSet(ctx, &params)
60-
58+
params := controllertypes.DefaultParams()
59+
if m.keeper.legacySubspace != nil {
60+
m.keeper.legacySubspace.GetParamSetIfExists(ctx, &params)
61+
}
6162
m.keeper.SetParams(ctx, params)
6263
m.keeper.Logger(ctx).Info("successfully migrated ica/controller submodule to self-manage params")
6364
}

modules/apps/27-interchain-accounts/controller/keeper/migrations_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,23 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() {
9292
},
9393
icacontrollertypes.DefaultParams(),
9494
},
95+
{
96+
"success: no legacy params pre-migration",
97+
func() {
98+
suite.chainA.GetSimApp().ICAControllerKeeper = icacontrollerkeeper.NewKeeper(
99+
suite.chainA.Codec,
100+
suite.chainA.GetSimApp().GetKey(icacontrollertypes.StoreKey),
101+
nil, // assign a nil legacy param subspace
102+
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
103+
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
104+
suite.chainA.GetSimApp().IBCKeeper.PortKeeper,
105+
suite.chainA.GetSimApp().ScopedICAControllerKeeper,
106+
suite.chainA.GetSimApp().MsgServiceRouter(),
107+
suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
108+
)
109+
},
110+
icacontrollertypes.DefaultParams(),
111+
},
95112
}
96113

97114
for _, tc := range testCases {

modules/apps/27-interchain-accounts/host/keeper/migrations.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ func NewMigrator(k *Keeper) Migrator {
2121
// MigrateParams migrates the host submodule's parameters from the x/params to self store.
2222
func (m Migrator) MigrateParams(ctx sdk.Context) error {
2323
if m.keeper != nil {
24-
var params types.Params
25-
m.keeper.legacySubspace.GetParamSet(ctx, &params)
26-
24+
params := types.DefaultParams()
25+
if m.keeper.legacySubspace != nil {
26+
m.keeper.legacySubspace.GetParamSetIfExists(ctx, &params)
27+
}
2728
if err := params.Validate(); err != nil {
2829
return err
2930
}

modules/apps/27-interchain-accounts/host/keeper/migrations_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package keeper_test
33
import (
44
"fmt"
55

6+
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
7+
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
8+
69
icahostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper"
710
icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
811
)
@@ -22,6 +25,24 @@ func (suite *KeeperTestSuite) TestMigratorMigrateParams() {
2225
},
2326
icahosttypes.DefaultParams(),
2427
},
28+
{
29+
"success: no legacy params pre-migration",
30+
func() {
31+
suite.chainA.GetSimApp().ICAHostKeeper = icahostkeeper.NewKeeper(
32+
suite.chainA.Codec,
33+
suite.chainA.GetSimApp().GetKey(icahosttypes.StoreKey),
34+
nil, // assign a nil legacy param subspace
35+
suite.chainA.GetSimApp().IBCFeeKeeper,
36+
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
37+
suite.chainA.GetSimApp().IBCKeeper.PortKeeper,
38+
suite.chainA.GetSimApp().AccountKeeper,
39+
suite.chainA.GetSimApp().ScopedICAHostKeeper,
40+
suite.chainA.GetSimApp().MsgServiceRouter(),
41+
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
42+
)
43+
},
44+
icahosttypes.DefaultParams(),
45+
},
2546
}
2647

2748
for _, tc := range testCases {

modules/apps/27-interchain-accounts/types/expected_keepers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,5 @@ type PortKeeper interface {
3737
// ParamSubspace defines the expected Subspace interface for module parameters.
3838
type ParamSubspace interface {
3939
GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet)
40+
GetParamSetIfExists(ctx sdk.Context, ps paramtypes.ParamSet)
4041
}

0 commit comments

Comments
 (0)