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

refactor: standardise sub keeper usage in core ibc #6081

Merged
merged 6 commits into from
Apr 10, 2024
Merged

refactor: standardise sub keeper usage in core ibc #6081

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
41a6f7f
refactor: standardise sub keeper usage in core ibc
damiannolan Apr 3, 2024
1167e6b
Merge branch 'main' into damian/6058-standardise-sub-keeper-usage
damiannolan Apr 8, 2024
17ac5a1
Merge branch 'main' into damian/6058-standardise-sub-keeper-usage
crodriguezvega Apr 10, 2024
5800bad
testing: type assertion nits
damiannolan Apr 10, 2024
e70e6cb
Merge branch 'main' into damian/6058-standardise-sub-keeper-usage
damiannolan Apr 10, 2024
1c1799a
refactor: use pointer receivers on core module methods
damiannolan Apr 10, 2024
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
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,6 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
suite.chainA.GetSimApp().ICAControllerKeeper.WithICS4Wrapper(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper)
ics4Wrapper = suite.chainA.GetSimApp().ICAControllerKeeper.GetICS4Wrapper()

_, isChannelKeeper = ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper = ics4Wrapper.(*channelkeeper.Keeper)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().True(isChannelKeeper)
}
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,14 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {

_, isFeeKeeper := ics4Wrapper.(ibcfeekeeper.Keeper)
suite.Require().True(isFeeKeeper)
_, isChannelKeeper := ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper := ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().False(isChannelKeeper)

// set the ics4 wrapper to the channel keeper
suite.chainA.GetSimApp().ICAHostKeeper.WithICS4Wrapper(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper)
ics4Wrapper = suite.chainA.GetSimApp().ICAHostKeeper.GetICS4Wrapper()

_, isChannelKeeper = ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper = ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().True(isChannelKeeper)
_, isFeeKeeper = ics4Wrapper.(ibcfeekeeper.Keeper)
suite.Require().False(isFeeKeeper)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
// test if the ics4 wrapper is the channel keeper initially
ics4Wrapper := suite.chainA.GetSimApp().IBCFeeKeeper.GetICS4Wrapper()

_, isChannelKeeper := ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper := ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().True(isChannelKeeper)
_, isFeeKeeper := ics4Wrapper.(keeper.Keeper)
suite.Require().False(isFeeKeeper)
Expand All @@ -307,6 +307,6 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {

_, isFeeKeeper = ics4Wrapper.(keeper.Keeper)
suite.Require().True(isFeeKeeper)
_, isChannelKeeper = ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper = ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().False(isChannelKeeper)
}
2 changes: 1 addition & 1 deletion modules/apps/callbacks/ibc_middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *CallbacksTestSuite) TestWithICS4Wrapper() {
cbsMiddleware.WithICS4Wrapper(s.chainA.App.GetIBCKeeper().ChannelKeeper)
ics4Wrapper := cbsMiddleware.GetICS4Wrapper()

s.Require().IsType(channelkeeper.Keeper{}, ics4Wrapper)
s.Require().IsType(&channelkeeper.Keeper{}, ics4Wrapper)
Copy link
Contributor

Choose a reason for hiding this comment

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

Update type assertion to expect a pointer to channelkeeper.Keeper.

- s.Require().IsType(&channelkeeper.Keeper{}, ics4Wrapper)
+ s.Require().IsType((*channelkeeper.Keeper)(nil), ics4Wrapper)

This change aligns with the PR's objective to standardize the usage of reference types for sub-keepers, ensuring consistency across the IBC module.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
s.Require().IsType(&channelkeeper.Keeper{}, ics4Wrapper)
s.Require().IsType((*channelkeeper.Keeper)(nil), ics4Wrapper)

}

func (s *CallbacksTestSuite) TestSendPacket() {
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/transfer/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,13 +341,13 @@ func (suite *KeeperTestSuite) TestWithICS4Wrapper() {
// test if the ics4 wrapper is the channel keeper initially
ics4Wrapper := suite.chainA.GetSimApp().TransferKeeper.GetICS4Wrapper()

_, isChannelKeeper := ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper := ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().False(isChannelKeeper)

// set the ics4 wrapper to the channel keeper
suite.chainA.GetSimApp().TransferKeeper.WithICS4Wrapper(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper)
ics4Wrapper = suite.chainA.GetSimApp().TransferKeeper.GetICS4Wrapper()

_, isChannelKeeper = ics4Wrapper.(channelkeeper.Keeper)
_, isChannelKeeper = ics4Wrapper.(*channelkeeper.Keeper)
suite.Require().True(isChannelKeeper)
}
4 changes: 2 additions & 2 deletions modules/core/02-client/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

// InitGenesis initializes the ibc client submodule's state from a provided genesis
// state.
func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
func InitGenesis(ctx sdk.Context, k *keeper.Keeper, gs types.GenesisState) {
if err := gs.Params.Validate(); err != nil {
panic(fmt.Errorf("invalid ibc client genesis state parameters: %v", err))
}
Expand Down Expand Up @@ -61,7 +61,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
// ExportGenesis returns the ibc client submodule's exported genesis.
// NOTE: the export process is not optimized, it will iterate three
// times over the 02-client sub-store.
func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState {
func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) types.GenesisState {
genClients := k.GetAllGenesisClients(ctx)
clientsMetadata, err := k.GetAllClientMetadata(ctx, genClients)
if err != nil {
Expand Down
4 changes: 2 additions & 2 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 @@ -33,12 +33,12 @@ type Keeper struct {
}

// NewKeeper creates a new NewKeeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, consensusHost types.ConsensusHost, uk types.UpgradeKeeper) Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, consensusHost types.ConsensusHost, uk types.UpgradeKeeper) *Keeper {
router := types.NewRouter(key)
localhostModule := localhost.NewLightClientModule(cdc, key)
router.AddRoute(exported.Localhost, localhostModule)

return Keeper{
return &Keeper{
storeKey: key,
cdc: cdc,
router: router,
Expand Down
4 changes: 2 additions & 2 deletions modules/core/02-client/migrations/v7/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, ibctesting.DefaultSolomachineClientID, "testing", 1)
solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4)

clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper)
clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)

// manually generate old proto buf definitions and set in genesis
// NOTE: we cannot use 'ExportGenesis' for the solo machines since we are
Expand Down Expand Up @@ -109,7 +109,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
// 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(ibcexported.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)
expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)

cdc, ok := suite.chainA.App.AppCodec().(codec.ProtoCodecMarshaler)
suite.Require().True(ok)
Expand Down
2 changes: 1 addition & 1 deletion modules/core/02-client/types/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (suite *TypesTestSuite) TestMarshalGenesisState() {
err := path.EndpointA.UpdateClient()
suite.Require().NoError(err)

genesis := client.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper)
genesis := client.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)

bz, err := cdc.MarshalJSON(&genesis)
suite.Require().NoError(err)
Expand Down
4 changes: 2 additions & 2 deletions modules/core/03-connection/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

// InitGenesis initializes the ibc connection submodule's state from a provided genesis
// state.
func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
func InitGenesis(ctx sdk.Context, k *keeper.Keeper, gs types.GenesisState) {
for _, connection := range gs.Connections {
conn := types.NewConnectionEnd(connection.State, connection.ClientId, connection.Counterparty, connection.Versions, connection.DelayPeriod)
k.SetConnection(ctx, connection.Id, conn)
Expand All @@ -24,7 +24,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
}

// ExportGenesis returns the ibc connection submodule's exported genesis.
func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState {
func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) types.GenesisState {
return types.GenesisState{
Connections: k.GetAllConnections(ctx),
ClientConnectionPaths: k.GetAllClientConnectionPaths(ctx),
Expand Down
4 changes: 2 additions & 2 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 @@ -29,8 +29,8 @@ type Keeper struct {
}

// NewKeeper creates a new IBC connection Keeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, ck types.ClientKeeper) Keeper {
return Keeper{
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, ck types.ClientKeeper) *Keeper {
return &Keeper{
storeKey: key,
cdc: cdc,
legacySubspace: legacySubspace,
Expand Down
4 changes: 2 additions & 2 deletions modules/core/03-connection/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import (

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper Keeper
keeper *Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper Keeper) Migrator {
func NewMigrator(keeper *Keeper) Migrator {
return Migrator{keeper: keeper}
}

Expand Down
4 changes: 2 additions & 2 deletions modules/core/04-channel/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// InitGenesis initializes the ibc channel submodule's state from a provided genesis
// state.
func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
func InitGenesis(ctx sdk.Context, k *keeper.Keeper, gs types.GenesisState) {
if err := gs.Params.Validate(); err != nil {
panic(fmt.Sprintf("invalid ibc channel genesis state parameters: %v", err))
}
Expand Down Expand Up @@ -42,7 +42,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
}

// ExportGenesis returns the ibc channel submodule's exported genesis.
func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState {
func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) types.GenesisState {
return types.GenesisState{
Channels: k.GetAllChannels(ctx),
Acknowledgements: k.GetAllPacketAcks(ctx),
Expand Down
4 changes: 2 additions & 2 deletions modules/core/04-channel/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ func NewKeeper(
cdc codec.BinaryCodec, key storetypes.StoreKey,
clientKeeper types.ClientKeeper, connectionKeeper types.ConnectionKeeper,
portKeeper types.PortKeeper, scopedKeeper exported.ScopedKeeper,
) Keeper {
return Keeper{
) *Keeper {
return &Keeper{
storeKey: key,
cdc: cdc,
clientKeeper: clientKeeper,
Expand Down
4 changes: 2 additions & 2 deletions modules/core/04-channel/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper Keeper
keeper *Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper Keeper) Migrator {
func NewMigrator(keeper *Keeper) Migrator {
return Migrator{keeper: keeper}
}

Expand Down
4 changes: 2 additions & 2 deletions modules/core/05-port/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ type Keeper struct {
}

// NewKeeper creates a new IBC connection Keeper instance
func NewKeeper(sck exported.ScopedKeeper) Keeper {
return Keeper{
func NewKeeper(sck exported.ScopedKeeper) *Keeper {
return &Keeper{
scopedKeeper: sck,
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/core/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import (
// InitGenesis initializes the ibc state from a provided genesis
// state.
func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs *types.GenesisState) {
client.InitGenesis(ctx, *k.ClientKeeper, gs.ClientGenesis)
client.InitGenesis(ctx, k.ClientKeeper, gs.ClientGenesis)
connection.InitGenesis(ctx, k.ConnectionKeeper, gs.ConnectionGenesis)
channel.InitGenesis(ctx, k.ChannelKeeper, gs.ChannelGenesis)
}

// ExportGenesis returns the ibc exported genesis.
func ExportGenesis(ctx sdk.Context, k keeper.Keeper) *types.GenesisState {
return &types.GenesisState{
ClientGenesis: client.ExportGenesis(ctx, *k.ClientKeeper),
ClientGenesis: client.ExportGenesis(ctx, k.ClientKeeper),
ConnectionGenesis: connection.ExportGenesis(ctx, k.ConnectionKeeper),
ChannelGenesis: channel.ExportGenesis(ctx, k.ChannelKeeper),
}
Expand Down
12 changes: 6 additions & 6 deletions modules/core/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ type Keeper struct {
cdc codec.BinaryCodec

ClientKeeper *clientkeeper.Keeper
ConnectionKeeper connectionkeeper.Keeper
ChannelKeeper channelkeeper.Keeper
ConnectionKeeper *connectionkeeper.Keeper
ChannelKeeper *channelkeeper.Keeper
PortKeeper *portkeeper.Keeper
Router *porttypes.Router

Expand Down Expand Up @@ -62,16 +62,16 @@ func NewKeeper(
}

clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, consensusHost, upgradeKeeper)
connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, &clientKeeper)
connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, clientKeeper)
portKeeper := portkeeper.NewKeeper(scopedKeeper)
channelKeeper := channelkeeper.NewKeeper(cdc, key, &clientKeeper, &connectionKeeper, &portKeeper, scopedKeeper)
channelKeeper := channelkeeper.NewKeeper(cdc, key, clientKeeper, connectionKeeper, portKeeper, scopedKeeper)

return &Keeper{
cdc: cdc,
ClientKeeper: &clientKeeper,
ClientKeeper: clientKeeper,
ConnectionKeeper: connectionKeeper,
ChannelKeeper: channelKeeper,
PortKeeper: &portKeeper,
PortKeeper: portKeeper,
authority: authority,
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/core/migrations/v7/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, ibctesting.DefaultSolomachineClientID, "testing", 1)
solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4)

clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper)
clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)

// manually generate old proto buf definitions and set in genesis
// NOTE: we cannot use 'ExportGenesis' for the solo machines since we are
Expand Down Expand Up @@ -135,7 +135,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
// 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(ibcexported.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)
expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)

cdc := suite.chainA.App.AppCodec().(*codec.ProtoCodec)

Expand Down
Loading