Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp!: use expected interface for legacy params subspace #4811

Merged
merged 2 commits into from
Oct 4, 2023
Merged

imp!: use expected interface for legacy params subspace #4811

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72b02c7
imp: use expected interface for legacy params subspace
damiannolan Oct 4, 2023
16ecdb5
chore: define GetParamSet on expected interface and rm explicit type …
damiannolan Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions modules/apps/27-interchain-accounts/controller/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types"
Expand All @@ -28,7 +27,7 @@ import (
type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.Codec
legacySubspace paramtypes.Subspace
legacySubspace icatypes.ParamSubspace
ics4Wrapper porttypes.ICS4Wrapper
channelKeeper icatypes.ChannelKeeper
portKeeper icatypes.PortKeeper
Expand All @@ -44,7 +43,7 @@ type Keeper struct {

// NewKeeper creates a new interchain accounts controller Keeper instance
func NewKeeper(
cdc codec.Codec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace,
cdc codec.Codec, key storetypes.StoreKey, legacySubspace icatypes.ParamSubspace,
ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
scopedKeeper exported.ScopedKeeper, msgRouter icatypes.MessageRouter, authority string,
) Keeper {
Expand Down
9 changes: 8 additions & 1 deletion modules/apps/27-interchain-accounts/controller/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

// Migrator is a struct for handling in-place store migrations.
Expand Down Expand Up @@ -55,8 +57,13 @@ 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 {
legacySubpsace, ok := m.keeper.legacySubspace.(paramtypes.Subspace)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", paramtypes.Subspace{}, m.keeper.legacySubspace)
}

var params controllertypes.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)
legacySubpsace.GetParamSet(ctx, &params)

m.keeper.SetParams(ctx, params)
m.keeper.Logger(ctx).Info("successfully migrated ica/controller submodule to self-manage params")
Expand Down
5 changes: 2 additions & 3 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
genesistypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/genesis/types"
Expand All @@ -27,7 +26,7 @@ import (
type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.Codec
legacySubspace paramtypes.Subspace
legacySubspace icatypes.ParamSubspace

ics4Wrapper porttypes.ICS4Wrapper
channelKeeper icatypes.ChannelKeeper
Expand All @@ -45,7 +44,7 @@ type Keeper struct {

// NewKeeper creates a new interchain accounts host Keeper instance
func NewKeeper(
cdc codec.Codec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace,
cdc codec.Codec, key storetypes.StoreKey, legacySubspace icatypes.ParamSubspace,
ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
accountKeeper icatypes.AccountKeeper, scopedKeeper exported.ScopedKeeper, msgRouter icatypes.MessageRouter,
authority string,
Expand Down
11 changes: 10 additions & 1 deletion modules/apps/27-interchain-accounts/host/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package keeper

import (
errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

// Migrator is a struct for handling in-place state migrations.
Expand All @@ -21,8 +25,13 @@ 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 {
legacySubpsace, ok := m.keeper.legacySubspace.(paramtypes.Subspace)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", paramtypes.Subspace{}, m.keeper.legacySubspace)
}

var params types.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)
legacySubpsace.GetParamSet(ctx, &params)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not include this function in the interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that if you'd prefer. I essentially went with copy/pasta of the same interface that the sdk had used and doing a strict type conversion in the migrations so an error will be returned explicitly if needs be

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On reflection, you get the same compiler safety by defining this as the expected interface func and not doing a strict type check 😅 I'll make the change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only difference is that you'd get a runtime panic in migrations when passing nil, as opposed to a runtime error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code panics anyways on migration error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the other subspace fns in the interface needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, they were boilerpalte. Updated to use GetParamSet on the interface now.


if err := params.Validate(); err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions modules/apps/27-interchain-accounts/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,9 @@ type PortKeeper interface {
BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability
IsBound(ctx sdk.Context, portID string) bool
}

// ParamSubspace defines the expected Subspace interface for module parameters.
type ParamSubspace interface {
Get(ctx sdk.Context, key []byte, ptr interface{})
Set(ctx sdk.Context, key []byte, param interface{})
}
5 changes: 2 additions & 3 deletions modules/apps/transfer/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

tmbytes "github.com/cometbft/cometbft/libs/bytes"

Expand All @@ -28,7 +27,7 @@ import (
type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.BinaryCodec
legacySubspace paramtypes.Subspace
legacySubspace types.ParamSubspace

ics4Wrapper porttypes.ICS4Wrapper
channelKeeper types.ChannelKeeper
Expand All @@ -46,7 +45,7 @@ type Keeper struct {
func NewKeeper(
cdc codec.BinaryCodec,
key storetypes.StoreKey,
legacySubspace paramtypes.Subspace,
legacySubspace types.ParamSubspace,
ics4Wrapper porttypes.ICS4Wrapper,
channelKeeper types.ChannelKeeper,
portKeeper types.PortKeeper,
Expand Down
11 changes: 10 additions & 1 deletion modules/apps/transfer/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@ package keeper
import (
"fmt"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

// Migrator is a struct for handling in-place store migrations.
Expand All @@ -22,8 +26,13 @@ func NewMigrator(keeper Keeper) Migrator {

// MigrateParams migrates the transfer module's parameters from the x/params to self store.
func (m Migrator) MigrateParams(ctx sdk.Context) error {
legacySubpsace, ok := m.keeper.legacySubspace.(paramtypes.Subspace)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", paramtypes.Subspace{}, m.keeper.legacySubspace)
}

var params types.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)
legacySubpsace.GetParamSet(ctx, &params)

m.keeper.SetParams(ctx, params)
m.keeper.Logger(ctx).Info("successfully migrated transfer app self-manage params")
Expand Down
6 changes: 6 additions & 0 deletions modules/apps/transfer/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,9 @@ type ConnectionKeeper interface {
type PortKeeper interface {
BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability
}

// ParamSubspace defines the expected Subspace interface for module parameters.
type ParamSubspace interface {
Get(ctx sdk.Context, key []byte, ptr interface{})
Set(ctx sdk.Context, key []byte, param interface{})
}
5 changes: 2 additions & 3 deletions modules/core/02-client/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

"github.com/cometbft/cometbft/light"

Expand All @@ -32,13 +31,13 @@ import (
type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.BinaryCodec
legacySubspace paramtypes.Subspace
legacySubspace types.ParamSubspace
stakingKeeper types.StakingKeeper
upgradeKeeper types.UpgradeKeeper
}

// NewKeeper creates a new NewKeeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
return Keeper{
storeKey: key,
cdc: cdc,
Expand Down
11 changes: 10 additions & 1 deletion modules/core/02-client/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package keeper

import (
errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

v7 "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

// Migrator is a struct for handling in-place store migrations.
Expand Down Expand Up @@ -37,8 +41,13 @@ func (m Migrator) Migrate3to4(ctx sdk.Context) error {
// This migration takes the parameters that are currently stored and managed by x/params
// and stores them directly in the ibc module's state.
func (m Migrator) MigrateParams(ctx sdk.Context) error {
legacySubpsace, ok := m.keeper.legacySubspace.(paramtypes.Subspace)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", paramtypes.Subspace{}, m.keeper.legacySubspace)
}

var params types.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)
legacySubpsace.GetParamSet(ctx, &params)
if err := params.Validate(); err != nil {
return err
}
Expand Down
7 changes: 7 additions & 0 deletions modules/core/02-client/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

upgradetypes "cosmossdk.io/x/upgrade/types"

sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

Expand All @@ -25,3 +26,9 @@ type UpgradeKeeper interface {
SetUpgradedConsensusState(ctx context.Context, planHeight int64, bz []byte) error
ScheduleUpgrade(ctx context.Context, plan upgradetypes.Plan) error
}

// ParamSubspace defines the expected Subspace interface for module parameters.
type ParamSubspace interface {
Get(ctx sdk.Context, key []byte, ptr interface{})
Set(ctx sdk.Context, key []byte, param interface{})
}
5 changes: 2 additions & 3 deletions modules/core/03-connection/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
"github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
Expand All @@ -24,13 +23,13 @@ type Keeper struct {
types.QueryServer

storeKey storetypes.StoreKey
legacySubspace paramtypes.Subspace
legacySubspace types.ParamSubspace
cdc codec.BinaryCodec
clientKeeper types.ClientKeeper
}

// NewKeeper creates a new IBC connection Keeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, ck types.ClientKeeper) Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, ck types.ClientKeeper) Keeper {
return Keeper{
storeKey: key,
cdc: cdc,
Expand Down
11 changes: 10 additions & 1 deletion modules/core/03-connection/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package keeper

import (
errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

connectionv7 "github.com/cosmos/ibc-go/v8/modules/core/03-connection/migrations/v7"
"github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

// Migrator is a struct for handling in-place store migrations.
Expand All @@ -28,8 +32,13 @@ func (m Migrator) Migrate3to4(ctx sdk.Context) error {
// This migration takes the parameters that are currently stored and managed by x/params
// and stores them directly in the ibc module's state.
func (m Migrator) MigrateParams(ctx sdk.Context) error {
legacySubpsace, ok := m.keeper.legacySubspace.(paramtypes.Subspace)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", paramtypes.Subspace{}, m.keeper.legacySubspace)
}

var params types.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)
legacySubpsace.GetParamSet(ctx, &params)
if err := params.Validate(); err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions modules/core/03-connection/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,9 @@ type ClientKeeper interface {
IterateClientStates(ctx sdk.Context, prefix []byte, cb func(string, exported.ClientState) bool)
ClientStore(ctx sdk.Context, clientID string) storetypes.KVStore
}

// ParamSubspace defines the expected Subspace interface for module parameters.
type ParamSubspace interface {
Get(ctx sdk.Context, key []byte, ptr interface{})
Set(ctx sdk.Context, key []byte, param interface{})
}
3 changes: 1 addition & 2 deletions modules/core/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/codec"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
clientkeeper "github.com/cosmos/ibc-go/v8/modules/core/02-client/keeper"
Expand Down Expand Up @@ -40,7 +39,7 @@ type Keeper struct {

// NewKeeper creates a new ibc Keeper
func NewKeeper(
cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace types.ParamSubspace,
stakingKeeper clienttypes.StakingKeeper, upgradeKeeper clienttypes.UpgradeKeeper,
scopedKeeper capabilitykeeper.ScopedKeeper, authority string,
) *Keeper {
Expand Down
9 changes: 9 additions & 0 deletions modules/core/types/expected_interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package types

import sdk "github.com/cosmos/cosmos-sdk/types"

// ParamSubspace defines the expected Subspace interface for module parameters.
type ParamSubspace interface {
Get(ctx sdk.Context, key []byte, ptr interface{})
Set(ctx sdk.Context, key []byte, param interface{})
}