Skip to content

Commit

Permalink
chore (api)!: remove capabilities from WriteAcknowledgement (#7225)
Browse files Browse the repository at this point in the history
* chore (api)!: remove capabilities from WriteAcknowledgment.

* revert wrong change

* Add changelog and docs placeholder
  • Loading branch information
bznein authored Aug 29, 2024
1 parent 03723d4 commit d771177
Show file tree
Hide file tree
Showing 15 changed files with 17 additions and 63 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ 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`.
* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7225) Remove capabilities from `WriteAcknowledgement`.

### State Machine Breaking

Expand Down
1 change: 1 addition & 0 deletions docs/docs/05-migrations/14-v9-to-v10.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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).
- (TODO: expand later) Removal of capabilities in `WriteAcknowledgement` [\#7225](https://github.com/cosmos/ibc-go/pull/7213).

### ICS27 - Interchain Accounts

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ func (IBCMiddleware) SendPacket(
// WriteAcknowledgement implements the ICS4 Wrapper interface
func (IBCMiddleware) WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet ibcexported.PacketI,
ack ibcexported.Acknowledgement,
) error {
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 @@ -471,11 +471,10 @@ func (im IBCMiddleware) SendPacket(
// WriteAcknowledgement implements the ICS4 Wrapper interface
func (im IBCMiddleware) WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
ack exported.Acknowledgement,
) error {
return im.keeper.WriteAcknowledgement(ctx, chanCap, packet, ack)
return im.keeper.WriteAcknowledgement(ctx, packet, ack)
}

// GetAppVersion returns the application version of the underlying application
Expand Down
7 changes: 3 additions & 4 deletions modules/apps/29-fee/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

errorsmod "cosmossdk.io/errors"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v9/modules/apps/29-fee/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"
Expand All @@ -27,10 +26,10 @@ func (k Keeper) SendPacket(

// WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
// ICS29 WriteAcknowledgement is used for asynchronous acknowledgements
func (k Keeper) WriteAcknowledgement(ctx context.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error {
func (k Keeper) WriteAcknowledgement(ctx context.Context, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error {
if !k.IsFeeEnabled(ctx, packet.GetDestPort(), packet.GetDestChannel()) {
// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, acknowledgement)
return k.ics4Wrapper.WriteAcknowledgement(ctx, packet, acknowledgement)
}

packetID := channeltypes.NewPacketID(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
Expand All @@ -50,7 +49,7 @@ func (k Keeper) WriteAcknowledgement(ctx context.Context, chanCap *capabilitytyp
k.DeleteForwardRelayerAddress(ctx, packetID)

// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
return k.ics4Wrapper.WriteAcknowledgement(ctx, packet, ack)
}

// GetAppVersion returns the underlying application version.
Expand Down
6 changes: 2 additions & 4 deletions modules/apps/29-fee/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
)

ack := channeltypes.NewResultAcknowledgement([]byte("success"))
chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)

// malleate test case
tc.malleate()

err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)
err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), packet, ack)

if tc.expErr == nil {
suite.Require().NoError(err)
Expand Down Expand Up @@ -95,9 +94,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsyncFeeDisabled() {
)

ack := channeltypes.NewResultAcknowledgement([]byte("success"))
chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)

err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)
err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), packet, ack)
suite.Require().NoError(err)

packetAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
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 @@ -246,11 +246,10 @@ func (im IBCMiddleware) OnRecvPacket(ctx context.Context, channelVersion string,
// reverted via a panic.
func (im IBCMiddleware) WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet ibcexported.PacketI,
ack ibcexported.Acknowledgement,
) error {
err := im.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
err := im.ics4Wrapper.WriteAcknowledgement(ctx, packet, ack)
if err != nil {
return 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 @@ -810,14 +810,12 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
ctx = s.chainB.GetContext()
gasLimit := ctx.GasMeter().Limit()

chanCap := s.chainB.GetChannelCapability(s.path.EndpointB.ChannelConfig.PortID, s.path.EndpointB.ChannelID)

tc.malleate()

// callbacks module is routed as top level middleware
transferICS4Wrapper := GetSimApp(s.chainB).TransferKeeper.GetICS4Wrapper()

err := transferICS4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
err := transferICS4Wrapper.WriteAcknowledgement(ctx, packet, ack)

expPass := tc.expError == nil
s.AssertHasExecutedExpectedCallback(tc.callbackType, expPass)
Expand Down
9 changes: 1 addition & 8 deletions modules/apps/transfer/keeper/forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,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 @@ -48,13 +47,7 @@ func (k Keeper) forwardPacket(ctx context.Context, data types.FungibleTokenPacke

// acknowledgeForwardedPacket writes the async acknowledgement for forwardedPacket
func (k Keeper) acknowledgeForwardedPacket(ctx context.Context, forwardedPacket, packet channeltypes.Packet, ack channeltypes.Acknowledgement) error {
sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
capability, ok := k.scopedKeeper.GetCapability(sdkCtx, host.ChannelCapabilityPath(forwardedPacket.DestinationPort, forwardedPacket.DestinationChannel))
if !ok {
return errorsmod.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
}

if err := k.ics4Wrapper.WriteAcknowledgement(ctx, capability, forwardedPacket, ack); err != nil {
if err := k.ics4Wrapper.WriteAcknowledgement(ctx, forwardedPacket, ack); err != nil {
return err
}

Expand Down
9 changes: 0 additions & 9 deletions modules/core/04-channel/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ func (k *Keeper) applyReplayProtection(ctx sdk.Context, packet types.Packet, cha
// previously by RecvPacket.
func (k *Keeper) WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
acknowledgement exported.Acknowledgement,
) error {
Expand All @@ -303,15 +302,7 @@ func (k *Keeper) WriteAcknowledgement(
return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected one of [%s, %s, %s], got %s", types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE, channel.State)
}

// Authenticate capability to ensure caller has authority to receive packet on this channel
capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel())
sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
if !k.scopedKeeper.AuthenticateCapability(sdkCtx, chanCap, capName) {
return errorsmod.Wrapf(
types.ErrInvalidChannelCapability,
"channel capability failed authentication for capability name %s", capName,
)
}

// REPLAY PROTECTION: The recvStartSequence will prevent historical proofs from allowing replay
// attacks on packets processed in previous lifecycles of a channel. After a successful channel
Expand Down
28 changes: 4 additions & 24 deletions modules/core/04-channel/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,10 +667,9 @@ func (suite *KeeperTestSuite) TestRecvPacket() {

func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
var (
path *ibctesting.Path
ack exported.Acknowledgement
packet exported.PacketI
channelCap *capabilitytypes.Capability
path *ibctesting.Path
ack exported.Acknowledgement
packet exported.PacketI
)

testCases := []testCase{
Expand All @@ -680,7 +679,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.MockAcknowledgement
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
true,
},
Expand All @@ -692,7 +690,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
ack = ibcmock.MockAcknowledgement

path.EndpointB.UpdateChannel(func(channel *types.Channel) { channel.State = types.FLUSHING })
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
true,
},
Expand All @@ -704,7 +701,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
ack = ibcmock.MockAcknowledgement

path.EndpointB.UpdateChannel(func(channel *types.Channel) { channel.State = types.FLUSHCOMPLETE })
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
true,
},
Expand All @@ -713,34 +709,21 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.InvalidID, ibctesting.InvalidID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.MockAcknowledgement
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
}, false},
{"channel not open", func() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.MockAcknowledgement

path.EndpointB.UpdateChannel(func(channel *types.Channel) { channel.State = types.CLOSED })
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
}, false},
{
"capability authentication failed",
func() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.MockAcknowledgement
channelCap = capabilitytypes.NewCapability(3)
},
false,
},
{
"no-op, already acked",
func() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.MockAcknowledgement
suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack.Acknowledgement())
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
false,
},
Expand All @@ -750,7 +733,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = ibcmock.NewEmptyAcknowledgement()
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
false,
},
Expand All @@ -760,7 +742,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
path.Setup()
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
ack = nil
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
false,
},
Expand All @@ -772,7 +753,6 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
suite.Require().NoError(err)
packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
ack = ibcmock.MockAcknowledgement

// set recv seq start to indicate packet was processed in previous upgrade
Expand All @@ -789,7 +769,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {

tc.malleate()

err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(suite.chainB.GetContext(), channelCap, packet, ack)
err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(suite.chainB.GetContext(), packet, ack)

if tc.expPass {
suite.Require().NoError(err)
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 @@ -175,7 +175,6 @@ type ICS4Wrapper interface {

WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
ack exported.Acknowledgement,
) error
Expand Down
2 changes: 1 addition & 1 deletion modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack
// NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the
// acknowledgement is nil.
if ack != nil {
if err := k.ChannelKeeper.WriteAcknowledgement(ctx, capability, msg.Packet, ack); err != nil {
if err := k.ChannelKeeper.WriteAcknowledgement(ctx, msg.Packet, ack); err != nil {
return nil, err
}
}
Expand Down
4 changes: 1 addition & 3 deletions testing/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,8 @@ func (endpoint *Endpoint) RecvPacketWithResult(packet channeltypes.Packet) (*abc
// WriteAcknowledgement writes an acknowledgement on the channel associated with the endpoint.
// The counterparty client is updated.
func (endpoint *Endpoint) WriteAcknowledgement(ack exported.Acknowledgement, packet exported.PacketI) error {
channelCap := endpoint.Chain.GetChannelCapability(packet.GetDestPort(), packet.GetDestChannel())

// no need to send message, acting as a handler
err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(endpoint.Chain.GetContext(), channelCap, packet, ack)
err := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.WriteAcknowledgement(endpoint.Chain.GetContext(), packet, ack)
if err != nil {
return 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 @@ -189,7 +189,6 @@ func (BlockUpgradeMiddleware) SendPacket(
// WriteAcknowledgement implements the ICS4 Wrapper interface
func (BlockUpgradeMiddleware) WriteAcknowledgement(
ctx context.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
ack exported.Acknowledgement,
) error {
Expand Down

0 comments on commit d771177

Please sign in to comment.