diff --git a/x/ibc/04-channel/keeper/packet.go b/x/ibc/04-channel/keeper/packet.go index 1475d68c9a..18cff6324c 100644 --- a/x/ibc/04-channel/keeper/packet.go +++ b/x/ibc/04-channel/keeper/packet.go @@ -187,6 +187,7 @@ func (k Keeper) RecvPacket( // CONTRACT: each packet handler function should call WriteAcknowledgement at the end of the execution func (k Keeper) PacketExecuted( ctx sdk.Context, + chanCap *capability.Capability, packet exported.PacketI, acknowledgement []byte, ) error { @@ -203,6 +204,11 @@ func (k Keeper) PacketExecuted( ) } + capName := ibctypes.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()) + if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { + return sdkerrors.Wrap(types.ErrInvalidChannelCapability, "channel capability failed authentication") + } + if acknowledgement != nil || channel.Ordering == exported.UNORDERED { k.SetPacketAcknowledgement( ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), diff --git a/x/ibc/04-channel/keeper/packet_test.go b/x/ibc/04-channel/keeper/packet_test.go index c38d68961e..2dc79bb196 100644 --- a/x/ibc/04-channel/keeper/packet_test.go +++ b/x/ibc/04-channel/keeper/packet_test.go @@ -193,6 +193,7 @@ func (suite *KeeperTestSuite) TestPacketExecuted() { counterparty := types.NewCounterparty(testPort2, testChannel2) var packet types.Packet + var channelCap *capability.Capability testCases := []testCase{ {"success: UNORDERED", func() { packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), 100) @@ -218,15 +219,26 @@ func (suite *KeeperTestSuite) TestPacketExecuted() { suite.chainA.createChannel(testPort2, testChannel2, testPort1, testChannel1, exported.OPEN, exported.ORDERED, testConnectionIDA) suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceRecv(suite.chainA.GetContext(), testPort2, testChannel2, 5) }, false}, + {"capability not found", func() { + packet = types.NewPacket(mockSuccessPacket{}.GetBytes(), 1, testPort1, testChannel1, counterparty.GetPortID(), counterparty.GetChannelID(), 100) + suite.chainA.createChannel(testPort2, testChannel2, testPort1, testChannel1, exported.OPEN, exported.UNORDERED, testConnectionIDA) + suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceRecv(suite.chainA.GetContext(), testPort2, testChannel2, 1) + channelCap = capability.NewCapability(3) + }, false}, } for i, tc := range testCases { tc := tc suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() { suite.SetupTest() // reset + + var err error + channelCap, err = suite.chainA.App.ScopedIBCKeeper.NewCapability(suite.chainA.GetContext(), ibctypes.ChannelCapabilityPath(testPort2, testChannel2)) + suite.Require().NoError(err, "could not create capability") + tc.malleate() - err := suite.chainA.App.IBCKeeper.ChannelKeeper.PacketExecuted(suite.chainA.GetContext(), packet, mockSuccessPacket{}.GetBytes()) + err = suite.chainA.App.IBCKeeper.ChannelKeeper.PacketExecuted(suite.chainA.GetContext(), channelCap, packet, mockSuccessPacket{}.GetBytes()) if tc.expPass { suite.Require().NoError(err) @@ -242,7 +254,9 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { var packet types.Packet packetKey := ibctypes.KeyPacketAcknowledgement(testPort2, testChannel2, 1) - ack := transfertypes.AckDataTransfer{}.GetBytes() + ack := transfertypes.FungibleTokenPacketAcknowledgement{ + Success: true, + }.GetBytes() testCases := []testCase{ {"success", func() { diff --git a/x/ibc/04-channel/keeper/timeout.go b/x/ibc/04-channel/keeper/timeout.go index 21234f9a26..8ca20555ff 100644 --- a/x/ibc/04-channel/keeper/timeout.go +++ b/x/ibc/04-channel/keeper/timeout.go @@ -6,10 +6,12 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/capability" connection "github.com/cosmos/cosmos-sdk/x/ibc/03-connection" "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/exported" "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types" commitmentexported "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported" + ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types" ) // TimeoutPacket is called by a module which originally attempted to send a @@ -122,17 +124,15 @@ func (k Keeper) TimeoutPacket( } // TimeoutExecuted deletes the commitment send from this chain after it verifies timeout -func (k Keeper) TimeoutExecuted(ctx sdk.Context, packet exported.PacketI) error { +func (k Keeper) TimeoutExecuted(ctx sdk.Context, chanCap *capability.Capability, packet exported.PacketI) error { channel, found := k.GetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) if !found { return sdkerrors.Wrapf(types.ErrChannelNotFound, packet.GetSourcePort(), packet.GetSourceChannel()) } - // TODO: blocked by #5542 - // _, found = k.GetChannelCapability(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) - // if !found { - // return types.ErrChannelCapabilityNotFound - // } + if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, ibctypes.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel())) { + return sdkerrors.Wrap(types.ErrChannelCapabilityNotFound, "caller does not own capability for channel") + } k.deletePacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) diff --git a/x/ibc/04-channel/keeper/timeout_test.go b/x/ibc/04-channel/keeper/timeout_test.go index 06bd8a7660..9d60a0ce14 100644 --- a/x/ibc/04-channel/keeper/timeout_test.go +++ b/x/ibc/04-channel/keeper/timeout_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "fmt" + "github.com/cosmos/cosmos-sdk/x/capability" connectionexported "github.com/cosmos/cosmos-sdk/x/ibc/03-connection/exported" "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/exported" "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types" @@ -107,21 +108,34 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { func (suite *KeeperTestSuite) TestTimeoutExecuted() { var packet types.Packet + var chanCap *capability.Capability testCases := []testCase{ {"success ORDERED", func() { packet = types.NewPacket(newMockTimeoutPacket().GetBytes(), 1, testPort1, testChannel1, testPort2, testChannel2, 3) suite.chainA.createChannel(testPort1, testChannel1, testPort2, testChannel2, exported.OPEN, exported.ORDERED, testConnectionIDA) }, true}, {"channel not found", func() {}, false}, + {"incorrect capability", func() { + packet = types.NewPacket(newMockTimeoutPacket().GetBytes(), 1, testPort1, testChannel1, testPort2, testChannel2, 3) + suite.chainA.createChannel(testPort1, testChannel1, testPort2, testChannel2, exported.OPEN, exported.ORDERED, testConnectionIDA) + chanCap = capability.NewCapability(1) + }, false}, } for i, tc := range testCases { tc := tc suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() { suite.SetupTest() // reset + + var err error + chanCap, err = suite.chainA.App.ScopedIBCKeeper.NewCapability( + suite.chainA.GetContext(), ibctypes.ChannelCapabilityPath(testPort1, testChannel1), + ) + suite.Require().NoError(err, "could not create capability") + tc.malleate() - err := suite.chainA.App.IBCKeeper.ChannelKeeper.TimeoutExecuted(suite.chainA.GetContext(), packet) + err = suite.chainA.App.IBCKeeper.ChannelKeeper.TimeoutExecuted(suite.chainA.GetContext(), chanCap, packet) if tc.expPass { suite.Require().NoError(err) diff --git a/x/ibc/20-transfer/alias.go b/x/ibc/20-transfer/alias.go index 033d9b1861..2999371ce3 100644 --- a/x/ibc/20-transfer/alias.go +++ b/x/ibc/20-transfer/alias.go @@ -12,12 +12,20 @@ import ( ) const ( - DefaultPacketTimeout = keeper.DefaultPacketTimeout - AttributeKeyReceiver = types.AttributeKeyReceiver - ModuleName = types.ModuleName - StoreKey = types.StoreKey - RouterKey = types.RouterKey - QuerierRoute = types.QuerierRoute + DefaultPacketTimeout = keeper.DefaultPacketTimeout + EventTypeTimeout = types.EventTypeTimeout + EventTypePacket = types.EventTypePacket + EventTypeChannelClose = types.EventTypeChannelClose + AttributeKeyReceiver = types.AttributeKeyReceiver + AttributeKeyValue = types.AttributeKeyValue + AttributeKeyRefundReceiver = types.AttributeKeyRefundReceiver + AttributeKeyRefundValue = types.AttributeKeyRefundValue + AttributeKeyAckSuccess = types.AttributeKeyAckSuccess + AttributeKeyAckError = types.AttributeKeyAckError + ModuleName = types.ModuleName + StoreKey = types.StoreKey + RouterKey = types.RouterKey + QuerierRoute = types.QuerierRoute ) var ( @@ -35,13 +43,13 @@ var ( ) type ( - Keeper = keeper.Keeper - BankKeeper = types.BankKeeper - ChannelKeeper = types.ChannelKeeper - ClientKeeper = types.ClientKeeper - ConnectionKeeper = types.ConnectionKeeper - SupplyKeeper = types.SupplyKeeper - FungibleTokenPacketData = types.FungibleTokenPacketData - MsgTransfer = types.MsgTransfer - AckDataTransfer = types.AckDataTransfer + Keeper = keeper.Keeper + BankKeeper = types.BankKeeper + ChannelKeeper = types.ChannelKeeper + ClientKeeper = types.ClientKeeper + ConnectionKeeper = types.ConnectionKeeper + SupplyKeeper = types.SupplyKeeper + FungibleTokenPacketData = types.FungibleTokenPacketData + FungibleTokenPacketAcknowledgement = types.FungibleTokenPacketAcknowledgement + MsgTransfer = types.MsgTransfer ) diff --git a/x/ibc/20-transfer/handler.go b/x/ibc/20-transfer/handler.go index 09935da060..e44a00bb6d 100644 --- a/x/ibc/20-transfer/handler.go +++ b/x/ibc/20-transfer/handler.go @@ -3,8 +3,6 @@ package transfer import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - - channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types" ) // NewHandler returns sdk.Handler for IBC token transfer module messages @@ -40,58 +38,3 @@ func handleMsgTransfer(ctx sdk.Context, k Keeper, msg MsgTransfer) (*sdk.Result, Events: ctx.EventManager().Events().ToABCIEvents(), }, nil } - -// See onRecvPacket in spec: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#packet-relay -func handlePacketDataTransfer( - ctx sdk.Context, k Keeper, packet channeltypes.Packet, data FungibleTokenPacketData, -) (*sdk.Result, error) { - if err := k.ReceiveTransfer(ctx, packet, data); err != nil { - // NOTE (cwgoes): How do we want to handle this case? Maybe we should be more lenient, - // it's safe to leave the channel open I think. - - // TODO: handle packet receipt that due to an error (specify) - // the receiving chain couldn't process the transfer - - // source chain sent invalid packet, shutdown our channel end - if err := k.ChanCloseInit(ctx, packet.DestinationPort, packet.DestinationChannel); err != nil { - return nil, err - } - return nil, err - } - - acknowledgement := AckDataTransfer{} - if err := k.PacketExecuted(ctx, packet, acknowledgement.GetBytes()); err != nil { - return nil, err - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - sdk.EventTypeMessage, - sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory), - ), - ) - - return &sdk.Result{ - Events: ctx.EventManager().Events().ToABCIEvents(), - }, nil -} - -// See onTimeoutPacket in spec: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#packet-relay -func handleTimeoutDataTransfer( - ctx sdk.Context, k Keeper, packet channeltypes.Packet, data FungibleTokenPacketData, -) (*sdk.Result, error) { - if err := k.TimeoutTransfer(ctx, packet, data); err != nil { - // This shouldn't happen, since we've already validated that we've sent the packet. - panic(err) - } - - if err := k.TimeoutExecuted(ctx, packet); err != nil { - // This shouldn't happen, since we've already validated that we've sent the packet. - // TODO: Figure out what happens if the capability authorisation changes. - panic(err) - } - - return &sdk.Result{ - Events: ctx.EventManager().Events().ToABCIEvents(), - }, nil -} diff --git a/x/ibc/20-transfer/keeper/keeper.go b/x/ibc/20-transfer/keeper/keeper.go index ffc6550a47..2fe9324736 100644 --- a/x/ibc/20-transfer/keeper/keeper.go +++ b/x/ibc/20-transfer/keeper/keeper.go @@ -70,8 +70,13 @@ func (k Keeper) GetTransferAccount(ctx sdk.Context) supplyexported.ModuleAccount // PacketExecuted defines a wrapper function for the channel Keeper's function // in order to expose it to the ICS20 transfer handler. +// Keeper retreives channel capability and passes it into channel keeper for authentication func (k Keeper) PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement []byte) error { - return k.channelKeeper.PacketExecuted(ctx, packet, acknowledgement) + chanCap, ok := k.scopedKeeper.GetCapability(ctx, ibctypes.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel())) + if !ok { + return sdkerrors.Wrap(channel.ErrChannelCapabilityNotFound, "channel capability could not be retrieved for packet") + } + return k.channelKeeper.PacketExecuted(ctx, chanCap, packet, acknowledgement) } // ChanCloseInit defines a wrapper function for the channel Keeper's function @@ -92,12 +97,6 @@ func (k Keeper) BindPort(ctx sdk.Context, portID string) error { return k.ClaimCapability(ctx, cap, porttypes.PortPath(portID)) } -// TimeoutExecuted defines a wrapper function for the channel Keeper's function -// in order to expose it to the ICS20 transfer handler. -func (k Keeper) TimeoutExecuted(ctx sdk.Context, packet channelexported.PacketI) error { - return k.channelKeeper.TimeoutExecuted(ctx, packet) -} - // ClaimCapability allows the transfer module that can claim a capability that IBC module // passes to it func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capability.Capability, name string) error { diff --git a/x/ibc/20-transfer/keeper/relay.go b/x/ibc/20-transfer/keeper/relay.go index cf7d8be4da..4f6f4417fb 100644 --- a/x/ibc/20-transfer/keeper/relay.go +++ b/x/ibc/20-transfer/keeper/relay.go @@ -46,16 +46,6 @@ func (k Keeper) SendTransfer( return k.createOutgoingPacket(ctx, sequence, sourcePort, sourceChannel, destinationPort, destinationChannel, destHeight, amount, sender, receiver) } -// ReceiveTransfer handles transfer receiving logic. -func (k Keeper) ReceiveTransfer(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { - return k.onRecvPacket(ctx, packet, data) -} - -// TimeoutTransfer handles transfer timeout logic. -func (k Keeper) TimeoutTransfer(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { - return k.onTimeoutPacket(ctx, packet, data) -} - // See spec for this function: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#packet-relay func (k Keeper) createOutgoingPacket( ctx sdk.Context, @@ -149,7 +139,7 @@ func (k Keeper) createOutgoingPacket( return k.channelKeeper.SendPacket(ctx, channelCap, packet) } -func (k Keeper) onRecvPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { // NOTE: packet data type already checked in handler.go if len(data.Amount) != 1 { @@ -191,7 +181,18 @@ func (k Keeper) onRecvPacket(ctx sdk.Context, packet channel.Packet, data types. return k.bankKeeper.SendCoins(ctx, escrowAddress, data.Receiver, coins) } -func (k Keeper) onTimeoutPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData, ack types.FungibleTokenPacketAcknowledgement) error { + if !ack.Success { + return k.refundPacketAmount(ctx, packet, data) + } + return nil +} + +func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { + return k.refundPacketAmount(ctx, packet, data) +} + +func (k Keeper) refundPacketAmount(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData) error { // NOTE: packet data type already checked in handler.go if len(data.Amount) != 1 { diff --git a/x/ibc/20-transfer/keeper/relay_test.go b/x/ibc/20-transfer/keeper/relay_test.go index df878f8fe6..f42b4225eb 100644 --- a/x/ibc/20-transfer/keeper/relay_test.go +++ b/x/ibc/20-transfer/keeper/relay_test.go @@ -106,7 +106,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { } } -func (suite *KeeperTestSuite) TestReceiveTransfer() { +func (suite *KeeperTestSuite) TestOnRecvPacket() { data := types.NewFungibleTokenPacketData(prefixCoins2, testAddr1, testAddr2) testCases := []struct { @@ -150,7 +150,7 @@ func (suite *KeeperTestSuite) TestReceiveTransfer() { suite.SetupTest() // reset tc.malleate() - err := suite.chainA.App.TransferKeeper.ReceiveTransfer(suite.chainA.GetContext(), packet, data) + err := suite.chainA.App.TransferKeeper.OnRecvPacket(suite.chainA.GetContext(), packet, data) if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) @@ -161,37 +161,39 @@ func (suite *KeeperTestSuite) TestReceiveTransfer() { } } -func (suite *KeeperTestSuite) TestTimeoutTransfer() { +// TestOnAcknowledgementPacket tests that successful acknowledgement is a no-op +// and failure acknowledment leads to refund +func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1, testAddr2) testCoins2 := sdk.NewCoins(sdk.NewCoin("testportid/secondchannel/atom", sdk.NewInt(100))) + successAck := types.FungibleTokenPacketAcknowledgement{ + Success: true, + } + failedAck := types.FungibleTokenPacketAcknowledgement{ + Success: false, + Error: "failed packet transfer", + } + testCases := []struct { msg string + ack types.FungibleTokenPacketAcknowledgement malleate func() - expPass bool + source bool + success bool // success of ack }{ - {"successful timeout from source chain", + {"success ack causes no-op", successAck, + func() {}, true, true}, + {"successful refund from source chain", failedAck, func() { escrow := types.GetEscrowAddress(testPort2, testChannel2) _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(100)))) suite.Require().NoError(err) - }, true}, - {"successful timeout from external chain", + }, true, false}, + {"successful refund from external chain", failedAck, func() { data.Amount = testCoins2 - }, true}, - {"no source prefix on coin denom", - func() { - data.Amount = prefixCoins2 - }, false}, - {"unescrow failed", - func() { - }, false}, - {"mint failed", - func() { - data.Amount[0].Denom = prefixCoins[0].Denom - data.Amount[0].Amount = sdk.ZeroInt() - }, false}, + }, false, false}, } packet := channeltypes.NewPacket(data.GetBytes(), 1, testPort1, testChannel1, testPort2, testChannel2, 100) @@ -201,12 +203,97 @@ func (suite *KeeperTestSuite) TestTimeoutTransfer() { i := i suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { suite.SetupTest() // reset + + preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), testAddr1, prefixCoins[0].Denom) + tc.malleate() - err := suite.chainA.App.TransferKeeper.TimeoutTransfer(suite.chainA.GetContext(), packet, data) + var denom string + if tc.source { + prefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) + denom = prefixCoins[0].Denom[len(prefix):] + } else { + denom = data.Amount[0].Denom + } + + err := suite.chainA.App.TransferKeeper.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, data, tc.ack) + suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) + + postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), testAddr1, denom) + deltaAmount := postCoin.Amount.Sub(preCoin.Amount) + + if tc.success { + suite.Require().Equal(sdk.ZeroInt(), deltaAmount, "successful ack changed balance") + } else { + suite.Require().Equal(prefixCoins[0].Amount, deltaAmount, "failed ack did not trigger refund") + } + }) + } +} + +// TestOnTimeoutPacket test private refundPacket function since it is a simple wrapper over it +func (suite *KeeperTestSuite) TestOnTimeoutPacket() { + data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1, testAddr2) + testCoins2 := sdk.NewCoins(sdk.NewCoin("testportid/secondchannel/atom", sdk.NewInt(100))) + + testCases := []struct { + msg string + malleate func() + source bool + expPass bool + }{ + {"successful timeout from source chain", + func() { + escrow := types.GetEscrowAddress(testPort2, testChannel2) + _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(100)))) + suite.Require().NoError(err) + }, true, true}, + {"successful timeout from external chain", + func() { + data.Amount = testCoins2 + }, false, true}, + {"no source prefix on coin denom", + func() { + data.Amount = prefixCoins2 + }, false, false}, + {"unescrow failed", + func() { + }, true, false}, + {"mint failed", + func() { + data.Amount[0].Denom = prefixCoins[0].Denom + data.Amount[0].Amount = sdk.ZeroInt() + }, true, false}, + } + + packet := channeltypes.NewPacket(data.GetBytes(), 1, testPort1, testChannel1, testPort2, testChannel2, 100) + + for i, tc := range testCases { + tc := tc + i := i + suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { + suite.SetupTest() // reset + + preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), testAddr1, prefixCoins[0].Denom) + + tc.malleate() + + var denom string + if tc.source { + prefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) + denom = prefixCoins[0].Denom[len(prefix):] + } else { + denom = data.Amount[0].Denom + } + + err := suite.chainA.App.TransferKeeper.OnTimeoutPacket(suite.chainA.GetContext(), packet, data) + + postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), testAddr1, denom) + deltaAmount := postCoin.Amount.Sub(preCoin.Amount) if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) + suite.Require().Equal(prefixCoins[0].Amount.Int64(), deltaAmount.Int64(), "failed ack did not trigger refund") } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg) } diff --git a/x/ibc/20-transfer/module.go b/x/ibc/20-transfer/module.go index 83f3d0260e..a3c5a35562 100644 --- a/x/ibc/20-transfer/module.go +++ b/x/ibc/20-transfer/module.go @@ -2,6 +2,7 @@ package transfer import ( "encoding/json" + "fmt" "github.com/gorilla/mux" "github.com/spf13/cobra" @@ -219,7 +220,8 @@ func (am AppModule) OnChanCloseInit( portID, channelID string, ) error { - return nil + // Disallow user-initiated channel closing for transfer channels + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel") } func (am AppModule) OnChanCloseConfirm( @@ -238,15 +240,75 @@ func (am AppModule) OnRecvPacket( if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) } - return handlePacketDataTransfer(ctx, am.keeper, packet, data) + acknowledgement := FungibleTokenPacketAcknowledgement{ + Success: true, + Error: "", + } + if err := am.keeper.OnRecvPacket(ctx, packet, data); err != nil { + acknowledgement = FungibleTokenPacketAcknowledgement{ + Success: false, + Error: err.Error(), + } + } + + if err := am.keeper.PacketExecuted(ctx, packet, acknowledgement.GetBytes()); err != nil { + return nil, err + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + EventTypePacket, + sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory), + sdk.NewAttribute(AttributeKeyReceiver, data.Receiver.String()), + sdk.NewAttribute(AttributeKeyValue, data.Amount.String()), + ), + ) + + return &sdk.Result{ + Events: ctx.EventManager().Events().ToABCIEvents(), + }, nil } func (am AppModule) OnAcknowledgementPacket( ctx sdk.Context, packet channeltypes.Packet, - acknowledment []byte, + acknowledgement []byte, ) (*sdk.Result, error) { - return nil, nil + var ack FungibleTokenPacketAcknowledgement + if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) + } + var data FungibleTokenPacketData + if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + } + + if err := am.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil { + return nil, err + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + EventTypePacket, + sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory), + sdk.NewAttribute(AttributeKeyReceiver, data.Receiver.String()), + sdk.NewAttribute(AttributeKeyValue, data.Amount.String()), + sdk.NewAttribute(AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success)), + ), + ) + + if !ack.Success { + ctx.EventManager().EmitEvent( + sdk.NewEvent( + EventTypePacket, + sdk.NewAttribute(AttributeKeyAckError, ack.Error), + ), + ) + } + + return &sdk.Result{ + Events: ctx.EventManager().Events().ToABCIEvents(), + }, nil } func (am AppModule) OnTimeoutPacket( @@ -257,5 +319,21 @@ func (am AppModule) OnTimeoutPacket( if err := types.ModuleCdc.UnmarshalBinaryBare(packet.GetData(), &data); err != nil { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) } - return handleTimeoutDataTransfer(ctx, am.keeper, packet, data) + // refund tokens + if err := am.keeper.OnTimeoutPacket(ctx, packet, data); err != nil { + return nil, err + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + EventTypeTimeout, + sdk.NewAttribute(AttributeKeyRefundReceiver, data.Sender.String()), + sdk.NewAttribute(AttributeKeyRefundValue, data.Amount.String()), + sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory), + ), + ) + + return &sdk.Result{ + Events: ctx.EventManager().Events().ToABCIEvents(), + }, nil } diff --git a/x/ibc/20-transfer/types/events.go b/x/ibc/20-transfer/types/events.go index f233b49377..6b1f6295bb 100644 --- a/x/ibc/20-transfer/types/events.go +++ b/x/ibc/20-transfer/types/events.go @@ -8,7 +8,16 @@ import ( // IBC transfer events const ( - AttributeKeyReceiver = "receiver" + EventTypeTimeout = "timeout" + EventTypePacket = "fungible_token_packet" + EventTypeChannelClose = "channel_closed" + + AttributeKeyReceiver = "receiver" + AttributeKeyValue = "value" + AttributeKeyRefundReceiver = "refund_receiver" + AttributeKeyRefundValue = "refund_value" + AttributeKeyAckSuccess = "success" + AttributeKeyAckError = "error" ) // IBC transfer events vars diff --git a/x/ibc/20-transfer/types/expected_keepers.go b/x/ibc/20-transfer/types/expected_keepers.go index 9e924ee870..f37c1424e6 100644 --- a/x/ibc/20-transfer/types/expected_keepers.go +++ b/x/ibc/20-transfer/types/expected_keepers.go @@ -20,9 +20,8 @@ type ChannelKeeper interface { GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channel.Channel, found bool) GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) SendPacket(ctx sdk.Context, channelCap *capability.Capability, packet channelexported.PacketI) error - PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement []byte) error + PacketExecuted(ctx sdk.Context, chanCap *capability.Capability, packet channelexported.PacketI, acknowledgement []byte) error ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap *capability.Capability) error - TimeoutExecuted(ctx sdk.Context, packet channelexported.PacketI) error } // ClientKeeper defines the expected IBC client keeper diff --git a/x/ibc/20-transfer/types/packet.go b/x/ibc/20-transfer/types/packet.go index 9cb6d1b812..f510c3027b 100644 --- a/x/ibc/20-transfer/types/packet.go +++ b/x/ibc/20-transfer/types/packet.go @@ -59,11 +59,15 @@ func (ftpd FungibleTokenPacketData) GetBytes() []byte { return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(ftpd)) } -// AckDataTransfer is a no-op packet +// FungibleTokenPacketAcknowledgement contains a boolean success flag and an optional error msg +// error msg is empty string on success // See spec for onAcknowledgePacket: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#packet-relay -type AckDataTransfer struct{} +type FungibleTokenPacketAcknowledgement struct { + Success bool `json:"success" yaml:"success"` + Error string `json:"error" yaml:"error"` +} // GetBytes is a helper for serialising -func (AckDataTransfer) GetBytes() []byte { - return []byte("fungible token transfer ack") +func (ack FungibleTokenPacketAcknowledgement) GetBytes() []byte { + return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(ack)) } diff --git a/x/ibc/ante/ante_test.go b/x/ibc/ante/ante_test.go index ced2b14ee5..72e20f398b 100644 --- a/x/ibc/ante/ante_test.go +++ b/x/ibc/ante/ante_test.go @@ -119,7 +119,12 @@ func (suite *HandlerTestSuite) TestHandleMsgPacketOrdered() { suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceRecv(cctx, cpportid, cpchanid, uint64(i)) _, err := handler(cctx, suite.newTx(msg), false) if err == nil { - err = suite.chainA.App.IBCKeeper.ChannelKeeper.PacketExecuted(cctx, packet, packet.Data) + // retrieve channelCapability from scopedIBCKeeper and pass into PacketExecuted + chanCap, ok := suite.chainA.App.ScopedIBCKeeper.GetCapability(cctx, ibctypes.ChannelCapabilityPath( + packet.GetDestPort(), packet.GetDestChannel()), + ) + suite.Require().True(ok, "could not retrieve capability") + err = suite.chainA.App.IBCKeeper.ChannelKeeper.PacketExecuted(cctx, chanCap, packet, packet.Data) } if i == 1 { suite.NoError(err, "%d", i) // successfully executed @@ -339,6 +344,7 @@ func (chain *TestChain) createChannel( ) ctx := chain.GetContext() chain.App.IBCKeeper.ChannelKeeper.SetChannel(ctx, portID, channelID, channel) + chain.App.ScopedIBCKeeper.NewCapability(ctx, ibctypes.ChannelCapabilityPath(portID, channelID)) return channel } diff --git a/x/ibc/handler.go b/x/ibc/handler.go index d71f73e458..cb3d6c7ea5 100644 --- a/x/ibc/handler.go +++ b/x/ibc/handler.go @@ -183,7 +183,7 @@ func NewHandler(k Keeper) sdk.Handler { case channel.MsgTimeout: // Lookup module by channel capability - module, _, ok := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel) + module, cap, ok := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel) if !ok { return nil, sdkerrors.Wrap(channel.ErrChannelCapabilityNotFound, "could not retrieve module from channel capability") } @@ -193,7 +193,16 @@ func NewHandler(k Keeper) sdk.Handler { if !ok { return nil, sdkerrors.Wrapf(port.ErrInvalidRoute, "route not found to module: %s", module) } - return cbs.OnTimeoutPacket(ctx, msg.Packet) + res, err := cbs.OnTimeoutPacket(ctx, msg.Packet) + if err != nil { + + return nil, err + } + err = k.ChannelKeeper.TimeoutExecuted(ctx, cap, msg.Packet) + if err != nil { + return nil, err + } + return res, err default: return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized IBC message type: %T", msg)