Skip to content

Commit

Permalink
chore (api)!: remove capabilities from SendPacket (#7213)
Browse files Browse the repository at this point in the history
* chore (api)\!: remove capabilities from SendPacket

* add changelog + note to add migration documentation

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
  • Loading branch information
bznein and crodriguezvega authored Aug 29, 2024
1 parent 049bef9 commit 0613ec8
Show file tree
Hide file tree
Showing 15 changed files with 12 additions and 73 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking

* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7213) Remove capabilities from `SendPacket`.

### State Machine Breaking

### Improvements
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/05-migrations/14-v9-to-v10.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ There are four sections based on the four potential user groups of this document

## IBC Apps

- (TODO: expand later) Removal of capabilities in `SendPacket` [\#7213](https://github.com/cosmos/ibc-go/pull/7213).

### ICS27 - Interchain Accounts

The channel capability migration introduced in v6 has been removed. Chains must upgrade from v6 or higher.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx context.Context, portID, channelID
// SendPacket implements the ICS4 Wrapper interface
func (IBCMiddleware) SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
)

// SendTx takes pre-built packet data containing messages to be executed on the host chain from an authentication module and attempts to send the packet.
Expand All @@ -39,11 +38,6 @@ func (k Keeper) sendTx(ctx sdk.Context, connectionID, portID string, icaPacketDa
return 0, errorsmod.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel on connection %s for port %s", connectionID, portID)
}

chanCap, found := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, activeChannelID))
if !found {
return 0, errorsmod.Wrapf(capabilitytypes.ErrCapabilityNotFound, "failed to find capability: %s", host.ChannelCapabilityPath(portID, activeChannelID))
}

if uint64(ctx.BlockTime().UnixNano()) >= timeoutTimestamp {
return 0, icatypes.ErrInvalidTimeoutTimestamp
}
Expand All @@ -52,7 +46,7 @@ func (k Keeper) sendTx(ctx sdk.Context, connectionID, portID string, icaPacketDa
return 0, errorsmod.Wrap(err, "invalid interchain account packet data")
}

sequence, err := k.ics4Wrapper.SendPacket(ctx, chanCap, portID, activeChannelID, clienttypes.ZeroHeight(), timeoutTimestamp, icaPacketData.GetBytes())
sequence, err := k.ics4Wrapper.SendPacket(ctx, portID, activeChannelID, clienttypes.ZeroHeight(), timeoutTimestamp, icaPacketData.GetBytes())
if err != nil {
return 0, err
}
Expand Down
3 changes: 1 addition & 2 deletions modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,14 +459,13 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx context.Context, portID, channelID
// SendPacket implements the ICS4 Wrapper interface
func (im IBCMiddleware) SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
return im.keeper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
return im.keeper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
}

// WriteAcknowledgement implements the ICS4 Wrapper interface
Expand Down
3 changes: 1 addition & 2 deletions modules/apps/29-fee/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ import (
// SendPacket wraps the ICS4Wrapper SendPacket function
func (k Keeper) SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
return k.ics4Wrapper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
return k.ics4Wrapper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
}

// WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
Expand Down
3 changes: 1 addition & 2 deletions modules/apps/callbacks/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,13 @@ func (im *IBCMiddleware) GetICS4Wrapper() porttypes.ICS4Wrapper {
// the packet send is rejected.
func (im IBCMiddleware) SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
seq, err := im.ics4Wrapper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
seq, err := im.ics4Wrapper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
if err != nil {
return 0, err
}
Expand Down
4 changes: 1 addition & 3 deletions modules/apps/callbacks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@ func (s *CallbacksTestSuite) TestSendPacket() {
ibctesting.EmptyForwardingPacketData,
)

chanCap := s.path.EndpointA.Chain.GetChannelCapability(s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID)

tc.malleate()

ctx := s.chainA.GetContext()
Expand All @@ -202,7 +200,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
err error
)
sendPacket := func() {
seq, err = transferICS4Wrapper.SendPacket(ctx, chanCap, s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, s.chainB.GetTimeoutHeight(), 0, packetData.GetBytes())
seq, err = transferICS4Wrapper.SendPacket(ctx, s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, s.chainB.GetTimeoutHeight(), 0, packetData.GetBytes())
}

expPass := tc.expValue == nil
Expand Down
7 changes: 1 addition & 6 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/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"
)

Expand Down Expand Up @@ -91,10 +90,6 @@ func (k Keeper) sendTransfer(

// begin createOutgoingPacket logic
// See spec for this logic: https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#packet-relay
channelCap, ok := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(sourcePort, sourceChannel))
if !ok {
return 0, errorsmod.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
}

tokens := make([]types.Token, 0, len(coins))

Expand Down Expand Up @@ -147,7 +142,7 @@ func (k Keeper) sendTransfer(
return 0, err
}

sequence, err := k.ics4Wrapper.SendPacket(ctx, channelCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, packetDataBytes)
sequence, err := k.ics4Wrapper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, packetDataBytes)
if err != nil {
return 0, err
}
Expand Down
10 changes: 0 additions & 10 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,6 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
},
sdkerrors.ErrInsufficientFunds,
},
{
"failure: channel capability not found",
func() {
capability := suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)

// Release channel capability
suite.chainA.GetSimApp().ScopedTransferKeeper.ReleaseCapability(suite.chainA.GetContext(), capability) //nolint:errcheck // ignore error for testing
},
channeltypes.ErrChannelCapabilityNotFound,
},
{
"failure: forwarding hops is not empty with ics20-1",
func() {
Expand Down
4 changes: 0 additions & 4 deletions modules/core/04-channel/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
// is returned if one occurs.
func (k *Keeper) SendPacket(
ctx context.Context,
channelCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
Expand All @@ -40,9 +39,6 @@ func (k *Keeper) SendPacket(
}

sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
if !k.scopedKeeper.AuthenticateCapability(sdkCtx, channelCap, host.ChannelCapabilityPath(sourcePort, sourceChannel)) {
return 0, errorsmod.Wrapf(types.ErrChannelCapabilityNotFound, "caller does not own capability for channel, port ID (%s) channel ID (%s)", sourcePort, sourceChannel)
}

sequence, found := k.GetNextSequenceSend(ctx, sourcePort, sourceChannel)
if !found {
Expand Down
32 changes: 1 addition & 31 deletions modules/core/04-channel/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,17 @@ func (suite *KeeperTestSuite) TestSendPacket() {
packetData []byte
timeoutHeight clienttypes.Height
timeoutTimestamp uint64
channelCap *capabilitytypes.Capability
)

testCases := []testCase{
{"success: UNORDERED channel", func() {
path.Setup()
sourceChannel = path.EndpointA.ChannelID

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, true},
{"success: ORDERED channel", func() {
path.SetChannelOrdered()
path.Setup()
sourceChannel = path.EndpointA.ChannelID

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, true},
{"success with solomachine: UNORDERED channel", func() {
path.Setup()
Expand All @@ -66,8 +61,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {
path.EndpointA.ClientID = clienttypes.FormatClientIdentifier(exported.Solomachine, 10)
path.EndpointA.SetClientState(solomachine.ClientState())
path.EndpointA.UpdateConnection(func(c *connectiontypes.ConnectionEnd) { c.ClientId = path.EndpointA.ClientID })

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, true},
{"success with solomachine: ORDERED channel", func() {
path.SetChannelOrdered()
Expand All @@ -80,21 +73,17 @@ func (suite *KeeperTestSuite) TestSendPacket() {
path.EndpointA.SetClientState(solomachine.ClientState())

path.EndpointA.UpdateConnection(func(c *connectiontypes.ConnectionEnd) { c.ClientId = path.EndpointA.ClientID })

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, true},
{"packet basic validation failed, empty packet data", func() {
path.Setup()
sourceChannel = path.EndpointA.ChannelID

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
packetData = []byte{}
}, false},
{"channel not found", func() {
// use wrong channel naming
path.Setup()
sourceChannel = ibctesting.InvalidID
channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"channel is in CLOSED state", func() {
path.Setup()
Expand All @@ -120,17 +109,13 @@ func (suite *KeeperTestSuite) TestSendPacket() {
sourceChannel = path.EndpointA.ChannelID

path.EndpointA.UpdateChannel(func(channel *types.Channel) { channel.ConnectionHops[0] = "invalid-connection" })

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"client state not found", func() {
path.Setup()
sourceChannel = path.EndpointA.ChannelID

// change connection client ID
path.EndpointA.UpdateConnection(func(c *connectiontypes.ConnectionEnd) { c.ClientId = ibctesting.InvalidID })

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"client state is frozen", func() {
path.Setup()
Expand All @@ -144,8 +129,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {
// freeze client
cs.FrozenHeight = clienttypes.NewHeight(0, 1)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), connection.ClientId, cs)

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"client state zero height", func() {
path.Setup()
Expand All @@ -162,8 +145,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {

cs.LatestHeight = clienttypes.ZeroHeight()
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), connection.ClientId, cs)

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"timeout height passed", func() {
path.Setup()
Expand All @@ -172,7 +153,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {
var ok bool
timeoutHeight, ok = path.EndpointA.GetClientLatestHeight().(clienttypes.Height)
suite.Require().True(ok)
channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"timeout timestamp passed", func() {
path.Setup()
Expand All @@ -184,7 +164,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {

timeoutHeight = disabledTimeoutHeight
timeoutTimestamp = timestamp
channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"timeout timestamp passed with solomachine", func() {
path.Setup()
Expand All @@ -202,8 +181,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {
sourceChannel = path.EndpointA.ChannelID
timeoutHeight = disabledTimeoutHeight
timeoutTimestamp = timestamp

channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"next sequence send not found", func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
Expand All @@ -217,13 +194,6 @@ func (suite *KeeperTestSuite) TestSendPacket() {
types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, path.EndpointA.ChannelConfig.Version),
)
suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false},
{"channel capability not found", func() {
path.Setup()
sourceChannel = path.EndpointA.ChannelID

channelCap = capabilitytypes.NewCapability(5)
}, false},
{
"channel is in FLUSH_COMPLETE state",
Expand Down Expand Up @@ -272,7 +242,7 @@ func (suite *KeeperTestSuite) TestSendPacket() {
// only check if nextSequenceSend exists in no error case since it is a tested error case above.
expectedSequence, ok := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceSend(suite.chainA.GetContext(), sourcePort, sourceChannel)

sequence, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.SendPacket(suite.chainA.GetContext(), channelCap,
sequence, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.SendPacket(suite.chainA.GetContext(),
sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, packetData)

if tc.expPass {
Expand Down
1 change: 0 additions & 1 deletion modules/core/05-port/types/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ type UpgradableModule interface {
type ICS4Wrapper interface {
SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
Expand Down
4 changes: 1 addition & 3 deletions testing/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,8 @@ func (endpoint *Endpoint) SendPacket(
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
channelCap := endpoint.Chain.GetChannelCapability(endpoint.ChannelConfig.PortID, endpoint.ChannelID)

// no need to send message, acting as a module
sequence, err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.SendPacket(endpoint.Chain.GetContext(), channelCap, endpoint.ChannelConfig.PortID, endpoint.ChannelID, timeoutHeight, timeoutTimestamp, data)
sequence, err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.SendPacket(endpoint.Chain.GetContext(), endpoint.ChannelConfig.PortID, endpoint.ChannelID, timeoutHeight, timeoutTimestamp, data)
if err != nil {
return 0, err
}
Expand Down
1 change: 0 additions & 1 deletion testing/mock/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx context.Context, channelVer
// SendPacket implements the ICS4 Wrapper interface
func (BlockUpgradeMiddleware) SendPacket(
ctx context.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
Expand Down

0 comments on commit 0613ec8

Please sign in to comment.