Skip to content

Commit

Permalink
chore: remove unused capabilities methods and clean up tests. (#7239)
Browse files Browse the repository at this point in the history
* remove unneeded methods

* Remvoed capabilites, need to remove tests

* fix tests

* Changelod and further removal
  • Loading branch information
Nikolas De Giorgis authored Sep 4, 2024
1 parent fcd2bb1 commit a953d38
Show file tree
Hide file tree
Showing 14 changed files with 24 additions and 241 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7213) Remove capabilities from `SendPacket`.
* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7225) Remove capabilities from `WriteAcknowledgement`.
* (core, apps) [\#7232](https://github.com/cosmos/ibc-go/pull/7232) Remove capabilities from channel handshake methods.
* (core, apps) [\#7232](https://github.com/cosmos/ibc-go/pull/7232) Remove capabilities from channel handshake methods. TODO list all changes
* (core/04-channel) [\#7239](https://github.com/cosmos/ibc-go/pull/7239) Removed function `LookupModuleByChannel`
* (core/24-host) [\#7239](https://github.com/cosmos/ibc-go/pull/7239) Removed function `ChannelCapabilityPath`
* (apps/27-interchain-accounts) [\#7239](https://github.com/cosmos/ibc-go/pull/7239) The following functions have been removed: `AuthenticateCapability`, `ClaimCapability`
* (apps/transfer) [\#7239](https://github.com/cosmos/ibc-go/pull/7239) The following functions have been removed: `BindPort`, `AuthenticateCapability`, `ClaimCapability`

### State Machine Breaking

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper_test
import (
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

Expand All @@ -23,15 +22,6 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() {
{
"success", func() {}, nil,
},
{
"port is already bound for owner but capability is claimed by another module",
func() {
capability := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), TestPortID)
err := suite.chainA.GetSimApp().TransferKeeper.ClaimCapability(suite.chainA.GetContext(), capability, host.PortPath(TestPortID))
suite.Require().NoError(err)
},
icatypes.ErrPortAlreadyBound,
},
{
"fails to generate port-id",
func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
connectiontypes "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)
Expand Down Expand Up @@ -64,15 +63,6 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
},
icatypes.ErrInvalidAccountAddress,
},
{
"port is already bound for owner but capability is claimed by another module",
func() {
capability := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), TestPortID)
err := suite.chainA.GetSimApp().TransferKeeper.ClaimCapability(suite.chainA.GetContext(), capability, host.PortPath(TestPortID))
suite.Require().NoError(err)
},
icatypes.ErrPortAlreadyBound,
},
}

for _, ordering := range []channeltypes.Order{channeltypes.UNORDERED, channeltypes.ORDERED} {
Expand Down
49 changes: 10 additions & 39 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() {
// ensure channel on chainB is set in state
suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, *channel)

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

version, err := cbs.OnChanOpenTry(suite.chainB.GetContext(), channel.Ordering, channel.ConnectionHops,
Expand Down Expand Up @@ -312,10 +309,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() {

tc.malleate()

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

err = cbs.OnChanOpenConfirm(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
Expand All @@ -341,10 +335,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanCloseInit() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

err = cbs.OnChanCloseInit(
Expand Down Expand Up @@ -382,10 +373,8 @@ func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() {
suite.Require().NoError(err)

tc.malleate() // malleate mutates test data
module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

err = cbs.OnChanCloseConfirm(
Expand Down Expand Up @@ -493,10 +482,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {

tc.malleate()

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

ctx := suite.chainB.GetContext()
Expand Down Expand Up @@ -567,10 +553,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {

tc.malleate() // malleate mutates test data

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

packet := channeltypes.NewPacket(
Expand Down Expand Up @@ -622,10 +605,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {

tc.malleate() // malleate mutates test data

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)

packet := channeltypes.NewPacket(
Expand Down Expand Up @@ -663,10 +643,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
suite.Require().NoError(err)

// call application callback directly
module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)
cbs, ok := app.(porttypes.UpgradableModule)
suite.Require().True(ok)
Expand Down Expand Up @@ -724,10 +701,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {

tc.malleate() // malleate mutates test data

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)
cbs, ok := app.(porttypes.UpgradableModule)
suite.Require().True(ok)
Expand Down Expand Up @@ -764,10 +738,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
suite.Require().NoError(err)

// call application callback directly
module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(path.EndpointB.ChannelConfig.PortID)
suite.Require().True(ok)
cbs, ok := app.(porttypes.UpgradableModule)
suite.Require().True(ok)
Expand Down
12 changes: 0 additions & 12 deletions modules/apps/27-interchain-accounts/host/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,12 @@ import (

genesistypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/genesis/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
)

// InitGenesis initializes the interchain accounts host application state from a provided genesis state
func InitGenesis(ctx context.Context, keeper Keeper, state genesistypes.HostGenesisState) {
keeper.setPort(ctx, state.Port)

// generate port capability if it does not already exist
if !keeper.hasCapability(ctx, state.Port) {
// use the port keeper to generate a new capability
capability := keeper.portKeeper.BindPort(ctx, state.Port)

// use the host scoped keeper to claim the port capability
if err := keeper.ClaimCapability(ctx, capability, host.PortPath(state.Port)); err != nil {
panic(fmt.Errorf("could not claim port capability: %v", err))
}
}

for _, ch := range state.ActiveChannels {
keeper.SetActiveChannelID(ctx, ch.ConnectionId, ch.PortId, ch.ChannelId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/host/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

Expand Down Expand Up @@ -46,10 +45,6 @@ func (suite *KeeperTestSuite) TestInitGenesis() {

store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(types.StoreKey))
suite.Require().True(store.Has(icatypes.KeyPort(icatypes.HostPortID)))

capability, found := suite.chainA.GetSimApp().ScopedICAHostKeeper.GetCapability(suite.chainA.GetContext(), host.PortPath(icatypes.HostPortID))
suite.Require().True(found)
suite.Require().NotNil(capability)
}

func (suite *KeeperTestSuite) TestGenesisParams() {
Expand Down
18 changes: 0 additions & 18 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ import (
"github.com/cosmos/cosmos-sdk/runtime"
sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
genesistypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/genesis/types"
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/host/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
)
Expand Down Expand Up @@ -124,22 +122,6 @@ func (k Keeper) setPort(ctx context.Context, portID string) {
}
}

// hasCapability checks if the interchain account host module owns the port capability for the desired port
func (k Keeper) hasCapability(ctx context.Context, portID string) bool {
_, ok := k.scopedKeeper.GetCapability(ctx, host.PortPath(portID))
return ok
}

// AuthenticateCapability wraps the scopedKeeper's AuthenticateCapability function
func (k Keeper) AuthenticateCapability(ctx context.Context, cap *capabilitytypes.Capability, name string) bool {
return k.scopedKeeper.AuthenticateCapability(ctx, cap, name)
}

// ClaimCapability wraps the scopedKeeper's ClaimCapability function
func (k Keeper) ClaimCapability(ctx context.Context, cap *capabilitytypes.Capability, name string) error {
return k.scopedKeeper.ClaimCapability(ctx, cap, name)
}

// GetAppVersion calls the ICS4Wrapper GetAppVersion function.
func (k Keeper) GetAppVersion(ctx context.Context, portID, channelID string) (string, bool) {
return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
Expand Down
37 changes: 8 additions & 29 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
}
counterpartyVersion = types.V2

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(ibctesting.TransferPort)
suite.Require().True(ok)

tc.malleate() // explicitly change fields in channel and testChannel
Expand Down Expand Up @@ -232,15 +229,12 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
path.EndpointA.ChannelID = ibctesting.FirstChannelID
counterpartyVersion = types.V2

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(ibctesting.TransferPort)
suite.Require().True(ok)

tc.malleate() // explicitly change fields in channel and testChannel

err = cbs.OnChanOpenAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointA.Counterparty.ChannelID, counterpartyVersion)
err := cbs.OnChanOpenAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointA.Counterparty.ChannelID, counterpartyVersion)

expPass := tc.expError == nil
if expPass {
Expand Down Expand Up @@ -372,10 +366,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
packet = channeltypes.NewPacket(packetData.GetBytes(), seq, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.ZeroHeight(), suite.chainA.GetTimeoutTimestamp())

ctx := suite.chainB.GetContext()
module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(ctx, ibctesting.TransferPort)
suite.Require().NoError(err)

cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(ibctesting.TransferPort)
suite.Require().True(ok)

tc.malleate() // change fields in packet
Expand Down Expand Up @@ -439,10 +430,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() {
"already timed-out packet",
sdk.NewCoins(ibctesting.TestCoin),
func() {
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(ibctesting.TransferPort)
suite.Require().True(ok)

suite.Require().NoError(cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, suite.chainA.SenderAccount.GetAddress()))
Expand Down Expand Up @@ -477,10 +465,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() {
packet, err = ibctesting.ParsePacketFromEvents(res.Events)
suite.Require().NoError(err)

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(ibctesting.TransferPort)
suite.Require().True(ok)

tc.malleate() // change fields in packet
Expand Down Expand Up @@ -627,10 +612,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() {

tc.malleate()

module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), types.PortID)
suite.Require().NoError(err)

app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(types.PortID)
suite.Require().True(ok)

cbs, ok := app.(porttypes.UpgradableModule)
Expand Down Expand Up @@ -698,10 +680,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeAck() {

tc.malleate()

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), types.PortID)
suite.Require().NoError(err)

app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(types.PortID)
suite.Require().True(ok)

cbs, ok := app.(porttypes.UpgradableModule)
Expand Down
13 changes: 0 additions & 13 deletions modules/apps/transfer/keeper/genesis.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

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

"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
Expand All @@ -17,17 +15,6 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
k.setDenomMetadata(ctx, denom)
}

// Only try to bind to port if it is not already bound, since we may already own
// port capability from capability InitGenesis
if !k.hasCapability(ctx, state.PortId) {
// transfer module binds to the transfer port on InitChain
// and claims the returned capability
err := k.BindPort(ctx, state.PortId)
if err != nil {
panic(fmt.Errorf("could not claim port capability: %v", err))
}
}

k.SetParams(ctx, state.Params)

// Every denom will have only one total escrow amount, since any
Expand Down
Loading

0 comments on commit a953d38

Please sign in to comment.